Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
22 changes: 22 additions & 0 deletions Lib/test/test_types.py
Original file line number Diff line number Diff line change
Expand Up @@ -864,6 +864,28 @@ def __prepare__(*args):
self.assertIs(ns, expected_ns)
self.assertEqual(len(kwds), 0)

def test_bad___prepare__(self):
# __prepare__() must return a mapping.
class BadMeta(type):
@classmethod
def __prepare__(*args):
return None
with self.assertRaisesRegex(TypeError,
r'^BadMeta\.__prepare__\(\) must '
r'return a mapping, not NoneType$'):
class Foo(metaclass=BadMeta):
pass
# Also test the case in which the metaclass is not a type.
class BadMeta:
@classmethod
def __prepare__(*args):
return None
with self.assertRaisesRegex(TypeError,
r'^<metaclass>\.__prepare__\(\) must '
r'return a mapping, not NoneType$'):
class Bar(metaclass=BadMeta()):
pass

def test_metaclass_derivation(self):
# issue1294232: correct metaclass calculation
new_calls = [] # to check the order of __new__ calls
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
Raise a `TypeError` with a helpful error message when class creation fails
due to a metaclass with a bad ``__prepare__()`` method. Patch by Oren Milman.
7 changes: 7 additions & 0 deletions Python/bltinmodule.c
Original file line number Diff line number Diff line change
Expand Up @@ -157,6 +157,13 @@ builtin___build_class__(PyObject *self, PyObject **args, Py_ssize_t nargs,
Py_DECREF(bases);
return NULL;
}
if (!PyMapping_Check(ns)) {
PyErr_Format(PyExc_TypeError,
"%.200s.__prepare__() must return a mapping, not %.200s",
isclass ? ((PyTypeObject *)meta)->tp_name : "<metaclass>",

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Why this code is as it is?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I thought that printing the name of the problematic metaclass would be helpful to the user.
Now ISTM that I should have checked the __name__ attribute.
Do you think I should add that? Or should we just not include the name of the metaclass in the error message?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Why not use ((PyTypeObject *)meta)->tp_name in all cases?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

IIUC, if meta is not a type object, then ((PyTypeObject *)meta)->tp_name might contain arbitrary data, according to the type of meta, which might cause a crash (as indeed happens when running my second test).

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

In what circumstances meta is not a type object?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This code is an example:

class BadMetaclass:
    def __prepare__(*args):
        pass

class Bar(metaclass=BadMetaclass()):
    pass

I don't know about usecases for that (though I am certainly not a metaclasses expert), but IIUC, the C code explicitly handles such cases.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Maybe use meta->ob_type->tp_name in this case?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

When metaclass isn't a type, it can be an arbitrary callable, so I think it's reasonable to just hardcode in <metaclass> for that case.

Py_TYPE(ns)->tp_name);
goto error;
}
cell = PyEval_EvalCodeEx(PyFunction_GET_CODE(func), PyFunction_GET_GLOBALS(func), ns,
NULL, 0, NULL, 0, NULL, 0, NULL,
PyFunction_GET_CLOSURE(func));
Expand Down