Remove simplejson dependency

In addition to removing an unnecessary dependency, this closes another
hole that was allowing raw bytes to appear in user-facing messages.

Change-Id: Ia0b76426a38e5a5c368c4c7e7ba2aef286758aca

requirements.txt

@@ -1,4 +1,3 @@
 futures>=2.1.3
 requests>=1.1
-simplejson>=2.0.9
 six>=1.5.2

swiftclient/client.py

@@ -21,10 +21,6 @@ import socket
 import requests
 import logging
 import warnings
-try:
-    from simplejson import loads as json_loads
-except ImportError:
-    from json import loads as json_loads
 
 from distutils.version import StrictVersion
 from requests.exceptions import RequestException, SSLError
@@ -35,7 +31,8 @@ import six
 
 from swiftclient import version as swiftclient_version
 from swiftclient.exceptions import ClientException
-from swiftclient.utils import LengthWrapper, ReadableToIterable
+from swiftclient.utils import (
+    LengthWrapper, ReadableToIterable, parse_api_response)
 
 AUTH_VERSIONS_V1 = ('1.0', '1', 1)
 AUTH_VERSIONS_V2 = ('2.0', '2', 2)
@@ -520,7 +517,7 @@ def get_account(url, token, marker=None, limit=None, prefix=None,
                               http_response_content=body)
     if resp.status == 204:
         return resp_headers, []
-    return resp_headers, json_loads(body)
+    return resp_headers, parse_api_response(resp_headers, body)
 
 
 def head_account(url, token, http_conn=None):
@@ -670,7 +667,7 @@ def get_container(url, token, container, marker=None, limit=None,
         resp_headers[header.lower()] = value
     if resp.status == 204:
         return resp_headers, []
-    return resp_headers, json_loads(body)
+    return resp_headers, parse_api_response(resp_headers, body)
 
 
 def head_container(url, token, container, http_conn=None, headers=None):
@@ -1154,7 +1151,10 @@ def get_capabilities(http_conn):
                               http_host=conn.host, http_path=parsed.path,
                               http_status=resp.status, http_reason=resp.reason,
                               http_response_content=body)
-    return json_loads(body)
+    resp_headers = {}
+    for header, value in resp.getheaders():
+        resp_headers[header.lower()] = value
+    return parse_api_response(resp_headers, body)
 
 
 class Connection(object):

swiftclient/service.py

@@ -29,10 +29,7 @@ from six.moves.queue import Empty as QueueEmpty
 from six.moves.urllib.parse import quote, unquote
 from six import Iterator, string_types
 
-try:
+import json
-    import simplejson as json
-except ImportError:
-    import json
 
 
 from swiftclient import Connection
@@ -40,7 +37,8 @@ from swiftclient.command_helpers import (
     stat_account, stat_container, stat_object
 )
 from swiftclient.utils import (
-    config_true_value, ReadableToIterable, LengthWrapper, EMPTY_ETAG
+    config_true_value, ReadableToIterable, LengthWrapper, EMPTY_ETAG,
+    parse_api_response
 )
 from swiftclient.exceptions import ClientException
 from swiftclient.multithreading import MultiThreadingManager
@@ -1515,9 +1513,10 @@ class SwiftService(object):
                 else:
                     raise part["error"]
         elif config_true_value(headers.get('x-static-large-object')):
-            _, manifest_data = conn.get_object(
+            manifest_headers, manifest_data = conn.get_object(
                 container, obj, query_string='multipart-manifest=get')
-            for chunk in json.loads(manifest_data):
+            manifest_data = parse_api_response(manifest_headers, manifest_data)
+            for chunk in manifest_data:
                 if chunk.get('sub_slo'):
                     scont, sobj = chunk['name'].lstrip('/').split('/', 1)
                     chunks.extend(self._get_chunk_data(

swiftclient/utils.py

@@ -15,6 +15,7 @@
 """Miscellaneous utility functions for use with Swift."""
 import hashlib
 import hmac
+import json
 import logging
 import time
 
@@ -28,7 +29,7 @@ def config_true_value(value):
     """
     Returns True if the value is either True or a string in TRUE_VALUES.
     Returns False otherwise.
-    This function come from swift.common.utils.config_true_value()
+    This function comes from swift.common.utils.config_true_value()
     """
     return value is True or \
         (isinstance(value, six.string_types) and value.lower() in TRUE_VALUES)
@@ -108,6 +109,16 @@ def generate_temp_url(path, seconds, key, method):
                 exp=expiration))
 
 
+def parse_api_response(headers, body):
+    charset = 'utf-8'
+    # Swift *should* be speaking UTF-8, but check content-type just in case
+    content_type = headers.get('content-type', '')
+    if '; charset=' in content_type:
+        charset = content_type.split('; charset=', 1)[1].split(';', 1)[0]
+
+    return json.loads(body.decode(charset))
+
+
 class NoopMD5(object):
     def __init__(self, *a, **kw):
         pass

tests/unit/test_service.py

@@ -910,8 +910,8 @@ class TestServiceUpload(testtools.TestCase):
                 'etag': md5(submanifest_etag.encode('ascii') +
                             seg_etag.encode('ascii')).hexdigest()}
             mock_conn.get_object.side_effect = [
-                (None, manifest),
+                ({}, manifest.encode('ascii')),
-                (None, submanifest)]
+                ({}, submanifest.encode('ascii'))]
             type(mock_conn).attempts = mock.PropertyMock(return_value=2)
 
             s = SwiftService()

tests/unit/test_shell.py

@@ -15,7 +15,6 @@
 from genericpath import getmtime
 
 import hashlib
-import json
 import mock
 import os
 import tempfile
@@ -31,8 +30,9 @@ import swiftclient.utils
 
 from os.path import basename, dirname
 from tests.unit.test_swiftclient import MockHttpTest
-from tests.unit.utils import (CaptureOutput, fake_get_auth_keystone,
+from tests.unit.utils import (
-                              _make_fake_import_keystone_client, FakeKeystone)
+    CaptureOutput, fake_get_auth_keystone, _make_fake_import_keystone_client,
+    FakeKeystone, StubResponse)
 from swiftclient.utils import EMPTY_ETAG
 
 
@@ -474,9 +474,11 @@ class TestShell(unittest.TestCase):
             {'x-static-large-object': 'false',  # For the 2nd delete call
              'content-length': '2'}
         ]
-        connection.return_value.get_object.return_value = ({}, json.dumps(
+        connection.return_value.get_object.return_value = (
-            [{'name': 'container1/old_seg1'}, {'name': 'container2/old_seg2'}]
+            {},
-        ))
+            b'[{"name": "container1/old_seg1"},'
+            b' {"name": "container2/old_seg2"}]'
+        )
         connection.return_value.put_object.return_value = EMPTY_ETAG
         swiftclient.shell.main(argv)
         connection.return_value.put_object.assert_called_with(
@@ -1763,13 +1765,10 @@ class TestCrossAccountObjectAccess(TestBase, MockHttpTest):
 
     def test_list_with_read_access(self):
         req_handler = self._fake_cross_account_auth(True, False)
-        resp_body = '{}'
+        resp_body = b'{}'
-        m = hashlib.md5()
+        resp = StubResponse(403, resp_body, {
-        m.update(resp_body.encode())
+            'etag': hashlib.md5(resp_body).hexdigest()})
-        etag = m.hexdigest()
+        fake_conn = self.fake_http_connection(resp, on_request=req_handler)
-        fake_conn = self.fake_http_connection(403, on_request=req_handler,
-                                              etags=[etag],
-                                              body=resp_body)
 
         args, env = self._make_cmd('download', cmd_args=[self.cont])
         with mock.patch('swiftclient.client._import_keystone_client',

tests/unit/test_swiftclient.py

@@ -14,7 +14,6 @@
 # limitations under the License.
 
 import logging
-import json
 
 try:
     from unittest import mock
@@ -28,7 +27,6 @@ import warnings
 import tempfile
 from hashlib import md5
 from six.moves.urllib.parse import urlparse
-from six.moves import reload_module
 
 from .utils import (MockHttpTest, fake_get_auth_keystone, StubResponse,
                     FakeKeystone, _make_fake_import_keystone_client)
@@ -65,36 +63,6 @@ class TestClientException(testtools.TestCase):
             self.assertTrue(value in str(exc))
 
 
-class TestJsonImport(testtools.TestCase):
-
-    def tearDown(self):
-        reload_module(json)
-
-        try:
-            import simplejson
-        except ImportError:
-            pass
-        else:
-            reload_module(simplejson)
-        super(TestJsonImport, self).tearDown()
-
-    def test_any(self):
-        self.assertTrue(hasattr(c, 'json_loads'))
-
-    def test_no_simplejson_falls_back_to_stdlib_when_reloaded(self):
-        # break simplejson
-        try:
-            import simplejson
-        except ImportError:
-            # not installed, so we don't have to break it for these tests
-            pass
-        else:
-            delattr(simplejson, 'loads')  # break simple json
-            reload_module(c)  # reload to repopulate json_loads
-
-        self.assertEqual(c.json_loads, json.loads)
-
-
 class MockHttpResponse(object):
     def __init__(self, status=0, headers=None, verify=False):
         self.status = status
@@ -936,7 +904,7 @@ class TestDeleteObject(MockHttpTest):
 class TestGetCapabilities(MockHttpTest):
 
     def test_ok(self):
-        conn = self.fake_http_connection(200, body='{}')
+        conn = self.fake_http_connection(200, body=b'{}')
         http_conn = conn('http://www.test.com/info')
         info = c.get_capabilities(http_conn)
         self.assertRequests([
@@ -957,7 +925,7 @@ class TestGetCapabilities(MockHttpTest):
         }
         auth_v1_response = StubResponse(headers=auth_headers)
         stub_info = {'swift': {'fake': True}}
-        info_response = StubResponse(body=json.dumps(stub_info))
+        info_response = StubResponse(body=b'{"swift":{"fake":true}}')
         fake_conn = self.fake_http_connection(auth_v1_response, info_response)
 
         conn = c.Connection('http://auth.example.com/auth/v1.0',
@@ -975,7 +943,7 @@ class TestGetCapabilities(MockHttpTest):
         fake_keystone = fake_get_auth_keystone(
             storage_url='http://storage.example.com/v1/AUTH_test')
         stub_info = {'swift': {'fake': True}}
-        info_response = StubResponse(body=json.dumps(stub_info))
+        info_response = StubResponse(body=b'{"swift":{"fake":true}}')
         fake_conn = self.fake_http_connection(info_response)
 
         os_options = {'project_id': 'test'}
@@ -993,7 +961,7 @@ class TestGetCapabilities(MockHttpTest):
 
     def test_conn_get_capabilities_with_url_param(self):
         stub_info = {'swift': {'fake': True}}
-        info_response = StubResponse(body=json.dumps(stub_info))
+        info_response = StubResponse(body=b'{"swift":{"fake":true}}')
         fake_conn = self.fake_http_connection(info_response)
 
         conn = c.Connection('http://auth.example.com/auth/v1.0',
@@ -1009,7 +977,7 @@ class TestGetCapabilities(MockHttpTest):
 
     def test_conn_get_capabilities_with_preauthurl_param(self):
         stub_info = {'swift': {'fake': True}}
-        info_response = StubResponse(body=json.dumps(stub_info))
+        info_response = StubResponse(body=b'{"swift":{"fake":true}}')
         fake_conn = self.fake_http_connection(info_response)
 
         storage_url = 'http://storage.example.com/v1/AUTH_test'
@@ -1025,7 +993,7 @@ class TestGetCapabilities(MockHttpTest):
 
     def test_conn_get_capabilities_with_os_options(self):
         stub_info = {'swift': {'fake': True}}
-        info_response = StubResponse(body=json.dumps(stub_info))
+        info_response = StubResponse(body=b'{"swift":{"fake":true}}')
         fake_conn = self.fake_http_connection(info_response)
 
         storage_url = 'http://storage.example.com/v1/AUTH_test'
@@ -1165,7 +1133,7 @@ class TestConnection(MockHttpTest):
 
             for method, args in method_signatures:
                 c.http_connection = self.fake_http_connection(
-                    200, body='[]', storage_url=static_url)
+                    200, body=b'[]', storage_url=static_url)
                 method(*args)
                 self.assertEqual(len(self.request_log), 1)
                 for request in self.iter_request_log():