Under Python 3; fix bug that caused invalid Content-ID header when custom request_id supplied (#545)

Fixes bug under Python 3 caused by incorrect RFC2822 Content-ID header folding (see #164, #536)

Allows custom request ids to contain the '+' character also.

Closes #164

Closes #536

Author: mcdonc

googleapiclient/http.py

@@ -1171,7 +1171,10 @@ def _id_to_header(self, id_):
     if self._base_id is None:
       self._base_id = uuid.uuid4()
 
-    return '<%s+%s>' % (self._base_id, quote(id_))
+    # NB: we intentionally leave whitespace between base/id and '+', so RFC2822
+    # line folding works properly on Python 3; see
+    # https://github.com/google/google-api-python-client/issues/164
+    return '<%s + %s>' % (self._base_id, quote(id_))
 
   def _header_to_id(self, header):
     """Convert a Content-ID header value to an id.
@@ -1192,7 +1195,7 @@ def _header_to_id(self, header):
       raise BatchError("Invalid value for Content-ID: %s" % header)
     if '+' not in header:
       raise BatchError("Invalid value for Content-ID: %s" % header)
-    base, id_ = header[1:-1].rsplit('+', 1)
+    base, id_ = header[1:-1].split(' + ', 1)
 
     return unquote(id_)
 
@@ -1302,8 +1305,8 @@ def add(self, request, callback=None, request_id=None):
         request id, and the second is the deserialized response object. The
         third is an googleapiclient.errors.HttpError exception object if an HTTP error
         occurred while processing the request, or None if no errors occurred.
-      request_id: string, A unique id for the request. The id will be passed to
-        the callback with the response.
+      request_id: string, A unique id for the request. The id will be passed
+        to the callback with the response.
 
     Returns:
       None

tests/test_http.py

@@ -635,7 +635,7 @@ def test_media_io_base_download_unknown_media_size(self):
 BATCH_RESPONSE = b"""--batch_foobarbaz
 Content-Type: application/http
 Content-Transfer-Encoding: binary
-Content-ID: <randomness+1>
+Content-ID: <randomness + 1>
 
 HTTP/1.1 200 OK
 Content-Type: application/json
@@ -645,7 +645,7 @@ def test_media_io_base_download_unknown_media_size(self):
 --batch_foobarbaz
 Content-Type: application/http
 Content-Transfer-Encoding: binary
-Content-ID: <randomness+2>
+Content-ID: <randomness + 2>
 
 HTTP/1.1 200 OK
 Content-Type: application/json
@@ -657,7 +657,7 @@ def test_media_io_base_download_unknown_media_size(self):
 BATCH_ERROR_RESPONSE = b"""--batch_foobarbaz
 Content-Type: application/http
 Content-Transfer-Encoding: binary
-Content-ID: <randomness+1>
+Content-ID: <randomness + 1>
 
 HTTP/1.1 200 OK
 Content-Type: application/json
@@ -667,7 +667,7 @@ def test_media_io_base_download_unknown_media_size(self):
 --batch_foobarbaz
 Content-Type: application/http
 Content-Transfer-Encoding: binary
-Content-ID: <randomness+2>
+Content-ID: <randomness + 2>
 
 HTTP/1.1 403 Access Not Configured
 Content-Type: application/json
@@ -693,7 +693,7 @@ def test_media_io_base_download_unknown_media_size(self):
 BATCH_RESPONSE_WITH_401 = b"""--batch_foobarbaz
 Content-Type: application/http
 Content-Transfer-Encoding: binary
-Content-ID: <randomness+1>
+Content-ID: <randomness + 1>
 
 HTTP/1.1 401 Authorization Required
 Content-Type: application/json
@@ -704,7 +704,7 @@ def test_media_io_base_download_unknown_media_size(self):
 --batch_foobarbaz
 Content-Type: application/http
 Content-Transfer-Encoding: binary
-Content-ID: <randomness+2>
+Content-ID: <randomness + 2>
 
 HTTP/1.1 200 OK
 Content-Type: application/json
@@ -716,7 +716,7 @@ def test_media_io_base_download_unknown_media_size(self):
 BATCH_SINGLE_RESPONSE = b"""--batch_foobarbaz
 Content-Type: application/http
 Content-Transfer-Encoding: binary
-Content-ID: <randomness+1>
+Content-ID: <randomness + 1>
 
 HTTP/1.1 200 OK
 Content-Type: application/json
@@ -1211,6 +1211,39 @@ def test_execute_request_body(self):
       header = parts[1].splitlines()[1]
       self.assertEqual('Content-Type: application/http', header)
 
+  def test_execute_request_body_with_custom_long_request_ids(self):
+    batch = BatchHttpRequest()
+
+    batch.add(self.request1, request_id='abc'*20)
+    batch.add(self.request2, request_id='def'*20)
+    http = HttpMockSequence([
+      ({'status': '200',
+        'content-type': 'multipart/mixed; boundary="batch_foobarbaz"'},
+        'echo_request_body'),
+      ])
+    try:
+      batch.execute(http=http)
+      self.fail('Should raise exception')
+    except BatchError as e:
+      boundary, _ = e.content.split(None, 1)
+      self.assertEqual('--', boundary[:2])
+      parts = e.content.split(boundary)
+      self.assertEqual(4, len(parts))
+      self.assertEqual('', parts[0])
+      self.assertEqual('--', parts[3].rstrip())
+      for partindex, request_id in ((1, 'abc'*20), (2, 'def'*20)):
+        lines = parts[partindex].splitlines()
+        for n, line in enumerate(lines):
+          if line.startswith('Content-ID:'):
+            # assert correct header folding
+            self.assertTrue(line.endswith('+'), line)
+            header_continuation = lines[n+1]
+            self.assertEqual(
+              header_continuation,
+              ' %s>' % request_id,
+              header_continuation
+            )
+
   def test_execute_initial_refresh_oauth2(self):
     batch = BatchHttpRequest()
     callbacks = Callbacks()