pythongh-113320: Reduce the number of dangerous getattr() calls when constructing protocol classes (python#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!

Author: AlexWaygood

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):
 

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
 
 

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.