From 7b9f8a1bde65874bb18276775b2571e7f29b28d7 Mon Sep 17 00:00:00 2001 From: AlexWaygood Date: Fri, 22 Dec 2023 11:31:51 +0000 Subject: [PATCH 1/3] gh-113320: Reduce the number of dangerous `getattr()` calls when constructing protocol classes --- Lib/test/test_typing.py | 42 +++++++++++++++++- Lib/typing.py | 44 +++++++++++-------- ...-12-22-11-30-57.gh-issue-113320.Vp5suS.rst | 4 ++ 3 files changed, 70 insertions(+), 20 deletions(-) create mode 100644 Misc/NEWS.d/next/Library/2023-12-22-11-30-57.gh-issue-113320.Vp5suS.rst diff --git a/Lib/test/test_typing.py b/Lib/test/test_typing.py index 2d10c39840ddf3..b5989a74f404ed 100644 --- a/Lib/test/test_typing.py +++ b/Lib/test/test_typing.py @@ -3448,8 +3448,8 @@ def meth(self): pass 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)) self.assertEqual(get_protocol_members(P), {"x"}) self.assertEqual(get_protocol_members(PR), {"meth"}) @@ -3472,6 +3472,12 @@ def meth(self): pass vars(NonPR).keys(), vars(D).keys() | acceptable_extra_attrs ) + def test_nonruntime_protocols_not_unnecessarily_introspected(self) -> None: + class Foo(Protocol): + x = 1 + + self.assertFalse(hasattr(Foo, "__non_callable_proto_members__")) + def test_custom_subclasshook(self): class P(Protocol): x = 1 @@ -4105,6 +4111,7 @@ def method(self) -> None: ... self.assertNotIsInstance(42, ProtocolWithMixedMembers) def test_protocol_issubclass_error_message(self): + @runtime_checkable class Vec2D(Protocol): x: float y: float @@ -4120,6 +4127,37 @@ def square_norm(self) -> float: with self.assertRaisesRegex(TypeError, re.escape(expected_error_message)): issubclass(int, Vec2D) + 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 for non-runtime protocols + self.assertEqual(get_protocol_members(Commentable), {"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): diff --git a/Lib/typing.py b/Lib/typing.py index d7d793539b35b1..2ab900b409a366 100644 --- a/Lib/typing.py +++ b/Lib/typing.py @@ -1670,7 +1670,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({ @@ -1833,11 +1833,6 @@ def __init__(cls, *args, **kwargs): 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: @@ -1846,25 +1841,22 @@ def __subclasscheck__(cls, other): getattr(cls, '_is_protocol', False) and not _allow_reckless_class_checks() ): + if not getattr(cls, '_is_runtime_protocol', False): + _type_check_issubclass_arg_1(other) + raise TypeError( + "Instance and class checks can only be used with " + "@runtime_checkable protocols" + ) if ( - not cls.__callable_proto_members_only__ + cls.__non_callable_proto_members__ and cls.__dict__.get("__subclasshook__") is _proto_hook ): _type_check_issubclass_arg_1(other) - non_method_attrs = sorted( - attr for attr in cls.__protocol_attrs__ - if not callable(getattr(cls, attr, None)) - ) + non_method_attrs = sorted(cls.__non_callable_proto_members__) raise TypeError( "Protocols with non-method members don't support issubclass()." f" Non-method members: {str(non_method_attrs)[1:-1]}." ) - if not getattr(cls, '_is_runtime_protocol', False): - _type_check_issubclass_arg_1(other) - raise TypeError( - "Instance and class checks can only be used with " - "@runtime_checkable protocols" - ) return _abc_subclasscheck(cls, other) def __instancecheck__(cls, instance): @@ -1892,7 +1884,7 @@ def __instancecheck__(cls, instance): val = getattr_static(instance, attr) except AttributeError: break - if val is None and callable(getattr(cls, attr, None)): + if val is None and attr not in cls.__non_callable_proto_members__: break else: return True @@ -2114,6 +2106,22 @@ def close(self): ... 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 diff --git a/Misc/NEWS.d/next/Library/2023-12-22-11-30-57.gh-issue-113320.Vp5suS.rst b/Misc/NEWS.d/next/Library/2023-12-22-11-30-57.gh-issue-113320.Vp5suS.rst new file mode 100644 index 00000000000000..6cf74f335d4d7d --- /dev/null +++ b/Misc/NEWS.d/next/Library/2023-12-22-11-30-57.gh-issue-113320.Vp5suS.rst @@ -0,0 +1,4 @@ +Fix regression in Python 3.12 where :class:`~typing.Protocol` classes that +were not marked as :func:`runtime-checkable ` +would be unnecessarily introspected, potentially causing exceptions to be +raised if the protocol had problematic members. Patch by Alex Waygood. From 2d8d40ac6cc7daad5136cd15cb63f12346864f6b Mon Sep 17 00:00:00 2001 From: Alex Waygood Date: Sat, 23 Dec 2023 09:19:24 +0000 Subject: [PATCH 2/3] Better comment --- Lib/test/test_typing.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/Lib/test/test_typing.py b/Lib/test/test_typing.py index b5989a74f404ed..40b13d37a3adc7 100644 --- a/Lib/test/test_typing.py +++ b/Lib/test/test_typing.py @@ -4136,7 +4136,9 @@ class Commentable(Protocol): evil = classproperty() # recognised as a protocol attr, - # but not actually accessed for non-runtime protocols + # but not actually accessed by the protocol metaclass + # (which would raise RuntimeError) for non-runtime protocols. + # See gh-113320 self.assertEqual(get_protocol_members(Commentable), {"evil"}) def test_runtime_protocol_interaction_with_evil_classproperty(self): From e0e108b43f50e9428888a08f3fdb922110860288 Mon Sep 17 00:00:00 2001 From: AlexWaygood Date: Fri, 5 Jan 2024 00:43:27 +0000 Subject: [PATCH 3/3] Address review + add a couple of comments --- Lib/test/test_typing.py | 6 ------ Lib/typing.py | 2 ++ 2 files changed, 2 insertions(+), 6 deletions(-) diff --git a/Lib/test/test_typing.py b/Lib/test/test_typing.py index 40b13d37a3adc7..8edab0cd6e34db 100644 --- a/Lib/test/test_typing.py +++ b/Lib/test/test_typing.py @@ -3472,12 +3472,6 @@ def meth(self): pass vars(NonPR).keys(), vars(D).keys() | acceptable_extra_attrs ) - def test_nonruntime_protocols_not_unnecessarily_introspected(self) -> None: - class Foo(Protocol): - x = 1 - - self.assertFalse(hasattr(Foo, "__non_callable_proto_members__")) - def test_custom_subclasshook(self): class P(Protocol): x = 1 diff --git a/Lib/typing.py b/Lib/typing.py index 2ab900b409a366..d278b4effc7eba 100644 --- a/Lib/typing.py +++ b/Lib/typing.py @@ -1848,6 +1848,7 @@ def __subclasscheck__(cls, other): "@runtime_checkable protocols" ) if ( + # this attribute is set by @runtime_checkable: cls.__non_callable_proto_members__ and cls.__dict__.get("__subclasshook__") is _proto_hook ): @@ -1884,6 +1885,7 @@ def __instancecheck__(cls, instance): val = getattr_static(instance, attr) except AttributeError: break + # this attribute is set by @runtime_checkable: if val is None and attr not in cls.__non_callable_proto_members__: break else: