s3api: Better logging for non-JSON when trying to do bucket listings

Change-Id: Ib79bfd23b7b6f40673396aafa47a0f8c4f533a97

swift/common/middleware/s3api/controllers/bucket.py

@@ -23,7 +23,8 @@ from swift.common import swob
 from swift.common.http import HTTP_OK
 from swift.common.middleware.versioned_writes.object_versioning import \
     DELETE_MARKER_CONTENT_TYPE
-from swift.common.utils import json, public, config_true_value, Timestamp
+from swift.common.utils import json, public, config_true_value, Timestamp, \
+    cap_length
 from swift.common.registry import get_swift_info
 
 from swift.common.middleware.s3api.controllers.base import Controller
@@ -343,7 +344,12 @@ class BucketController(Controller):
 
         resp = req.get_response(self.app, query=query)
 
-        objects = json.loads(resp.body)
+        try:
+            objects = json.loads(resp.body)
+        except (TypeError, ValueError):
+            self.logger.error('Got non-JSON response trying to list %s: %r',
+                              req.path, cap_length(resp.body, 60))
+            raise
 
         is_truncated = max_keys > 0 and len(objects) > max_keys
         objects = objects[:max_keys]

test/unit/common/middleware/s3api/test_bucket.py

@@ -183,6 +183,25 @@ class TestS3ApiBucket(S3ApiTestCase):
         code = self._test_method_error('GET', '/bucket', swob.HTTPServerError)
         self.assertEqual(code, 'InternalError')
 
+    @s3acl
+    def test_bucket_GET_non_json(self):
+        # Suppose some middleware accidentally makes it return txt instead
+        resp_body = b'\n'.join([b'obj%d' % i for i in range(100)])
+        self.swift.register('GET', '/v1/AUTH_test/bucket', swob.HTTPOk, {},
+                            resp_body)
+        # When we do our GET...
+        req = Request.blank('/bucket',
+                            headers={'Authorization': 'AWS test:tester:hmac',
+                                     'Date': self.get_date_header()})
+        status, headers, body = self.call_s3api(req)
+        # ...there isn't much choice but to error...
+        self.assertEqual(self._get_error_code(body), 'InternalError')
+        # ... but we should at least log the body to aid in debugging
+        self.assertIn(
+            'Got non-JSON response trying to list /bucket: %r'
+            % (resp_body[:60] + b'...'),
+            self.s3api.logger.get_lines_for_level('error'))
+
     def test_bucket_GET(self):
         bucket_name = 'junk'
         req = Request.blank('/%s' % bucket_name,