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

@@ -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
+  The arguments are a string and optional globals and locals.  If provided,
-  \var{expression} argument is parsed and evaluated as a Python
+  \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

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

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 @@ class BuiltinTest(unittest.TestCase):
         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')

Lib/test/test_descrtut.py

@@ -78,16 +78,6 @@ statement or the built-in function eval():
     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 "<stdin>", line 1, in ?
-      File "<string>", line 1, in ?
-    NameError: name 'foo' is not defined
-    >>>
-
 Now I'll show that defaultdict instances have dynamic instance variables,
 just like classic classes:
 

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.
 

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;

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;
 	}

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",
+	if (!PyArg_UnpackTuple(args, "eval", 1, 3, &cmd, &globals, &locals))
-			&cmd,
-			&PyDict_Type, &globals,
-			&PyDict_Type, &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\
+The globals must be a dictionary and locals can be any mappping,\n\
-globals and locals.  If only globals is given, locals defaults to it.");
+defaulting to the current globals and locals.\n\
+If only globals is given, locals defaults to it.\n");
 
 
 static PyObject *

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