Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ensuring response from httplib2 is treated as str in Python 2 and bytes in Python3 #724

Closed
2 changes: 1 addition & 1 deletion gcloud/datastore/test_connection.py
Original file line number Diff line number Diff line change
Expand Up @@ -152,7 +152,7 @@ def test__request_not_200(self):
METHOD = 'METHOD'
DATA = 'DATA'
conn = self._makeOne()
conn._http = Http({'status': '400'}, 'Entity value is indexed.')
conn._http = Http({'status': '400'}, b'Entity value is indexed.')
with self.assertRaises(BadRequest) as e:
conn._request(DATASET_ID, METHOD, DATA)
expected_message = '400 Entity value is indexed.'
Expand Down
15 changes: 8 additions & 7 deletions gcloud/exceptions.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
"""

import json
import six

_HTTP_CODE_TO_EXCEPTION = {} # populated at end of module

Expand Down Expand Up @@ -171,18 +172,18 @@ def make_exception(response, content, use_json=True):
:rtype: instance of :class:`GCloudError`, or a concrete subclass.
:returns: Exception specific to the error response.
"""
message = content
errors = ()

if isinstance(content, str):
if isinstance(content, six.binary_type):
content_decoded = content.decode('utf-8')
if use_json:
payload = json.loads(content)
payload = json.loads(content_decoded)
else:
payload = {}
payload = {
'message': content_decoded
}
else:
payload = content

message = payload.get('message', message)
message = payload.get('message', '')
errors = payload.get('error', {}).get('errors', ())

try:
Expand Down
6 changes: 5 additions & 1 deletion gcloud/storage/connection.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@

import json

import six
from six.moves.urllib.parse import urlencode # pylint: disable=F0401

from gcloud import connection as base_connection
Expand Down Expand Up @@ -231,10 +232,13 @@ def api_request(self, method, path, query_params=None,
if not 200 <= response.status < 300:
raise make_exception(response, content)

if not isinstance(content, six.binary_type):
raise TypeError('Expected binary type, got %s' % type(content))

if content and expect_json:
content_type = response.get('content-type', '')
if not content_type.startswith('application/json'):
raise TypeError('Expected JSON, got %s' % content_type)
return json.loads(content)
return json.loads(content.decode('utf-8'))

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.


return content
15 changes: 8 additions & 7 deletions gcloud/storage/test_api.py
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ def test_miss(self):
])
http = conn._http = Http(
{'status': '404', 'content-type': 'application/json'},
'{}',
b'{}',
)
bucket = self._callFUT(NONESUCH, connection=conn)
self.assertEqual(bucket, None)
Expand All @@ -58,7 +58,7 @@ def _lookup_bucket_hit_helper(self, use_default=False):
])
http = conn._http = Http(
{'status': '200', 'content-type': 'application/json'},
'{"name": "%s"}' % BLOB_NAME,
'{{"name": "{0}"}}'.format(BLOB_NAME).encode('utf-8'),
)

if use_default:
Expand Down Expand Up @@ -98,7 +98,7 @@ def test_empty(self):
])
http = conn._http = Http(
{'status': '200', 'content-type': 'application/json'},
'{}',
b'{}',
)
buckets = list(self._callFUT(conn))
self.assertEqual(len(buckets), 0)
Expand All @@ -119,7 +119,8 @@ def _get_all_buckets_non_empty_helper(self, use_default=False):
])
http = conn._http = Http(
{'status': '200', 'content-type': 'application/json'},
'{"items": [{"name": "%s"}]}' % BUCKET_NAME,
'{{"items": [{{"name": "{0}"}}]}}'.format(BUCKET_NAME)
.encode('utf-8'),
)

if use_default:
Expand Down Expand Up @@ -161,7 +162,7 @@ def test_miss(self):
])
http = conn._http = Http(
{'status': '404', 'content-type': 'application/json'},
'{}',
b'{}',
)
self.assertRaises(NotFound, self._callFUT, NONESUCH, connection=conn)
self.assertEqual(http._called_with['method'], 'GET')
Expand All @@ -183,7 +184,7 @@ def _get_bucket_hit_helper(self, use_default=False):
])
http = conn._http = Http(
{'status': '200', 'content-type': 'application/json'},
'{"name": "%s"}' % BLOB_NAME,
'{{"name": "{0}"}}'.format(BLOB_NAME).encode('utf-8'),
)

if use_default:
Expand Down Expand Up @@ -226,7 +227,7 @@ def _create_bucket_success_helper(self, use_default=False):
])
http = conn._http = Http(
{'status': '200', 'content-type': 'application/json'},
'{"name": "%s"}' % BLOB_NAME,
'{{"name": "{0}"}}'.format(BLOB_NAME).encode('utf-8'),
)

if use_default:
Expand Down
33 changes: 21 additions & 12 deletions gcloud/storage/test_connection.py
Original file line number Diff line number Diff line change
Expand Up @@ -106,12 +106,12 @@ def test__make_request_no_data_no_content_type_no_headers(self):
URI = 'http://example.com/test'
http = conn._http = Http(
{'status': '200', 'content-type': 'text/plain'},
'',
b'',
)
headers, content = conn._make_request('GET', URI)
self.assertEqual(headers['status'], '200')
self.assertEqual(headers['content-type'], 'text/plain')
self.assertEqual(content, '')
self.assertEqual(content, b'')
self.assertEqual(http._called_with['method'], 'GET')
self.assertEqual(http._called_with['uri'], URI)
self.assertEqual(http._called_with['body'], None)
Expand All @@ -128,7 +128,7 @@ def test__make_request_w_data_no_extra_headers(self):
URI = 'http://example.com/test'
http = conn._http = Http(
{'status': '200', 'content-type': 'text/plain'},
'',
b'',
)
conn._make_request('GET', URI, {}, 'application/json')
self.assertEqual(http._called_with['method'], 'GET')
Expand All @@ -148,7 +148,7 @@ def test__make_request_w_extra_headers(self):
URI = 'http://example.com/test'
http = conn._http = Http(
{'status': '200', 'content-type': 'text/plain'},
'',
b'',
)
conn._make_request('GET', URI, headers={'X-Foo': 'foo'})
self.assertEqual(http._called_with['method'], 'GET')
Expand All @@ -173,7 +173,7 @@ def test_api_request_defaults(self):
])
http = conn._http = Http(
{'status': '200', 'content-type': 'application/json'},
'{}',
b'{}',
)
self.assertEqual(conn.api_request('GET', PATH), {})
self.assertEqual(http._called_with['method'], 'GET')
Expand All @@ -191,7 +191,7 @@ def test_api_request_w_non_json_response(self):
conn = self._makeOne(PROJECT)
conn._http = Http(
{'status': '200', 'content-type': 'text/plain'},
'CONTENT',
b'CONTENT',
)

self.assertRaises(TypeError, conn.api_request, 'GET', '/')
Expand All @@ -201,10 +201,10 @@ def test_api_request_wo_json_expected(self):
conn = self._makeOne(PROJECT)
conn._http = Http(
{'status': '200', 'content-type': 'text/plain'},
'CONTENT',
b'CONTENT',
)
self.assertEqual(conn.api_request('GET', '/', expect_json=False),
'CONTENT')
b'CONTENT')

def test_api_request_w_query_params(self):
from six.moves.urllib.parse import parse_qsl
Expand All @@ -213,7 +213,7 @@ def test_api_request_w_query_params(self):
conn = self._makeOne(PROJECT)
http = conn._http = Http(
{'status': '200', 'content-type': 'application/json'},
'{}',
b'{}',
)
self.assertEqual(conn.api_request('GET', '/', {'foo': 'bar'}), {})
self.assertEqual(http._called_with['method'], 'GET')
Expand Down Expand Up @@ -247,7 +247,7 @@ def test_api_request_w_data(self):
])
http = conn._http = Http(
{'status': '200', 'content-type': 'application/json'},
'{}',
b'{}',
)
self.assertEqual(conn.api_request('POST', '/', data=DATA), {})
self.assertEqual(http._called_with['method'], 'POST')
Expand All @@ -267,7 +267,7 @@ def test_api_request_w_404(self):
conn = self._makeOne(PROJECT)
conn._http = Http(
{'status': '404', 'content-type': 'text/plain'},
'{}'
b'{}',
)
self.assertRaises(NotFound, conn.api_request, 'GET', '/')

Expand All @@ -277,10 +277,19 @@ def test_api_request_w_500(self):
conn = self._makeOne(PROJECT)
conn._http = Http(
{'status': '500', 'content-type': 'text/plain'},
'{}',
b'{}',
)
self.assertRaises(InternalServerError, conn.api_request, 'GET', '/')

def test_api_request_non_binary_response(self):
PROJECT = 'project'
conn = self._makeOne(PROJECT)
conn._http = Http(
{'status': '200', 'content-type': 'application/json'},
u'CONTENT',
)
self.assertRaises(TypeError, conn.api_request, 'GET', '/')


class Http(object):

Expand Down
2 changes: 1 addition & 1 deletion gcloud/test_exceptions.py
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ def _callFUT(self, response, content):
def test_hit_w_content_as_str(self):
from gcloud.exceptions import NotFound
response = _Response(404)
content = '{"message": "Not Found"}'
content = b'{"message": "Not Found"}'
exception = self._callFUT(response, content)
self.assertTrue(isinstance(exception, NotFound))
self.assertEqual(exception.message, 'Not Found')
Expand Down