This issue tracker has been migrated to GitHub ,
and is currently read-only.
For more information,
see the GitHub FAQs in the Python's Developer Guide.
Created on 2012年03月22日 01:35 by vstinner, last changed 2022年04月11日 14:57 by admin. This issue is now closed.
| Files | ||||
|---|---|---|---|---|
| File name | Uploaded | Description | Edit | |
| builtins-3.patch | vstinner, 2012年04月15日 23:49 | review | ||
| builtins-4.patch | vstinner, 2012年04月18日 11:54 | review | ||
| Messages (14) | |||
|---|---|---|---|
| msg156534 - (view) | Author: STINNER Victor (vstinner) * (Python committer) | Date: 2012年03月22日 01:35 | |
CPython expects __builtins__ to be a dict, but it is interesting to be able to use another type. For example, my pysandbox project (sandbox to secure Python) requires a read-only mapping for __builtins__. The PEP 416 was rejected, so there is no builtin frozendict type, but it looks like the dictproxy type will be exposed as a public type. Attached patch uses PyDict_CheckExact() to check if __builtins__ is a dict and add a "slow-path" for other types. The overhead on runtime performance should be very low (near zero), PyDict_CheckExact() just dereference a pointer (to read the object type) and compare two pointers. The patch depends on issue #14383 patch (identifier.patch) for the __build_class__ identifier. I can write a new patch without this dependency if needed. |
|||
| msg156537 - (view) | Author: STINNER Victor (vstinner) * (Python committer) | Date: 2012年03月22日 01:50 | |
See the issue #14386 which exposes dictproxy as a public type. |
|||
| msg156539 - (view) | Author: STINNER Victor (vstinner) * (Python committer) | Date: 2012年03月22日 02:03 | |
Example combining patches of #14385 and #14386 to run code with read-only __builtins__: ----------- test.py ------------- ns={'__builtins__': __builtins__.__dict__} exec(compile("__builtins__['superglobal']=1; print(superglobal)", "test", "exec"), ns) ns={'__builtins__': dictproxy(__builtins__.__dict__)} exec(compile("__builtins__['superglobal']=2; print(superglobal)", "test", "exec"), ns) ----------- end of test.py ----- Output: -------- $ ./python test.py 1 Traceback (most recent call last): File "x.py", line 4, in <module> exec(compile("__builtins__['superglobal']=1; print(superglobal)", "test", "exec"), ns) File "test", line 1, in <module> TypeError: 'dictproxy' object does not support item assignment -------- Note: this protection is not enough to secure Python, but it is an important part of a Python sandbox. |
|||
| msg156552 - (view) | Author: STINNER Victor (vstinner) * (Python committer) | Date: 2012年03月22日 12:38 | |
> Note: this protection is not enough to secure Python, > but it is an important part of a Python sandbox. Oh, and by the way, I workaround the lack of read-only mapping in pysandbox by removing dict methods: dict.__init__(), dict.clear(), dict.update(), etc. This is a problem because these methods are useful in Python. |
|||
| msg156791 - (view) | Author: STINNER Victor (vstinner) * (Python committer) | Date: 2012年03月25日 23:59 | |
With my patch, Python doesn't check __builtins__ type whereas ceval.c replaces any lookup error by a NameError. Example:
$ ./python
Python 3.3.0a1+ (default:f8d01c8baf6a+, Mar 26 2012, 01:44:48)
>>> code=compile("print('Hello World!')", "", "exec")
>>> exec(code,{'__builtins__': {'print': print}})
Hello World!
>>> exec(code,{'__builtins__': {}})
NameError: name 'print' is not defined
>>> exec(code,{'__builtins__': 1})
NameError: name 'print' is not defined
It should only replace the current exception by NameError if the current exception is a LookupError.
And my patch on LOAD_GLOBAL is not correct, it does still call PyDict_GetItem. I'm waiting until #14383 is done before writing a new patch.
|
|||
| msg157573 - (view) | Author: STINNER Victor (vstinner) * (Python committer) | Date: 2012年04月05日 12:09 | |
New version: - if __build_class__ is missing, raise a NameError instead of surprising ImportError - add tests - if PyObject_GetItem on __builtins__ or globals fail, only raise NameError if the exception is a KeyError Before my patch, a new dict was created for builtins if __builtins__ exists in global but is not a dict. With my patch, the __builtins__ is kept and the type is checked at runtime. If __builtins__ is not a mapping, an exception is raised on lookup in ceval.c. We may check __builtins__ type in PyFrame_New() using: PyDict_Check(builtins) || (PyMapping_Check(mapping) && !PyList_Check(mapping) && !PyTuple_Check(mapping)) (PyDict_Check(builtins) is checked first for performance) |
|||
| msg158379 - (view) | Author: STINNER Victor (vstinner) * (Python committer) | Date: 2012年04月15日 23:49 | |
Oops, patch version 2 was not correct: I forgot a { ... } in ceval.c.
New patch fixing this issue but leaves also the LOAD_GLOBAL code unchanged : keep the goto and don't try to factorize the 3 last instructions. LOAD_GLOBAL is really critical in performance.
With patch version 3, the overall overhead is +0.4% according to pybench.
|
|||
| msg158606 - (view) | Author: STINNER Victor (vstinner) * (Python committer) | Date: 2012年04月18日 11:54 | |
- assert(!builtins || PyDict_Check(builtins)); + assert(!builtins); Oops, the assert must be replaced with assert(builtins != NULL) -> fixed in patch version 4. |
|||
| msg158679 - (view) | Author: Antoine Pitrou (pitrou) * (Python committer) | Date: 2012年04月18日 22:26 | |
This looks fine. |
|||
| msg158681 - (view) | Author: Roundup Robot (python-dev) (Python triager) | Date: 2012年04月18日 23:00 | |
New changeset e3ab8aa0216c by Victor Stinner in branch 'default': Issue #14385: Support other types than dict for __builtins__ http://hg.python.org/cpython/rev/e3ab8aa0216c |
|||
| msg169650 - (view) | Author: Martijn Pieters (mjpieters) * | Date: 2012年09月01日 17:35 | |
I note that the documentation still states a dictionary is required for globals. Should that not be updated as well? See http://docs.python.org/py3k/library/functions.html#exec |
|||
| msg169651 - (view) | Author: Martijn Pieters (mjpieters) * | Date: 2012年09月01日 17:37 | |
Apologies, I meant to link to the dev docs: http://docs.python.org/dev/library/functions.html#exec |
|||
| msg337245 - (view) | Author: Kevin Shweh (Kevin Shweh) | Date: 2019年03月05日 23:54 | |
The patch for this issue changed LOAD_GLOBAL to use PyObject_GetItem when globals() is a dict subclass, but LOAD_NAME, STORE_GLOBAL, and DELETE_GLOBAL weren't changed. (LOAD_NAME uses PyObject_GetItem for builtins now, but not for globals.)
This means that global lookup doesn't respect overridden __getitem__ inside a class statement (unless you explicitly declare the name global with a global statement, in which case LOAD_GLOBAL gets used instead of LOAD_NAME).
I don't have a strong opinion on whether STORE_GLOBAL or DELETE_GLOBAL should respect overridden __setitem__ or __delitem__, but the inconsistency between LOAD_GLOBAL and LOAD_NAME seems like a bug that should be fixed.
For reference, in the following code, the first 3 exec calls successfully print 5, and the last exec call fails, due to the LOAD_GLOBAL/LOAD_NAME inconsistency:
class Foo(dict):
def __getitem__(self, index):
return 5 if index == 'y' else super().__getitem__(index)
exec('print(y)', Foo())
exec('global y; print(y)', Foo())
exec('''
class UsesLOAD_NAME:
global y
print(y)''', Foo())
exec('''
class UsesLOAD_NAME:
print(y)''', Foo())
|
|||
| msg337262 - (view) | Author: STINNER Victor (vstinner) * (Python committer) | Date: 2019年03月06日 01:35 | |
This issues is now closed. Please open a new issue. |
|||
| History | |||
|---|---|---|---|
| Date | User | Action | Args |
| 2022年04月11日 14:57:28 | admin | set | github: 58593 |
| 2019年03月06日 01:35:04 | vstinner | set | messages: + msg337262 |
| 2019年03月05日 23:54:37 | Kevin Shweh | set | nosy:
+ Kevin Shweh messages: + msg337245 |
| 2012年09月01日 17:37:01 | mjpieters | set | messages: + msg169651 |
| 2012年09月01日 17:35:55 | mjpieters | set | nosy:
+ mjpieters messages: + msg169650 |
| 2012年04月18日 23:02:25 | vstinner | set | status: open -> closed resolution: fixed |
| 2012年04月18日 23:00:20 | python-dev | set | nosy:
+ python-dev messages: + msg158681 |
| 2012年04月18日 22:26:39 | pitrou | set | messages: + msg158679 |
| 2012年04月18日 11:54:07 | vstinner | set | files:
+ builtins-4.patch messages: + msg158606 |
| 2012年04月15日 23:53:11 | vstinner | set | nosy:
+ pitrou |
| 2012年04月15日 23:49:45 | vstinner | set | files: - builtins-2.patch |
| 2012年04月15日 23:49:24 | vstinner | set | files:
+ builtins-3.patch messages: + msg158379 |
| 2012年04月15日 22:31:17 | vstinner | set | files: - builtins.patch |
| 2012年04月05日 12:09:41 | vstinner | set | files:
+ builtins-2.patch messages: + msg157573 |
| 2012年03月25日 23:59:41 | vstinner | set | messages: + msg156791 |
| 2012年03月22日 12:38:15 | vstinner | set | messages: + msg156552 |
| 2012年03月22日 02:03:35 | vstinner | set | messages: + msg156539 |
| 2012年03月22日 01:50:27 | vstinner | set | messages: + msg156537 |
| 2012年03月22日 01:35:34 | vstinner | create | |