diff --git a/swift/common/middleware/bulk.py b/swift/common/middleware/bulk.py index 72f472f003..2ca73d7be8 100644 --- a/swift/common/middleware/bulk.py +++ b/swift/common/middleware/bulk.py @@ -675,7 +675,11 @@ class Bulk(object): 'tar.bz2': 'bz2'}.get(extract_type.lower().strip('.')) if archive_type is not None: resp = HTTPOk(request=req) - out_content_type = req.accept.best_match(ACCEPTABLE_FORMATS) + try: + out_content_type = req.accept.best_match( + ACCEPTABLE_FORMATS) + except ValueError: + out_content_type = None # Ignore invalid header if out_content_type: resp.content_type = out_content_type resp.app_iter = self.handle_extract_iter( @@ -684,7 +688,10 @@ class Bulk(object): resp = HTTPBadRequest("Unsupported archive format") if 'bulk-delete' in req.params and req.method in ['POST', 'DELETE']: resp = HTTPOk(request=req) - out_content_type = req.accept.best_match(ACCEPTABLE_FORMATS) + try: + out_content_type = req.accept.best_match(ACCEPTABLE_FORMATS) + except ValueError: + out_content_type = None # Ignore invalid header if out_content_type: resp.content_type = out_content_type resp.app_iter = self.handle_delete_iter( diff --git a/swift/common/middleware/listing_formats.py b/swift/common/middleware/listing_formats.py index 53d5070429..436507aa89 100644 --- a/swift/common/middleware/listing_formats.py +++ b/swift/common/middleware/listing_formats.py @@ -21,7 +21,7 @@ from swift.common.constraints import valid_api_version from swift.common.http import HTTP_NO_CONTENT from swift.common.request_helpers import get_param from swift.common.swob import HTTPException, HTTPNotAcceptable, Request, \ - RESPONSE_REASONS + RESPONSE_REASONS, HTTPBadRequest #: Mapping of query string ``format=`` values to their corresponding @@ -51,8 +51,11 @@ def get_listing_content_type(req): if query_format: req.accept = FORMAT2CONTENT_TYPE.get( query_format.lower(), FORMAT2CONTENT_TYPE['plain']) - out_content_type = req.accept.best_match( - ['text/plain', 'application/json', 'application/xml', 'text/xml']) + try: + out_content_type = req.accept.best_match( + ['text/plain', 'application/json', 'application/xml', 'text/xml']) + except ValueError: + raise HTTPBadRequest(request=req, body='Invalid Accept header') if not out_content_type: raise HTTPNotAcceptable(request=req) return out_content_type diff --git a/swift/common/middleware/slo.py b/swift/common/middleware/slo.py index fdc2e9efc1..63b664913e 100644 --- a/swift/common/middleware/slo.py +++ b/swift/common/middleware/slo.py @@ -929,7 +929,10 @@ class StaticLargeObject(object): 'Number of segments must be <= %d' % self.max_manifest_segments) total_size = 0 - out_content_type = req.accept.best_match(ACCEPTABLE_FORMATS) + try: + out_content_type = req.accept.best_match(ACCEPTABLE_FORMATS) + except ValueError: + out_content_type = 'text/plain' # Ignore invalid header if not out_content_type: out_content_type = 'text/plain' data_for_storage = [] @@ -1154,7 +1157,10 @@ class StaticLargeObject(object): """ req.headers['Content-Type'] = None # Ignore content-type from client resp = HTTPOk(request=req) - out_content_type = req.accept.best_match(ACCEPTABLE_FORMATS) + try: + out_content_type = req.accept.best_match(ACCEPTABLE_FORMATS) + except ValueError: + out_content_type = None # Ignore invalid header if out_content_type: resp.content_type = out_content_type resp.app_iter = self.bulk_deleter.handle_delete_iter( diff --git a/swift/common/swob.py b/swift/common/swob.py index eb585a2846..08726a28db 100644 --- a/swift/common/swob.py +++ b/swift/common/swob.py @@ -691,11 +691,9 @@ class Accept(object): Returns None if no available options are acceptable to the client. :param options: a list of content-types the server can respond with + :raises ValueError: if the header is malformed """ - try: - types = self._get_types() - except ValueError: - return None + types = self._get_types() if not types and options: return options[0] for pattern in types: diff --git a/test/unit/account/test_server.py b/test/unit/account/test_server.py index cef4efc345..2c00773441 100644 --- a/test/unit/account/test_server.py +++ b/test/unit/account/test_server.py @@ -353,6 +353,13 @@ class TestAccountController(unittest.TestCase): resp = req.get_response(self.controller) self.assertEqual(resp.status_int, 406) + def test_HEAD_invalid_accept(self): + req = Request.blank('/sda1/p/a', environ={'REQUEST_METHOD': 'HEAD'}, + headers={'Accept': 'application/plain;q=1;q=0.5'}) + resp = req.get_response(self.controller) + self.assertEqual(resp.status_int, 400) + self.assertEqual(resp.body, '') + def test_HEAD_invalid_format(self): format = '%D1%BD%8A9' # invalid UTF-8; should be %E1%BD%8A9 (E -> D) req = Request.blank('/sda1/p/a?format=' + format, @@ -787,6 +794,13 @@ class TestAccountController(unittest.TestCase): self.assertEqual(resp.headers['Content-Type'], 'application/xml; charset=utf-8') + def test_GET_invalid_accept(self): + req = Request.blank('/sda1/p/a', environ={'REQUEST_METHOD': 'GET'}, + headers={'Accept': 'application/plain;q=foo'}) + resp = req.get_response(self.controller) + self.assertEqual(resp.status_int, 400) + self.assertEqual(resp.body, 'Invalid Accept header') + def test_GET_over_limit(self): req = Request.blank( '/sda1/p/a?limit=%d' % (constraints.ACCOUNT_LISTING_LIMIT + 1), @@ -2096,8 +2110,8 @@ class TestAccountController(unittest.TestCase): StoragePolicy(2, 'two', False), StoragePolicy(3, 'three', False)]) class TestNonLegacyDefaultStoragePolicy(TestAccountController): - pass + if __name__ == '__main__': unittest.main() diff --git a/test/unit/common/test_swob.py b/test/unit/common/test_swob.py index 8ed19c8ab5..81b8eb4c63 100644 --- a/test/unit/common/test_swob.py +++ b/test/unit/common/test_swob.py @@ -358,11 +358,11 @@ class TestAccept(unittest.TestCase): 'text /plain', 'text\x7f/plain', 'text/plain;a=b=c', 'text/plain;q=1;q=2', + 'text/plain;q=not-a-number', 'text/plain; ubq="unbalanced " quotes"'): acc = swift.common.swob.Accept(accept) - match = acc.best_match(['text/plain', 'application/xml', - 'text/xml']) - self.assertIsNone(match) + with self.assertRaises(ValueError): + acc.best_match(['text/plain', 'application/xml', 'text/xml']) def test_repr(self): acc = swift.common.swob.Accept("application/json") diff --git a/test/unit/container/test_server.py b/test/unit/container/test_server.py index 502f73948e..a30a455873 100644 --- a/test/unit/container/test_server.py +++ b/test/unit/container/test_server.py @@ -312,6 +312,14 @@ class TestContainerController(unittest.TestCase): resp = req.get_response(self.controller) self.assertEqual(resp.status_int, 406) + def test_HEAD_invalid_accept(self): + req = Request.blank( + '/sda1/p/a/c', environ={'REQUEST_METHOD': 'HEAD'}, + headers={'Accept': 'application/plain;q'}) + resp = req.get_response(self.controller) + self.assertEqual(resp.status_int, 400) + self.assertEqual(resp.body, '') + def test_HEAD_invalid_format(self): format = '%D1%BD%8A9' # invalid UTF-8; should be %E1%BD%8A9 (E -> D) req = Request.blank( @@ -2367,6 +2375,14 @@ class TestContainerController(unittest.TestCase): self.assertEqual(resp.content_type, 'text/xml') self.assertEqual(resp.body, xml_body) + def test_GET_invalid_accept(self): + req = Request.blank( + '/sda1/p/a/c', environ={'REQUEST_METHOD': 'GET'}, + headers={'Accept': 'application/plain;q'}) + resp = req.get_response(self.controller) + self.assertEqual(resp.status_int, 400) + self.assertEqual(resp.body, 'Invalid Accept header') + def test_GET_marker(self): # make a container req = Request.blank( diff --git a/test/unit/proxy/test_server.py b/test/unit/proxy/test_server.py index 5c84293409..975674112a 100644 --- a/test/unit/proxy/test_server.py +++ b/test/unit/proxy/test_server.py @@ -9271,6 +9271,24 @@ class TestAccountControllerFakeGetResponse(unittest.TestCase): resp = req.get_response(self.app) self.assertEqual(406, resp.status_int) + def test_GET_autocreate_bad_accept(self): + with save_globals(): + set_http_connect(*([404] * 100)) # nonexistent: all backends 404 + req = Request.blank('/v1/a', headers={"Accept": "a/b;q=nope"}, + environ={'REQUEST_METHOD': 'GET', + 'PATH_INFO': '/v1/a'}) + resp = req.get_response(self.app) + self.assertEqual(400, resp.status_int) + self.assertEqual('Invalid Accept header', resp.body) + + set_http_connect(*([404] * 100)) # nonexistent: all backends 404 + req = Request.blank('/v1/a', headers={"Accept": "a/b;q=0.5;q=1"}, + environ={'REQUEST_METHOD': 'GET', + 'PATH_INFO': '/v1/a'}) + resp = req.get_response(self.app) + self.assertEqual(400, resp.status_int) + self.assertEqual('Invalid Accept header', resp.body) + def test_GET_autocreate_format_invalid_utf8(self): with save_globals(): set_http_connect(*([404] * 100)) # nonexistent: all backends 404