https://github.com/python/cpython/commit/ed6ea3ea79fac68b127c7eb457c7ecb996461010
commit: ed6ea3ea79fac68b127c7eb457c7ecb996461010
branch: main
author: Alex Waygood <[email protected]>
committer: AlexWaygood <[email protected]>
date: 2024-01-05T01:01:48Z
summary:
gh-113320: Reduce the number of dangerous `getattr()` calls when constructing
protocol classes (#113401)
- 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!
files:
A Misc/NEWS.d/next/Library/2023-12-22-11-30-57.gh-issue-113320.Vp5suS.rst
M Lib/test/test_typing.py
M Lib/typing.py
diff --git a/Lib/test/test_typing.py b/Lib/test/test_typing.py
index 2d10c39840ddf3..8edab0cd6e34db 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"})
@@ -4105,6 +4105,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 +4121,39 @@ 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 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):
+ 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..d278b4effc7eba 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,23 @@ 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__
+ # this attribute is set by @runtime_checkable:
+ 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 +1885,8 @@ def __instancecheck__(cls, instance):
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
@@ -2114,6 +2108,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 <typing.runtime_checkable>`
+would be unnecessarily introspected, potentially causing exceptions to be
+raised if the protocol had problematic members. Patch by Alex Waygood.
_______________________________________________
Python-checkins mailing list -- [email protected]
To unsubscribe send an email to [email protected]
https://mail.python.org/mailman3/lists/python-checkins.python.org/
Member address: [email protected]