Changed some ValueError's to KeyError and IndexError.

Corrected code for invalid conversion specifier.
Added tests to verify.

Modified string.Formatter to correctly expand format_spec's,
and added a limit to recursion depth.  Added _vformat()
method to support both of these.

3 files changed, 31 insertions, 25 deletions

diff --git a/Lib/string.py b/Lib/string.py
index 9b00a62..03179fb 100644
--- a/Lib/string.py
+++ b/Lib/string.py
@@ -202,6 +202,13 @@ class Formatter:
 
     def vformat(self, format_string, args, kwargs):
         used_args = set()
+        result = self._vformat(format_string, args, kwargs, used_args, 2)
+        self.check_unused_args(used_args, args, kwargs)
+        return result
+
+    def _vformat(self, format_string, args, kwargs, used_args, recursion_depth):
+        if recursion_depth < 0:
+            raise ValueError('Max string recursion exceeded')
         result = []
         for literal_text, field_name, format_spec, conversion in \
                 self.parse(format_string):
@@ -223,10 +230,13 @@ class Formatter:
                 # do any conversion on the resulting object
                 obj = self.convert_field(obj, conversion)
 
+                # expand the format spec, if needed
+                format_spec = self._vformat(format_spec, args, kwargs,
+                                            used_args, recursion_depth-1)
+
                 # format the object and append to the result
                 result.append(self.format_field(obj, format_spec))
 
-        self.check_unused_args(used_args, args, kwargs)
         return ''.join(result)
 
 
@@ -251,9 +261,9 @@ class Formatter:
             return repr(value)
         elif conversion == 's':
             return str(value)
-        else:
-            assert conversion is None
+        elif conversion is None:
             return value
+        raise ValueError("Unknown converion specifier {0!s}".format(conversion))
 
 
     # returns an iterable that contains tuples of the form:
diff --git a/Lib/test/test_unicode.py b/Lib/test/test_unicode.py
index 52b7b4f..64cca3f 100644
--- a/Lib/test/test_unicode.py
+++ b/Lib/test/test_unicode.py
@@ -542,29 +542,30 @@ class UnicodeTest(
         self.assertRaises(ValueError, 'a}'.format)
         self.assertRaises(ValueError, '{a'.format)
         self.assertRaises(ValueError, '}a'.format)
-        self.assertRaises(ValueError, '{0}'.format)
-        self.assertRaises(ValueError, '{1}'.format, 'abc')
-        self.assertRaises(ValueError, '{x}'.format)
+        self.assertRaises(IndexError, '{0}'.format)
+        self.assertRaises(IndexError, '{1}'.format, 'abc')
+        self.assertRaises(KeyError,   '{x}'.format)
         self.assertRaises(ValueError, "}{".format)
         self.assertRaises(ValueError, "{".format)
         self.assertRaises(ValueError, "}".format)
         self.assertRaises(ValueError, "abc{0:{}".format)
         self.assertRaises(ValueError, "{0".format)
-        self.assertRaises(ValueError, "{0.}".format)
-        self.assertRaises(ValueError, "{0[}".format)
+        self.assertRaises(IndexError, "{0.}".format)
+        self.assertRaises(ValueError, "{0.}".format, 0)
+        self.assertRaises(IndexError, "{0[}".format)
         self.assertRaises(ValueError, "{0[}".format, [])
-        self.assertRaises(ValueError, "{0]}".format)
-        self.assertRaises(ValueError, "{0.[]}".format)
+        self.assertRaises(KeyError,   "{0]}".format)
+        self.assertRaises(ValueError, "{0.[]}".format, 0)
         self.assertRaises(ValueError, "{0..foo}".format, 0)
-        self.assertRaises(ValueError, "{0[0}".format)
-        self.assertRaises(ValueError, "{0[0:foo}".format)
-        self.assertRaises(ValueError, "{c]}".format)
-        self.assertRaises(ValueError, "{{ {{{0}}".format)
-        self.assertRaises(ValueError, "{0}}".format)
-        self.assertRaises(ValueError, "{foo}".format, bar=3)
+        self.assertRaises(ValueError, "{0[0}".format, 0)
+        self.assertRaises(ValueError, "{0[0:foo}".format, 0)
+        self.assertRaises(KeyError,   "{c]}".format)
+        self.assertRaises(ValueError, "{{ {{{0}}".format, 0)
+        self.assertRaises(ValueError, "{0}}".format, 0)
+        self.assertRaises(KeyError,   "{foo}".format, bar=3)
         self.assertRaises(ValueError, "{0!x}".format, 3)
-        self.assertRaises(ValueError, "{0!}".format)
-        self.assertRaises(ValueError, "{0!rs}".format)
+        self.assertRaises(ValueError, "{0!}".format, 0)
+        self.assertRaises(ValueError, "{0!rs}".format, 0)
         self.assertRaises(ValueError, "{!}".format)
         self.assertRaises(ValueError, "{:}".format)
         self.assertRaises(ValueError, "{:s}".format)
diff --git a/Objects/stringlib/string_format.h b/Objects/stringlib/string_format.h
index de700f6..ea8b0e7 100644
--- a/Objects/stringlib/string_format.h
+++ b/Objects/stringlib/string_format.h
@@ -414,8 +414,7 @@ get_field_object(SubString *input, PyObject *args, PyObject *kwargs)
         if (key == NULL)
             goto error;
         if ((kwargs == NULL) || (obj = PyDict_GetItem(kwargs, key)) == NULL) {
-            PyErr_SetString(PyExc_ValueError, "Keyword argument not found "
-                            "in format string");
+            PyErr_SetObject(PyExc_KeyError, key);
             Py_DECREF(key);
             goto error;
         }
@@ -425,12 +424,8 @@ get_field_object(SubString *input, PyObject *args, PyObject *kwargs)
     else {
         /* look up in args */
         obj = PySequence_GetItem(args, index);
-        if (obj == NULL) {
-            /* translate IndexError to a ValueError */
-            PyErr_SetString(PyExc_ValueError, "Not enough positional arguments "
-                            "in format string");
+        if (obj == NULL)
             goto error;
-        }
     }
 
     /* iterate over the rest of the field_name */