Skip to content

Commit f069ab4

Browse files
committed
Treat properties with private setters as read-only from Python
Python assignment to a CLR property resolved the setter with nonPublic: true, so a property declared { get; private set; } was silently writable from Python: the read-only guard in tp_descr_set never fired and reflection invoked the private setter, mutating state the class never exposed. Align property setter binding with the accessibility policy in ClassManager.ShouldBindMethod (public, protected, protected internal): CacheAccessors now discards a resolved setter that fails that check, so assignment raises TypeError 'property is read-only'. Protected setters remain writable from Python subclasses.
1 parent 998138b commit f069ab4

2 files changed

Lines changed: 77 additions & 0 deletions

File tree

src/embed_tests/TestPropertyAccess.cs

Lines changed: 70 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,12 +34,16 @@ public class Fixture
3434
public string PublicReadOnlyProperty { get; } = "Default value";
3535
protected string ProtectedReadOnlyProperty { get; } = "Default value";
3636

37+
public string PublicPropertyWithPrivateSetter { get; private set; } = "Default value";
38+
3739
public static string PublicStaticProperty { get; set; } = "Default value";
3840
protected static string ProtectedStaticProperty { get; set; } = "Default value";
3941

4042
public static string PublicStaticReadOnlyProperty { get; } = "Default value";
4143
protected static string ProtectedStaticReadOnlyProperty { get; } = "Default value";
4244

45+
public static string PublicStaticPropertyWithPrivateSetter { get; private set; } = "Default value";
46+
4347
public string PublicField = "Default value";
4448
protected string ProtectedField = "Default value";
4549

@@ -58,6 +62,11 @@ public static Fixture Create()
5862
{
5963
return new Fixture();
6064
}
65+
66+
public static void ResetPublicStaticPropertyWithPrivateSetter()
67+
{
68+
PublicStaticPropertyWithPrivateSetter = "Default value";
69+
}
6170
}
6271

6372
public class NonStaticConstHolder
@@ -321,6 +330,32 @@ def SetValue(self, fixture):
321330
}
322331
}
323332

333+
[Test]
334+
public void TestSetPublicPropertyWithPrivateSetterFails()
335+
{
336+
dynamic model = PyModule.FromString("module", @"
337+
from clr import AddReference
338+
AddReference(""System"")
339+
AddReference(""Python.EmbeddingTest"")
340+
341+
from Python.EmbeddingTest import *
342+
343+
class TestSetPublicPropertyWithPrivateSetterFails:
344+
def SetValue(self, fixture):
345+
fixture.PublicPropertyWithPrivateSetter = 'New value'
346+
").GetAttr("TestSetPublicPropertyWithPrivateSetterFails").Invoke();
347+
348+
var fixture = new Fixture();
349+
350+
using (Py.GIL())
351+
{
352+
var exception = Assert.Throws<PythonException>(() => model.SetValue(fixture));
353+
Assert.AreEqual("TypeError", exception.Type.Name);
354+
Assert.That(exception.Message, Does.Contain("read-only"));
355+
Assert.AreEqual("Default value", fixture.PublicPropertyWithPrivateSetter);
356+
}
357+
}
358+
324359
[Test]
325360
public void TestGetPublicReadOnlyPropertyFailsWhenAccessedOnClass()
326361
{
@@ -515,6 +550,41 @@ def SetValue(self):
515550
}
516551
}
517552

553+
[Test]
554+
public void TestSetPublicStaticPropertyWithPrivateSetterFails()
555+
{
556+
dynamic model = PyModule.FromString("module", @"
557+
from clr import AddReference
558+
AddReference(""System"")
559+
AddReference(""Python.EmbeddingTest"")
560+
561+
from Python.EmbeddingTest import *
562+
563+
class TestSetPublicStaticPropertyWithPrivateSetterFails:
564+
def SetValue(self):
565+
TestPropertyAccess.Fixture.PublicStaticPropertyWithPrivateSetter = 'New value'
566+
").GetAttr("TestSetPublicStaticPropertyWithPrivateSetterFails").Invoke();
567+
568+
using (Py.GIL())
569+
{
570+
string valueAfterSet;
571+
PythonException exception = null;
572+
try
573+
{
574+
exception = Assert.Throws<PythonException>(() => model.SetValue());
575+
}
576+
finally
577+
{
578+
valueAfterSet = Fixture.PublicStaticPropertyWithPrivateSetter;
579+
Fixture.ResetPublicStaticPropertyWithPrivateSetter();
580+
}
581+
582+
Assert.AreEqual("TypeError", exception.Type.Name);
583+
Assert.That(exception.Message, Does.Contain("read-only"));
584+
Assert.AreEqual("Default value", valueAfterSet);
585+
}
586+
}
587+
518588
[Test]
519589
public void TestGetProtectedStaticReadOnlyPropertyWorks()
520590
{

src/runtime/Types/PropertyObject.cs

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,13 @@ void CacheAccessors()
4040
PropertyInfo md = info.Value;
4141
getter = md.GetGetMethod(true) ?? md.GetBaseGetMethod(true);
4242
setter = md.GetSetMethod(true) ?? md.GetBaseSetMethod(true);
43+
if (setter != null && !ClassManager.ShouldBindMethod(setter))
44+
{
45+
// A private/internal setter is not part of the surface exposed to Python:
46+
// assignment must treat the property as read-only rather than silently
47+
// writing through the non-public setter via reflection.
48+
setter = null;
49+
}
4350
}
4451

4552

0 commit comments

Comments
 (0)