From 064ee2b58337b7fd3ae5fca343b1f6fd3bd86ed7 Mon Sep 17 00:00:00 2001 From: Michael Barton Date: Thu, 29 Nov 2012 13:29:00 -0800 Subject: [PATCH] 406 if we can't satisfy Accept The container and account servers should respond with 406 if the Accept header isn't satisfiable. This behavior is defined in RFC 2616 section 14.1. Change-Id: I8a67ccafe33dc70ef4f7794686a54fbc8581f4dc --- swift/account/server.py | 26 +++++-------- swift/common/swob.py | 60 ++++++++++++++++++------------ swift/container/server.py | 26 +++++-------- test/unit/account/test_server.py | 3 +- test/unit/common/test_swob.py | 21 ++++++++--- test/unit/container/test_server.py | 7 ++-- 6 files changed, 73 insertions(+), 70 deletions(-) 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 df94233360..4112357ae6 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 44c41c98b8..6762bd23ff 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',