From 34099370037e5ab9f03576e93af364dfd65f1519 Mon Sep 17 00:00:00 2001
From: Kefu Chai <kchai@redhat.com>
Date: Fri, 19 Jul 2019 01:29:52 +0800
Subject: [PATCH] mgr: check for unicode passed to "set_health_checks()"

in python2, in addition to `str`, a string could also be unicode
string. there is chance that user passes a unicode string to
`set_health_checks()`, so let's check for unicode string also.

in python3, all strings are `unicode` string, and `PyString_Check()` is
defined as an alias of `PyUnicode_Check()`, so we are fine with python3.

the wrapper method of `PyString_ToString()` returns a pair of converted
string and a bool. if it fails to detect a string, an empty string and
false are returned.

Signed-off-by: Kefu Chai <kchai@redhat.com>
---
 src/mgr/BaseMgrModule.cc | 16 ++++++++--------
 src/mgr/PythonCompat.h   | 17 +++++++++++++++++
 2 files changed, 25 insertions(+), 8 deletions(-)

diff --git a/src/mgr/BaseMgrModule.cc b/src/mgr/BaseMgrModule.cc
index 02a37f6143f..ddc387f6f55 100644
--- a/src/mgr/BaseMgrModule.cc
+++ b/src/mgr/BaseMgrModule.cc
@@ -284,24 +284,23 @@ ceph_set_health_checks(BaseMgrModule *self, PyObject *args)
       }
       string ks(k);
       if (ks == "severity") {
-	if (!PyString_Check(v)) {
+	if (auto [vs, valid] = PyString_ToString(v); !valid) {
 	  derr << __func__ << " check " << check_name
 	       << " severity value not string" << dendl;
 	  continue;
-	}
-	string vs(PyString_AsString(v));
-	if (vs == "warning") {
+	} else if (vs == "warning") {
 	  severity = HEALTH_WARN;
 	} else if (vs == "error") {
 	  severity = HEALTH_ERR;
 	}
       } else if (ks == "summary") {
-	if (!PyString_Check(v) && !PyUnicode_Check(v)) {
+	if (auto [vs, valid] = PyString_ToString(v); !valid) {
 	  derr << __func__ << " check " << check_name
 	       << " summary value not [unicode] string" << dendl;
 	  continue;
+	} else {
+	  summary = std::move(vs);
 	}
-	summary = PyString_AsString(v);
       } else if (ks == "detail") {
 	if (!PyList_Check(v)) {
 	  derr << __func__ << " check " << check_name
@@ -310,12 +309,13 @@ ceph_set_health_checks(BaseMgrModule *self, PyObject *args)
 	}
 	for (int k = 0; k < PyList_Size(v); ++k) {
 	  PyObject *di = PyList_GET_ITEM(v, k);
-	  if (!PyString_Check(di) && !PyUnicode_Check(di)) {
+	  if (auto [vs, valid] = PyString_ToString(di); !valid) {
 	    derr << __func__ << " check " << check_name
 		 << " detail item " << k << " not a [unicode] string" << dendl;
 	    continue;
+	  } else {
+	    detail.push_back(std::move(vs));
 	  }
-	  detail.push_back(PyString_AsString(di));
 	}
       } else {
 	derr << __func__ << " check " << check_name
diff --git a/src/mgr/PythonCompat.h b/src/mgr/PythonCompat.h
index 4ffb2eee8a5..c2929851de8 100644
--- a/src/mgr/PythonCompat.h
+++ b/src/mgr/PythonCompat.h
@@ -4,6 +4,7 @@
 #pragma once
 
 #include <Python.h>
+#include <string>
 
 // Python's pyconfig-64.h conflicts with ceph's acconfig.h
 #undef HAVE_SYS_WAIT_H
@@ -36,3 +37,19 @@ inline PyObject* PyInt_FromString(const char *str, char **pend, int base) {
 }
 #define PyString_Type PyUnicode_Type
 #endif
+
+inline std::pair<std::string, bool> PyString_ToString(PyObject *o) {
+#if PY_MAJOR_VERSION >= 3
+  if (PyUnicode_Check(o)) {
+    return {PyUnicode_AsUTF8(o), true};
+  } else {
+    return {{}, false};
+  }
+#else
+  if (PyString_Check(o) || PyUnicode_Check(o)) {
+    return {PyString_AsString(o), true};
+  } else {
+    return {{}, false};
+  }
+#endif
+}
-- 
2.39.5