CompoundObjectBinding : Fix null dereference

Boost Python will happily accept `None` as a `const char *`, and that was getting passed to the `InternedString` constructor which requires a non-null pointer. By telling Boost Python we want an InternedString in the first place it goes via our registered converter which is smart enough to reject `None`.

Author: johnhaddon

Changes

@@ -6,6 +6,11 @@ Improvements
 
 - USDScene : Added basic loading of UsdGeomNurbsCurves, converting them to CurvesPrimitives.
 
+Fixes
+-----
+
+- CompoundObject : Fixed crashes in Python bindings caused by passing `None` as a key.
+
 10.5.4.2 (relative to 10.5.4.1)
 ========
 

src/IECorePython/CompoundObjectBinding.cpp

@@ -101,47 +101,36 @@ static unsigned int len( const CompoundObject &o )
 	return o.members().size();
 }
 
-static ObjectPtr getItem( const CompoundObject &o, const char *n )
+static ObjectPtr getItem( const CompoundObject &o, const InternedString &n )
 {
 	CompoundObject::ObjectMap::const_iterator it = o.members().find( n );
 	if( it==o.members().end() )
 	{
-		PyErr_SetString( PyExc_KeyError, n );
+		PyErr_SetString( PyExc_KeyError, n.c_str() );
 		throw_error_already_set();
 	}
 	return it->second;
 }
 
-static void setItem( CompoundObject &o, const char *n, Object &v )
+static void setItem( CompoundObject &o, const InternedString &n, Object &v )
 {
 	o.members()[n] = &v;
 }
 
-static void delItem( CompoundObject &o, const char *n )
+static void delItem( CompoundObject &o, const InternedString &n )
 {
 	CompoundObject::ObjectMap::iterator it = o.members().find( n );
 	if( it==o.members().end() )
 	{
-		PyErr_SetString( PyExc_KeyError, n );
+		PyErr_SetString( PyExc_KeyError, n.c_str() );
 		throw_error_already_set();
 	}
 	o.members().erase( it );
 }
 
-static bool contains( const CompoundObject &o, const char *n )
+static bool contains( const CompoundObject &o, const InternedString &n )
 {
-	CompoundObject::ObjectMap::const_iterator it = o.members().find( n );
-	if( it==o.members().end() )
-	{
-		return false;
-	}
-	return true;
-}
-
-static bool has_key( const CompoundObject &o, const char *n )
-{
-	CompoundObject::ObjectMap::const_iterator it = o.members().find( n );
-	return ( it != o.members().end() );
+	return o.members().find( n ) != o.members().end();
 }
 
 static boost::python::list keys( const CompoundObject &o )
@@ -226,7 +215,7 @@ class CompoundObjectFromPythonDict
 			{
 				key = keys[i];
 				value = v[key];
-				extract<const char *> keyElem(key);
+				extract<InternedString> keyElem(key);
 				if (!keyElem.check())
 				{
 					PyErr_SetString(PyExc_TypeError, "Incompatible key type. Only strings accepted.");
@@ -279,7 +268,7 @@ static CompoundObjectPtr copyConstructor( ConstCompoundObjectPtr other )
 }
 
 /// binding for get method
-static ObjectPtr get( const CompoundObject &o, const char *key, ObjectPtr defaultValue )
+static ObjectPtr get( const CompoundObject &o, const IECore::InternedString &key, ObjectPtr defaultValue )
 {
 	CompoundObject::ObjectMap::const_iterator it = o.members().find( key );
 	if ( it == o.members().end() )
@@ -307,7 +296,7 @@ void bindCompoundObject()
 		.def( "__setitem__", &setItem )
 		.def( "__delitem__", &delItem )
 		.def( "__contains__", &contains )
-		.def( "has_key", &has_key )
+		.def( "has_key", &contains )
 		.def( "keys", &keys )
 		.def( "values", &values )
 		.def( "items", &items )

test/IECore/CompoundObject.py

@@ -223,6 +223,24 @@ def testHash( self ) :
 				self.assertEqual( h, o.hash() )
 			h = o.hash()
 
+	def testNoneKey( self ) :
+
+		o = IECore.CompoundObject()
+		with self.assertRaisesRegex( Exception, r"Python argument types" ) :
+			o.get( None )
+		with self.assertRaisesRegex( Exception, r"Python argument types" ) :
+			o[None]
+		with self.assertRaisesRegex( Exception, r"Python argument types" ) :
+			o[None] = IECore.IntData( 10 )
+		with self.assertRaisesRegex( Exception, r"Python argument types" ) :
+			del o[None]
+		with self.assertRaisesRegex( Exception, r"Python argument types" ) :
+			None in o
+		with self.assertRaisesRegex( Exception, r"Python argument types" ) :
+			o.has_key( None )
+		with self.assertRaisesRegex( Exception, r"Incompatible key type" ) :
+			IECore.CompoundObject( { None : IECore.IntData( 10 ) } )
+
 if __name__ == "__main__":
         unittest.main()