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

namedtuple's __new__.__globals__['__builtins__'] is None #87268

Closed
douglas-raillard-arm mannequin opened this issue Feb 2, 2021 · 9 comments
Closed

namedtuple's __new__.__globals__['__builtins__'] is None #87268

douglas-raillard-arm mannequin opened this issue Feb 2, 2021 · 9 comments
Assignees
Labels
3.9 only security fixes 3.10 only security fixes stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error

Comments

@douglas-raillard-arm
Copy link
Mannequin

douglas-raillard-arm mannequin commented Feb 2, 2021

BPO 43102
Nosy @rhettinger, @miss-islington, @douglas-raillard-arm
PRs
  • bpo-43102: Set namedtuple __new__'s internal builtins to a dict. #24439
  • [3.9] bpo-43102: Set namedtuple __new__'s internal builtins to a dict. (GH-24439) #24452
  • 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 = 'https://github.com/rhettinger'
    closed_at = <Date 2021-02-05.00:13:47.270>
    created_at = <Date 2021-02-02.14:46:44.895>
    labels = ['type-bug', 'library', '3.9', '3.10']
    title = "namedtuple's __new__.__globals__['__builtins__'] is None"
    updated_at = <Date 2021-02-05.10:03:20.084>
    user = 'https://github.com/douglas-raillard-arm'

    bugs.python.org fields:

    activity = <Date 2021-02-05.10:03:20.084>
    actor = 'douglas-raillard-arm'
    assignee = 'rhettinger'
    closed = True
    closed_date = <Date 2021-02-05.00:13:47.270>
    closer = 'rhettinger'
    components = ['Library (Lib)']
    creation = <Date 2021-02-02.14:46:44.895>
    creator = 'douglas-raillard-arm'
    dependencies = []
    files = []
    hgrepos = []
    issue_num = 43102
    keywords = ['patch']
    message_count = 9.0
    messages = ['386145', '386147', '386163', '386185', '386211', '386504', '386505', '386506', '386519']
    nosy_count = 3.0
    nosy_names = ['rhettinger', 'miss-islington', 'douglas-raillard-arm']
    pr_nums = ['24439', '24452']
    priority = 'normal'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'behavior'
    url = 'https://bugs.python.org/issue43102'
    versions = ['Python 3.9', 'Python 3.10']

    @douglas-raillard-arm
    Copy link
    Mannequin Author

    douglas-raillard-arm mannequin commented Feb 2, 2021

    When creating a namedtuple such as this one:

        from collections import namedtuple
    
        class C(namedtuple('C', ('hello', 'world'))):
            pass
    
        print(C.__new__.__globals__)

    The globals' dict of __new__ contains a "__builtins__" key which is set to None in collections/init.py:

        namespace = {
            '_tuple_new': tuple_new,
            '__builtins__': None,
            '__name__': f'namedtuple_{typename}',
        }

    When such globals are used with eval(), it will raise a TypeError such as:

        >>> eval('X', {'__builtins__': None})
        Traceback (most recent call last):
          File "<stdin>", line 1, in <module>
          File "<string>", line 1, in <module>
        TypeError: 'NoneType' object is not subscriptable

    If an empty dict was used instead, we get the expected exception:

        >>> eval('X', {'__builtins__': {}})
        Traceback (most recent call last):
          File "<stdin>", line 1, in <module>
          File "<string>", line 1, in <module>
        NameError: name 'X' is not defined

    Given that both ways allow preventing references to any builtin, please consider switching to an empty dict. Also, even though this is documented as implementation detail, this would agree more with the current documentation stating:

    The value of __builtins__ is normally either this module or the value of this module’s __dict__ attribute
    

    https://docs.python.org/3/library/builtins.html

    @douglas-raillard-arm douglas-raillard-arm mannequin added 3.9 only security fixes stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error labels Feb 2, 2021
    @rhettinger
    Copy link
    Contributor

    Thanks for the report. I'll look at it in detail this evening. Off-hand the suggest sounds reasonable.

    @rhettinger rhettinger added 3.10 only security fixes and removed 3.9 only security fixes labels Feb 2, 2021
    @rhettinger rhettinger self-assigned this Feb 2, 2021
    @rhettinger rhettinger added 3.10 only security fixes and removed 3.9 only security fixes labels Feb 2, 2021
    @rhettinger rhettinger self-assigned this Feb 2, 2021
    @rhettinger
    Copy link
    Contributor

    Out of curiosity, how did you find this? Did it arise in the course of trying to solve a real problem or were you just reading the code and and thought you might do it differently?

    Unless there's a real problem, I prefer None because it is immutable (safer), because it doesn't incur the time and memory cost of creating a new object, and because it does a good job of indicating that builtins are shut-off and not intended to be accessed. The primary reason the line is there at all is to make eval() as safe as possible. There is no public named tuple functionality that depends on it, so no user should ever encounter it unless they are just investigating implementation details. In particular, they should never see the error message in your post.

    @douglas-raillard-arm
    Copy link
    Mannequin Author

    douglas-raillard-arm mannequin commented Feb 3, 2021

    I did hit the issue while porting a tool to Python 3.9:
    ARM-software/lisa@a4cd3aa

    It basically infers valid Python expressions from type annotations (some sort
    of inverse type checker) and run them while serializing all intermediate
    subexpressions for debugging. This allows a whole test suite to be plain
    classes and functions (usable out of context) to be assembled into "pipelines"
    without extra user work. The reason I ran into that problem are:

    1. I'm scanning whole modules for annotations, so it is exposed to lots of things
    2. In order to be able to infer expressions, it has to augment existing
      annotations when it is unambiguous. In our case, since __new__ is more or
      less a classmethod (i.e. it takes the class as first argument even if
      strictly speaking it's a staticmethod), it added an annotation with the class
      name (extracted from __qualname__).

    It implements PEP-563 to evaluate the annotions, which describes how to get the
    globals for a function and class and that they can be fed to eval(). Using
    typing.get_type_hints() is not really possible since annotations need to be
    augmented, and fancy type hints like Optional are unusable for my purpose since
    there typing.get_args() was only added to Python 3.8 (I need to support >=
    3.6).

    Maybe an alternative would be to use a module-level types.MappingProxyType
    instance ? This way there is no extra object created for each namedtuple and
    since it's read-only it should be as safe as None.

    @douglas-raillard-arm douglas-raillard-arm mannequin added 3.9 only security fixes and removed 3.10 only security fixes labels Feb 3, 2021
    @rhettinger
    Copy link
    Contributor

    Wow, your project looks super interesting.

    @rhettinger
    Copy link
    Contributor

    New changeset b6d68aa by Raymond Hettinger in branch 'master':
    bpo-43102: Set namedtuple __new__'s internal builtins to a dict. (GH-24439)
    b6d68aa

    @rhettinger
    Copy link
    Contributor

    New changeset 29584aa by Miss Islington (bot) in branch '3.9':
    bpo-43102: Set namedtuple __new__'s internal builtins to a dict. (GH-24439) (GH-24452)
    29584aa

    @rhettinger
    Copy link
    Contributor

    Okay, it's done. Thanks for the motivating use case.

    @rhettinger rhettinger added the 3.10 only security fixes label Feb 5, 2021
    @rhettinger rhettinger added the 3.10 only security fixes label Feb 5, 2021
    @douglas-raillard-arm
    Copy link
    Mannequin Author

    douglas-raillard-arm mannequin commented Feb 5, 2021

    Thanks for looking into this issue

    @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.9 only security fixes 3.10 only security fixes stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error
    Projects
    None yet
    Development

    No branches or pull requests

    1 participant