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

@@ -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(
-            headers['Content-Type'] = req.accept.best_match(
+            ['text/plain', 'application/json', 'application/xml', 'text/xml'])
-                ['text/plain', 'application/json', 'application/xml',
+        if not headers['Content-Type']:
-                 'text/xml'],
+            return HTTPNotAcceptable(request=req)
-                default_match='text/plain')
-        except AssertionError, err:
-            return HTTPBadRequest(body='bad accept header: %s' % req.accept,
-                                  content_type='text/plain', 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(
-            out_content_type = req.accept.best_match(
+            ['text/plain', 'application/json', 'application/xml', 'text/xml'])
-                ['text/plain', 'application/json', 'application/xml',
+        if not out_content_type:
-                 'text/xml'],
+            return HTTPNotAcceptable(request=req)
-                default_match='text/plain')
-        except AssertionError, err:
-            return HTTPBadRequest(body='bad accept header: %s' % req.accept,
-                                  content_type='text/plain', request=req)
         account_list = broker.list_containers_iter(limit, marker, end_marker,
                                                    prefix, delimiter)
         if out_content_type == 'application/json':

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(','):
+        if not self.headerval:
-            quality = 1.0
+            return []
-            if '; q=' in typ:
+        for typ in self.headerval.split(','):
-                typ, quality = typ.split('; q=')
+            type_parms = self.acc_pattern.findall(typ)
-            elif ';q=' in typ:
+            if not type_parms:
-                typ, quality = typ.split(';q=')
+                raise ValueError('Invalid accept header')
-            quality = float(quality)
+            typ, subtype, parms, quality = type_parms[0]
-            if typ.startswith('*/'):
+            quality = float(quality or '1.0')
-                quality -= 0.01
+            pattern = '^' + \
-            elif typ.endswith('/*'):
+                (self.token if typ == '*' else re.escape(typ)) + '/' + \
-                quality -= 0.01
+                (self.token if subtype == '*' else re.escape(subtype)) + '$'
-            elif '*' in typ:
+            types.append((pattern, quality, '*' not in (typ, subtype)))
-                raise AssertionError('bad accept header')
+        # sort candidates by quality, then whether or not there were globs
-            pattern = '[a-zA-Z0-9-]+'.join([re.escape(x) for x in
+        types.sort(reverse=True, key=lambda t: (t[1], t[2]))
-                                            typ.strip().split('*')])
+        return [t[0] for t in types]
-            types.append((quality, re.compile(pattern), typ))
-        types.sort(reverse=True, key=lambda t: t[0])
-        return types
 
-    def best_match(self, options, default_match='text/plain'):
+    def best_match(self, options):
-        for quality, pattern, typ in self._get_types():
+        """
+        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]

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(
-            headers['Content-Type'] = req.accept.best_match(
+            ['text/plain', 'application/json', 'application/xml', 'text/xml'])
-                ['text/plain', 'application/json', 'application/xml',
+        if not headers['Content-Type']:
-                 'text/xml'],
+            return HTTPNotAcceptable(request=req)
-                default_match='text/plain')
-        except AssertionError, err:
-            return HTTPBadRequest(body='bad accept header: %s' % req.accept,
-                                  content_type='text/plain', 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(
-            out_content_type = req.accept.best_match(
+            ['text/plain', 'application/json', 'application/xml', 'text/xml'])
-                ['text/plain', 'application/json', 'application/xml',
+        if not out_content_type:
-                 'text/xml'],
+            return HTTPNotAcceptable(request=req)
-                default_match='text/plain')
-        except AssertionError, err:
-            return HTTPBadRequest(body='bad accept header: %s' % req.accept,
-                                  content_type='text/plain', request=req)
         container_list = broker.list_objects_iter(limit, marker, end_marker,
                                                   prefix, delimiter, path)
         if out_content_type == 'application/json':

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.status_int, 406)
-        self.assertEquals(resp.body, 'bad accept header: application/xml*')
 
     def test_GET_prefix_delimeter_plain(self):
         req = Request.blank('/sda1/p/a', environ={'REQUEST_METHOD': 'PUT',

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'],
+                                    'application/xml', 'text/xml'])
-                                   default_match='text/plain')
             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'],
+                                    'application/xml', 'text/xml'])
-                                   default_match='text/plain')
             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):

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.status_int, 406)
-        self.assertEquals(resp.body, 'bad accept header: application/xml*')
+
-        
     def test_GET_limit(self):
         # make a container
         req = Request.blank('/sda1/p/a/c', environ={'REQUEST_METHOD': 'PUT',