Implement different workaround for https://bugs.python.org/issue27777

suggested by jsm/Joseph Myers.

Subclass cgi.FieldStorage into BinaryFieldStorage and use that class
rather then monkey patching cgi.FieldStorage.make_file. This should
eliminate race conditions issues inherent with the prior way I tried
to do it by flipping the class method back and forth at runtime.

Also import new class into rest_common.py and use it in place of old
mechanism.

Author: rouilj

roundup/cgi/client.py

@@ -230,7 +230,18 @@ def update(self, set_cookie=False, expire=None):
         if set_cookie:
             self.client.add_cookie(self.cookie_name, self._sid, expire=expire)
 
+class BinaryFieldStorage(cgi.FieldStorage):
+    '''This class works around the bug https://bugs.python.org/issue27777.
 
+       cgi.FieldStorage must save all data as binary/bytes. This is
+       needed for handling json and xml data blobs under python
+       3. Under python 2, str and binary are interchangable, not so
+       under 3.
+    '''
+    def make_file(self, mode=None):
+        ''' work around https://bugs.python.org/issue27777 '''
+        import tempfile
+        return tempfile.TemporaryFile("wb+")
 
 class Client:
     """Instantiate to handle one CGI request.
@@ -376,18 +387,10 @@ def __init__(self, instance, request, env, form=None, translator=None):
                                 self.env['REQUEST_METHOD'])
 
             # cgi.FieldStorage must save all data as
-            # binary/bytes. This is needed for handling json and xml
-            # data blobs under python 3. Under python 2, str and binary
-            # are interchangable, not so under 3.
-            def make_file(self):
-                ''' work around https://bugs.python.org/issue27777 '''
-                import tempfile
-                return tempfile.TemporaryFile("wb+")
-
-            saved_make_file = cgi.FieldStorage.make_file
-            cgi.FieldStorage.make_file = make_file
-            self.form = cgi.FieldStorage(fp=request.rfile, environ=env)
-            cgi.FieldStorage.make_file = saved_make_file
+            # binary/bytes. Subclass BinaryFieldStorage does this.
+            # It's a workaround for a bug in cgi.FieldStorage. See class
+            # def for details.
+            self.form = BinaryFieldStorage(fp=request.rfile, environ=env)
             # In some case (e.g. content-type application/xml), cgi
             # will not parse anything. Fake a list property in this case
             if self.form.list is None:

test/rest_common.py

@@ -399,11 +399,6 @@ def testEtagProcessing(self):
             else:
                 self.assertEqual(self.dummy_client.response_code, 412)
 
-    def make_file(self, arg=None):
-        ''' work around https://bugs.python.org/issue27777 '''
-        import tempfile
-        return tempfile.TemporaryFile("wb+")
-
     def testDispatch(self):
         """
         run changes through rest dispatch(). This also tests
@@ -412,12 +407,6 @@ def testDispatch(self):
         process.
         """
 
-        # Override the make_file so it is always set to binary
-        # read mode. This is needed so we can send a json 
-        # body.
-        saved_make_file = cgi.FieldStorage.make_file
-        cgi.FieldStorage.make_file = self.make_file
-
         # TEST #1
         # PUT: joe's 'realname' using json data.
         # simulate: /rest/data/user/<id>/realname
@@ -438,7 +427,7 @@ def testDispatch(self):
         #  FieldStorage(None, None, 'string') rather than
         #  FieldStorage(None, None, [])
         body_file=BytesIO(body)  # FieldStorage needs a file
-        form = cgi.FieldStorage(body_file,
+        form = client.BinaryFieldStorage(body_file,
                                 headers=headers,
                                 environ=env)
         self.server.client.request.headers.get=self.get_header
@@ -465,7 +454,7 @@ def testDispatch(self):
         }
         self.headers=None  # have FieldStorage get len from env.
         body_file=BytesIO(body)  # FieldStorage needs a file
-        form = cgi.FieldStorage(body_file,
+        form = client.BinaryFieldStorage(body_file,
                                 headers=None,
                                 environ=env)
         self.server.client.request.headers.get=self.get_header
@@ -544,7 +533,7 @@ def testDispatch(self):
         }
         self.headers=headers
         body_file=BytesIO(body)  # FieldStorage needs a file
-        form = cgi.FieldStorage(body_file,
+        form = client.BinaryFieldStorage(body_file,
                                 headers=headers,
                                 environ=env)
         self.server.client.request.headers.get=self.get_header
@@ -565,7 +554,7 @@ def testDispatch(self):
             etag))
         # reuse env and headers from prior test.
         body_file=BytesIO(body)  # FieldStorage needs a file
-        form = cgi.FieldStorage(body_file,
+        form = client.BinaryFieldStorage(body_file,
                                 headers=headers,
                                 environ=env)
         self.server.client.request.headers.get=self.get_header
@@ -598,7 +587,7 @@ def testDispatch(self):
         }
         self.headers=headers
         body_file=BytesIO(body)  # FieldStorage needs a file
-        form = cgi.FieldStorage(body_file,
+        form = client.BinaryFieldStorage(body_file,
                                 headers=headers,
                                 environ=env)
         self.server.client.request.headers.get=self.get_header
@@ -617,9 +606,6 @@ def testDispatch(self):
                          'foo bar')
         del(self.headers)
 
-        # reset the make_file method in the class
-        cgi.FieldStorage.make_file = saved_make_file
-
     def testPut(self):
         """
         Change joe's 'realname'