Merge "s3api: Improve checksum-mismatch detection"

swift/common/middleware/s3api/s3request.py

@@ -120,6 +120,17 @@ def _header_acl_property(resource):
                     doc='Get and set the %s acl property' % resource)
 
 
+class S3InputSHA256Mismatch(BaseException):
+    """
+    Client provided a X-Amz-Content-SHA256, but it doesn't match the data.
+
+    Inherit from BaseException (rather than Exception) so it cuts from the
+    proxy-server app (which will presumably be the one reading the input)
+    through all the layers of the pipeline back to us. It should never escape
+    the s3api middleware.
+    """
+
+
 class HashingInput(object):
     """
     wsgi.input wrapper to verify the hash of the input as it's read.
@@ -141,7 +152,7 @@ class HashingInput(object):
                 self._hasher.hexdigest() != self._expected):
             self.close()
             # Since we don't return the last chunk, the PUT never completes
-            raise swob.HTTPUnprocessableEntity(
+            raise S3InputSHA256Mismatch(
                 'The X-Amz-Content-SHA56 you specified did not match '
                 'what we received.')
         return chunk
@@ -910,13 +921,8 @@ class S3Request(swob.Request):
             # Limit the read similar to how SLO handles manifests
             try:
                 body = self.body_file.read(max_length)
-            except swob.HTTPException as err:
-                if err.status_int == HTTP_UNPROCESSABLE_ENTITY:
-                    # Special case for HashingInput check
-                    raise BadDigest(
-                        'The X-Amz-Content-SHA56 you specified did not '
-                        'match what we received.')
-                raise
+            except S3InputSHA256Mismatch as err:
+                raise BadDigest(err.args[0])
         else:
             # No (or zero) Content-Length provided, and not chunked transfer;
             # no body. Assume zero-length, and enforce a required body below.
@@ -1410,13 +1416,11 @@ 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
+        except S3InputSHA256Mismatch as err:
+            # 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
+            raise BadDigest(err.args[0])
         else:
             # reuse account
             _, self.account, _ = split_path(sw_resp.environ['PATH_INFO'],

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

@@ -770,6 +770,18 @@ class BaseS3ApiObj(object):
                          req.environ.get('swift.backend_path'))
 
     def test_object_PUT_v4_bad_hash(self):
+        orig_app = self.s3api.app
+
+        def error_catching_app(env, start_response):
+            try:
+                return orig_app(env, start_response)
+            except Exception:
+                self.logger.exception('uh oh')
+                start_response('599 Uh Oh', [])
+                return [b'']
+
+        self.s3api.app = error_catching_app
+
         req = Request.blank(
             '/bucket/object',
             environ={'REQUEST_METHOD': 'PUT'},

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

@@ -29,7 +29,8 @@ from swift.common.middleware.s3api.subresource import ACL, User, Owner, \
     Grant, encode_acl
 from test.unit.common.middleware.s3api.test_s3api import S3ApiTestCase
 from swift.common.middleware.s3api.s3request import S3Request, \
-    S3AclRequest, SigV4Request, SIGV4_X_AMZ_DATE_FORMAT, HashingInput
+    S3AclRequest, SigV4Request, SIGV4_X_AMZ_DATE_FORMAT, HashingInput, \
+    S3InputSHA256Mismatch
 from swift.common.middleware.s3api.s3response import InvalidArgument, \
     NoSuchBucket, InternalError, ServiceUnavailable, \
     AccessDenied, SignatureDoesNotMatch, RequestTimeTooSkewed, BadDigest, \
@@ -1405,9 +1406,11 @@ class TestHashingInput(S3ApiTestCase):
         self.assertEqual(b'1234', wrapped.read(4))
         self.assertEqual(b'56', wrapped.read(2))
         # even though the hash matches, there was more data than we expected
-        with self.assertRaises(swob.HTTPException) as raised:
+        with self.assertRaises(S3InputSHA256Mismatch) as raised:
             wrapped.read(3)
-        self.assertEqual(raised.exception.status, '422 Unprocessable Entity')
+        self.assertIsInstance(raised.exception, BaseException)
+        # won't get caught by most things in a pipeline
+        self.assertNotIsInstance(raised.exception, Exception)
         # the error causes us to close the input
         self.assertTrue(wrapped._input.closed)
 
@@ -1419,9 +1422,8 @@ class TestHashingInput(S3ApiTestCase):
         self.assertEqual(b'1234', wrapped.read(4))
         self.assertEqual(b'56', wrapped.read(2))
         # even though the hash matches, there was more data than we expected
-        with self.assertRaises(swob.HTTPException) as raised:
+        with self.assertRaises(S3InputSHA256Mismatch):
             wrapped.read(4)
-        self.assertEqual(raised.exception.status, '422 Unprocessable Entity')
         self.assertTrue(wrapped._input.closed)
 
     def test_bad_hash(self):
@@ -1431,17 +1433,14 @@ class TestHashingInput(S3ApiTestCase):
             md5(raw, usedforsecurity=False).hexdigest())
         self.assertEqual(b'1234', wrapped.read(4))
         self.assertEqual(b'5678', wrapped.read(4))
-        with self.assertRaises(swob.HTTPException) as raised:
+        with self.assertRaises(S3InputSHA256Mismatch):
             wrapped.read(4)
-        self.assertEqual(raised.exception.status, '422 Unprocessable Entity')
         self.assertTrue(wrapped._input.closed)
 
     def test_empty_bad_hash(self):
         wrapped = HashingInput(BytesIO(b''), 0, hashlib.sha256, 'nope')
-        with self.assertRaises(swob.HTTPException) as raised:
+        with self.assertRaises(S3InputSHA256Mismatch):
             wrapped.read(3)
-        self.assertEqual(raised.exception.status, '422 Unprocessable Entity')
-        # the error causes us to close the input
         self.assertTrue(wrapped._input.closed)