Skip to content

bpo-34441: Fix ABC.__subclasscheck__ crash on a class with invalid __subclasses__ #8835

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 9 commits into from
Aug 20, 2018
Merged

bpo-34441: Fix ABC.__subclasscheck__ crash on a class with invalid __subclasses__ #8835

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
377d01b
bpo-34441: Fix ABC.__subclasscheck__ crash on a class with non-callab…
Aug 20, 2018
fa3bdae
Add tests for various unusual __subclasses__
Aug 20, 2018
a3cd050
Fix NEWS entry and reformat it with blurb
Aug 20, 2018
432f6df
End sentences with a full stop
Aug 20, 2018
56b21e8
Test an invalid __subclasses__ signature with a valid return type
Aug 20, 2018
7df825e
Use subTest() to distinguish iterations
Aug 20, 2018
dcf2e04
Generalize NEWS entry
Aug 20, 2018
7d3c5ec
bs -> func
Aug 20, 2018
72cb031
Use nested with statements for readability
Aug 20, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions Lib/test/test_abc.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -410,6 +410,13 @@ class C:
with self.assertRaises(TypeError):
issubclass(C(), A)

# bpo-34441: Check that issubclass() doesn't crash on bogus classes
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would be nice to add also tests for following cases:

  • __subclasses__ is a callable with wrong signature.
  • __subclasses__ raises an exception explicitly.
  • __subclasses__ returns not a list.
  • __subclasses__ returns a list containing not a type.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, I've added those cases.

class S(metaclass=abc_ABCMeta):
__subclasses__ = None

with self.assertRaises(TypeError):
issubclass(int, S)

def test_all_new_methods_are_called(self):
class A(metaclass=abc_ABCMeta):
pass
Expand Down
2 changes: 2 additions & 0 deletions Misc/NEWS.d/next/Library/2018-08-20-16-48-32.bpo-34441._zx9lU.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
Fix crash when an ``ABC``-derived class with non-callable ``__subclasses__`` is passed as the
second argument to ``issubclass()``.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

``issubclass()`` -> :func:`issubclass`

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add "Patch by Your Name." and wrap lines at 80 chars.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed, thank you.

3 changes: 3 additions & 0 deletions Modules/_abc.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -665,6 +665,9 @@ _abc__abc_subclasscheck_impl(PyObject *module, PyObject *self,

/* 6. Check if it's a subclass of a subclass (recursive). */
subclasses = PyObject_CallMethod(self, "__subclasses__", NULL);
if (subclasses == NULL) {
goto end;
}
if (!PyList_Check(subclasses)) {
PyErr_SetString(PyExc_TypeError, "__subclasses__() must return a list");
goto end;
Expand Down