Merge "xlo: Drain error responses"

swift/common/middleware/dlo.py

@@ -130,7 +130,8 @@ from swift.common.swob import Request, Response, \
     HTTPRequestedRangeNotSatisfiable, HTTPBadRequest, HTTPConflict, \
     str_to_wsgi, wsgi_to_str, wsgi_quote, wsgi_unquote, normalize_etag
 from swift.common.utils import get_logger, \
-    RateLimitedIterator, quote, close_if_possible, closing_if_possible
+    RateLimitedIterator, quote, close_if_possible, closing_if_possible, \
+    drain_and_close
 from swift.common.request_helpers import SegmentedIterable, \
     update_ignore_range_header
 from swift.common.wsgi import WSGIContext, make_subrequest, load_app_config
@@ -376,6 +377,10 @@ class GetContext(WSGIContext):
         # make sure this response is for a dynamic large object manifest
         for header, value in self._response_headers:
             if (header.lower() == 'x-object-manifest'):
+                content_length = self._response_header_value('content-length')
+                if content_length is not None and int(content_length) < 1024:
+                    # Go ahead and consume small bodies
+                    drain_and_close(resp_iter)
                 close_if_possible(resp_iter)
                 response = self.get_or_head_response(
                     req, wsgi_to_str(wsgi_unquote(value)))

swift/common/middleware/slo.py

@@ -547,11 +547,15 @@ class SloGetContext(WSGIContext):
         sub_resp = sub_req.get_response(self.slo.app)
 
         if not sub_resp.is_success:
-            close_if_possible(sub_resp.app_iter)
+            # Error message should be short
-            raise ListingIterError(
+            body = sub_resp.body
-                'while fetching %s, GET of submanifest %s '
+            if not six.PY2:
-                'failed with status %d' % (req.path, sub_req.path,
+                body = body.decode('utf-8')
-                                           sub_resp.status_int))
+            msg = ('while fetching %s, GET of submanifest %s '
+                   'failed with status %d (%s)')
+            raise ListingIterError(msg % (
+                req.path, sub_req.path, sub_resp.status_int,
+                body if len(body) <= 60 else body[:57] + '...'))
 
         try:
             with closing_if_possible(sub_resp.app_iter):

swift/common/request_helpers.py

@@ -564,11 +564,16 @@ class SegmentedIterable(object):
             seg_req = data_or_req
             seg_resp = seg_req.get_response(self.app)
             if not is_success(seg_resp.status_int):
-                close_if_possible(seg_resp.app_iter)
+                # Error body should be short
+                body = seg_resp.body
+                if not six.PY2:
+                    body = body.decode('utf8')
                 raise SegmentError(
                     'While processing manifest %s, '
-                    'got %d while retrieving %s' %
+                    'got %d (%s) while retrieving %s' %
-                    (self.name, seg_resp.status_int, seg_req.path))
+                    (self.name, seg_resp.status_int,
+                     body if len(body) <= 60 else body[:57] + '...',
+                     seg_req.path))
 
             elif ((seg_etag and (seg_resp.etag != seg_etag)) or
                     (seg_size and (seg_resp.content_length != seg_size) and

test/unit/common/middleware/test_dlo.py

@@ -356,9 +356,28 @@ class TestDloGetManifest(DloTestCase):
             md5hex("aaaaa") + md5hex("bbbbb") + md5hex("ccccc") +
             md5hex("ddddd") + md5hex("eeeee"))
         self.assertEqual(headers.get("Etag"), expected_etag)
+        self.assertEqual(self.app.unread_requests, {})
+
+    def test_get_big_manifest(self):
+        self.app.register(
+            'GET', '/v1/AUTH_test/mancon/big-manifest',
+            swob.HTTPOk, {'Content-Length': '17000', 'Etag': 'manifest-etag',
+                          'X-Object-Manifest': 'c/seg'},
+            b'manifest-contents' * 1000)
+        req = swob.Request.blank('/v1/AUTH_test/mancon/big-manifest',
+                                 environ={'REQUEST_METHOD': 'GET'})
+        status, headers, body = self.call_dlo(req)
+        headers = HeaderKeyDict(headers)
+        self.assertEqual(status, "200 OK")
+        self.assertEqual(headers["Content-Length"], "25")
+        self.assertEqual(body, b'aaaaabbbbbcccccdddddeeeee')
+        expected_etag = '"%s"' % md5hex(
+            md5hex("aaaaa") + md5hex("bbbbb") + md5hex("ccccc") +
+            md5hex("ddddd") + md5hex("eeeee"))
+        self.assertEqual(headers.get("Etag"), expected_etag)
         self.assertEqual(self.app.unread_requests, {
             # Since we don't know how big this will be, we just disconnect
-            ('GET', '/v1/AUTH_test/mancon/manifest'): 1,
+            ('GET', '/v1/AUTH_test/mancon/big-manifest'): 1,
         })
 
     def test_get_range_on_segment_boundaries(self):
@@ -573,9 +592,11 @@ class TestDloGetManifest(DloTestCase):
                                  environ={'REQUEST_METHOD': 'GET'})
         status, headers, body = self.call_dlo(req)
         self.assertEqual(status, "409 Conflict")
+        self.assertEqual(self.app.unread_requests, {})
         self.assertEqual(self.dlo.logger.get_lines_for_level('error'), [
             'While processing manifest /v1/AUTH_test/mancon/manifest, '
-            'got 403 while retrieving /v1/AUTH_test/c/seg_01',
+            'got 403 (<html><h1>Forbidden</h1><p>Access was denied to this '
+            'reso...) while retrieving /v1/AUTH_test/c/seg_01',
         ])
 
     def test_error_fetching_second_segment(self):
@@ -591,9 +612,11 @@ class TestDloGetManifest(DloTestCase):
         self.assertEqual(status, "200 OK")
         # first segment made it out
         self.assertEqual(body, b'aaaaa')
+        self.assertEqual(self.app.unread_requests, {})
         self.assertEqual(self.dlo.logger.get_lines_for_level('error'), [
             'While processing manifest /v1/AUTH_test/mancon/manifest, '
-            'got 403 while retrieving /v1/AUTH_test/c/seg_02',
+            'got 403 (<html><h1>Forbidden</h1><p>Access was denied to this '
+            'reso...) while retrieving /v1/AUTH_test/c/seg_02',
         ])
 
     def test_error_listing_container_first_listing_request(self):

test/unit/common/middleware/test_slo.py

@@ -3255,9 +3255,12 @@ class TestSloGetManifest(SloTestCase):
         headers = HeaderKeyDict(headers)
 
         self.assertEqual(status, '200 OK')
+        self.assertEqual(b"aaaaabbbbbbbbbb", body)
+        self.assertEqual(self.app.unread_requests, {})
         self.assertEqual(self.slo.logger.get_lines_for_level('error'), [
             'While processing manifest /v1/AUTH_test/gettest/manifest-abcd, '
-            'got 401 while retrieving /v1/AUTH_test/gettest/c_15'
+            'got 401 (<html><h1>Unauthorized</h1><p>This server could not '
+            'verif...) while retrieving /v1/AUTH_test/gettest/c_15'
         ])
         self.assertEqual(self.app.calls, [
             ('GET', '/v1/AUTH_test/gettest/manifest-abcd'),
@@ -3277,10 +3280,12 @@ class TestSloGetManifest(SloTestCase):
 
         self.assertEqual("200 OK", status)
         self.assertEqual(b"aaaaa", body)
+        self.assertEqual(self.app.unread_requests, {})
         self.assertEqual(self.slo.logger.get_lines_for_level('error'), [
             'while fetching /v1/AUTH_test/gettest/manifest-abcd, GET of '
             'submanifest /v1/AUTH_test/gettest/manifest-bc failed with '
-            'status 401'
+            'status 401 (<html><h1>Unauthorized</h1><p>This server could '
+            'not verif...)'
         ])
         self.assertEqual(self.app.calls, [
             ('GET', '/v1/AUTH_test/gettest/manifest-abcd'),
@@ -3311,10 +3316,12 @@ class TestSloGetManifest(SloTestCase):
         status, headers, body = self.call_slo(req)
 
         self.assertEqual('409 Conflict', status)
+        self.assertEqual(self.app.unread_requests, {})
         self.assertEqual(self.slo.logger.get_lines_for_level('error'), [
             'while fetching /v1/AUTH_test/gettest/manifest-manifest-a, GET '
             'of submanifest /v1/AUTH_test/gettest/manifest-a failed with '
-            'status 403'
+            'status 403 (<html><h1>Forbidden</h1><p>Access was denied to '
+            'this reso...)'
         ])
 
     def test_invalid_json_submanifest(self):
@@ -3473,9 +3480,11 @@ class TestSloGetManifest(SloTestCase):
         status, headers, body = self.call_slo(req)
 
         self.assertEqual('409 Conflict', status)
+        self.assertEqual(self.app.unread_requests, {})
         self.assertEqual(self.slo.logger.get_lines_for_level('error'), [
             'While processing manifest /v1/AUTH_test/gettest/'
-            'manifest-not-exists, got 404 while retrieving /v1/AUTH_test/'
+            'manifest-not-exists, got 404 (<html><h1>Not Found</h1><p>The '
+            'resource could not be foun...) while retrieving /v1/AUTH_test/'
             'gettest/not_exists_obj'
         ])