Merge pull request #74 from jhonabreul/bug-python-object-reference-security-dynamic-properties

Keep dynamic class properties as Python objects

Author: jhonabreul

src/embed_tests/TestPropertyAccess.cs

@@ -1096,6 +1096,67 @@ def GetValue(self, fixture):
             }
         }
 
+        public class CSharpTestClass
+        {
+            public string CSharpProperty { get; set; }
+        }
+
+        [Test]
+        public void TestKeepsPythonReferenceForDynamicPropertiesFromPythonClassDerivedFromCSharpClass()
+        {
+            var expectedCSharpPropertyValue = "C# property";
+            var expectedPythonPropertyValue = "Python property";
+
+            var testModule = PyModule.FromString("module", $@"
+from clr import AddReference
+AddReference(""Python.EmbeddingTest"")
+AddReference(""System"")
+
+from Python.EmbeddingTest import TestPropertyAccess
+
+class PythonTestClass(TestPropertyAccess.CSharpTestClass):
+    def __init__(self):
+        super().__init__()
+
+def SetPythonObjectToFixture(fixture: TestPropertyAccess.DynamicFixture) -> None:
+    obj = PythonTestClass()
+    obj.CSharpProperty = '{expectedCSharpPropertyValue}'
+    obj.PythonProperty = '{expectedPythonPropertyValue}'
+    fixture.PythonClassObject = obj
+
+def AssertPythonClassObjectType(fixture: TestPropertyAccess.DynamicFixture) -> None:
+    if type(fixture.PythonClassObject) != PythonTestClass:
+        raise Exception('PythonClassObject is not of type PythonTestClass')
+
+def AccessCSharpProperty(fixture: TestPropertyAccess.DynamicFixture) -> str:
+    return fixture.PythonClassObject.CSharpProperty
+
+def AccessPythonProperty(fixture: TestPropertyAccess.DynamicFixture) -> str:
+    return fixture.PythonClassObject.PythonProperty
+");
+
+            dynamic fixture = new DynamicFixture();
+
+            using (Py.GIL())
+            {
+                dynamic SetPythonObjectToFixture = testModule.GetAttr("SetPythonObjectToFixture");
+                SetPythonObjectToFixture(fixture);
+
+                dynamic AssertPythonClassObjectType = testModule.GetAttr("AssertPythonClassObjectType");
+                Assert.DoesNotThrow(() => AssertPythonClassObjectType(fixture));
+
+                // Access the C# class property
+                dynamic AccessCSharpProperty = testModule.GetAttr("AccessCSharpProperty");
+                Assert.AreEqual(expectedCSharpPropertyValue, AccessCSharpProperty(fixture).As<string>());
+                Assert.AreEqual(expectedCSharpPropertyValue, fixture.PythonClassObject.CSharpProperty.As<string>());
+
+                // Access the Python class property
+                dynamic AccessPythonProperty = testModule.GetAttr("AccessPythonProperty");
+                Assert.AreEqual(expectedPythonPropertyValue, AccessPythonProperty(fixture).As<string>());
+                Assert.AreEqual(expectedPythonPropertyValue, fixture.PythonClassObject.PythonProperty.As<string>());
+            }
+        }
+
         private static TestCaseData[] DynamicPropertiesSetterTestCases() => new[]
         {
             new TestCaseData("True", null),
@@ -1136,10 +1197,15 @@ def GetPythonValue(self):
             using (Py.GIL())
             {
                 model.SetValue(fixture);
+
                 var expectedAsPyObject = model.GetPythonValue() as PyObject;
-                var expected = expectedType != null ? expectedAsPyObject.AsManagedObject(expectedType) : expectedAsPyObject;
+                Assert.AreEqual(expectedAsPyObject, fixture.DynamicProperty);
+
+                if (expectedType != null)
+                {
+                    Assert.AreEqual(expectedAsPyObject.AsManagedObject(expectedType), fixture.DynamicProperty.AsManagedObject(expectedType));
+                }
 
-                Assert.AreEqual(expected, fixture.DynamicProperty);
             }
         }
 

src/perf_tests/Python.PerformanceTests.csproj

@@ -13,7 +13,7 @@
       <IncludeAssets>runtime; build; native; contentfiles; analyzers; buildtransitive</IncludeAssets>
     </PackageReference>
     <PackageReference Include="Microsoft.NET.Test.Sdk" Version="16.*" />
-    <PackageReference Include="quantconnect.pythonnet" Version="2.0.20" GeneratePathProperty="true">
+    <PackageReference Include="quantconnect.pythonnet" Version="2.0.21" GeneratePathProperty="true">
       <IncludeAssets>compile</IncludeAssets>
     </PackageReference>
   </ItemGroup>
@@ -25,7 +25,7 @@
   </Target>
 
   <Target Name="CopyBaseline" AfterTargets="Build">
-    <Copy SourceFiles="$(NuGetPackageRoot)quantconnect.pythonnet\2.0.20\lib\net5.0\Python.Runtime.dll" DestinationFolder="$(OutDir)baseline" />
+    <Copy SourceFiles="$(NuGetPackageRoot)quantconnect.pythonnet\2.0.21\lib\net5.0\Python.Runtime.dll" DestinationFolder="$(OutDir)baseline" />
   </Target>
 
   <Target Name="CopyNewBuild" AfterTargets="Build">

src/runtime/Properties/AssemblyInfo.cs

@@ -4,5 +4,5 @@
 [assembly: InternalsVisibleTo("Python.EmbeddingTest, PublicKey=00240000048000009400000006020000002400005253413100040000110000005ffd8f49fb44ab0641b3fd8d55e749f716e6dd901032295db641eb98ee46063cbe0d4a1d121ef0bc2af95f8a7438d7a80a3531316e6b75c2dae92fb05a99f03bf7e0c03980e1c3cfb74ba690aca2f3339ef329313bcc5dccced125a4ffdc4531dcef914602cd5878dc5fbb4d4c73ddfbc133f840231343e013762884d6143189")]
 [assembly: InternalsVisibleTo("Python.Test, PublicKey=00240000048000009400000006020000002400005253413100040000110000005ffd8f49fb44ab0641b3fd8d55e749f716e6dd901032295db641eb98ee46063cbe0d4a1d121ef0bc2af95f8a7438d7a80a3531316e6b75c2dae92fb05a99f03bf7e0c03980e1c3cfb74ba690aca2f3339ef329313bcc5dccced125a4ffdc4531dcef914602cd5878dc5fbb4d4c73ddfbc133f840231343e013762884d6143189")]
 
-[assembly: AssemblyVersion("2.0.20")]
-[assembly: AssemblyFileVersion("2.0.20")]
+[assembly: AssemblyVersion("2.0.21")]
+[assembly: AssemblyFileVersion("2.0.21")]

src/runtime/Python.Runtime.csproj

@@ -5,7 +5,7 @@
     <RootNamespace>Python.Runtime</RootNamespace>
     <AssemblyName>Python.Runtime</AssemblyName>
     <PackageId>QuantConnect.pythonnet</PackageId>
-    <Version>2.0.20</Version>
+    <Version>2.0.21</Version>
     <GenerateAssemblyInfo>false</GenerateAssemblyInfo>
     <PackageLicenseFile>LICENSE</PackageLicenseFile>
     <RepositoryUrl>https://github.com/pythonnet/pythonnet</RepositoryUrl>

src/runtime/Types/DynamicClassObject.cs

@@ -112,13 +112,10 @@ public static int tp_setattro(BorrowedReference ob, BorrowedReference key, Borro
                 return Runtime.PyObject_GenericSetAttr(ob, key, val);
             }
 
-            // If the value is a managed object, we get it from the reference. If it is a Python object, we assign it as is.
-            var value = ((CLRObject)GetManagedObject(val))?.inst ?? PyObject.FromNullableReference(val);
-
             var callsite = SetAttrCallSite(name, clrObjectType);
             try
             {
-                callsite.Target(callsite, clrObj.inst, value);
+                callsite.Target(callsite, clrObj.inst, PyObject.FromNullableReference(val));
             }
             // Catch C# exceptions and raise them as Python exceptions.
             catch (Exception exception)