Now a from submitted via POST that also has a query string

will contain both FieldStorage and MiniFieldStorage items.

Fixes #1817.

diff --git a/Doc/library/cgi.rst b/Doc/library/cgi.rst
index 4c619e4b771fa748b9b724a2ab6c822fa10b6e60..17cd750207b057819e41518f407dc7d201d875db 100644 (file)
--- a/Doc/library/cgi.rst
+++ b/Doc/library/cgi.rst
@@ -165,6 +165,8 @@ data part of type :mimetype:`application/x-www-form-urlencoded`), the items will
 actually be instances of the class :class:`MiniFieldStorage`.  In this case, the
 :attr:`list`, :attr:`file`, and :attr:`filename` attributes are always ``None``.
 
+A form submitted via POST that also has a query string will contain both
+:class:`FieldStorage` and :class:`MiniFieldStorage` items.
 
 Higher Level Interface
 ----------------------

diff --git a/Lib/cgi.py b/Lib/cgi.py
index 8760f960549d977f7a4bd0d31d96316da3568003..561f5adc566e5b05f962616962eb28ca7d5be26a 100755 (executable)
--- a/Lib/cgi.py
+++ b/Lib/cgi.py
@@ -456,6 +456,7 @@ class FieldStorage:
         self.strict_parsing = strict_parsing
         if 'REQUEST_METHOD' in environ:
             method = environ['REQUEST_METHOD'].upper()
+        self.qs_on_post = None
         if method == 'GET' or method == 'HEAD':
             if 'QUERY_STRING' in environ:
                 qs = environ['QUERY_STRING']
@@ -474,6 +475,8 @@ class FieldStorage:
                 headers['content-type'] = "application/x-www-form-urlencoded"
             if 'CONTENT_TYPE' in environ:
                 headers['content-type'] = environ['CONTENT_TYPE']
+            if 'QUERY_STRING' in environ:
+                self.qs_on_post = environ['QUERY_STRING']
             if 'CONTENT_LENGTH' in environ:
                 headers['content-length'] = environ['CONTENT_LENGTH']
         self.fp = fp or sys.stdin
@@ -631,6 +634,8 @@ class FieldStorage:
     def read_urlencoded(self):
         """Internal: read data in query string format."""
         qs = self.fp.read(self.length)
+        if self.qs_on_post:
+            qs += '&' + self.qs_on_post
         self.list = list = []
         for key, value in parse_qsl(qs, self.keep_blank_values,
                                     self.strict_parsing):
@@ -645,6 +650,12 @@ class FieldStorage:
         if not valid_boundary(ib):
             raise ValueError, 'Invalid boundary in multipart form: %r' % (ib,)
         self.list = []
+        if self.qs_on_post:
+            for key, value in parse_qsl(self.qs_on_post, self.keep_blank_values,
+                                        self.strict_parsing):
+                self.list.append(MiniFieldStorage(key, value))
+            FieldStorageClass = None
+
         klass = self.FieldStorageClass or self.__class__
         part = klass(self.fp, {}, ib,
                      environ, keep_blank_values, strict_parsing)

diff --git a/Lib/test/test_cgi.py b/Lib/test/test_cgi.py
index c3c2c6cddc0a143ada73fed24cbd591ba5438472..042e50713a3a0ff1e849cc0fc7e9f23a3936b3e7 100644 (file)
--- a/Lib/test/test_cgi.py
+++ b/Lib/test/test_cgi.py
@@ -130,6 +130,17 @@ def first_elts(list):
 def first_second_elts(list):
     return map(lambda p:(p[0], p[1][0]), list)
 
+def gen_result(data, environ):
+    fake_stdin = StringIO(data)
+    fake_stdin.seek(0)
+    form = cgi.FieldStorage(fp=fake_stdin, environ=environ)
+
+    result = {}
+    for k, v in dict(form).items():
+        result[k] = type(v) is list and form.getlist(k) or v.value
+
+    return result
+
 class CgiTests(unittest.TestCase):
 
     def test_qsl(self):
@@ -278,6 +289,83 @@ Content-Disposition: form-data; name="submit"
                 got = getattr(fs.list[x], k)
                 self.assertEquals(got, exp)
 
+    _qs_result = {
+        'key1': 'value1',
+        'key2': ['value2x', 'value2y'],
+        'key3': 'value3',
+        'key4': 'value4'
+    }
+    def testQSAndUrlEncode(self):
+        data = "key2=value2x&key3=value3&key4=value4"
+        environ = {
+            'CONTENT_LENGTH':   str(len(data)),
+            'CONTENT_TYPE':     'application/x-www-form-urlencoded',
+            'QUERY_STRING':     'key1=value1&key2=value2y',
+            'REQUEST_METHOD':   'POST',
+        }
+        v = gen_result(data, environ)
+        self.assertEqual(self._qs_result, v)
+
+    def testQSAndFormData(self):
+        data = """
+---123
+Content-Disposition: form-data; name="key2"
+
+value2y
+---123
+Content-Disposition: form-data; name="key3"
+
+value3
+---123
+Content-Disposition: form-data; name="key4"
+
+value4
+---123--
+"""
+        environ = {
+            'CONTENT_LENGTH':   str(len(data)),
+            'CONTENT_TYPE':     'multipart/form-data; boundary=-123',
+            'QUERY_STRING':     'key1=value1&key2=value2x',
+            'REQUEST_METHOD':   'POST',
+        }
+        v = gen_result(data, environ)
+        self.assertEqual(self._qs_result, v)
+
+    def testQSAndFormDataFile(self):
+        data = """
+---123
+Content-Disposition: form-data; name="key2"
+
+value2y
+---123
+Content-Disposition: form-data; name="key3"
+
+value3
+---123
+Content-Disposition: form-data; name="key4"
+
+value4
+---123
+Content-Disposition: form-data; name="upload"; filename="fake.txt"
+Content-Type: text/plain
+
+this is the content of the fake file
+
+---123--
+"""
+        environ = {
+            'CONTENT_LENGTH':   str(len(data)),
+            'CONTENT_TYPE':     'multipart/form-data; boundary=-123',
+            'QUERY_STRING':     'key1=value1&key2=value2x',
+            'REQUEST_METHOD':   'POST',
+        }
+        result = self._qs_result.copy()
+        result.update({
+            'upload': 'this is the content of the fake file\n'
+        })
+        v = gen_result(data, environ)
+        self.assertEqual(result, v)
+
 def test_main():
     run_unittest(CgiTests)
 

diff --git a/Misc/NEWS b/Misc/NEWS
index c5dfe73065f91f996ae4b047a072c1fe4eaa29ac..c98a2af3d1aaaf63c9e5e43116688f13feb1b335 100644 (file)
--- a/Misc/NEWS
+++ b/Misc/NEWS
@@ -112,6 +112,8 @@ Library
   structures, to match the behaviour of 2.5 and 3.0 (now follows the common
   sense).
 
+- Issue #1817: cgi now correctly handles the querystring on POST requests
+
 - Issue #3136: fileConfig()'s disabling of old loggers is now conditional via
   an optional disable_existing_loggers parameter, but the default value is
   such that the old behaviour is preserved. Thanks to Leandro Lucarella for