Merge pull request #1711 from wxWidgets/fix-issue1706

Add `IsNull()` checks for all property methods in GraphicsObject classes

etg/graphics.py

@@ -201,11 +201,6 @@ def run():
     c.find('GetCurrentPoint').findOverload('wxDouble *x, wxDouble *y').ignore()
     c.mustHaveApp()
 
-    c.find('GetNativePath').setCppCode("""\
-        if (self->IsNull())
-            return (void*)0;
-        return self->GetNativePath();
-        """)
 
     #---------------------------------------------
     c = module.find('wxGraphicsRenderer')
@@ -288,12 +283,6 @@ def run():
     c.find('TransformPoint.x').inOut = True
     c.find('TransformPoint.y').inOut = True
 
-    c.find('GetNativeMatrix').setCppCode("""\
-        if (self->IsNull())
-            return (void*)0;
-        return self->GetNativeMatrix();
-        """)
-
 
     #---------------------------------------------
     c = module.find('wxGraphicsGradientStops')
@@ -307,11 +296,6 @@ def run():
 
     #---------------------------------------------
     c = module.find('wxGraphicsBitmap')
-    c.find('GetNativeBitmap').setCppCode("""\
-        if (self->IsNull())
-            return (void*)0;
-        return self->GetNativeBitmap();
-        """)
 
 
     #---------------------------------------------
@@ -336,6 +320,35 @@ def run():
 
     #-----------------------------------------------------------------
     tools.doCommonTweaks(module)
+
+    # Add some code to check obj.IsNull() to all methods that are used as getters for a
+    # PropertyDef. This is needed because it seems that most methods in GraphicsOpbects
+    # assume that the dev has already checked that the object is valid and so don't check
+    # it themselves. But when turned into a Python property they will automatically be called
+    # when introspecting the property values in things like wxNullGraphicsFOO. This cas
+    # easily result in a fatal crash. The tweak below will raise a ValueError exception in
+    # these cases before it gets to the crashy parts.
+    checkIsNull = """\
+        if (sipCpp->IsNull()) {{
+            wxPyErr_SetString(PyExc_ValueError, "The {} is not valid (likely an uninitialized or null instance)");
+            return NULL;
+        }}
+        """
+    for module_item in module.items:
+        if isinstance(module_item, etgtools.ClassDef):
+            klass = module_item
+            if 'wxGraphicsObject' in [klass.name] + klass.bases:
+                for item in klass.items:
+                    if isinstance(item, etgtools.PropertyDef):
+                        method = klass.find(item.getter)
+                        method.preMethodCode = checkIsNull.format(klass.pyName)
+                        if item.setter:
+                            method = klass.find(item.setter)
+                            method.preMethodCode = checkIsNull.format(klass.pyName)
+
+
+
+    #-----------------------------------------------------------------
     tools.runGenerators(module)
 
 

etgtools/extractors.py

@@ -291,7 +291,7 @@ class FunctionDef(BaseDef, FixWxPrefix):
         self.transferThis = False     # ownership of 'this' pointer transfered to C++
         self.cppCode = None           # Use this code instead of the default wrapper
         self.noArgParser = False      # set the NoargParser annotation
-        self.mustHaveAppFlag = False
+        self.preMethodCode = None
 
         self.__dict__.update(kw)
         if element is not None:
@@ -556,7 +556,11 @@ class FunctionDef(BaseDef, FixWxPrefix):
 
 
     def mustHaveApp(self, value=True):
-        self.mustHaveAppFlag = value
+        if value:
+            self.preMethodCode = "if (!wxPyCheckForApp()) return NULL;\n"
+        else:
+            self.preMethodCode = None
+
 
 #---------------------------------------------------------------------------
 
@@ -679,7 +683,7 @@ class ClassDef(BaseDef):
         self.innerclasses = []
         self.isInner = False        # Is this a nested class?
         self.klass = None           # if so, then this is the outer class
-        self.mustHaveAppFlag = False
+        self.preMethodCode = None
 
         # Stuff that needs to be generated after the class instead of within
         # it. Some back-end generators need to put stuff inside the class, and
@@ -1133,7 +1137,10 @@ private:
         self.addItem(wig)
 
     def mustHaveApp(self, value=True):
-        self.mustHaveAppFlag = value
+        if value:
+            self.preMethodCode = "if (!wxPyCheckForApp()) return NULL;\n"
+        else:
+            self.preMethodCode = None
 
 
     def copyFromClass(self, klass, name):

etgtools/sip_generator.py

@@ -194,9 +194,9 @@ from .%s import *
                 # SIP appends them all together.
                 _needDocstring = False
 
-            if function.mustHaveAppFlag:
+            if function.preMethodCode:
                 stream.write('%PreMethodCode\n')
-                stream.write(nci("if (!wxPyCheckForApp()) return NULL;\n", 4))
+                stream.write(nci(function.preMethodCode, 4))
                 stream.write('%End\n')
 
             if function.cppCode:
@@ -426,11 +426,11 @@ from .%s import *
         if klass.ignored:
             return
 
-        # Propagate mustHaveApp setting to the ctors
-        if klass.mustHaveAppFlag:
+        # Propagate preMethodCode setting to the ctors
+        if klass.preMethodCode:
             for item in klass.allItems():
                 if isinstance(item, extractors.MethodDef) and item.isCtor:
-                    item.mustHaveApp(True)
+                    item.preMethodCode = klass.preMethodCode
 
         # write the class header
         if klass.templateParams:
@@ -680,9 +680,9 @@ from .%s import *
                 # SIP appends them all together.
                 _needDocstring = False
 
-            if method.mustHaveAppFlag:
+            if method.preMethodCode:
                 stream.write('%s%%PreMethodCode\n' % indent)
-                stream.write(nci("if (!wxPyCheckForApp()) return NULL;\n", len(indent)+4))
+                stream.write(nci(method.preMethodCode, len(indent)+4))
                 stream.write('%s%%End\n' % indent)
 
             if method.cppCode: