Switched headers to be normalized as strings, not bytes, in keeping with httplib2.

foozlevazquez · foozlevazquez · commit f41eb746a3ae · 2015-02-20T23:31:06.000-07:00
diff --git a/oauth2client/client.py b/oauth2client/client.py
@@ -397,9 +397,32 @@ def delete(self):
     finally:
       self.release_lock()
 
+# Since strings ARE unicode in Python3, "cleaning" the string doesn't mean
+# just converting to str.
+#
+#  As per the httplib2 documentation:
+#
+#    ** THE RESPONSE HEADERS ARE STRINGS, BUT THE CONTENT BODY IS BYTES **
+#
+def _clean_header(s):
+  """Always returns something of type str.  Raise UnicodeEncodeError if
+  unconvertable to ascii."""
+
+  if not isinstance(s, str):
+    # str(b'foo') will return "b'foo'" in Py3, not what we want.
+    if isinstance(s, bytes):
+      # Binary string in Py3
+      s = s.decode('utf-8')
+    else:
+      s = str(s)
+
+  # We're trying to generate the UnicodeEncodeError here if the string is
+  # unconvertable, AND keep this a string in both Py2 and Py3.
+  s = str((s.encode('ascii')).decode('utf-8'))
+  return s
 
 def clean_headers(headers):
-  """Forces header keys and values to be strings, i.e not unicode.
+  """Forces header keys and values to be strings suitable for httplib2.
 
   The httplib module just concats the header keys and values in a way that may
   make the message header a unicode string, which, if it then tries to
@@ -414,8 +437,8 @@ def clean_headers(headers):
   clean = {}
   try:
     for k, v in six.iteritems(headers):
-      clean_k = k if isinstance(k, bytes) else str(k).encode('ascii')
-      clean_v = v if isinstance(v, bytes) else str(v).encode('ascii')
+      clean_k = _clean_header(k)
+      clean_v = _clean_header(v)
       clean[clean_k] = clean_v
   except UnicodeEncodeError:
     raise NonAsciiHeaderError(k + ': ' + v)
diff --git a/tests/test_appengine.py b/tests/test_appengine.py
@@ -29,7 +29,11 @@
 import time
 import unittest
 import urllib
-import urlparse
+
+try:
+  import urlparse
+except ImportError:
+  from urllib.parse import urlparse
 
 import dev_appserver
 dev_appserver.fix_sys_path()
diff --git a/tests/test_jwt.py b/tests/test_jwt.py
@@ -220,7 +220,7 @@ def test_credentials_good(self):
     ])
     http = credentials.authorize(http)
     resp, content = http.request('http://example.org')
-    self.assertEqual(b'Bearer 1/3w', content[b'Authorization'])
+    self.assertEqual('Bearer 1/3w', content['Authorization'])
 
   def test_credentials_to_from_json(self):
     private_key = datafile('privatekey.%s' % self.format)
@@ -257,7 +257,7 @@ def test_credentials_refresh_without_storage(self):
 
     content = self._credentials_refresh(credentials)
 
-    self.assertEqual(b'Bearer 3/3w', content[b'Authorization'])
+    self.assertEqual('Bearer 3/3w', content['Authorization'])
 
   def test_credentials_refresh_with_storage(self):
     private_key = datafile('privatekey.%s' % self.format)
@@ -275,7 +275,7 @@ def test_credentials_refresh_with_storage(self):
 
     content = self._credentials_refresh(credentials)
 
-    self.assertEqual(b'Bearer 3/3w', content[b'Authorization'])
+    self.assertEqual('Bearer 3/3w', content['Authorization'])
     os.unlink(filename)
 
 
diff --git a/tests/test_oauth2client.py b/tests/test_oauth2client.py
@@ -20,6 +20,7 @@
 Unit tests for oauth2client.
 """
 
+# pylint: disable=bad-indentation
 __author__ = 'jcgregorio@google.com (Joe Gregorio)'
 
 import base64
@@ -531,7 +532,7 @@ def test_token_refresh_success(self):
       ])
       http = self.credentials.authorize(http)
       resp, content = http.request('http://example.com')
-      self.assertEqual(b'Bearer 1/3w', content[b'Authorization'])
+      self.assertEqual('Bearer 1/3w', content['Authorization'])
       self.assertFalse(self.credentials.access_token_expired)
       self.assertEqual(token_response, self.credentials.token_response)
 
@@ -599,17 +600,17 @@ def test_unicode_header_checks(self):
 
     # First, test that we correctly encode basic objects, making sure
     # to include a bytes object. Note that oauth2client will normalize
-    # everything to bytes, no matter what python version we're in.
+    # everything to strings, no matter what python version we're in.
     http = credentials.authorize(HttpMock(headers={'status': '200'}))
     headers = {u'foo': 3, b'bar': True, 'baz': b'abc'}
-    cleaned_headers = {b'foo': b'3', b'bar': b'True', b'baz': b'abc'}
+    cleaned_headers = {'foo': '3', 'bar': 'True', 'baz': 'abc'}
     http.request(u'http://example.com', method=u'GET', headers=headers)
     for k, v in cleaned_headers.items():
       self.assertTrue(k in http.headers)
       self.assertEqual(v, http.headers[k])
 
     # Next, test that we do fail on unicode.
-    unicode_str = six.unichr(40960) + 'abcd'
+    unicode_str = u'\u2602' + 'abcd'
     self.assertRaises(
         NonAsciiHeaderError,
         http.request,
@@ -631,14 +632,21 @@ def test_no_unicode_in_request_params(self):
     http = HttpMock(headers={'status': '200'})
     http = credentials.authorize(http)
     http.request(u'http://example.com', method=u'GET', headers={u'foo': u'bar'})
+
+    # oauth2client uses httplib2 and httplib2 says:
+    # "** THE RESPONSE HEADERS ARE STRINGS, BUT THE CONTENT BODY IS BYTES **"
+    # and from https://github.com/jcgregorio/httplib2/wiki/Examples-Python3
+    # "In httplib2, the response headers are strings..."
+    #
+    # So, the headers should be of type str.
     for k, v in six.iteritems(http.headers):
-      self.assertEqual(six.binary_type, type(k))
-      self.assertEqual(six.binary_type, type(v))
+      self.assertTrue(isinstance(k, str))
+      self.assertTrue(isinstance(v, str))
 
     # Test again with unicode strings that can't simply be converted to ASCII.
     try:
       http.request(
-          u'http://example.com', method=u'GET', headers={u'foo': u'\N{COMET}'})
+          u'http://example.com', method=u'GET', headers={u'foo': u'\u2602'})
       self.fail('Expected exception to be raised.')
     except NonAsciiHeaderError:
       pass
@@ -724,7 +732,7 @@ def test_auth_header_sent(self):
       ])
     http = self.credentials.authorize(http)
     resp, content = http.request('http://example.com')
-    self.assertEqual(b'Bearer foo', content[b'Authorization'])
+    self.assertEqual('Bearer foo', content['Authorization'])
 
 
 class TestAssertionCredentials(unittest.TestCase):
@@ -755,7 +763,7 @@ def test_assertion_refresh(self):
       ])
     http = self.credentials.authorize(http)
     resp, content = http.request('http://example.com')
-    self.assertEqual(b'Bearer 1/3w', content[b'Authorization'])
+    self.assertEqual('Bearer 1/3w', content['Authorization'])
 
   def test_token_revoke_success(self):
     _token_revoke_test_helper(
