Merge "406 if we can't satisfy Accept"

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',