s3api: set swift.backend_path when returning 422

The S3Request class generally tries to set 's3api.backend_path' in the
original request's environ to the path cited by the response to any
swift subrequests that it issues, so that the backend path is logged
by proxy_logging middleware. For example, a multi-part upload request
will be logged usng the segments container and segment object name.

The S3Request class may also check the hash of an object as it is
uploaded using the HashingInput class. If a hash mismatch is detected,
an HTTPUnprocessableEntity exception is raised which previously caused
the setting of 's3api.backend_path' to be bypassed. This patch
therefore modifies the exception handling clause to set
's3api.backend_path' to the swift subrequest's path.

Change-Id: I0ccc6828174bc869ba0604521bdaed0ebc37a408

swift/common/middleware/s3api/s3request.py

@@ -1353,15 +1353,18 @@ class S3Request(swob.Request):
         try:
             sw_resp = sw_req.get_response(app)
         except swob.HTTPException as err:
+            # Maybe a 422 from HashingInput? Put something in
+            # s3api.backend_path - hopefully by now any modifications to the
+            # path (e.g. tenant to account translation) will have been made by
+            # auth middleware
+            self.environ['s3api.backend_path'] = sw_req.environ['PATH_INFO']
             sw_resp = err
         else:
             # reuse account
             _, self.account, _ = split_path(sw_resp.environ['PATH_INFO'],
                                             2, 3, True)
-            # Propagate swift.backend_path in environ for middleware
-            # in pipeline that need Swift PATH_INFO like ceilometermiddleware.
-            self.environ['s3api.backend_path'] = \
-                sw_resp.environ['PATH_INFO']
+            # Update s3.backend_path from the response environ
+            self.environ['s3api.backend_path'] = sw_resp.environ['PATH_INFO']
             # Propogate backend headers back into our req headers for logging
             for k, v in sw_req.headers.items():
                 if k.lower().startswith('x-backend-'):

test/unit/common/middleware/helpers.py

@@ -166,6 +166,19 @@ class FakeSwift(object):
                 ignore_range_meta.split(',')).intersection(headers.keys()):
             req.headers.pop('range', None)
 
+        # Update req.headers before capturing the request
+        if method in ('GET', 'HEAD') and obj:
+            req.headers['X-Backend-Storage-Policy-Index'] = headers.get(
+                'x-backend-storage-policy-index', '2')
+
+        # Capture the request before reading the body, in case the iter raises
+        # an exception.
+        # note: tests may assume this copy of req_headers is case insensitive
+        # so we deliberately use a HeaderKeyDict
+        req_headers_copy = HeaderKeyDict(req.headers)
+        self._calls.append(
+            FakeSwiftCall(method, path, req_headers_copy))
+
         req_body = None  # generally, we don't care and let eventlet discard()
         if (cont and not obj and method == 'UPDATE') or (
                 obj and method == 'PUT'):
@@ -177,6 +190,7 @@ class FakeSwift(object):
                 footers = HeaderKeyDict()
                 env['swift.callback.update_footers'](footers)
                 req.headers.update(footers)
+                req_headers_copy.update(footers)
             etag = md5(req_body, usedforsecurity=False).hexdigest()
             headers.setdefault('Etag', etag)
             headers.setdefault('Content-Length', len(req_body))
@@ -202,15 +216,6 @@ class FakeSwift(object):
                          k.lower == 'content-type')))
             self.uploaded[path] = new_metadata, data
 
-        # simulate object GET/HEAD
-        elif method in ('GET', 'HEAD') and obj:
-            req.headers['X-Backend-Storage-Policy-Index'] = headers.get(
-                'x-backend-storage-policy-index', '2')
-
-        # note: tests may assume this copy of req_headers is case insensitive
-        # so we deliberately use a HeaderKeyDict
-        self._calls.append(
-            FakeSwiftCall(method, path, HeaderKeyDict(req.headers)))
         self.req_bodies.append(req_body)
 
         # Apply conditional etag overrides

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

@@ -174,6 +174,33 @@ class TestS3ApiMultiUpload(S3ApiTestCase):
             ('PUT', '/v1/AUTH_test/bucket+segments/object/X/1'),
         ], self.swift.calls)
 
+    def test_bucket_upload_part_v4_bad_hash(self):
+        authz_header = 'AWS4-HMAC-SHA256 ' + ', '.join([
+            'Credential=test:tester/%s/us-east-1/s3/aws4_request' %
+            self.get_v4_amz_date_header().split('T', 1)[0],
+            'SignedHeaders=host;x-amz-date',
+            'Signature=X',
+        ])
+        req = Request.blank(
+            '/bucket/object?partNumber=1&uploadId=X',
+            method='PUT',
+            headers={'Authorization': authz_header,
+                     'X-Amz-Date': self.get_v4_amz_date_header(),
+                     'X-Amz-Content-SHA256': 'not_the_hash'},
+            body=b'test')
+        with patch('swift.common.middleware.s3api.s3request.'
+                   'get_container_info',
+                   lambda env, app, swift_source: {'status': 204}):
+            status, headers, body = self.call_s3api(req)
+        self.assertEqual(status, '400 Bad Request')
+        self.assertEqual(self._get_error_code(body), 'BadDigest')
+        self.assertEqual([
+            ('HEAD', '/v1/AUTH_test/bucket+segments/object/X'),
+            ('PUT', '/v1/AUTH_test/bucket+segments/object/X/1'),
+        ], self.swift.calls)
+        self.assertEqual('/v1/AUTH_test/bucket+segments/object/X/1',
+                         req.environ.get('swift.backend_path'))
+
     @s3acl
     def test_object_multipart_uploads_list(self):
         req = Request.blank('/bucket/object?uploads',
@@ -1321,6 +1348,8 @@ class TestS3ApiMultiUpload(S3ApiTestCase):
         status, headers, body = self.call_s3api(req)
         self.assertEqual('400 Bad Request', status)
         self.assertEqual(self._get_error_code(body), 'BadDigest')
+        self.assertEqual('/v1/AUTH_test/bucket+segments/object/X',
+                         req.environ.get('swift.backend_path'))
 
     def test_object_multipart_upload_upper_sha256(self):
         upper_sha = hashlib.sha256(

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

@@ -694,6 +694,8 @@ class TestS3ApiObj(S3ApiTestCase):
         _, _, headers = self.swift.calls_with_headers[-1]
         # No way to determine ETag to send
         self.assertNotIn('etag', headers)
+        self.assertEqual('/v1/AUTH_test/bucket/object',
+                         req.environ.get('swift.backend_path'))
 
     @s3acl
     def test_object_PUT_v4_bad_hash(self):
@@ -717,6 +719,8 @@ class TestS3ApiObj(S3ApiTestCase):
         status, headers, body = self.call_s3api(req)
         self.assertEqual(status.split()[0], '400')
         self.assertEqual(self._get_error_code(body), 'BadDigest')
+        self.assertEqual('/v1/AUTH_test/bucket/object',
+                         req.environ.get('swift.backend_path'))
 
     @s3acl
     def test_object_PUT_v4_unsigned_payload(self):

test/unit/proxy/controllers/test_obj.py

@@ -1552,6 +1552,31 @@ class TestReplicatedObjController(CommonObjectControllerMixin,
         for conn in conns:
             self.assertTrue(conn.closed)
 
+    def test_PUT_insufficient_data_from_client(self):
+        class FakeReader(object):
+            def read(self, size):
+                raise Timeout()
+        conns = []
+
+        def capture_expect(conn):
+            # stash connections so that we can verify they all get closed
+            conns.append(conn)
+
+        req = swob.Request.blank('/v1/a/c/o.jpg', method='PUT',
+                                 body='7 bytes')
+        req.headers['content-length'] = '99'
+        with set_http_connect(201, 201, 201, give_expect=capture_expect):
+            resp = req.get_response(self.app)
+
+        self.assertEqual(resp.status_int, 499)
+        warning_lines = self.app.logger.get_lines_for_level('warning')
+        self.assertEqual(1, len(warning_lines))
+        self.assertIn('Client disconnected without sending enough data',
+                      warning_lines[0])
+        self.assertEqual(self.replicas(), len(conns))
+        for conn in conns:
+            self.assertTrue(conn.closed)
+
     def test_PUT_exception_during_transfer_data(self):
         class FakeReader(object):
             def read(self, size):