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
This commit is contained in:
parent
72e413a9ef
commit
b94d0db9aa
|
@ -139,7 +139,7 @@ from swift.common import utils
|
||||||
from swift.common.utils import get_logger, \
|
from swift.common.utils import get_logger, \
|
||||||
config_true_value, FileLikeIter, read_conf_dir, close_if_possible
|
config_true_value, FileLikeIter, read_conf_dir, close_if_possible
|
||||||
from swift.common.swob import Request, HTTPPreconditionFailed, \
|
from swift.common.swob import Request, HTTPPreconditionFailed, \
|
||||||
HTTPRequestEntityTooLarge, HTTPBadRequest
|
HTTPRequestEntityTooLarge, HTTPBadRequest, HTTPException
|
||||||
from swift.common.http import HTTP_MULTIPLE_CHOICES, HTTP_CREATED, \
|
from swift.common.http import HTTP_MULTIPLE_CHOICES, HTTP_CREATED, \
|
||||||
is_success, HTTP_OK
|
is_success, HTTP_OK
|
||||||
from swift.common.constraints import check_account_format, MAX_FILE_SIZE
|
from swift.common.constraints import check_account_format, MAX_FILE_SIZE
|
||||||
|
@ -323,16 +323,20 @@ class ServerSideCopyMiddleware(object):
|
||||||
# the client actually sent.
|
# the client actually sent.
|
||||||
req.environ['swift.orig_req_method'] = req.method
|
req.environ['swift.orig_req_method'] = req.method
|
||||||
|
|
||||||
if req.method == 'PUT' and req.headers.get('X-Copy-From'):
|
try:
|
||||||
return self.handle_PUT(req, start_response)
|
if req.method == 'PUT' and req.headers.get('X-Copy-From'):
|
||||||
elif req.method == 'COPY':
|
return self.handle_PUT(req, start_response)
|
||||||
return self.handle_COPY(req, start_response)
|
elif req.method == 'COPY':
|
||||||
elif req.method == 'POST' and self.object_post_as_copy:
|
return self.handle_COPY(req, start_response)
|
||||||
return self.handle_object_post_as_copy(req, start_response)
|
elif req.method == 'POST' and self.object_post_as_copy:
|
||||||
elif req.method == 'OPTIONS':
|
return self.handle_object_post_as_copy(req, start_response)
|
||||||
# Does not interfere with OPTIONS response from (account,container)
|
elif req.method == 'OPTIONS':
|
||||||
# servers and /info response.
|
# Does not interfere with OPTIONS response from
|
||||||
return self.handle_OPTIONS(req, start_response)
|
# (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)
|
return self.app(env, start_response)
|
||||||
|
|
||||||
|
|
|
@ -1600,6 +1600,12 @@ class TestFile(Base):
|
||||||
cfg={'destination': Utils.create_name()}))
|
cfg={'destination': Utils.create_name()}))
|
||||||
self.assert_status(412)
|
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):
|
def testCopyFromHeader(self):
|
||||||
source_filename = Utils.create_name()
|
source_filename = Utils.create_name()
|
||||||
file_item = self.env.container.file(source_filename)
|
file_item = self.env.container.file(source_filename)
|
||||||
|
|
|
@ -13,8 +13,7 @@
|
||||||
|
|
||||||
import unittest
|
import unittest
|
||||||
|
|
||||||
from swift.common.swob import Request, wsgify, HTTPForbidden, \
|
from swift.common.swob import Request, wsgify, HTTPForbidden
|
||||||
HTTPException
|
|
||||||
|
|
||||||
from swift.common.middleware import account_quotas, copy
|
from swift.common.middleware import account_quotas, copy
|
||||||
|
|
||||||
|
@ -491,9 +490,8 @@ class AccountQuotaCopyingTestCases(unittest.TestCase):
|
||||||
environ={'REQUEST_METHOD': 'PUT',
|
environ={'REQUEST_METHOD': 'PUT',
|
||||||
'swift.cache': cache},
|
'swift.cache': cache},
|
||||||
headers={'x-copy-from': 'bad_path'})
|
headers={'x-copy-from': 'bad_path'})
|
||||||
with self.assertRaises(HTTPException) as catcher:
|
res = req.get_response(self.copy_filter)
|
||||||
req.get_response(self.copy_filter)
|
self.assertEqual(res.status_int, 412)
|
||||||
self.assertEqual(412, catcher.exception.status_int)
|
|
||||||
|
|
||||||
if __name__ == '__main__':
|
if __name__ == '__main__':
|
||||||
unittest.main()
|
unittest.main()
|
||||||
|
|
|
@ -521,24 +521,16 @@ class TestServerSideCopyMiddleware(unittest.TestCase):
|
||||||
req = Request.blank('/v1/a/c/o', environ={'REQUEST_METHOD': 'PUT'},
|
req = Request.blank('/v1/a/c/o', environ={'REQUEST_METHOD': 'PUT'},
|
||||||
headers={'Content-Length': '0',
|
headers={'Content-Length': '0',
|
||||||
'X-Copy-From': '/c'})
|
'X-Copy-From': '/c'})
|
||||||
try:
|
status, headers, body = self.call_ssc(req)
|
||||||
status, headers, body = self.call_ssc(req)
|
self.assertEqual(status, '412 Precondition Failed')
|
||||||
except HTTPException as resp:
|
|
||||||
self.assertEqual("412 Precondition Failed", str(resp))
|
|
||||||
else:
|
|
||||||
self.fail("Expecting HTTPException.")
|
|
||||||
|
|
||||||
def test_copy_with_no_object_in_x_copy_from_and_account(self):
|
def test_copy_with_no_object_in_x_copy_from_and_account(self):
|
||||||
req = Request.blank('/v1/a/c/o', environ={'REQUEST_METHOD': 'PUT'},
|
req = Request.blank('/v1/a/c/o', environ={'REQUEST_METHOD': 'PUT'},
|
||||||
headers={'Content-Length': '0',
|
headers={'Content-Length': '0',
|
||||||
'X-Copy-From': '/c',
|
'X-Copy-From': '/c',
|
||||||
'X-Copy-From-Account': 'a'})
|
'X-Copy-From-Account': 'a'})
|
||||||
try:
|
status, headers, body = self.call_ssc(req)
|
||||||
status, headers, body = self.call_ssc(req)
|
self.assertEqual(status, '412 Precondition Failed')
|
||||||
except HTTPException as resp:
|
|
||||||
self.assertEqual("412 Precondition Failed", str(resp))
|
|
||||||
else:
|
|
||||||
self.fail("Expecting HTTPException.")
|
|
||||||
|
|
||||||
def test_copy_with_bad_x_copy_from_account(self):
|
def test_copy_with_bad_x_copy_from_account(self):
|
||||||
req = Request.blank('/v1/a/c/o',
|
req = Request.blank('/v1/a/c/o',
|
||||||
|
@ -546,12 +538,8 @@ class TestServerSideCopyMiddleware(unittest.TestCase):
|
||||||
headers={'Content-Length': '0',
|
headers={'Content-Length': '0',
|
||||||
'X-Copy-From': '/c/o',
|
'X-Copy-From': '/c/o',
|
||||||
'X-Copy-From-Account': '/i/am/bad'})
|
'X-Copy-From-Account': '/i/am/bad'})
|
||||||
try:
|
status, headers, body = self.call_ssc(req)
|
||||||
status, headers, body = self.call_ssc(req)
|
self.assertEqual(status, '412 Precondition Failed')
|
||||||
except HTTPException as resp:
|
|
||||||
self.assertEqual("412 Precondition Failed", str(resp))
|
|
||||||
else:
|
|
||||||
self.fail("Expecting HTTPException.")
|
|
||||||
|
|
||||||
def test_copy_server_error_reading_source(self):
|
def test_copy_server_error_reading_source(self):
|
||||||
self.app.register('GET', '/v1/a/c/o', swob.HTTPServiceUnavailable, {})
|
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',
|
req = Request.blank('/v1/a/c/o',
|
||||||
environ={'REQUEST_METHOD': 'COPY'},
|
environ={'REQUEST_METHOD': 'COPY'},
|
||||||
headers={'Destination': 'c_o'})
|
headers={'Destination': 'c_o'})
|
||||||
try:
|
status, headers, body = self.call_ssc(req)
|
||||||
status, headers, body = self.call_ssc(req)
|
|
||||||
except HTTPException as resp:
|
self.assertEqual(status, '412 Precondition Failed')
|
||||||
self.assertEqual("412 Precondition Failed", str(resp))
|
|
||||||
else:
|
|
||||||
self.fail("Expecting HTTPException.")
|
|
||||||
|
|
||||||
def test_COPY_account_no_object_in_destination(self):
|
def test_COPY_account_no_object_in_destination(self):
|
||||||
req = Request.blank('/v1/a/c/o',
|
req = Request.blank('/v1/a/c/o',
|
||||||
environ={'REQUEST_METHOD': 'COPY'},
|
environ={'REQUEST_METHOD': 'COPY'},
|
||||||
headers={'Destination': 'c_o',
|
headers={'Destination': 'c_o',
|
||||||
'Destination-Account': 'a1'})
|
'Destination-Account': 'a1'})
|
||||||
try:
|
status, headers, body = self.call_ssc(req)
|
||||||
status, headers, body = self.call_ssc(req)
|
|
||||||
except HTTPException as resp:
|
self.assertEqual(status, '412 Precondition Failed')
|
||||||
self.assertEqual("412 Precondition Failed", str(resp))
|
|
||||||
else:
|
|
||||||
self.fail("Expecting HTTPException.")
|
|
||||||
|
|
||||||
def test_COPY_account_bad_destination_account(self):
|
def test_COPY_account_bad_destination_account(self):
|
||||||
req = Request.blank('/v1/a/c/o',
|
req = Request.blank('/v1/a/c/o',
|
||||||
environ={'REQUEST_METHOD': 'COPY'},
|
environ={'REQUEST_METHOD': 'COPY'},
|
||||||
headers={'Destination': '/c/o',
|
headers={'Destination': '/c/o',
|
||||||
'Destination-Account': '/i/am/bad'})
|
'Destination-Account': '/i/am/bad'})
|
||||||
try:
|
status, headers, body = self.call_ssc(req)
|
||||||
status, headers, body = self.call_ssc(req)
|
|
||||||
except HTTPException as resp:
|
self.assertEqual(status, '412 Precondition Failed')
|
||||||
self.assertEqual("412 Precondition Failed", str(resp))
|
|
||||||
else:
|
|
||||||
self.fail("Expecting HTTPException.")
|
|
||||||
|
|
||||||
def test_COPY_server_error_reading_source(self):
|
def test_COPY_server_error_reading_source(self):
|
||||||
self.app.register('GET', '/v1/a/c/o', swob.HTTPServiceUnavailable, {})
|
self.app.register('GET', '/v1/a/c/o', swob.HTTPServiceUnavailable, {})
|
||||||
|
|
|
@ -15,7 +15,7 @@
|
||||||
|
|
||||||
import unittest
|
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 swift.common.middleware import container_quotas, copy
|
||||||
from test.unit.common.middleware.helpers import FakeSwift
|
from test.unit.common.middleware.helpers import FakeSwift
|
||||||
|
|
||||||
|
@ -315,9 +315,8 @@ class ContainerQuotaCopyingTestCases(unittest.TestCase):
|
||||||
environ={'REQUEST_METHOD': 'PUT',
|
environ={'REQUEST_METHOD': 'PUT',
|
||||||
'swift.cache': cache},
|
'swift.cache': cache},
|
||||||
headers={'x-copy-from': 'bad_path'})
|
headers={'x-copy-from': 'bad_path'})
|
||||||
with self.assertRaises(HTTPException) as catcher:
|
res = req.get_response(self.copy_filter)
|
||||||
req.get_response(self.copy_filter)
|
self.assertEqual(res.status_int, 412)
|
||||||
self.assertEqual(412, catcher.exception.status_int)
|
|
||||||
|
|
||||||
def test_exceed_counts_quota_copy_from(self):
|
def test_exceed_counts_quota_copy_from(self):
|
||||||
self.app.register('GET', '/v1/a/c2/o2', HTTPOk,
|
self.app.register('GET', '/v1/a/c2/o2', HTTPOk,
|
||||||
|
|
Loading…
Reference in New Issue