[issue18287] PyType_Ready() should sanity-check the tp_name field

[Donald Stufft]

Changes by Donald Stufft :


--
pull_requests: +1009

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue18287] PyType_Ready() should sanity-check the tp_name field

[Roundup Robot]

Roundup Robot added the comment:

New changeset 5c459b0f2b75 by Serhiy Storchaka in branch '3.5':
Issue #18287: PyType_Ready() now checks that tp_name is not NULL.
https://hg.python.org/cpython/rev/5c459b0f2b75

New changeset ba76dd106e66 by Serhiy Storchaka in branch '2.7':
Issue #18287: PyType_Ready() now checks that tp_name is not NULL.
https://hg.python.org/cpython/rev/ba76dd106e66

New changeset 0b726193ec3c by Serhiy Storchaka in branch '3.6':
Issue #18287: PyType_Ready() now checks that tp_name is not NULL.
https://hg.python.org/cpython/rev/0b726193ec3c

New changeset a60d0e80cc1d by Serhiy Storchaka in branch 'default':
Issue #18287: PyType_Ready() now checks that tp_name is not NULL.
https://hg.python.org/cpython/rev/a60d0e80cc1d

--
nosy: +python-dev

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue18287] PyType_Ready() should sanity-check the tp_name field

[Serhiy Storchaka]

Serhiy Storchaka added the comment:

Thank you Niklas for your report and patch.

--
resolution:  -> fixed
stage: patch review -> resolved
status: open -> closed

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue18287] PyType_Ready() should sanity-check the tp_name field

[Serhiy Storchaka]

Serhiy Storchaka added the comment:

The patch LGTM, but I think SystemError is more appropriate.

--
assignee:  -> serhiy.storchaka

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue18287] PyType_Ready() should sanity-check the tp_name field

[Serhiy Storchaka]

Changes by Serhiy Storchaka :


--
nosy: +serhiy.storchaka
versions: +Python 3.7 -Python 3.4

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue18287] PyType_Ready() should sanity-check the tp_name field

[Rose Ames]

Rose Ames added the comment:

There's still no check on tp_name.  The patch looks reasonable, applies 
cleanly, compiles, and doesn't break any tests - suggest it be merged.

--
nosy: +superluser

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue18287] PyType_Ready() should sanity-check the tp_name field

[Niklas Koep]

Changes by Niklas Koep niklas.k...@gmail.com:


Removed file: http://bugs.python.org/file30678/patch

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue18287
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue18287] PyType_Ready() should sanity-check the tp_name field

[Antoine Pitrou]

Changes by Antoine Pitrou pit...@free.fr:


--
nosy: +amaury.forgeotdarc, benjamin.peterson
stage:  - patch review
versions: +Python 2.7, Python 3.3, Python 3.4 -Python 3.5

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue18287
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue18287] PyType_Ready() should sanity-check the tp_name field

[Amaury Forgeot d'Arc]

Amaury Forgeot d'Arc added the comment:

The patch is not complete: PyType_Ready() returns -1 but no no exception is set.

--

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue18287
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue18287] PyType_Ready() should sanity-check the tp_name field

[Niklas Koep]

Niklas Koep added the comment:

Oh, you're right, of course. I completely forgot that any other case which 
jumps to the error label assumes an appropriate exception has already been set. 
I attached a new patch which raises a TypeError. Is there a better way to 
identify the type which is affected by this exception? Since we're complaining 
about the missing tp_name field we obviously can't supply the name in the 
exception.

--
Added file: http://bugs.python.org/file30724/patch.default

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue18287
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue18287] PyType_Ready() should sanity-check the tp_name field

[Amaury Forgeot d'Arc]

Amaury Forgeot d'Arc added the comment:

Since this error can occur only during the development of a C extension, I 
would not worry too much. The traceback will already indicate the imported 
module, this is much better than a segfault later in pydoc.

--

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue18287
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue18287] PyType_Ready() should sanity-check the tp_name field

[Niklas Koep]

New submission from Niklas Koep:

I noticed that defining a new type where the tp_name field is NULL causes 
segfaults, for instance, when calling pydoc on the extension module. This 
particular segfault traces back to type_module() in Objects/typeobject.c where 
tp_name is passed to strrchr(). Raising an appropriate exception from 
PyType_Ready() seems sensible to avoid this kind of issue. The field is also 
used in two calls to PyErr_Format() in PyType_Ready() itself where it'll cause 
segfaults if not handled properly.

While we're on the subject, pydoc doesn't list the type documentation  if the 
tp_name string does not have a dot in it. I didn't research this any further as 
omitting dots seems to be valid according to the docs. However, at the very 
least it seems like this side effect should be mentioned in the documentation 
to avoid confusion when someone omits/forgets the 
package.subpackage.module.type hierarchy in the field definition.

I attached a tiny patch which just jumps to PyType_Ready()'s error label if the 
field is NULL. I also added a comment about pydoc in the two places of the 
documentation I could think of where tp_name is mentioned.

--
components: Interpreter Core
files: patch
messages: 191705
nosy: nkoep
priority: normal
severity: normal
status: open
title: PyType_Ready() should sanity-check the tp_name field
type: crash
versions: Python 3.5
Added file: http://bugs.python.org/file30678/patch

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue18287
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com