From 214b1c3aaea3e83302df9ea37a37b3c7548b92b1 Mon Sep 17 00:00:00 2001 From: Raymond Hettinger Date: Fri, 2 Jul 2004 06:41:07 +0000 Subject: [PATCH] SF Bug #215126: Over restricted type checking on eval() function The builtin eval() function now accepts any mapping for the locals argument. Time sensitive steps guarded by PyDict_CheckExact() to keep from slowing down the normal case. My timings so no measurable impact. --- Doc/lib/libfuncs.tex | 8 ++++-- Include/frameobject.h | 2 +- Lib/test/test_builtin.py | 56 ++++++++++++++++++++++++++++++++++++++- Lib/test/test_descrtut.py | 10 ------- Misc/NEWS | 2 ++ Objects/frameobject.c | 15 ++++++----- Objects/object.c | 2 +- Python/bltinmodule.c | 19 ++++++++----- Python/ceval.c | 20 +++++++++++--- 9 files changed, 103 insertions(+), 31 deletions(-) diff --git a/Doc/lib/libfuncs.tex b/Doc/lib/libfuncs.tex index fa9cd55bfd6..0c5b0e359ed 100644 --- a/Doc/lib/libfuncs.tex +++ b/Doc/lib/libfuncs.tex @@ -291,8 +291,12 @@ class C: \end{funcdesc} \begin{funcdesc}{eval}{expression\optional{, globals\optional{, locals}}} - The arguments are a string and two optional dictionaries. The - \var{expression} argument is parsed and evaluated as a Python + The arguments are a string and optional globals and locals. If provided, + \var{globals} must be a dictionary. If provided, \var{locals} can be + any mapping object. \versionchanged[formerly \var{locals} was required + to be a dictionary]{2.4} + + The \var{expression} argument is parsed and evaluated as a Python expression (technically speaking, a condition list) using the \var{globals} and \var{locals} dictionaries as global and local name space. If the \var{globals} dictionary is present and lacks diff --git a/Include/frameobject.h b/Include/frameobject.h index 2820e88bb0e..7dc14e32040 100644 --- a/Include/frameobject.h +++ b/Include/frameobject.h @@ -19,7 +19,7 @@ typedef struct _frame { PyCodeObject *f_code; /* code segment */ PyObject *f_builtins; /* builtin symbol table (PyDictObject) */ PyObject *f_globals; /* global symbol table (PyDictObject) */ - PyObject *f_locals; /* local symbol table (PyDictObject) */ + PyObject *f_locals; /* local symbol table (any mapping) */ PyObject **f_valuestack; /* points after the last local */ /* Next free slot in f_valuestack. Frame creation sets to f_valuestack. Frame evaluation usually NULLs it, but a frame that yields sets it diff --git a/Lib/test/test_builtin.py b/Lib/test/test_builtin.py index 9aa24c2331a..8fea0ca6845 100644 --- a/Lib/test/test_builtin.py +++ b/Lib/test/test_builtin.py @@ -3,7 +3,7 @@ import test.test_support, unittest from test.test_support import fcmp, have_unicode, TESTFN, unlink -import sys, warnings, cStringIO, random +import sys, warnings, cStringIO, random, UserDict warnings.filterwarnings("ignore", "hex../oct.. of negative int", FutureWarning, __name__) warnings.filterwarnings("ignore", "integer argument expected", @@ -262,6 +262,60 @@ def test_eval(self): self.assertRaises(TypeError, eval) self.assertRaises(TypeError, eval, ()) + def test_general_eval(self): + # Tests that general mappings can be used for the locals argument + + class M: + "Test mapping interface versus possible calls from eval()." + def __getitem__(self, key): + if key == 'a': + return 12 + raise KeyError + def keys(self): + return list('xyz') + + m = M() + g = globals() + self.assertEqual(eval('a', g, m), 12) + self.assertRaises(NameError, eval, 'b', g, m) + self.assertEqual(eval('dir()', g, m), list('xyz')) + self.assertEqual(eval('globals()', g, m), g) + self.assertEqual(eval('locals()', g, m), m) + + # Verify that dict subclasses work as well + class D(dict): + def __getitem__(self, key): + if key == 'a': + return 12 + return dict.__getitem__(self, key) + def keys(self): + return list('xyz') + + d = D() + self.assertEqual(eval('a', g, d), 12) + self.assertRaises(NameError, eval, 'b', g, d) + self.assertEqual(eval('dir()', g, d), list('xyz')) + self.assertEqual(eval('globals()', g, d), g) + self.assertEqual(eval('locals()', g, d), d) + + # Verify locals stores (used by list comps) + eval('[locals() for i in (2,3)]', g, d) + eval('[locals() for i in (2,3)]', g, UserDict.UserDict()) + + class SpreadSheet: + "Sample application showing nested, calculated lookups." + _cells = {} + def __setitem__(self, key, formula): + self._cells[key] = formula + def __getitem__(self, key ): + return eval(self._cells[key], globals(), self) + + ss = SpreadSheet() + ss['a1'] = '5' + ss['a2'] = 'a1*6' + ss['a3'] = 'a2*7' + self.assertEqual(ss['a3'], 210) + # Done outside of the method test_z to get the correct scope z = 0 f = open(TESTFN, 'w') diff --git a/Lib/test/test_descrtut.py b/Lib/test/test_descrtut.py index 58b74514f36..851ce4a93cf 100644 --- a/Lib/test/test_descrtut.py +++ b/Lib/test/test_descrtut.py @@ -78,16 +78,6 @@ def merge(self, other): 3 >>> -However, our __getitem__() method is not used for variable access by the -interpreter: - - >>> exec "print foo" in a - Traceback (most recent call last): - File "", line 1, in ? - File "", line 1, in ? - NameError: name 'foo' is not defined - >>> - Now I'll show that defaultdict instances have dynamic instance variables, just like classic classes: diff --git a/Misc/NEWS b/Misc/NEWS index 7d219942d46..d01f20bb6b2 100644 --- a/Misc/NEWS +++ b/Misc/NEWS @@ -12,6 +12,8 @@ What's New in Python 2.4 alpha 1? Core and builtins ----------------- +- Bug #215126. The locals argument to eval() now accepts any mapping type. + - marshal now shares interned strings. This change introduces a new .pyc magic. diff --git a/Objects/frameobject.c b/Objects/frameobject.c index 81b38193cb7..bc8cae9fe61 100644 --- a/Objects/frameobject.c +++ b/Objects/frameobject.c @@ -544,7 +544,7 @@ PyFrame_New(PyThreadState *tstate, PyCodeObject *code, PyObject *globals, #ifdef Py_DEBUG if (code == NULL || globals == NULL || !PyDict_Check(globals) || - (locals != NULL && !PyDict_Check(locals))) { + (locals != NULL && !PyMapping_Check(locals))) { PyErr_BadInternalCall(); return NULL; } @@ -688,11 +688,11 @@ map_to_dict(PyObject *map, int nmap, PyObject *dict, PyObject **values, if (deref) value = PyCell_GET(value); if (value == NULL) { - if (PyDict_DelItem(dict, key) != 0) + if (PyObject_DelItem(dict, key) != 0) PyErr_Clear(); } else { - if (PyDict_SetItem(dict, key, value) != 0) + if (PyObject_SetItem(dict, key, value) != 0) PyErr_Clear(); } } @@ -705,7 +705,9 @@ dict_to_map(PyObject *map, int nmap, PyObject *dict, PyObject **values, int j; for (j = nmap; --j >= 0; ) { PyObject *key = PyTuple_GET_ITEM(map, j); - PyObject *value = PyDict_GetItem(dict, key); + PyObject *value = PyObject_GetItem(dict, key); + if (value == NULL) + PyErr_Clear(); if (deref) { if (value || clear) { if (PyCell_GET(values[j]) != value) { @@ -720,6 +722,7 @@ dict_to_map(PyObject *map, int nmap, PyObject *dict, PyObject **values, values[j] = value; } } + Py_XDECREF(value); } } @@ -742,7 +745,7 @@ PyFrame_FastToLocals(PyFrameObject *f) } } map = f->f_code->co_varnames; - if (!PyDict_Check(locals) || !PyTuple_Check(map)) + if (!PyTuple_Check(map)) return; PyErr_Fetch(&error_type, &error_value, &error_traceback); fast = f->f_localsplus; @@ -780,7 +783,7 @@ PyFrame_LocalsToFast(PyFrameObject *f, int clear) map = f->f_code->co_varnames; if (locals == NULL) return; - if (!PyDict_Check(locals) || !PyTuple_Check(map)) + if (!PyTuple_Check(map)) return; PyErr_Fetch(&error_type, &error_value, &error_traceback); fast = f->f_localsplus; diff --git a/Objects/object.c b/Objects/object.c index 22196d714fe..1c0efdd3054 100644 --- a/Objects/object.c +++ b/Objects/object.c @@ -1621,7 +1621,7 @@ PyObject_Dir(PyObject *arg) PyObject *locals = PyEval_GetLocals(); if (locals == NULL) goto error; - result = PyDict_Keys(locals); + result = PyMapping_Keys(locals); if (result == NULL) goto error; } diff --git a/Python/bltinmodule.c b/Python/bltinmodule.c index b3c8b09a31c..f4f8b7a1a7b 100644 --- a/Python/bltinmodule.c +++ b/Python/bltinmodule.c @@ -455,11 +455,17 @@ builtin_eval(PyObject *self, PyObject *args) char *str; PyCompilerFlags cf; - if (!PyArg_ParseTuple(args, "O|O!O!:eval", - &cmd, - &PyDict_Type, &globals, - &PyDict_Type, &locals)) + if (!PyArg_UnpackTuple(args, "eval", 1, 3, &cmd, &globals, &locals)) return NULL; + if (locals != Py_None && !PyMapping_Check(locals)) { + PyErr_SetString(PyExc_TypeError, "locals must be a mapping"); + return NULL; + } + if (globals != Py_None && !PyDict_Check(globals)) { + PyErr_SetString(PyExc_TypeError, PyMapping_Check(globals) ? + "globals must be a real dict; try eval(expr, {}, mapping)" + : "globals must be a dict"); + } if (globals == Py_None) { globals = PyEval_GetGlobals(); if (locals == Py_None) @@ -517,8 +523,9 @@ PyDoc_STRVAR(eval_doc, Evaluate the source in the context of globals and locals.\n\ The source may be a string representing a Python expression\n\ or a code object as returned by compile().\n\ -The globals and locals are dictionaries, defaulting to the current\n\ -globals and locals. If only globals is given, locals defaults to it."); +The globals must be a dictionary and locals can be any mappping,\n\ +defaulting to the current globals and locals.\n\ +If only globals is given, locals defaults to it.\n"); static PyObject * diff --git a/Python/ceval.c b/Python/ceval.c index ca7cea8a425..0c3a93dac59 100644 --- a/Python/ceval.c +++ b/Python/ceval.c @@ -1643,7 +1643,10 @@ PyEval_EvalFrame(PyFrameObject *f) w = GETITEM(names, oparg); v = POP(); if ((x = f->f_locals) != NULL) { - err = PyDict_SetItem(x, w, v); + if (PyDict_CheckExact(v)) + err = PyDict_SetItem(x, w, v); + else + err = PyObject_SetItem(x, w, v); Py_DECREF(v); if (err == 0) continue; break; @@ -1656,7 +1659,7 @@ PyEval_EvalFrame(PyFrameObject *f) case DELETE_NAME: w = GETITEM(names, oparg); if ((x = f->f_locals) != NULL) { - if ((err = PyDict_DelItem(x, w)) != 0) + if ((err = PyObject_DelItem(x, w)) != 0) format_exc_check_arg(PyExc_NameError, NAME_ERROR_MSG ,w); break; @@ -1733,13 +1736,22 @@ PyEval_EvalFrame(PyFrameObject *f) case LOAD_NAME: w = GETITEM(names, oparg); - if ((x = f->f_locals) == NULL) { + if ((v = f->f_locals) == NULL) { PyErr_Format(PyExc_SystemError, "no locals when loading %s", PyObject_REPR(w)); break; } - x = PyDict_GetItem(x, w); + if (PyDict_CheckExact(v)) + x = PyDict_GetItem(v, w); + else { + x = PyObject_GetItem(v, w); + if (x == NULL && PyErr_Occurred()) { + if (!PyErr_ExceptionMatches(PyExc_KeyError)) + break; + PyErr_Clear(); + } + } if (x == NULL) { x = PyDict_GetItem(f->f_globals, w); if (x == NULL) {