Respond 400 Bad Request when Accept headers fail to parse

Change-Id: I6eb4e4bca95e2ee4fecdb703394cb2419737922d
Closes-Bug: 1716509

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(

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(
+    try:
-        ['text/plain', 'application/json', 'application/xml', 'text/xml'])
+        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

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(

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()
-            types = self._get_types()
-        except ValueError:
-            return None
         if not types and options:
             return options[0]
         for pattern in types:

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()

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',
+            with self.assertRaises(ValueError):
-                                   'text/xml'])
+                acc.best_match(['text/plain', 'application/xml', 'text/xml'])
-            self.assertIsNone(match)
 
     def test_repr(self):
         acc = swift.common.swob.Accept("application/json")

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(

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