openstack/swift
Code Issues Proposed changes

Move parameter checking before disk accesses

Browse Source 
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 <peter.portante@redhat.com>
This commit is contained in:
Peter Portante 2013-05-25 16:30:07 -04:00
parent a2db3b6f32
commit 8d6869a6cc
4 changed files with 106 additions and 78 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

49
swift/account/server.py
View File

@ -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':

53
swift/container/server.py
View File

@ -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':

43
test/unit/account/test_server.py
View File

@ -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),

39
test/unit/container/test_server.py
View File

@ -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),