From 8d6869a6cca7e3f645aaa402a742f79e47dd9343 Mon Sep 17 00:00:00 2001 From: Peter Portante Date: Sat, 25 May 2013 16:30:07 -0400 Subject: [PATCH] Move parameter checking before disk accesses For HEAD and GET requests on both containers and accounts APIs we now perform parameter checking before we access the database. The unit tests were updated to show that the parameter checking is performed before accesses are performed. Change-Id: Ieb753316cdccabf45022e3d83522d87d34aa6b0e Signed-off-by: Peter Portante --- swift/account/server.py | 49 +++++++++++++-------------- swift/container/server.py | 53 +++++++++++++++--------------- test/unit/account/test_server.py | 43 +++++++++++++++--------- test/unit/container/test_server.py | 39 ++++++++++++++-------- 4 files changed, 106 insertions(+), 78 deletions(-) diff --git a/swift/account/server.py b/swift/account/server.py index 4c08fc1e78..0a6bf29ca9 100644 --- a/swift/account/server.py +++ b/swift/account/server.py @@ -160,6 +160,13 @@ class AccountController(object): except ValueError, err: return HTTPBadRequest(body=str(err), content_type='text/plain', request=req) + if get_param(req, 'format'): + req.accept = FORMAT2CONTENT_TYPE.get( + get_param(req, 'format').lower(), FORMAT2CONTENT_TYPE['plain']) + out_content_type = req.accept.best_match( + ['text/plain', 'application/json', 'application/xml', 'text/xml']) + if not out_content_type: + return HTTPNotAcceptable(request=req) if self.mount_check and not check_mount(self.root, drive): return HTTPInsufficientStorage(drive=drive, request=req) broker = self._get_account_broker(drive, part, account) @@ -177,13 +184,7 @@ class AccountController(object): headers.update((key, value) for key, (value, timestamp) in broker.metadata.iteritems() if value != '') - if get_param(req, 'format'): - req.accept = FORMAT2CONTENT_TYPE.get( - get_param(req, 'format').lower(), FORMAT2CONTENT_TYPE['plain']) - headers['Content-Type'] = req.accept.best_match( - ['text/plain', 'application/json', 'application/xml', 'text/xml']) - if not headers['Content-Type']: - return HTTPNotAcceptable(request=req) + headers['Content-Type'] = out_content_type return HTTPNoContent(request=req, headers=headers, charset='utf-8') @public @@ -196,23 +197,6 @@ class AccountController(object): except ValueError, err: return HTTPBadRequest(body=str(err), content_type='text/plain', request=req) - if self.mount_check and not check_mount(self.root, drive): - return HTTPInsufficientStorage(drive=drive, request=req) - broker = self._get_account_broker(drive, part, account) - broker.pending_timeout = 0.1 - broker.stale_reads_ok = True - if broker.is_deleted(): - return HTTPNotFound(request=req) - info = broker.get_info() - resp_headers = { - 'X-Account-Container-Count': info['container_count'], - 'X-Account-Object-Count': info['object_count'], - 'X-Account-Bytes-Used': info['bytes_used'], - 'X-Timestamp': info['created_at'], - 'X-PUT-Timestamp': info['put_timestamp']} - resp_headers.update((key, value) - for key, (value, timestamp) in - broker.metadata.iteritems() if value != '') try: prefix = get_param(req, 'prefix') delimiter = get_param(req, 'delimiter') @@ -240,6 +224,23 @@ class AccountController(object): ['text/plain', 'application/json', 'application/xml', 'text/xml']) if not out_content_type: return HTTPNotAcceptable(request=req) + if self.mount_check and not check_mount(self.root, drive): + return HTTPInsufficientStorage(drive=drive, request=req) + broker = self._get_account_broker(drive, part, account) + broker.pending_timeout = 0.1 + broker.stale_reads_ok = True + if broker.is_deleted(): + return HTTPNotFound(request=req) + info = broker.get_info() + resp_headers = { + 'X-Account-Container-Count': info['container_count'], + 'X-Account-Object-Count': info['object_count'], + 'X-Account-Bytes-Used': info['bytes_used'], + 'X-Timestamp': info['created_at'], + 'X-PUT-Timestamp': info['put_timestamp']} + resp_headers.update((key, value) + for key, (value, timestamp) in + broker.metadata.iteritems() if value != '') account_list = broker.list_containers_iter(limit, marker, end_marker, prefix, delimiter) if out_content_type == 'application/json': diff --git a/swift/container/server.py b/swift/container/server.py index 6aee0045ac..db6c4b3ce8 100644 --- a/swift/container/server.py +++ b/swift/container/server.py @@ -294,6 +294,13 @@ class ContainerController(object): except ValueError, err: return HTTPBadRequest(body=str(err), content_type='text/plain', request=req) + if get_param(req, 'format'): + req.accept = FORMAT2CONTENT_TYPE.get( + get_param(req, 'format').lower(), FORMAT2CONTENT_TYPE['plain']) + out_content_type = req.accept.best_match( + ['text/plain', 'application/json', 'application/xml', 'text/xml']) + if not out_content_type: + return HTTPNotAcceptable(request=req) if self.mount_check and not check_mount(self.root, drive): return HTTPInsufficientStorage(drive=drive, request=req) broker = self._get_container_broker(drive, part, account, container) @@ -313,13 +320,7 @@ class ContainerController(object): for key, (value, timestamp) in broker.metadata.iteritems() if value != '' and (key.lower() in self.save_headers or key.lower().startswith('x-container-meta-'))) - if get_param(req, 'format'): - req.accept = FORMAT2CONTENT_TYPE.get( - get_param(req, 'format').lower(), FORMAT2CONTENT_TYPE['plain']) - headers['Content-Type'] = req.accept.best_match( - ['text/plain', 'application/json', 'application/xml', 'text/xml']) - if not headers['Content-Type']: - return HTTPNotAcceptable(request=req) + headers['Content-Type'] = out_content_type return HTTPNoContent(request=req, headers=headers, charset='utf-8') def derive_content_type_metadata(self, content_type, size): @@ -352,25 +353,6 @@ class ContainerController(object): except ValueError, err: return HTTPBadRequest(body=str(err), content_type='text/plain', request=req) - if self.mount_check and not check_mount(self.root, drive): - return HTTPInsufficientStorage(drive=drive, request=req) - broker = self._get_container_broker(drive, part, account, container) - broker.pending_timeout = 0.1 - broker.stale_reads_ok = True - if broker.is_deleted(): - return HTTPNotFound(request=req) - info = broker.get_info() - resp_headers = { - 'X-Container-Object-Count': info['object_count'], - 'X-Container-Bytes-Used': info['bytes_used'], - 'X-Timestamp': info['created_at'], - 'X-PUT-Timestamp': info['put_timestamp'], - } - resp_headers.update( - (key, value) - for key, (value, timestamp) in broker.metadata.iteritems() - if value != '' and (key.lower() in self.save_headers or - key.lower().startswith('x-container-meta-'))) try: path = get_param(req, 'path') prefix = get_param(req, 'prefix') @@ -399,6 +381,25 @@ class ContainerController(object): ['text/plain', 'application/json', 'application/xml', 'text/xml']) if not out_content_type: return HTTPNotAcceptable(request=req) + if self.mount_check and not check_mount(self.root, drive): + return HTTPInsufficientStorage(drive=drive, request=req) + broker = self._get_container_broker(drive, part, account, container) + broker.pending_timeout = 0.1 + broker.stale_reads_ok = True + if broker.is_deleted(): + return HTTPNotFound(request=req) + info = broker.get_info() + resp_headers = { + 'X-Container-Object-Count': info['object_count'], + 'X-Container-Bytes-Used': info['bytes_used'], + 'X-Timestamp': info['created_at'], + 'X-PUT-Timestamp': info['put_timestamp'], + } + resp_headers.update( + (key, value) + for key, (value, timestamp) in broker.metadata.iteritems() + if value != '' and (key.lower() in self.save_headers or + key.lower().startswith('x-container-meta-'))) container_list = broker.list_objects_iter(limit, marker, end_marker, prefix, delimiter, path) if out_content_type == 'application/json': diff --git a/test/unit/account/test_server.py b/test/unit/account/test_server.py index d8188fdd8a..4c018b4a15 100644 --- a/test/unit/account/test_server.py +++ b/test/unit/account/test_server.py @@ -191,9 +191,6 @@ class TestAccountController(unittest.TestCase): self.assertEquals(resp.status_int, 400) def test_HEAD_invalid_content_type(self): - req = Request.blank('/sda1/p/a', environ={'REQUEST_METHOD': 'PUT', - 'HTTP_X_TIMESTAMP': '0'}) - self.controller.PUT(req) req = Request.blank('/sda1/p/a', environ={'REQUEST_METHOD': 'HEAD'}, headers={'Accept': 'application/plain'}) resp = self.controller.HEAD(req) @@ -429,9 +426,6 @@ class TestAccountController(unittest.TestCase): self.assertEquals(resp.status_int, 200) def test_GET_over_limit(self): - req = Request.blank('/sda1/p/a', environ={'REQUEST_METHOD': 'PUT', - 'HTTP_X_TIMESTAMP': '0'}) - self.controller.PUT(req) req = Request.blank('/sda1/p/a?limit=%d' % (ACCOUNT_LISTING_LIMIT + 1), environ={'REQUEST_METHOD': 'GET'}) resp = self.controller.GET(req) @@ -863,7 +857,14 @@ class TestAccountController(unittest.TestCase): resp = self.controller.GET(req) self.assertEquals(resp.status_int, 406) - def test_GET_prefix_delimeter_plain(self): + def test_GET_delimiter_too_long(self): + req = Request.blank('/sda1/p/a?delimiter=xx', + environ={'REQUEST_METHOD': 'GET', + 'HTTP_X_TIMESTAMP': '0'}) + resp = self.controller.GET(req) + self.assertEquals(resp.status_int, 412) + + def test_GET_prefix_delimiter_plain(self): req = Request.blank('/sda1/p/a', environ={'REQUEST_METHOD': 'PUT', 'HTTP_X_TIMESTAMP': '0'}) resp = self.controller.PUT(req) @@ -903,7 +904,7 @@ class TestAccountController(unittest.TestCase): self.assertEquals(resp.body.strip().split('\n'), ['sub.1.0', 'sub.1.1', 'sub.1.2']) - def test_GET_prefix_delimeter_json(self): + def test_GET_prefix_delimiter_json(self): req = Request.blank('/sda1/p/a', environ={'REQUEST_METHOD': 'PUT', 'HTTP_X_TIMESTAMP': '0'}) resp = self.controller.PUT(req) @@ -946,7 +947,7 @@ class TestAccountController(unittest.TestCase): for n in simplejson.loads(resp.body)], ['sub.1.0', 'sub.1.1', 'sub.1.2']) - def test_GET_prefix_delimeter_xml(self): + def test_GET_prefix_delimiter_xml(self): req = Request.blank('/sda1/p/a', environ={'REQUEST_METHOD': 'PUT', 'HTTP_X_TIMESTAMP': '0'}) resp = self.controller.PUT(req) @@ -1134,18 +1135,30 @@ class TestAccountController(unittest.TestCase): self.assertEquals(resp.status_int, 200) def test_params_utf8(self): - self.controller.PUT(Request.blank('/sda1/p/a', - headers={'X-Timestamp': normalize_timestamp(1)}, - environ={'REQUEST_METHOD': 'PUT'})) - for param in ('delimiter', 'limit', 'marker', 'prefix'): + # Bad UTF8 sequence, all parameters should cause 400 error + for param in ('delimiter', 'limit', 'marker', 'prefix', 'end_marker', + 'format'): req = Request.blank('/sda1/p/a?%s=\xce' % param, environ={'REQUEST_METHOD': 'GET'}) resp = self.controller.GET(req) - self.assertEquals(resp.status_int, 400) + self.assertEquals(resp.status_int, 400, + "%d on param %s" % (resp.status_int, param)) + # Good UTF8 sequence for delimiter, too long (1 byte delimiters only) + req = Request.blank('/sda1/p/a?delimiter=\xce\xa9', + environ={'REQUEST_METHOD': 'GET'}) + resp = self.controller.GET(req) + self.assertEquals(resp.status_int, 412, + "%d on param delimiter" % (resp.status_int)) + self.controller.PUT(Request.blank('/sda1/p/a', + headers={'X-Timestamp': normalize_timestamp(1)}, + environ={'REQUEST_METHOD': 'PUT'})) + # Good UTF8 sequence, ignored for limit, doesn't affect other queries + for param in ('limit', 'marker', 'prefix', 'end_marker', 'format'): req = Request.blank('/sda1/p/a?%s=\xce\xa9' % param, environ={'REQUEST_METHOD': 'GET'}) resp = self.controller.GET(req) - self.assert_(resp.status_int in (204, 412), resp.status_int) + self.assertEquals(resp.status_int, 204, + "%d on param %s" % (resp.status_int, param)) def test_put_auto_create(self): headers = {'x-put-timestamp': normalize_timestamp(1), diff --git a/test/unit/container/test_server.py b/test/unit/container/test_server.py index 201e9d8b86..4870003207 100644 --- a/test/unit/container/test_server.py +++ b/test/unit/container/test_server.py @@ -138,9 +138,6 @@ class TestContainerController(unittest.TestCase): self.assertEquals(resp.status_int, 507) def test_HEAD_invalid_content_type(self): - req = Request.blank('/sda1/p/a/c', environ={'REQUEST_METHOD': 'PUT', - 'HTTP_X_TIMESTAMP': '0'}) - self.controller.PUT(req) req = Request.blank('/sda1/p/a/c', environ={'REQUEST_METHOD': 'HEAD'}, headers={'Accept': 'application/plain'}) resp = self.controller.HEAD(req) @@ -702,10 +699,6 @@ class TestContainerController(unittest.TestCase): self.assertEquals(resp.status_int, 507) def test_GET_over_limit(self): - req = Request.blank('/sda1/p/a/c', environ={'REQUEST_METHOD': 'PUT'}, - headers={'X-Timestamp': '0'}) - resp = self.controller.PUT(req) - self.assertEquals(resp.status_int, 201) req = Request.blank('/sda1/p/a/c?limit=%d' % (container_server.CONTAINER_LISTING_LIMIT + 1), environ={'REQUEST_METHOD': 'GET'}) @@ -1029,6 +1022,13 @@ class TestContainerController(unittest.TestCase): resp = self.controller.GET(req) self.assertEquals(resp.body.split(), ['a1', 'a2', 'a3']) + def test_GET_delimiter_too_long(self): + req = Request.blank('/sda1/p/a/c?delimiter=xx', + environ={'REQUEST_METHOD': 'GET', + 'HTTP_X_TIMESTAMP': '0'}) + resp = self.controller.GET(req) + self.assertEquals(resp.status_int, 412) + def test_GET_delimiter(self): req = Request.blank('/sda1/p/a/c', environ={'REQUEST_METHOD': 'PUT', 'HTTP_X_TIMESTAMP': '0'}) @@ -1213,18 +1213,31 @@ class TestContainerController(unittest.TestCase): self.assertEquals(resp.status_int, 200) def test_params_utf8(self): - self.controller.PUT(Request.blank('/sda1/p/a/c', - headers={'X-Timestamp': normalize_timestamp(1)}, - environ={'REQUEST_METHOD': 'PUT'})) - for param in ('delimiter', 'limit', 'marker', 'path', 'prefix'): + # Bad UTF8 sequence, all parameters should cause 400 error + for param in ('delimiter', 'limit', 'marker', 'path', 'prefix', + 'end_marker', 'format'): req = Request.blank('/sda1/p/a/c?%s=\xce' % param, environ={'REQUEST_METHOD': 'GET'}) resp = self.controller.GET(req) - self.assertEquals(resp.status_int, 400) + self.assertEquals(resp.status_int, 400, + "%d on param %s" % (resp.status_int, param)) + # Good UTF8 sequence for delimiter, too long (1 byte delimiters only) + req = Request.blank('/sda1/p/a/c?delimiter=\xce\xa9', + environ={'REQUEST_METHOD': 'GET'}) + resp = self.controller.GET(req) + self.assertEquals(resp.status_int, 412, + "%d on param delimiter" % (resp.status_int)) + self.controller.PUT(Request.blank('/sda1/p/a/c', + headers={'X-Timestamp': normalize_timestamp(1)}, + environ={'REQUEST_METHOD': 'PUT'})) + # Good UTF8 sequence, ignored for limit, doesn't affect other queries + for param in ('limit', 'marker', 'path', 'prefix', 'end_marker', + 'format'): req = Request.blank('/sda1/p/a/c?%s=\xce\xa9' % param, environ={'REQUEST_METHOD': 'GET'}) resp = self.controller.GET(req) - self.assert_(resp.status_int in (204, 412), resp.status_int) + self.assertEquals(resp.status_int, 204, + "%d on param %s" % (resp.status_int, param)) def test_put_auto_create(self): headers = {'x-timestamp': normalize_timestamp(1),