SF bug #997050: Document, test, & check for non-string values in ConfigParser. Moved the new string-only restriction added in rev. 1.65 to the SafeConfigParser class, leaving existing ConfigParser & RawConfigParser behavior alone, and documented the conditions under which non-string values work.

4 files changed, 78 insertions, 21 deletions

diff --git a/Doc/lib/libcfgparser.tex b/Doc/lib/libcfgparser.tex
index bc15655c93..42a362eac2 100644
--- a/Doc/lib/libcfgparser.tex
+++ b/Doc/lib/libcfgparser.tex
@@ -238,10 +238,12 @@ option in the given \var{section}.
 \end{methoddesc}
 
 \begin{methoddesc}{set}{section, option, value}
-If the given section exists, set the given option to the specified value;
-otherwise raise \exception{NoSectionError}.  \var{value} must be a
-string (\class{str} or \class{unicode}); if not, \exception{TypeError}
-is raised.
+If the given section exists, set the given option to the specified
+value; otherwise raise \exception{NoSectionError}.  While it is
+possible to use \class{RawConfigParser} (or \class{ConfigParser} with
+\var{raw} parameters set to true) for \emph{internal} storage of
+non-string values, full functionality (including interpolation and
+output to files) can only be achieved using string values.
 \versionadded{1.6}
 \end{methoddesc}
 
@@ -281,8 +283,6 @@ option names case sensitive.
 
 The \class{ConfigParser} class extends some methods of the
 \class{RawConfigParser} interface, adding some optional arguments.
-The \class{SafeConfigParser} class implements the same extended
-interface.
 
 \begin{methoddesc}{get}{section, option\optional{, raw\optional{, vars}}}
 Get an \var{option} value for the named \var{section}.  All the
@@ -297,3 +297,17 @@ option in the given \var{section}. Optional arguments have the
 same meaning as for the \method{get()} method.
 \versionadded{2.3}
 \end{methoddesc}
+
+
+\subsection{SafeConfigParser Objects \label{SafeConfigParser-objects}}
+
+The \class{SafeConfigParser} class implements the same extended
+interface as \class{ConfigParser}, with the following addition:
+
+\begin{methoddesc}{set}{section, option, value}
+If the given section exists, set the given option to the specified
+value; otherwise raise \exception{NoSectionError}.  \var{value} must
+be a string (\class{str} or \class{unicode}); if not,
+\exception{TypeError} is raised.
+\versionadded{2.4}
+\end{methoddesc}
diff --git a/Lib/ConfigParser.py b/Lib/ConfigParser.py
index acbf3ea144..ade9614796 100644
--- a/Lib/ConfigParser.py
+++ b/Lib/ConfigParser.py
@@ -92,7 +92,8 @@ import re
 __all__ = ["NoSectionError", "DuplicateSectionError", "NoOptionError",
            "InterpolationError", "InterpolationDepthError",
            "InterpolationSyntaxError", "ParsingError",
-           "MissingSectionHeaderError", "ConfigParser", "SafeConfigParser",
+           "MissingSectionHeaderError",
+           "ConfigParser", "SafeConfigParser", "RawConfigParser",
            "DEFAULTSECT", "MAX_INTERPOLATION_DEPTH"]
 
 DEFAULTSECT = "DEFAULT"
@@ -348,8 +349,6 @@ class RawConfigParser:
 
     def set(self, section, option, value):
         """Set an option."""
-        if not isinstance(value, basestring):
-            raise TypeError("option values must be strings")
         if not section or section == DEFAULTSECT:
             sectdict = self._defaults
         else:
@@ -633,3 +632,9 @@ class SafeConfigParser(ConfigParser):
                 raise InterpolationSyntaxError(
                     option, section,
                     "'%%' must be followed by '%%' or '(', found: %r" % (rest,))
+
+    def set(self, section, option, value):
+        """Set an option.  Extend ConfigParser.set: check for string values."""
+        if not isinstance(value, basestring):
+            raise TypeError("option values must be strings")
+        ConfigParser.set(self, section, option, value)
diff --git a/Lib/test/test_cfgparser.py b/Lib/test/test_cfgparser.py
index 6b3e68a6d1..36640f114e 100644
--- a/Lib/test/test_cfgparser.py
+++ b/Lib/test/test_cfgparser.py
@@ -240,18 +240,6 @@ class TestCaseBase(unittest.TestCase):
             cf.set("sect", "option1", unicode("splat"))
             cf.set("sect", "option2", unicode("splat"))
 
-    def test_set_nonstring_types(self):
-        cf = self.fromstring("[sect]\n"
-                             "option1=foo\n")
-        # Check that we get a TypeError when setting non-string values
-        # in an existing section:
-        self.assertRaises(TypeError, cf.set, "sect", "option1", 1)
-        self.assertRaises(TypeError, cf.set, "sect", "option1", 1.0)
-        self.assertRaises(TypeError, cf.set, "sect", "option1", object())
-        self.assertRaises(TypeError, cf.set, "sect", "option2", 1)
-        self.assertRaises(TypeError, cf.set, "sect", "option2", 1.0)
-        self.assertRaises(TypeError, cf.set, "sect", "option2", object())
-
     def test_read_returns_file_list(self):
         file1 = test_support.findfile("cfgparser.1")
         # check when we pass a mix of readable and non-readable files:
@@ -344,6 +332,27 @@ class ConfigParserTestCase(TestCaseBase):
                                  ('key', '|value|'),
                                  ('name', 'value')])
 
+    def test_set_nonstring_types(self):
+        cf = self.newconfig()
+        cf.add_section('non-string')
+        cf.set('non-string', 'int', 1)
+        cf.set('non-string', 'list', [0, 1, 1, 2, 3, 5, 8, 13, '%('])
+        cf.set('non-string', 'dict', {'pi': 3.14159, '%(': 1,
+                                      '%(list)': '%(list)'})
+        cf.set('non-string', 'string_with_interpolation', '%(list)s')
+        self.assertEqual(cf.get('non-string', 'int', raw=True), 1)
+        self.assertRaises(TypeError, cf.get, 'non-string', 'int')
+        self.assertEqual(cf.get('non-string', 'list', raw=True),
+                         [0, 1, 1, 2, 3, 5, 8, 13, '%('])
+        self.assertRaises(TypeError, cf.get, 'non-string', 'list')
+        self.assertEqual(cf.get('non-string', 'dict', raw=True),
+                         {'pi': 3.14159, '%(': 1, '%(list)': '%(list)'})
+        self.assertRaises(TypeError, cf.get, 'non-string', 'dict')
+        self.assertEqual(cf.get('non-string', 'string_with_interpolation',
+                                raw=True), '%(list)s')
+        self.assertRaises(ValueError, cf.get, 'non-string',
+                          'string_with_interpolation', raw=False)
+
 
 class RawConfigParserTestCase(TestCaseBase):
     config_class = ConfigParser.RawConfigParser
@@ -368,6 +377,17 @@ class RawConfigParserTestCase(TestCaseBase):
                                  ('key', '|%(name)s|'),
                                  ('name', 'value')])
 
+    def test_set_nonstring_types(self):
+        cf = self.newconfig()
+        cf.add_section('non-string')
+        cf.set('non-string', 'int', 1)
+        cf.set('non-string', 'list', [0, 1, 1, 2, 3, 5, 8, 13])
+        cf.set('non-string', 'dict', {'pi': 3.14159})
+        self.assertEqual(cf.get('non-string', 'int'), 1)
+        self.assertEqual(cf.get('non-string', 'list'),
+                         [0, 1, 1, 2, 3, 5, 8, 13])
+        self.assertEqual(cf.get('non-string', 'dict'), {'pi': 3.14159})
+        
 
 class SafeConfigParserTestCase(ConfigParserTestCase):
     config_class = ConfigParser.SafeConfigParser
@@ -382,6 +402,18 @@ class SafeConfigParserTestCase(ConfigParserTestCase):
         self.assertEqual(cf.get("section", "ok"), "xxx/%s")
         self.assertEqual(cf.get("section", "not_ok"), "xxx/xxx/%s")
 
+    def test_set_nonstring_types(self):
+        cf = self.fromstring("[sect]\n"
+                             "option1=foo\n")
+        # Check that we get a TypeError when setting non-string values
+        # in an existing section:
+        self.assertRaises(TypeError, cf.set, "sect", "option1", 1)
+        self.assertRaises(TypeError, cf.set, "sect", "option1", 1.0)
+        self.assertRaises(TypeError, cf.set, "sect", "option1", object())
+        self.assertRaises(TypeError, cf.set, "sect", "option2", 1)
+        self.assertRaises(TypeError, cf.set, "sect", "option2", 1.0)
+        self.assertRaises(TypeError, cf.set, "sect", "option2", object())
+
 
 def test_main():
     test_support.run_unittest(
diff --git a/Misc/NEWS b/Misc/NEWS
index 634f661fbc..a382e70086 100644
--- a/Misc/NEWS
+++ b/Misc/NEWS
@@ -99,6 +99,12 @@ Library
   consistent with the handling of config file entries and runtime-set
   options.
 
+- SF bug #997050: Document, test, & check for non-string values in
+  ConfigParser.  Moved the new string-only restriction added in
+  rev. 1.65 to the SafeConfigParser class, leaving existing
+  ConfigParser & RawConfigParser behavior alone, and documented the
+  conditions under which non-string values work.
+
 Build
 -----