Skip to content

Commit 8435fbf

Browse files
authored
[3.12] gh-113320: Reduce the number of dangerous getattr() calls when constructing protocol classes (#113401) (#113722)
- Only attempt to figure out whether protocol members are "method members" or not if the class is marked as a runtime protocol. This information is irrelevant for non-runtime protocols; we can safely skip the risky introspection for them. - Only do the risky getattr() calls in one place (the runtime_checkable class decorator), rather than in three places (_ProtocolMeta.__init__, _ProtocolMeta.__instancecheck__ and _ProtocolMeta.__subclasscheck__). This reduces the number of locations in typing.py where the risky introspection could go wrong. - For runtime protocols, if determining whether a protocol member is callable or not fails, give a better error message. I think it's reasonable for us to reject runtime protocols that have members which raise strange exceptions when you try to access them. PEP-544 clearly states that all protocol member must be callable for issubclass() calls against the protocol to be valid -- and if a member raises when we try to access it, there's no way for us to figure out whether it's a callable member or not! (cherry-picked from commit ed6ea3e)
1 parent 6f90399 commit 8435fbf

File tree

3 files changed

+66
-15
lines changed

3 files changed

+66
-15
lines changed

Diff for: Lib/test/test_typing.py

+36-2
Original file line numberDiff line numberDiff line change
@@ -3449,8 +3449,8 @@ def meth(self): pass
34493449

34503450
self.assertNotIn("__protocol_attrs__", vars(NonP))
34513451
self.assertNotIn("__protocol_attrs__", vars(NonPR))
3452-
self.assertNotIn("__callable_proto_members_only__", vars(NonP))
3453-
self.assertNotIn("__callable_proto_members_only__", vars(NonPR))
3452+
self.assertNotIn("__non_callable_proto_members__", vars(NonP))
3453+
self.assertNotIn("__non_callable_proto_members__", vars(NonPR))
34543454

34553455
acceptable_extra_attrs = {
34563456
'_is_protocol', '_is_runtime_protocol', '__parameters__',
@@ -3995,6 +3995,40 @@ def method(self) -> None: ...
39953995
self.assertNotIsInstance(42, ProtocolWithMixedMembers)
39963996

39973997

3998+
def test_nonruntime_protocol_interaction_with_evil_classproperty(self):
3999+
class classproperty:
4000+
def __get__(self, instance, type):
4001+
raise RuntimeError("NO")
4002+
4003+
class Commentable(Protocol):
4004+
evil = classproperty()
4005+
4006+
# recognised as a protocol attr,
4007+
# but not actually accessed by the protocol metaclass
4008+
# (which would raise RuntimeError) for non-runtime protocols.
4009+
# See gh-113320
4010+
self.assertEqual(Commentable.__protocol_attrs__, {"evil"})
4011+
4012+
def test_runtime_protocol_interaction_with_evil_classproperty(self):
4013+
class CustomError(Exception): pass
4014+
4015+
class classproperty:
4016+
def __get__(self, instance, type):
4017+
raise CustomError
4018+
4019+
with self.assertRaises(TypeError) as cm:
4020+
@runtime_checkable
4021+
class Commentable(Protocol):
4022+
evil = classproperty()
4023+
4024+
exc = cm.exception
4025+
self.assertEqual(
4026+
exc.args[0],
4027+
"Failed to determine whether protocol member 'evil' is a method member"
4028+
)
4029+
self.assertIs(type(exc.__cause__), CustomError)
4030+
4031+
39984032
class GenericTests(BaseTestCase):
39994033

40004034
def test_basics(self):

Diff for: Lib/typing.py

+26-13
Original file line numberDiff line numberDiff line change
@@ -1683,7 +1683,7 @@ class _TypingEllipsis:
16831683
_TYPING_INTERNALS = frozenset({
16841684
'__parameters__', '__orig_bases__', '__orig_class__',
16851685
'_is_protocol', '_is_runtime_protocol', '__protocol_attrs__',
1686-
'__callable_proto_members_only__', '__type_params__',
1686+
'__non_callable_proto_members__', '__type_params__',
16871687
})
16881688

16891689
_SPECIAL_NAMES = frozenset({
@@ -1820,11 +1820,6 @@ def __init__(cls, *args, **kwargs):
18201820
super().__init__(*args, **kwargs)
18211821
if getattr(cls, "_is_protocol", False):
18221822
cls.__protocol_attrs__ = _get_protocol_attrs(cls)
1823-
# PEP 544 prohibits using issubclass()
1824-
# with protocols that have non-method members.
1825-
cls.__callable_proto_members_only__ = all(
1826-
callable(getattr(cls, attr, None)) for attr in cls.__protocol_attrs__
1827-
)
18281823

18291824
def __subclasscheck__(cls, other):
18301825
if cls is Protocol:
@@ -1836,18 +1831,19 @@ def __subclasscheck__(cls, other):
18361831
if not isinstance(other, type):
18371832
# Same error message as for issubclass(1, int).
18381833
raise TypeError('issubclass() arg 1 must be a class')
1834+
if not getattr(cls, '_is_runtime_protocol', False):
1835+
raise TypeError(
1836+
"Instance and class checks can only be used with "
1837+
"@runtime_checkable protocols"
1838+
)
18391839
if (
1840-
not cls.__callable_proto_members_only__
1840+
# this attribute is set by @runtime_checkable:
1841+
cls.__non_callable_proto_members__
18411842
and cls.__dict__.get("__subclasshook__") is _proto_hook
18421843
):
18431844
raise TypeError(
18441845
"Protocols with non-method members don't support issubclass()"
18451846
)
1846-
if not getattr(cls, '_is_runtime_protocol', False):
1847-
raise TypeError(
1848-
"Instance and class checks can only be used with "
1849-
"@runtime_checkable protocols"
1850-
)
18511847
return super().__subclasscheck__(other)
18521848

18531849
def __instancecheck__(cls, instance):
@@ -1875,7 +1871,8 @@ def __instancecheck__(cls, instance):
18751871
val = getattr_static(instance, attr)
18761872
except AttributeError:
18771873
break
1878-
if val is None and callable(getattr(cls, attr, None)):
1874+
# this attribute is set by @runtime_checkable:
1875+
if val is None and attr not in cls.__non_callable_proto_members__:
18791876
break
18801877
else:
18811878
return True
@@ -2113,6 +2110,22 @@ def close(self): ...
21132110
raise TypeError('@runtime_checkable can be only applied to protocol classes,'
21142111
' got %r' % cls)
21152112
cls._is_runtime_protocol = True
2113+
# PEP 544 prohibits using issubclass()
2114+
# with protocols that have non-method members.
2115+
# See gh-113320 for why we compute this attribute here,
2116+
# rather than in `_ProtocolMeta.__init__`
2117+
cls.__non_callable_proto_members__ = set()
2118+
for attr in cls.__protocol_attrs__:
2119+
try:
2120+
is_callable = callable(getattr(cls, attr, None))
2121+
except Exception as e:
2122+
raise TypeError(
2123+
f"Failed to determine whether protocol member {attr!r} "
2124+
"is a method member"
2125+
) from e
2126+
else:
2127+
if not is_callable:
2128+
cls.__non_callable_proto_members__.add(attr)
21162129
return cls
21172130

21182131

Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
Fix regression in Python 3.12 where :class:`~typing.Protocol` classes that
2+
were not marked as :func:`runtime-checkable <typing.runtime_checkable>`
3+
would be unnecessarily introspected, potentially causing exceptions to be
4+
raised if the protocol had problematic members. Patch by Alex Waygood.

0 commit comments

Comments
 (0)