assertion failure in imp.create_dynamic(), when spec.name is not a string

[orenmn]

BPO	31315
Nosy	@brettcannon, @ncoghlan, @ericsnowcurrently, @serhiy-storchaka, @orenmn
PRs	bpo-31315: Fix an assertion failure in imp.create_dynamic(), when spec.name is not a string #3257 [3.6] bpo-31315: Fix an assertion failure in imp.create_dynamic(), when spec.name is not a string. (GH-3257) #3653

^{Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.}

Show more details

GitHub fields:

assignee = None
closed_at = <Date 2017-09-19.12:54:26.111>
created_at = <Date 2017-08-31.16:25:55.661>
labels = ['extension-modules', '3.7', 'type-crash']
title = 'assertion failure in imp.create_dynamic(), when spec.name is not a string'
updated_at = <Date 2017-09-19.12:54:46.470>
user = 'https://github.com/orenmn'

bugs.python.org fields:

activity = <Date 2017-09-19.12:54:46.470>
actor = 'serhiy.storchaka'
assignee = 'none'
closed = True
closed_date = <Date 2017-09-19.12:54:26.111>
closer = 'serhiy.storchaka'
components = ['Extension Modules']
creation = <Date 2017-08-31.16:25:55.661>
creator = 'Oren Milman'
dependencies = []
files = []
hgrepos = []
issue_num = 31315
keywords = ['patch']
message_count = 8.0
messages = ['301050', '301056', '301057', '301061', '301109', '302516', '302518', '302519']
nosy_count = 5.0
nosy_names = ['brett.cannon', 'ncoghlan', 'eric.snow', 'serhiy.storchaka', 'Oren Milman']
pr_nums = ['3257', '3653']
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'crash'
url = 'https://bugs.python.org/issue31315'
versions = ['Python 3.6', 'Python 3.7']

[orenmn]

The following code causes an assertion failure in get_encoded_name(), which is
called by _PyImport_LoadDynamicModuleWithSpec() (in Python/importdl.c):

import imp

class BadSpec:
    name = 42
    origin = 'foo'

imp.create_dynamic(BadSpec())

this is because _PyImport_LoadDynamicModuleWithSpec() assumes that spec.name is
a string.

should we fix this (even though imp is deprecated)?

[brettcannon]

I'm about to go on vacation so I might not be right of mind to comment, but I think throwing a TypeError is valid if it's triggering an assertion error that is already there.

P.S. Thanks for all the fuzz testing you're doing, Oren!

[orenmn]

do you mean that we should fix it to raise a TypeError?

the assertion is there, but not explicitly.
get_encoded_name() calls PyUnicode_FindChar(), which calls
PyUnicode_READY(), which does assert(_PyUnicode_CHECK).

so i get:
>>> import imp
>>>
>>> class BadSpec:
...     name = 42
...     origin = 'foo'
...
>>> imp.create_dynamic(BadSpec())
Assertion failed: PyUnicode_Check(op), file ..\Objects\unicodeobject.c, line 380

[brettcannon]

Yes, I'm saying that instead of hitting the C-level assertion error an explicit TypeError should be raised (or some other change like calling str() on the object). Either way, a C-level assertion from valid Python code is bad. :)

[ericsnowcurrently]

On Thu, Aug 31, 2017 at 1:32 PM, Brett Cannon <report@bugs.python.org> wrote:

I think throwing a TypeError is valid if it's triggering an assertion error that is already there.

+1

P.S. Thanks for all the fuzz testing you're doing, Oren!

Also a big +1. :)

[serhiy-storchaka]

New changeset 9974e1b by Serhiy Storchaka (Oren Milman) in branch 'master':
bpo-31315: Fix an assertion failure in imp.create_dynamic(), when spec.name is not a string. (bpo-3257)
9974e1b

[serhiy-storchaka]

New changeset 99a51d4 by Serhiy Storchaka (Miss Islington (bot)) in branch '3.6':
[3.6] bpo-31315: Fix an assertion failure in imp.create_dynamic(), when spec.name is not a string. (GH-3257) (bpo-3653)
99a51d4

[serhiy-storchaka]

Thank you Oren!