Factor out code used by packaging commands for HTTP requests (#12169).

We now have one function to prepare multipart POST requests, and we use
CRLF, as recommended by the HTTP spec (#10150).  Initial patch by John
Edmonds.

Lib/packaging/command/register.py

@@ -10,7 +10,7 @@ import urllib.request
 
 from packaging import logger
 from packaging.util import (read_pypirc, generate_pypirc, DEFAULT_REPOSITORY,
-                            DEFAULT_REALM, get_pypirc_path)
+                            DEFAULT_REALM, get_pypirc_path, encode_multipart)
 from packaging.command.cmd import Command
 
 class register(Command):
@@ -231,29 +231,11 @@ Your selection [default 1]: ''')
         if 'name' in data:
             logger.info('Registering %s to %s', data['name'], self.repository)
         # Build up the MIME payload for the urllib2 POST data
-        boundary = '--------------GHSKFJDLGDS7543FJKLFHRE75642756743254'
+        content_type, body = encode_multipart(data.items(), [])
-        sep_boundary = '\n--' + boundary
-        end_boundary = sep_boundary + '--'
-        body = io.StringIO()
-        for key, value in data.items():
-            # handle multiple entries for the same name
-            if not isinstance(value, (tuple, list)):
-                value = [value]
-
-            for value in value:
-                body.write(sep_boundary)
-                body.write('\nContent-Disposition: form-data; name="%s"'%key)
-                body.write("\n\n")
-                body.write(value)
-                if value and value[-1] == '\r':
-                    body.write('\n')  # write an extra newline (lurve Macs)
-        body.write(end_boundary)
-        body.write("\n")
-        body = body.getvalue()
 
         # build the Request
         headers = {
-            'Content-type': 'multipart/form-data; boundary=%s; charset=utf-8'%boundary,
+            'Content-type': content_type,
             'Content-length': str(len(body))
         }
         req = urllib.request.Request(self.repository, body, headers)

Lib/packaging/command/upload.py

@@ -14,7 +14,7 @@ from urllib.request import urlopen, Request
 from packaging import logger
 from packaging.errors import PackagingOptionError
 from packaging.util import (spawn, read_pypirc, DEFAULT_REPOSITORY,
-                            DEFAULT_REALM)
+                            DEFAULT_REALM, encode_multipart)
 from packaging.command.cmd import Command
 
 
@@ -131,54 +131,22 @@ class upload(Command):
         auth = b"Basic " + standard_b64encode(user_pass)
 
         # Build up the MIME payload for the POST data
-        boundary = b'--------------GHSKFJDLGDS7543FJKLFHRE75642756743254'
+        files = []
-        sep_boundary = b'\n--' + boundary
+        for key in ('content', 'gpg_signature'):
-        end_boundary = sep_boundary + b'--'
+            if key in data:
-        body = BytesIO()
+                filename_, value = data.pop(key)
+                files.append((key, filename_, value))
 
-        file_fields = ('content', 'gpg_signature')
+        content_type, body = encode_multipart(data.items(), files)
-
-        for key, value in data.items():
-            # handle multiple entries for the same name
-            if not isinstance(value, tuple):
-                value = [value]
-
-            content_dispo = '\nContent-Disposition: form-data; name="%s"' % key
-
-            if key in file_fields:
-                filename_, content = value
-                filename_ = ';filename="%s"' % filename_
-                body.write(sep_boundary)
-                body.write(content_dispo.encode('utf-8'))
-                body.write(filename_.encode('utf-8'))
-                body.write(b"\n\n")
-                body.write(content)
-            else:
-                for value in value:
-                    value = str(value).encode('utf-8')
-                    body.write(sep_boundary)
-                    body.write(content_dispo.encode('utf-8'))
-                    body.write(b"\n\n")
-                    body.write(value)
-                    if value and value.endswith(b'\r'):
-                        # write an extra newline (lurve Macs)
-                        body.write(b'\n')
-
-        body.write(end_boundary)
-        body.write(b"\n")
-        body = body.getvalue()
 
         logger.info("Submitting %s to %s", filename, self.repository)
 
         # build the Request
-        headers = {'Content-type':
+        headers = {'Content-type': content_type,
-                        'multipart/form-data; boundary=%s' %
-                        boundary.decode('ascii'),
                    'Content-length': str(len(body)),
                    'Authorization': auth}
 
-        request = Request(self.repository, data=body,
+        request = Request(self.repository, body, headers)
-                          headers=headers)
         # send the data
         try:
             result = urlopen(request)

Lib/packaging/command/upload_docs.py

@@ -10,7 +10,8 @@ import urllib.parse
 from io import BytesIO
 
 from packaging import logger
-from packaging.util import read_pypirc, DEFAULT_REPOSITORY, DEFAULT_REALM
+from packaging.util import (read_pypirc, DEFAULT_REPOSITORY, DEFAULT_REALM,
+                            encode_multipart)
 from packaging.errors import PackagingFileError
 from packaging.command.cmd import Command
 
@@ -28,49 +29,6 @@ def zip_dir(directory):
     return destination
 
 
-# grabbed from
-#    http://code.activestate.com/recipes/
-#    146306-http-client-to-post-using-multipartform-data/
-# TODO factor this out for use by install and command/upload
-
-def encode_multipart(fields, files, boundary=None):
-    """
-    *fields* is a sequence of (name: str, value: str) elements for regular
-    form fields, *files* is a sequence of (name: str, filename: str, value:
-    bytes) elements for data to be uploaded as files.
-
-    Returns (content_type: bytes, body: bytes) ready for http.client.HTTP.
-    """
-    if boundary is None:
-        boundary = b'--------------GHSKFJDLGDS7543FJKLFHRE75642756743254'
-    elif not isinstance(boundary, bytes):
-        raise TypeError('boundary is not bytes but %r' % type(boundary))
-
-    l = []
-    for key, value in fields:
-        l.extend((
-            b'--' + boundary,
-            ('Content-Disposition: form-data; name="%s"' %
-             key).encode('utf-8'),
-            b'',
-            value.encode('utf-8')))
-
-    for key, filename, value in files:
-        l.extend((
-            b'--' + boundary,
-            ('Content-Disposition: form-data; name="%s"; filename="%s"' %
-             (key, filename)).encode('utf-8'),
-            b'',
-            value))
-    l.append(b'--' + boundary + b'--')
-    l.append(b'')
-
-    body = b'\r\n'.join(l)
-
-    content_type = b'multipart/form-data; boundary=' + boundary
-    return content_type, body
-
-
 class upload_docs(Command):
 
     description = "upload HTML documentation to PyPI"

Lib/packaging/tests/test_command_register.py

@@ -152,7 +152,7 @@ class RegisterTestCase(support.TempdirManager,
         req1 = dict(self.conn.reqs[0].headers)
         req2 = dict(self.conn.reqs[1].headers)
         self.assertEqual(req2['Content-length'], req1['Content-length'])
-        self.assertIn('xxx', self.conn.reqs[1].data)
+        self.assertIn(b'xxx', self.conn.reqs[1].data)
 
     def test_password_not_in_file(self):
 
@@ -180,8 +180,8 @@ class RegisterTestCase(support.TempdirManager,
         self.assertEqual(len(self.conn.reqs), 1)
         req = self.conn.reqs[0]
         headers = dict(req.headers)
-        self.assertEqual(headers['Content-length'], '608')
+        self.assertEqual(headers['Content-length'], '628')
-        self.assertIn('tarek', req.data)
+        self.assertIn(b'tarek', req.data)
 
     def test_password_reset(self):
         # this test runs choice 3
@@ -195,8 +195,8 @@ class RegisterTestCase(support.TempdirManager,
         self.assertEqual(len(self.conn.reqs), 1)
         req = self.conn.reqs[0]
         headers = dict(req.headers)
-        self.assertEqual(headers['Content-length'], '290')
+        self.assertEqual(headers['Content-length'], '298')
-        self.assertIn('tarek', req.data)
+        self.assertIn(b'tarek', req.data)
 
     @unittest.skipUnless(DOCUTILS_SUPPORT, 'needs docutils')
     def test_strict(self):

Lib/packaging/tests/test_command_upload_docs.py

@@ -9,8 +9,7 @@ except ImportError:
     _ssl = None
 
 from packaging.command import upload_docs as upload_docs_mod
-from packaging.command.upload_docs import (upload_docs, zip_dir,
+from packaging.command.upload_docs import upload_docs, zip_dir
-                                           encode_multipart)
 from packaging.dist import Distribution
 from packaging.errors import PackagingFileError, PackagingOptionError
 
@@ -23,23 +22,6 @@ except ImportError:
     PyPIServerTestCase = object
 
 
-EXPECTED_MULTIPART_OUTPUT = [
-    b'---x',
-    b'Content-Disposition: form-data; name="username"',
-    b'',
-    b'wok',
-    b'---x',
-    b'Content-Disposition: form-data; name="password"',
-    b'',
-    b'secret',
-    b'---x',
-    b'Content-Disposition: form-data; name="picture"; filename="wok.png"',
-    b'',
-    b'PNG89',
-    b'---x--',
-    b'',
-]
-
 PYPIRC = """\
 [distutils]
 index-servers = server1
@@ -108,13 +90,6 @@ class UploadDocsTestCase(support.TempdirManager,
         zip_f = zipfile.ZipFile(compressed)
         self.assertEqual(zip_f.namelist(), ['index.html', 'docs/index.html'])
 
-    def test_encode_multipart(self):
-        fields = [('username', 'wok'), ('password', 'secret')]
-        files = [('picture', 'wok.png', b'PNG89')]
-        content_type, body = encode_multipart(fields, files, b'-x')
-        self.assertEqual(b'multipart/form-data; boundary=-x', content_type)
-        self.assertEqual(EXPECTED_MULTIPART_OUTPUT, body.split(b'\r\n'))
-
     def prepare_command(self):
         self.cmd.upload_dir = self.prepare_sample_dir()
         self.cmd.ensure_finalized()

Lib/packaging/tests/test_util.py

@@ -19,7 +19,7 @@ from packaging.util import (
     get_compiler_versions, _MAC_OS_X_LD_VERSION, byte_compile, find_packages,
     spawn, get_pypirc_path, generate_pypirc, read_pypirc, resolve_name, iglob,
     RICH_GLOB, egginfo_to_distinfo, is_setuptools, is_distutils, is_packaging,
-    get_install_method, cfg_to_args)
+    get_install_method, cfg_to_args, encode_multipart)
 
 
 PYPIRC = """\
@@ -54,6 +54,23 @@ username:tarek
 password:xxx
 """
 
+EXPECTED_MULTIPART_OUTPUT = [
+    b'---x',
+    b'Content-Disposition: form-data; name="username"',
+    b'',
+    b'wok',
+    b'---x',
+    b'Content-Disposition: form-data; name="password"',
+    b'',
+    b'secret',
+    b'---x',
+    b'Content-Disposition: form-data; name="picture"; filename="wok.png"',
+    b'',
+    b'PNG89',
+    b'---x--',
+    b'',
+]
+
 
 class FakePopen:
     test_class = None
@@ -525,6 +542,13 @@ class UtilTestCase(support.EnvironRestorer,
         self.assertEqual(args['scripts'], dist.scripts)
         self.assertEqual(args['py_modules'], dist.py_modules)
 
+    def test_encode_multipart(self):
+        fields = [('username', 'wok'), ('password', 'secret')]
+        files = [('picture', 'wok.png', b'PNG89')]
+        content_type, body = encode_multipart(fields, files, b'-x')
+        self.assertEqual(b'multipart/form-data; boundary=-x', content_type)
+        self.assertEqual(EXPECTED_MULTIPART_OUTPUT, body.split(b'\r\n'))
+
 
 class GlobTestCaseBase(support.TempdirManager,
                        support.LoggingCatcher,

Lib/packaging/util.py

@@ -1487,3 +1487,50 @@ def _mkpath(name, mode=0o777, verbose=True, dry_run=False):
 
         _path_created.add(abs_head)
     return created_dirs
+
+
+def encode_multipart(fields, files, boundary=None):
+    """Prepare a multipart HTTP request.
+
+    *fields* is a sequence of (name: str, value: str) elements for regular
+    form fields, *files* is a sequence of (name: str, filename: str, value:
+    bytes) elements for data to be uploaded as files.
+
+    Returns (content_type: bytes, body: bytes) ready for http.client.HTTP.
+    """
+    # Taken from
+    # http://code.activestate.com/recipes/146306-http-client-to-post-using-multipartform-data/
+
+    if boundary is None:
+        boundary = b'--------------GHSKFJDLGDS7543FJKLFHRE75642756743254'
+    elif not isinstance(boundary, bytes):
+        raise TypeError('boundary must be bytes, not %r' % type(boundary))
+
+    l = []
+    for key, values in fields:
+        # handle multiple entries for the same name
+        if not isinstance(values, (tuple, list)):
+            values=[values]
+
+        for value in values:
+            l.extend((
+                b'--' + boundary,
+                ('Content-Disposition: form-data; name="%s"' %
+                 key).encode('utf-8'),
+                b'',
+                value.encode('utf-8')))
+
+    for key, filename, value in files:
+        l.extend((
+            b'--' + boundary,
+            ('Content-Disposition: form-data; name="%s"; filename="%s"' %
+             (key, filename)).encode('utf-8'),
+            b'',
+            value))
+
+    l.append(b'--' + boundary + b'--')
+    l.append(b'')
+
+    body = b'\r\n'.join(l)
+    content_type = b'multipart/form-data; boundary=' + boundary
+    return content_type, body

Misc/ACKS

@@ -263,6 +263,7 @@ Maxim Dzumanenko
 Walter Dörwald
 Hans Eckardt
 Rodolpho Eckhardt
+John Edmonds
 Grant Edwards
 John Ehresman
 Eric Eisner

Misc/NEWS

@@ -219,6 +219,9 @@ Core and Builtins
 Library
 -------
 
+- Issues #12169 and #10510: Factor out code used by various packaging commands
+  to make HTTP POST requests, and make sure it uses CRLF.
+
 - Issue #12016: Multibyte CJK decoders now resynchronize faster. They only
   ignore the first byte of an invalid byte sequence. For example,
   b'\xff\n'.decode('gb2312', 'replace') gives '\ufffd\n' instead of '\ufffd'.