mirror of
https://github.com/python/cpython.git
synced 2025-09-26 10:19:53 +00:00
[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 ed6ea3ea79
)
This commit is contained in:
parent
6f90399c23
commit
8435fbfe4e
3 changed files with 67 additions and 16 deletions
|
@ -3449,8 +3449,8 @@ class ProtocolTests(BaseTestCase):
|
|||
|
||||
self.assertNotIn("__protocol_attrs__", vars(NonP))
|
||||
self.assertNotIn("__protocol_attrs__", vars(NonPR))
|
||||
self.assertNotIn("__callable_proto_members_only__", vars(NonP))
|
||||
self.assertNotIn("__callable_proto_members_only__", vars(NonPR))
|
||||
self.assertNotIn("__non_callable_proto_members__", vars(NonP))
|
||||
self.assertNotIn("__non_callable_proto_members__", vars(NonPR))
|
||||
|
||||
acceptable_extra_attrs = {
|
||||
'_is_protocol', '_is_runtime_protocol', '__parameters__',
|
||||
|
@ -3995,6 +3995,40 @@ class ProtocolTests(BaseTestCase):
|
|||
self.assertNotIsInstance(42, ProtocolWithMixedMembers)
|
||||
|
||||
|
||||
def test_nonruntime_protocol_interaction_with_evil_classproperty(self):
|
||||
class classproperty:
|
||||
def __get__(self, instance, type):
|
||||
raise RuntimeError("NO")
|
||||
|
||||
class Commentable(Protocol):
|
||||
evil = classproperty()
|
||||
|
||||
# recognised as a protocol attr,
|
||||
# but not actually accessed by the protocol metaclass
|
||||
# (which would raise RuntimeError) for non-runtime protocols.
|
||||
# See gh-113320
|
||||
self.assertEqual(Commentable.__protocol_attrs__, {"evil"})
|
||||
|
||||
def test_runtime_protocol_interaction_with_evil_classproperty(self):
|
||||
class CustomError(Exception): pass
|
||||
|
||||
class classproperty:
|
||||
def __get__(self, instance, type):
|
||||
raise CustomError
|
||||
|
||||
with self.assertRaises(TypeError) as cm:
|
||||
@runtime_checkable
|
||||
class Commentable(Protocol):
|
||||
evil = classproperty()
|
||||
|
||||
exc = cm.exception
|
||||
self.assertEqual(
|
||||
exc.args[0],
|
||||
"Failed to determine whether protocol member 'evil' is a method member"
|
||||
)
|
||||
self.assertIs(type(exc.__cause__), CustomError)
|
||||
|
||||
|
||||
class GenericTests(BaseTestCase):
|
||||
|
||||
def test_basics(self):
|
||||
|
|
|
@ -1683,7 +1683,7 @@ class _TypingEllipsis:
|
|||
_TYPING_INTERNALS = frozenset({
|
||||
'__parameters__', '__orig_bases__', '__orig_class__',
|
||||
'_is_protocol', '_is_runtime_protocol', '__protocol_attrs__',
|
||||
'__callable_proto_members_only__', '__type_params__',
|
||||
'__non_callable_proto_members__', '__type_params__',
|
||||
})
|
||||
|
||||
_SPECIAL_NAMES = frozenset({
|
||||
|
@ -1820,11 +1820,6 @@ class _ProtocolMeta(ABCMeta):
|
|||
super().__init__(*args, **kwargs)
|
||||
if getattr(cls, "_is_protocol", False):
|
||||
cls.__protocol_attrs__ = _get_protocol_attrs(cls)
|
||||
# PEP 544 prohibits using issubclass()
|
||||
# with protocols that have non-method members.
|
||||
cls.__callable_proto_members_only__ = all(
|
||||
callable(getattr(cls, attr, None)) for attr in cls.__protocol_attrs__
|
||||
)
|
||||
|
||||
def __subclasscheck__(cls, other):
|
||||
if cls is Protocol:
|
||||
|
@ -1836,18 +1831,19 @@ class _ProtocolMeta(ABCMeta):
|
|||
if not isinstance(other, type):
|
||||
# Same error message as for issubclass(1, int).
|
||||
raise TypeError('issubclass() arg 1 must be a class')
|
||||
if (
|
||||
not cls.__callable_proto_members_only__
|
||||
and cls.__dict__.get("__subclasshook__") is _proto_hook
|
||||
):
|
||||
raise TypeError(
|
||||
"Protocols with non-method members don't support issubclass()"
|
||||
)
|
||||
if not getattr(cls, '_is_runtime_protocol', False):
|
||||
raise TypeError(
|
||||
"Instance and class checks can only be used with "
|
||||
"@runtime_checkable protocols"
|
||||
)
|
||||
if (
|
||||
# this attribute is set by @runtime_checkable:
|
||||
cls.__non_callable_proto_members__
|
||||
and cls.__dict__.get("__subclasshook__") is _proto_hook
|
||||
):
|
||||
raise TypeError(
|
||||
"Protocols with non-method members don't support issubclass()"
|
||||
)
|
||||
return super().__subclasscheck__(other)
|
||||
|
||||
def __instancecheck__(cls, instance):
|
||||
|
@ -1875,7 +1871,8 @@ class _ProtocolMeta(ABCMeta):
|
|||
val = getattr_static(instance, attr)
|
||||
except AttributeError:
|
||||
break
|
||||
if val is None and callable(getattr(cls, attr, None)):
|
||||
# this attribute is set by @runtime_checkable:
|
||||
if val is None and attr not in cls.__non_callable_proto_members__:
|
||||
break
|
||||
else:
|
||||
return True
|
||||
|
@ -2113,6 +2110,22 @@ def runtime_checkable(cls):
|
|||
raise TypeError('@runtime_checkable can be only applied to protocol classes,'
|
||||
' got %r' % cls)
|
||||
cls._is_runtime_protocol = True
|
||||
# PEP 544 prohibits using issubclass()
|
||||
# with protocols that have non-method members.
|
||||
# See gh-113320 for why we compute this attribute here,
|
||||
# rather than in `_ProtocolMeta.__init__`
|
||||
cls.__non_callable_proto_members__ = set()
|
||||
for attr in cls.__protocol_attrs__:
|
||||
try:
|
||||
is_callable = callable(getattr(cls, attr, None))
|
||||
except Exception as e:
|
||||
raise TypeError(
|
||||
f"Failed to determine whether protocol member {attr!r} "
|
||||
"is a method member"
|
||||
) from e
|
||||
else:
|
||||
if not is_callable:
|
||||
cls.__non_callable_proto_members__.add(attr)
|
||||
return cls
|
||||
|
||||
|
||||
|
|
|
@ -0,0 +1,4 @@
|
|||
Fix regression in Python 3.12 where :class:`~typing.Protocol` classes that
|
||||
were not marked as :func:`runtime-checkable <typing.runtime_checkable>`
|
||||
would be unnecessarily introspected, potentially causing exceptions to be
|
||||
raised if the protocol had problematic members. Patch by Alex Waygood.
|
Loading…
Add table
Add a link
Reference in a new issue