classification
Title: namedtuple's __new__.__globals__['__builtins__'] is None
Type: behavior Stage: resolved
Components: Library (Lib) Versions: Python 3.10, Python 3.9
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: rhettinger Nosy List: douglas-raillard-arm, miss-islington, rhettinger
Priority: normal Keywords: patch

Created on 2021-02-02 14:46 by douglas-raillard-arm, last changed 2021-02-05 10:03 by douglas-raillard-arm. This issue is now closed.

Pull Requests
URL Status Linked Edit
PR 24439 merged rhettinger, 2021-02-04 07:00
PR 24452 merged miss-islington, 2021-02-04 23:52
Messages (9)
msg386145 - (view) Author: Douglas Raillard (douglas-raillard-arm) Date: 2021-02-02 14:46
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
msg386147 - (view) Author: Raymond Hettinger (rhettinger) * (Python committer) Date: 2021-02-02 14:57
Thanks for the report.  I'll look at it in detail this evening.  Off-hand the suggest sounds reasonable.
msg386163 - (view) Author: Raymond Hettinger (rhettinger) * (Python committer) Date: 2021-02-02 19:51
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.
msg386185 - (view) Author: Douglas Raillard (douglas-raillard-arm) Date: 2021-02-03 10:16
I did hit the issue while porting a tool to Python 3.9:
https://github.com/ARM-software/lisa/pull/1585/commits/a4cd3aa1ad339ebfe59cc9e2ae290bb3788c900d

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 PEP563 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.
msg386211 - (view) Author: Raymond Hettinger (rhettinger) * (Python committer) Date: 2021-02-03 14:56
Wow, your project looks super interesting.
msg386504 - (view) Author: Raymond Hettinger (rhettinger) * (Python committer) Date: 2021-02-04 23:52
New changeset b6d68aa08baebb753534a26d537ac3c0d2c21c79 by Raymond Hettinger in branch 'master':
bpo-43102:  Set namedtuple __new__'s internal builtins to a dict. (GH-24439)
https://github.com/python/cpython/commit/b6d68aa08baebb753534a26d537ac3c0d2c21c79
msg386505 - (view) Author: Raymond Hettinger (rhettinger) * (Python committer) Date: 2021-02-05 00:12
New changeset 29584aa6acbc70091dc23636db51ee1696e65072 by Miss Islington (bot) in branch '3.9':
bpo-43102:  Set namedtuple __new__'s internal builtins to a dict. (GH-24439) (GH-24452)
https://github.com/python/cpython/commit/29584aa6acbc70091dc23636db51ee1696e65072
msg386506 - (view) Author: Raymond Hettinger (rhettinger) * (Python committer) Date: 2021-02-05 00:13
Okay, it's done.  Thanks for the motivating use case.
msg386519 - (view) Author: Douglas Raillard (douglas-raillard-arm) Date: 2021-02-05 10:03
Thanks for looking into this issue
History
Date User Action Args
2021-02-05 10:03:20douglas-raillard-armsetmessages: + msg386519
2021-02-05 00:13:47rhettingersetstatus: open -> closed
versions: + Python 3.10
messages: + msg386506

resolution: fixed
stage: patch review -> resolved
2021-02-05 00:12:45rhettingersetmessages: + msg386505
2021-02-04 23:52:32miss-islingtonsetnosy: + miss-islington
pull_requests: + pull_request23252
2021-02-04 23:52:23rhettingersetmessages: + msg386504
2021-02-04 07:00:04rhettingersetkeywords: + patch
stage: patch review
pull_requests: + pull_request23247
2021-02-03 14:56:48rhettingersetmessages: + msg386211
2021-02-03 10:16:59douglas-raillard-armsetmessages: + msg386185
versions: + Python 3.9, - Python 3.10
2021-02-02 19:51:09rhettingersetmessages: + msg386163
2021-02-02 14:57:28rhettingersetversions: + Python 3.10, - Python 3.9
nosy: + rhettinger

messages: + msg386147

assignee: rhettinger
2021-02-02 14:46:44douglas-raillard-armcreate