diff --git a/swift/account/server.py b/swift/account/server.py index ad6ecde3ba..5b3df742a5 100644 --- a/swift/account/server.py +++ b/swift/account/server.py @@ -35,7 +35,7 @@ from swift.common.swob import HTTPAccepted, HTTPBadRequest, \ HTTPCreated, HTTPForbidden, HTTPInternalServerError, \ HTTPMethodNotAllowed, HTTPNoContent, HTTPNotFound, \ HTTPPreconditionFailed, HTTPConflict, Request, Response, \ - HTTPInsufficientStorage + HTTPInsufficientStorage, HTTPNotAcceptable DATADIR = 'accounts' @@ -182,14 +182,10 @@ class AccountController(object): if get_param(req, 'format'): req.accept = FORMAT2CONTENT_TYPE.get( get_param(req, 'format').lower(), FORMAT2CONTENT_TYPE['plain']) - try: - headers['Content-Type'] = req.accept.best_match( - ['text/plain', 'application/json', 'application/xml', - 'text/xml'], - default_match='text/plain') - except AssertionError, err: - return HTTPBadRequest(body='bad accept header: %s' % req.accept, - content_type='text/plain', request=req) + headers['Content-Type'] = req.accept.best_match( + ['text/plain', 'application/json', 'application/xml', 'text/xml']) + if not headers['Content-Type']: + return HTTPNotAcceptable(request=req) return HTTPNoContent(request=req, headers=headers, charset='utf-8') @public @@ -242,14 +238,10 @@ class AccountController(object): if query_format: req.accept = FORMAT2CONTENT_TYPE.get(query_format.lower(), FORMAT2CONTENT_TYPE['plain']) - try: - out_content_type = req.accept.best_match( - ['text/plain', 'application/json', 'application/xml', - 'text/xml'], - default_match='text/plain') - except AssertionError, err: - return HTTPBadRequest(body='bad accept header: %s' % req.accept, - content_type='text/plain', request=req) + out_content_type = req.accept.best_match( + ['text/plain', 'application/json', 'application/xml', 'text/xml']) + if not out_content_type: + return HTTPNotAcceptable(request=req) account_list = broker.list_containers_iter(limit, marker, end_marker, prefix, delimiter) if out_content_type == 'application/json': diff --git a/swift/common/swob.py b/swift/common/swob.py index 6b61b9a153..1fcff0423f 100755 --- a/swift/common/swob.py +++ b/swift/common/swob.py @@ -586,38 +586,49 @@ class Accept(object): :param headerval: value of the header as a str """ + token = r'[^()<>@,;:\"/\[\]?={}\x00-\x20\x7f]+' # RFC 2616 2.2 + acc_pattern = re.compile(r'^\s*(' + token + r')/(' + token + + r')(;\s*q=([\d.]+))?\s*$') + def __init__(self, headerval): self.headerval = headerval def _get_types(self): - headerval = self.headerval or '*/*' - level = 1 types = [] - for typ in headerval.split(','): - quality = 1.0 - if '; q=' in typ: - typ, quality = typ.split('; q=') - elif ';q=' in typ: - typ, quality = typ.split(';q=') - quality = float(quality) - if typ.startswith('*/'): - quality -= 0.01 - elif typ.endswith('/*'): - quality -= 0.01 - elif '*' in typ: - raise AssertionError('bad accept header') - pattern = '[a-zA-Z0-9-]+'.join([re.escape(x) for x in - typ.strip().split('*')]) - types.append((quality, re.compile(pattern), typ)) - types.sort(reverse=True, key=lambda t: t[0]) - return types + if not self.headerval: + return [] + for typ in self.headerval.split(','): + type_parms = self.acc_pattern.findall(typ) + if not type_parms: + raise ValueError('Invalid accept header') + typ, subtype, parms, quality = type_parms[0] + quality = float(quality or '1.0') + pattern = '^' + \ + (self.token if typ == '*' else re.escape(typ)) + '/' + \ + (self.token if subtype == '*' else re.escape(subtype)) + '$' + types.append((pattern, quality, '*' not in (typ, subtype))) + # sort candidates by quality, then whether or not there were globs + types.sort(reverse=True, key=lambda t: (t[1], t[2])) + return [t[0] for t in types] - def best_match(self, options, default_match='text/plain'): - for quality, pattern, typ in self._get_types(): + def best_match(self, options): + """ + Returns the item from "options" that best matches the accept header. + Returns None if no available options are acceptable to the client. + + :param options: a list of content-types the server can respond with + """ + try: + types = self._get_types() + except ValueError: + return None + if not types and options: + return options[0] + for pattern in types: for option in options: - if pattern.match(option): + if re.match(pattern, option): return option - return default_match + return None def __repr__(self): return self.headerval @@ -1011,6 +1022,7 @@ HTTPUnauthorized = status_map[401] HTTPForbidden = status_map[403] HTTPMethodNotAllowed = status_map[405] HTTPNotFound = status_map[404] +HTTPNotAcceptable = status_map[406] HTTPRequestTimeout = status_map[408] HTTPConflict = status_map[409] HTTPLengthRequired = status_map[411] diff --git a/swift/container/server.py b/swift/container/server.py index 3e4b520058..604cd68733 100644 --- a/swift/container/server.py +++ b/swift/container/server.py @@ -38,7 +38,7 @@ from swift.common.http import HTTP_NOT_FOUND, is_success from swift.common.swob import HTTPAccepted, HTTPBadRequest, HTTPConflict, \ HTTPCreated, HTTPInternalServerError, HTTPNoContent, HTTPNotFound, \ HTTPPreconditionFailed, HTTPMethodNotAllowed, Request, Response, \ - HTTPInsufficientStorage + HTTPInsufficientStorage, HTTPNotAcceptable DATADIR = 'containers' @@ -280,14 +280,10 @@ class ContainerController(object): if get_param(req, 'format'): req.accept = FORMAT2CONTENT_TYPE.get( get_param(req, 'format').lower(), FORMAT2CONTENT_TYPE['plain']) - try: - headers['Content-Type'] = req.accept.best_match( - ['text/plain', 'application/json', 'application/xml', - 'text/xml'], - default_match='text/plain') - except AssertionError, err: - return HTTPBadRequest(body='bad accept header: %s' % req.accept, - content_type='text/plain', request=req) + headers['Content-Type'] = req.accept.best_match( + ['text/plain', 'application/json', 'application/xml', 'text/xml']) + if not headers['Content-Type']: + return HTTPNotAcceptable(request=req) return HTTPNoContent(request=req, headers=headers, charset='utf-8') @public @@ -344,14 +340,10 @@ class ContainerController(object): if query_format: req.accept = FORMAT2CONTENT_TYPE.get(query_format.lower(), FORMAT2CONTENT_TYPE['plain']) - try: - out_content_type = req.accept.best_match( - ['text/plain', 'application/json', 'application/xml', - 'text/xml'], - default_match='text/plain') - except AssertionError, err: - return HTTPBadRequest(body='bad accept header: %s' % req.accept, - content_type='text/plain', request=req) + out_content_type = req.accept.best_match( + ['text/plain', 'application/json', 'application/xml', 'text/xml']) + if not out_content_type: + return HTTPNotAcceptable(request=req) 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 79f6af585d..8ed6a5ae51 100644 --- a/test/unit/account/test_server.py +++ b/test/unit/account/test_server.py @@ -771,8 +771,7 @@ class TestAccountController(unittest.TestCase): req = Request.blank('/sda1/p/a', environ={'REQUEST_METHOD': 'GET'}) req.accept = 'application/xml*' resp = self.controller.GET(req) - self.assertEquals(resp.status_int, 400) - self.assertEquals(resp.body, 'bad accept header: application/xml*') + self.assertEquals(resp.status_int, 406) def test_GET_prefix_delimeter_plain(self): req = Request.blank('/sda1/p/a', environ={'REQUEST_METHOD': 'PUT', diff --git a/test/unit/common/test_swob.py b/test/unit/common/test_swob.py index f676daa7e8..ca065c0509 100755 --- a/test/unit/common/test_swob.py +++ b/test/unit/common/test_swob.py @@ -232,11 +232,12 @@ class TestMatch(unittest.TestCase): class TestAccept(unittest.TestCase): def test_accept_json(self): for accept in ('application/json', 'application/json;q=1.0,*/*;q=0.9', - '*/*;q=0.9,application/json;q=1.0', 'application/*'): + '*/*;q=0.9,application/json;q=1.0', 'application/*', + 'text/*,application/json', 'application/*,text/*', + 'application/json,text/xml'): acc = swift.common.swob.Accept(accept) match = acc.best_match(['text/plain', 'application/json', - 'application/xml', 'text/xml'], - default_match='text/plain') + 'application/xml', 'text/xml']) self.assertEquals(match, 'application/json') def test_accept_plain(self): @@ -245,8 +246,7 @@ class TestAccept(unittest.TestCase): 'text/plain,application/xml'): acc = swift.common.swob.Accept(accept) match = acc.best_match(['text/plain', 'application/json', - 'application/xml', 'text/xml'], - default_match='text/plain') + 'application/xml', 'text/xml']) self.assertEquals(match, 'text/plain') def test_accept_xml(self): @@ -254,9 +254,18 @@ class TestAccept(unittest.TestCase): '*/*;q=0.9,application/xml;q=1.0'): acc = swift.common.swob.Accept(accept) match = acc.best_match(['text/plain', 'application/xml', - 'text/xml'], default_match='text/plain') + 'text/xml']) self.assertEquals(match, 'application/xml') + def test_accept_invalid(self): + for accept in ('*', 'text/plain,,', 'some stuff', + 'application/xml;q=1.0;q=1.1', 'text/plain,*', + 'text /plain', 'text\x7f/plain'): + acc = swift.common.swob.Accept(accept) + match = acc.best_match(['text/plain', 'application/xml', + 'text/xml']) + self.assertEquals(match, None) + class TestRequest(unittest.TestCase): def test_blank(self): diff --git a/test/unit/container/test_server.py b/test/unit/container/test_server.py index a26a932faa..87dbaeb2b3 100644 --- a/test/unit/container/test_server.py +++ b/test/unit/container/test_server.py @@ -797,7 +797,7 @@ class TestContainerController(unittest.TestCase): resp = self.controller.GET(req) result = [x['content_type'] for x in simplejson.loads(resp.body)] self.assertEquals(result, [u'\u2603', 'text/plain; "utf-8"']) - + def test_GET_accept_not_valid(self): req = Request.blank('/sda1/p/a/c', environ={'REQUEST_METHOD': 'PUT', 'HTTP_X_TIMESTAMP': '0'}) @@ -812,9 +812,8 @@ class TestContainerController(unittest.TestCase): req = Request.blank('/sda1/p/a/c', environ={'REQUEST_METHOD': 'GET'}) req.accept = 'application/xml*' resp = self.controller.GET(req) - self.assertEquals(resp.status_int, 400) - self.assertEquals(resp.body, 'bad accept header: application/xml*') - + self.assertEquals(resp.status_int, 406) + def test_GET_limit(self): # make a container req = Request.blank('/sda1/p/a/c', environ={'REQUEST_METHOD': 'PUT',