Correctly send 412 Precondition Failed in copy middleware

Previously in copy middleware, if a user entered an invalid destination
path with an invalid `container/object` path the server would return
a 500 Internal Server Error. However, the correct response should be
a 412 Precondition Failed. This patch updates copy so that it catches
the 412 Precondition Failed exception and returns it to the client.

Closes-Bug: #1641980

Change-Id: Ic4677ae033d05b8730c6ad1041bd9c07268e11a9

5 files changed, 42 insertions, 56 deletions

diff --git a/swift/common/middleware/copy.py b/swift/common/middleware/copy.py
index 5265fe930..1422d6680 100644
--- a/swift/common/middleware/copy.py
+++ b/swift/common/middleware/copy.py
@@ -139,7 +139,7 @@ from swift.common import utils
 from swift.common.utils import get_logger, \
     config_true_value, FileLikeIter, read_conf_dir, close_if_possible
 from swift.common.swob import Request, HTTPPreconditionFailed, \
-    HTTPRequestEntityTooLarge, HTTPBadRequest
+    HTTPRequestEntityTooLarge, HTTPBadRequest, HTTPException
 from swift.common.http import HTTP_MULTIPLE_CHOICES, HTTP_CREATED, \
     is_success, HTTP_OK
 from swift.common.constraints import check_account_format, MAX_FILE_SIZE
@@ -323,16 +323,20 @@ class ServerSideCopyMiddleware(object):
         # the client actually sent.
         req.environ['swift.orig_req_method'] = req.method
 
-        if req.method == 'PUT' and req.headers.get('X-Copy-From'):
-            return self.handle_PUT(req, start_response)
-        elif req.method == 'COPY':
-            return self.handle_COPY(req, start_response)
-        elif req.method == 'POST' and self.object_post_as_copy:
-            return self.handle_object_post_as_copy(req, start_response)
-        elif req.method == 'OPTIONS':
-            # Does not interfere with OPTIONS response from (account,container)
-            # servers and /info response.
-            return self.handle_OPTIONS(req, start_response)
+        try:
+            if req.method == 'PUT' and req.headers.get('X-Copy-From'):
+                return self.handle_PUT(req, start_response)
+            elif req.method == 'COPY':
+                return self.handle_COPY(req, start_response)
+            elif req.method == 'POST' and self.object_post_as_copy:
+                return self.handle_object_post_as_copy(req, start_response)
+            elif req.method == 'OPTIONS':
+                # Does not interfere with OPTIONS response from
+                # (account,container) servers and /info response.
+                return self.handle_OPTIONS(req, start_response)
+
+        except HTTPException as e:
+            return e(req.environ, start_response)
 
         return self.app(env, start_response)
 
diff --git a/test/functional/tests.py b/test/functional/tests.py
index e18b3be8e..086bc6244 100644
--- a/test/functional/tests.py
+++ b/test/functional/tests.py
@@ -1600,6 +1600,12 @@ class TestFile(Base):
                          cfg={'destination': Utils.create_name()}))
         self.assert_status(412)
 
+        # too many slashes
+        self.assertFalse(file_item.copy(Utils.create_name(),
+                         Utils.create_name(),
+                         cfg={'destination': '//%s' % Utils.create_name()}))
+        self.assert_status(412)
+
     def testCopyFromHeader(self):
         source_filename = Utils.create_name()
         file_item = self.env.container.file(source_filename)
diff --git a/test/unit/common/middleware/test_account_quotas.py b/test/unit/common/middleware/test_account_quotas.py
index 3ebbe37bd..a59dc0c64 100644
--- a/test/unit/common/middleware/test_account_quotas.py
+++ b/test/unit/common/middleware/test_account_quotas.py
@@ -13,8 +13,7 @@
 
 import unittest
 
-from swift.common.swob import Request, wsgify, HTTPForbidden, \
-    HTTPException
+from swift.common.swob import Request, wsgify, HTTPForbidden
 
 from swift.common.middleware import account_quotas, copy
 
@@ -491,9 +490,8 @@ class AccountQuotaCopyingTestCases(unittest.TestCase):
                             environ={'REQUEST_METHOD': 'PUT',
                                      'swift.cache': cache},
                             headers={'x-copy-from': 'bad_path'})
-        with self.assertRaises(HTTPException) as catcher:
-            req.get_response(self.copy_filter)
-        self.assertEqual(412, catcher.exception.status_int)
+        res = req.get_response(self.copy_filter)
+        self.assertEqual(res.status_int, 412)
 
 if __name__ == '__main__':
     unittest.main()
diff --git a/test/unit/common/middleware/test_copy.py b/test/unit/common/middleware/test_copy.py
index 5715e1fbb..2dba7194a 100644
--- a/test/unit/common/middleware/test_copy.py
+++ b/test/unit/common/middleware/test_copy.py
@@ -521,24 +521,16 @@ class TestServerSideCopyMiddleware(unittest.TestCase):
         req = Request.blank('/v1/a/c/o', environ={'REQUEST_METHOD': 'PUT'},
                             headers={'Content-Length': '0',
                                      'X-Copy-From': '/c'})
-        try:
-            status, headers, body = self.call_ssc(req)
-        except HTTPException as resp:
-            self.assertEqual("412 Precondition Failed", str(resp))
-        else:
-            self.fail("Expecting HTTPException.")
+        status, headers, body = self.call_ssc(req)
+        self.assertEqual(status, '412 Precondition Failed')
 
     def test_copy_with_no_object_in_x_copy_from_and_account(self):
         req = Request.blank('/v1/a/c/o', environ={'REQUEST_METHOD': 'PUT'},
                             headers={'Content-Length': '0',
                                      'X-Copy-From': '/c',
                                      'X-Copy-From-Account': 'a'})
-        try:
-            status, headers, body = self.call_ssc(req)
-        except HTTPException as resp:
-            self.assertEqual("412 Precondition Failed", str(resp))
-        else:
-            self.fail("Expecting HTTPException.")
+        status, headers, body = self.call_ssc(req)
+        self.assertEqual(status, '412 Precondition Failed')
 
     def test_copy_with_bad_x_copy_from_account(self):
         req = Request.blank('/v1/a/c/o',
@@ -546,12 +538,8 @@ class TestServerSideCopyMiddleware(unittest.TestCase):
                             headers={'Content-Length': '0',
                                      'X-Copy-From': '/c/o',
                                      'X-Copy-From-Account': '/i/am/bad'})
-        try:
-            status, headers, body = self.call_ssc(req)
-        except HTTPException as resp:
-            self.assertEqual("412 Precondition Failed", str(resp))
-        else:
-            self.fail("Expecting HTTPException.")
+        status, headers, body = self.call_ssc(req)
+        self.assertEqual(status, '412 Precondition Failed')
 
     def test_copy_server_error_reading_source(self):
         self.app.register('GET', '/v1/a/c/o', swob.HTTPServiceUnavailable, {})
@@ -992,36 +980,27 @@ class TestServerSideCopyMiddleware(unittest.TestCase):
         req = Request.blank('/v1/a/c/o',
                             environ={'REQUEST_METHOD': 'COPY'},
                             headers={'Destination': 'c_o'})
-        try:
-            status, headers, body = self.call_ssc(req)
-        except HTTPException as resp:
-            self.assertEqual("412 Precondition Failed", str(resp))
-        else:
-            self.fail("Expecting HTTPException.")
+        status, headers, body = self.call_ssc(req)
+
+        self.assertEqual(status, '412 Precondition Failed')
 
     def test_COPY_account_no_object_in_destination(self):
         req = Request.blank('/v1/a/c/o',
                             environ={'REQUEST_METHOD': 'COPY'},
                             headers={'Destination': 'c_o',
                                      'Destination-Account': 'a1'})
-        try:
-            status, headers, body = self.call_ssc(req)
-        except HTTPException as resp:
-            self.assertEqual("412 Precondition Failed", str(resp))
-        else:
-            self.fail("Expecting HTTPException.")
+        status, headers, body = self.call_ssc(req)
+
+        self.assertEqual(status, '412 Precondition Failed')
 
     def test_COPY_account_bad_destination_account(self):
         req = Request.blank('/v1/a/c/o',
                             environ={'REQUEST_METHOD': 'COPY'},
                             headers={'Destination': '/c/o',
                                      'Destination-Account': '/i/am/bad'})
-        try:
-            status, headers, body = self.call_ssc(req)
-        except HTTPException as resp:
-            self.assertEqual("412 Precondition Failed", str(resp))
-        else:
-            self.fail("Expecting HTTPException.")
+        status, headers, body = self.call_ssc(req)
+
+        self.assertEqual(status, '412 Precondition Failed')
 
     def test_COPY_server_error_reading_source(self):
         self.app.register('GET', '/v1/a/c/o', swob.HTTPServiceUnavailable, {})
diff --git a/test/unit/common/middleware/test_quotas.py b/test/unit/common/middleware/test_quotas.py
index 0bec5cad5..1ee600cfc 100644
--- a/test/unit/common/middleware/test_quotas.py
+++ b/test/unit/common/middleware/test_quotas.py
@@ -15,7 +15,7 @@
 
 import unittest
 
-from swift.common.swob import Request, HTTPUnauthorized, HTTPOk, HTTPException
+from swift.common.swob import Request, HTTPUnauthorized, HTTPOk
 from swift.common.middleware import container_quotas, copy
 from test.unit.common.middleware.helpers import FakeSwift
 
@@ -315,9 +315,8 @@ class ContainerQuotaCopyingTestCases(unittest.TestCase):
                             environ={'REQUEST_METHOD': 'PUT',
                                      'swift.cache': cache},
                             headers={'x-copy-from': 'bad_path'})
-        with self.assertRaises(HTTPException) as catcher:
-            req.get_response(self.copy_filter)
-        self.assertEqual(412, catcher.exception.status_int)
+        res = req.get_response(self.copy_filter)
+        self.assertEqual(res.status_int, 412)
 
     def test_exceed_counts_quota_copy_from(self):
         self.app.register('GET', '/v1/a/c2/o2', HTTPOk,