changeset 45:ee76cced46a5

Fix issue #185 (segfaults on non-UTF8 special characters in request URI)
author Sebastien Jodogne <s.jodogne@gmail.com>
date Mon, 03 Aug 2020 18:13:10 +0200
parents 2468e2f2456c
children 3e2ff3616e57
files NEWS Sources/OnChangeCallback.cpp Sources/OnStoredInstanceCallback.cpp Sources/PythonLock.cpp Sources/PythonString.cpp Sources/PythonString.h Sources/RestCallbacks.cpp
diffstat 7 files changed, 72 insertions(+), 23 deletions(-) [+]
line wrap: on
line diff
--- a/NEWS	Wed Jul 08 15:22:12 2020 +0200
+++ b/NEWS	Mon Aug 03 18:13:10 2020 +0200
@@ -1,6 +1,8 @@
 Pending changes in the mainline
 ===============================
 
+* Fix issue #185 (segfaults on non-UTF8 special characters in request URI)
+
 
 Version 2.0 (2020-07-08)
 ========================
--- a/Sources/OnChangeCallback.cpp	Wed Jul 08 15:22:12 2020 +0200
+++ b/Sources/OnChangeCallback.cpp	Mon Aug 03 18:13:10 2020 +0200
@@ -19,7 +19,7 @@
 
 #include "OnChangeCallback.h"
 
-#include "PythonObject.h"
+#include "PythonString.h"
 
 #include "../Resources/Orthanc/Plugins/OrthancPluginCppWrapper.h"
 
@@ -149,10 +149,14 @@
         try
         {
           PythonLock lock;
+
+          PythonString resourceId(lock, change->GetResourceId());
+          
           PythonObject args(lock, PyTuple_New(3));
           PyTuple_SetItem(args.GetPyObject(), 0, PyLong_FromLong(change->GetChangeType()));
           PyTuple_SetItem(args.GetPyObject(), 1, PyLong_FromLong(change->GetResourceType()));
-          PyTuple_SetItem(args.GetPyObject(), 2, PyUnicode_FromString(change->GetResourceId().c_str()));
+          PyTuple_SetItem(args.GetPyObject(), 2, resourceId.Release());
+          
           PythonObject result(lock, PyObject_CallObject(changesCallback_, args.GetPyObject()));
 
           std::string traceback;
--- a/Sources/OnStoredInstanceCallback.cpp	Wed Jul 08 15:22:12 2020 +0200
+++ b/Sources/OnStoredInstanceCallback.cpp	Mon Aug 03 18:13:10 2020 +0200
@@ -19,7 +19,7 @@
 
 #include "OnStoredInstanceCallback.h"
 
-#include "PythonObject.h"
+#include "PythonString.h"
 #include "Autogenerated/sdk.h"
 
 #include "../Resources/Orthanc/Plugins/OrthancPluginCppWrapper.h"
@@ -48,9 +48,11 @@
     /**
      * Construct the arguments tuple (output, uri)
      **/
+    PythonString str(lock, instanceId);
+    
     PythonObject args2(lock, PyTuple_New(2));
     PyTuple_SetItem(args2.GetPyObject(), 0, pInst);
-    PyTuple_SetItem(args2.GetPyObject(), 1, PyUnicode_FromString(instanceId));
+    PyTuple_SetItem(args2.GetPyObject(), 1, str.Release());
 
     PythonObject result(lock, PyObject_CallObject(storedInstanceCallback_, args2.GetPyObject()));
 
--- a/Sources/PythonLock.cpp	Wed Jul 08 15:22:12 2020 +0200
+++ b/Sources/PythonLock.cpp	Mon Aug 03 18:13:10 2020 +0200
@@ -21,6 +21,7 @@
 
 #include "PythonFunction.h"
 #include "PythonModule.h"
+#include "PythonString.h"
 
 #include "../Resources/Orthanc/Plugins/OrthancPluginCppWrapper.h"
 
@@ -429,9 +430,8 @@
     ORTHANC_PLUGINS_THROW_EXCEPTION(InternalError);
   }
 
-  PyObject* pyPath = PyUnicode_FromString(path.c_str());
-  int result = PyList_Insert(sysPath, 0, pyPath);
-  Py_DECREF(pyPath);
+  PythonString pyPath(lock, path);
+  int result = PyList_Insert(sysPath, 0, pyPath.Release());
   
   if (result != 0)
   {
--- a/Sources/PythonString.cpp	Wed Jul 08 15:22:12 2020 +0200
+++ b/Sources/PythonString.cpp	Mon Aug 03 18:13:10 2020 +0200
@@ -21,13 +21,35 @@
 
 #include "../Resources/Orthanc/Plugins/OrthancPluginCppWrapper.h"
 
-PythonString::PythonString(PythonLock& lock,
-                           const std::string& utf8) :
-  string_(lock, PyUnicode_FromString(utf8.c_str()))
+
+void PythonString::SanityCheck()
 {
-  if (!string_.IsValid())
+  if (!string_->IsValid())
   {
-    OrthancPlugins::LogError("Cannot create a Python string");
+    OrthancPlugins::LogError("Cannot create a Python string, check that the string is properly encoded using UTF-8");
     ORTHANC_PLUGINS_THROW_EXCEPTION(InternalError);
   }
 }
+
+
+PythonString::PythonString(PythonLock& lock,
+                           const std::string& utf8)
+{
+  string_.reset(new PythonObject(lock, PyUnicode_FromString(utf8.c_str())));
+  SanityCheck();
+}
+
+
+PythonString::PythonString(PythonLock& lock,
+                           const char* utf8)
+{
+  if (utf8 == NULL)
+  {
+    ORTHANC_PLUGINS_THROW_EXCEPTION(NullPointer);
+  }
+  else
+  {
+    string_.reset(new PythonObject(lock, PyUnicode_FromString(utf8)));
+    SanityCheck();
+  }
+}
--- a/Sources/PythonString.h	Wed Jul 08 15:22:12 2020 +0200
+++ b/Sources/PythonString.h	Mon Aug 03 18:13:10 2020 +0200
@@ -21,18 +21,30 @@
 
 #include "PythonObject.h"
 
+#include <Compatibility.h>  // For std::unique_ptr
+
 // A Python string is always valid, or an exception was thrown on its creation
 class PythonString : public boost::noncopyable
 {
 private:
-  PythonObject  string_;
+  std::unique_ptr<PythonObject>  string_;
+
+  void SanityCheck();
 
 public:
   PythonString(PythonLock& lock,
                const std::string& utf8);
 
+  PythonString(PythonLock& lock,
+               const char* utf8);
+
   PyObject* GetPyObject() const
   {
-    return string_.GetPyObject();
+    return string_->GetPyObject();
+  }
+
+  PyObject* Release()
+  {
+    return string_->Release();
   }
 };
--- a/Sources/RestCallbacks.cpp	Wed Jul 08 15:22:12 2020 +0200
+++ b/Sources/RestCallbacks.cpp	Mon Aug 03 18:13:10 2020 +0200
@@ -19,7 +19,7 @@
 
 #include "RestCallbacks.h"
 
-#include "PythonObject.h"
+#include "PythonString.h"
 #include "Autogenerated/sdk.h"
 
 #include "../Resources/Orthanc/Plugins/OrthancPluginCppWrapper.h"
@@ -90,9 +90,11 @@
       /**
        * Construct the arguments tuple (output, uri)
        **/
+      PythonString str(lock, uri);
+      
       PythonObject args2(lock, PyTuple_New(2));
       PyTuple_SetItem(args2.GetPyObject(), 0, pInst);
-      PyTuple_SetItem(args2.GetPyObject(), 1, PyUnicode_FromString(uri));
+      PyTuple_SetItem(args2.GetPyObject(), 1, str.Release());
       // No need to decrement refcount with "PyTuple_SetItem()"
 
       /**
@@ -120,16 +122,21 @@
         default:
           ORTHANC_PLUGINS_THROW_EXCEPTION(ParameterOutOfRange);
       }
-      
+
       PythonObject kw(lock, PyDict_New());
-      PyDict_SetItemString(kw.GetPyObject(), "method", PyUnicode_FromString(method));
+
+      {
+        PythonString str(lock, method);
+        PyDict_SetItemString(kw.GetPyObject(), "method", str.Release());
+      }
 
       {
         PythonObject groups(lock, PyTuple_New(request->groupsCount));
 
         for (uint32_t i = 0; i < request->groupsCount; i++)
         {
-          PyTuple_SetItem(groups.GetPyObject(), i, PyUnicode_FromString(request->groups[i]));
+          PythonString str(lock, request->groups[i]);
+          PyTuple_SetItem(groups.GetPyObject(), i, str.Release());
         }
 
         PyDict_SetItemString(kw.GetPyObject(), "groups", groups.Release());
@@ -141,8 +148,8 @@
 
         for (uint32_t i = 0; i < request->getCount; i++)
         {
-          PyDict_SetItemString(get.GetPyObject(), request->getKeys[i],
-                               PyUnicode_FromString(request->getValues[i]));
+          PythonString value(lock, request->getValues[i]);
+          PyDict_SetItemString(get.GetPyObject(), request->getKeys[i], value.Release());
         }
 
         PyDict_SetItemString(kw.GetPyObject(), "get", get.Release());
@@ -153,8 +160,8 @@
 
         for (uint32_t i = 0; i < request->headersCount; i++)
         {
-          PyDict_SetItemString(headers.GetPyObject(), request->headersKeys[i],
-                               PyUnicode_FromString(request->headersValues[i]));
+          PythonString value(lock, request->headersValues[i]);
+          PyDict_SetItemString(headers.GetPyObject(), request->headersKeys[i], value.Release());
         }
 
         PyDict_SetItemString(kw.GetPyObject(), "headers", headers.Release());