Skip to content
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.

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

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

Closed
orenmn mannequin opened this issue Aug 31, 2017 · 8 comments
Closed

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

orenmn mannequin opened this issue Aug 31, 2017 · 8 comments
Labels
3.7 (EOL) end of life extension-modules C modules in the Modules dir type-crash A hard crash of the interpreter, possibly with a core dump

Comments

@orenmn
Copy link
Mannequin

orenmn mannequin commented Aug 31, 2017

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
    Copy link
    Mannequin Author

    orenmn mannequin commented Aug 31, 2017

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

    @orenmn orenmn mannequin added 3.7 (EOL) end of life extension-modules C modules in the Modules dir type-crash A hard crash of the interpreter, possibly with a core dump labels Aug 31, 2017
    @brettcannon
    Copy link
    Member

    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
    Copy link
    Mannequin Author

    orenmn mannequin commented Aug 31, 2017

    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
    Copy link
    Member

    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
    Copy link
    Member

    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
    Copy link
    Member

    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
    Copy link
    Member

    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
    Copy link
    Member

    Thank you Oren!

    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    3.7 (EOL) end of life extension-modules C modules in the Modules dir type-crash A hard crash of the interpreter, possibly with a core dump
    Projects
    None yet
    Development

    No branches or pull requests

    3 participants