Merge lp:~cjwatson/lazr.restful/web-service-caller-multipart-named-post into lp:lazr.restful

[Colin Watson]

Rework WebServiceCaller to send named POSTs as multipart/form-data.

Arguments that are instances of io.BufferedIOBase are sent as-is rather than being encoded as JSON. This allows robust use of binary arguments on both Python 2 and 3.

[Ioana Lasc (ilasc)]

looks good

review: Approve

=== modified file 'NEWS.rst'
--- NEWS.rst	2020-09-28 14:43:25 +0000
+++ NEWS.rst	2021-01-04 11:27:11 +0000
@@ -2,6 +2,15 @@
 NEWS for lazr.restful
 =====================
 
+0.24.0
+======
+
+Rework ``lazr.restful.testing.webservice.WebServiceCaller`` to send named
+POSTs as ``multipart/form-data`` requests.  Arguments that are instances of
+``io.BufferedIOBase`` are sent as-is rather than being encoded as JSON,
+allowing robust use of binary arguments on both Python 2 and 3
+(bug 1116954).
+
 0.23.0 (2020-09-28)
 ===================
 

=== modified file 'setup.py'
--- setup.py	2020-08-21 16:16:09 +0000
+++ setup.py	2021-01-04 11:27:11 +0000
@@ -61,7 +61,7 @@
         'pytz',
         'setuptools',
         'simplejson>=2.1.0',
-        'six',
+        'six>=1.12.0',
         'testtools',
         'van.testing',
         'wsgiref; python_version < "3"',

=== modified file 'src/lazr/restful/example/base/tests/hostedfile.txt'
--- src/lazr/restful/example/base/tests/hostedfile.txt	2020-09-07 09:54:10 +0000
+++ src/lazr/restful/example/base/tests/hostedfile.txt	2021-01-04 11:27:11 +0000
@@ -118,15 +118,22 @@
 real web service to define a more complex named operation that
 manipulates uploaded files.
 
-    >>> print(webservice.named_post(greens_url, 'replace_cover',
-    ...                             cover='\x01\x02'))
+    >>> import io
+    >>> print(webservice.named_post(
+    ...     greens_url, 'replace_cover',
+    ...     cover=io.BytesIO(b'\x01\x02\r\n\x81\r\x82\n')))
     HTTP/1.1 200 Ok
     ...
-    >>> print(webservice.get(greens_cover))
+    >>> result = webservice.get(greens_cover)
+    >>> print(result)
     HTTP/1.1 303 See Other
     ...
     Location: http://cookbooks.dev/devel/filemanager/2
     ...
+    >>> filemanager_url = result.getheader('location')
+    >>> response = webservice.get(filemanager_url)
+    >>> response.body == b'\x01\x02\r\n\x81\r\x82\n'
+    True
 
 Deleting a cover (with DELETE) disables the redirect.
 

=== modified file 'src/lazr/restful/testing/webservice.py'
--- src/lazr/restful/testing/webservice.py	2020-09-28 08:59:17 +0000
+++ src/lazr/restful/testing/webservice.py	2021-01-04 11:27:11 +0000
@@ -21,7 +21,10 @@
     'WebServiceTestPublication',
     ]
 
-from io import BytesIO
+from email.utils import quote as email_quote
+import io
+import random
+import re
 import simplejson
 import sys
 from types import ModuleType
@@ -32,7 +35,6 @@
 import six
 from six.moves.urllib.parse import (
     quote,
-    urlencode,
     urljoin,
     )
 from zope.component import (
@@ -83,10 +85,10 @@
     if environ is not None:
         test_environ.update(environ)
     if body is None:
-        body_instream = BytesIO(b'')
+        body_instream = io.BytesIO(b'')
     else:
         test_environ['CONTENT_LENGTH'] = len(body)
-        body_instream = BytesIO(body)
+        body_instream = io.BytesIO(body)
     request = config.createRequest(body_instream, test_environ)
 
     request.processInputs()
@@ -348,15 +350,103 @@
         return self._make_request_with_entity_body(
             path, 'POST', media_type, data, headers, api_version=api_version)
 
-    def _convertArgs(self, operation_name, args):
-        """Encode and convert keyword arguments."""
-        args['ws.op'] = operation_name
-        # To be properly marshalled all values must be strings or converted to
-        # JSON.
-        for key, value in args.items():
-            if not isinstance(value, six.string_types):
-                args[key] = simplejson.dumps(value)
-        return urlencode(args)
+    def _makeBoundary(self, all_parts):
+        """Make a random boundary that does not appear in `all_parts`."""
+        _width = len(repr(sys.maxsize - 1))
+        _fmt = '%%0%dd' % _width
+        token = random.randrange(sys.maxsize)
+        boundary = ('=' * 15) + (_fmt % token) + '=='
+        if all_parts is None:
+            return boundary
+        b = boundary
+        counter = 0
+        while True:
+            pattern = ('^--' + re.escape(b) + '(--)?$').encode('ascii')
+            if not re.search(pattern, all_parts, flags=re.MULTILINE):
+                break
+            b = boundary + '.' + str(counter)
+            counter += 1
+        return b
+
+    def _writeHeaders(self, buf, headers):
+        """Write MIME headers to a file object."""
+        for key, value in headers:
+            buf.write(key.encode('UTF-8'))
+            buf.write(b': ')
+            buf.write(value.encode('UTF-8'))
+            buf.write(b'\r\n')
+        buf.write(b'\r\n')
+
+    def _writeBoundary(self, buf, boundary, closing=False):
+        """Write a multipart boundary to a file object."""
+        buf.write(b'--')
+        buf.write(boundary.encode('UTF-8'))
+        if closing:
+            buf.write(b'--')
+        buf.write(b'\r\n')
+
+    def _convertArgs(self, args):
+        """Encode and convert keyword arguments to multipart/form-data.
+
+        This is very loosely based on the email module in the Python
+        standard library.  However, that module doesn't really support
+        directly embedding binary data in a form: various versions of Python
+        have mangled line separators in different ways, and none of them get
+        it quite right.  Since we only need a tiny subset of MIME here, it's
+        easier to implement it ourselves.
+
+        Binary argument values are supported, but must be passed as
+        instances of `io.BufferedIOBase` (e.g. `io.BytesIO`).
+
+        :return: a tuple of two elements: the Content-Type of the message,
+            and the entire encoded message as a byte string.
+        """
+        # Generate the subparts first so that we can calculate a safe boundary.
+        encoded_parts = []
+        for name, value in args.items():
+            buf = io.BytesIO()
+            if isinstance(value, io.BufferedIOBase):
+                ctype = 'application/octet-stream'
+                # RFC 7578 says that the filename parameter isn't mandatory
+                # in our case, but zope.publisher only considers a field to
+                # be a file upload if a filename was passed in.
+                cdisp = 'form-data; name="%s"; filename="%s"' % (
+                    email_quote(name), email_quote(name))
+            else:
+                ctype = 'text/plain; charset="utf-8"'
+                cdisp = 'form-data; name="%s"' % email_quote(name)
+            self._writeHeaders(buf, [
+                ('MIME-Version', '1.0'),
+                ('Content-Type', ctype),
+                ('Content-Disposition', cdisp),
+                ])
+            if isinstance(value, io.BufferedIOBase):
+                buf.write(value.read())
+            else:
+                if not isinstance(value, six.string_types):
+                    value = simplejson.dumps(value)
+                lines = re.split(r'\r\n|\r|\n', value)
+                for line in lines[:-1]:
+                    buf.write(six.ensure_binary(line))
+                    buf.write(b'\r\n')
+                buf.write(six.ensure_binary(lines[-1]))
+            encoded_parts.append(buf.getvalue())
+
+        # Create a suitable boundary.
+        boundary = self._makeBoundary(b'\r\n'.join(encoded_parts))
+
+        # Now we can write the multipart headers, followed by all the parts.
+        buf = io.BytesIO()
+        self._writeHeaders(buf, [])
+        for encoded_part in encoded_parts:
+            self._writeBoundary(buf, boundary)
+            buf.write(encoded_part)
+            buf.write(b'\r\n')
+        self._writeBoundary(buf, boundary, closing=True)
+
+        media_type = (
+            'multipart/form-data; boundary="%s"' % email_quote(boundary))
+        return media_type, buf.getvalue()
 
     def named_get(self, path_or_url, operation_name, headers=None,
                   api_version=None, **kwargs):
@@ -368,9 +458,10 @@
 
     def named_post(self, path, operation_name, headers=None,
                    api_version=None, **kwargs):
-        data = self._convertArgs(operation_name, kwargs)
-        return self.post(path, 'application/x-www-form-urlencoded', data,
-                         headers, api_version=api_version)
+        kwargs['ws.op'] = operation_name
+        media_type, data = self._convertArgs(kwargs)
+        return self.post(path, media_type, data, headers,
+                         api_version=api_version)
 
     def patch(self, path, media_type, data, headers=None,
               api_version=None):