Accept header parsing fixes. Now return first acceptable match rather

than last. If not acceptable match in accept, 406 error returns list
of acceptable types as text string. application/xml is listed in
acceptable types only if dicttoxml is installed.  Handle q > 1.0 by
demoting q factor to 0.0001 making it unusable.

Test cases for all this code. XML is commented out as we don't install
dicttoxml.py.

Author: rouilj

roundup/rest.py

@@ -241,6 +241,12 @@ def parse_accept_header(accept):
             value = value.strip()
             if key == "q":
                 q = float(value)
+                if q > 1.0:
+                    # Not sure what to do here. Can't find spec
+                    # about how to handle q > 1.0. Since invalid
+                    # I choose to make it lowest in priority.
+                    pass
+                    q = 0.0001
             else:
                 media_params.append((key, value))
         result.append((media_type, dict(media_params), q))
@@ -337,7 +343,6 @@ class RestfulInstance(object):
     __accepted_content_type = {
         "application/json": "json",
         "*/*": "json",
-        "application/xml": "xml"
     }
     __default_accept_type = "json"
 
@@ -361,6 +366,9 @@ def __init__(self, client, db):
         self.base_path = '%srest' % (self.db.config.TRACKER_WEB)
         self.data_path = self.base_path + '/data'
 
+        if dicttoxml: # add xml if supported
+            self.__accepted_content_type["application/xml"] = "xml"
+
     def props_from_args(self, cl, args, itemid=None, skip_protected=True):
         """Construct a list of properties from the given arguments,
         and return them after validation.
@@ -1729,9 +1737,18 @@ def dispatch(self, method, uri, input):
                     method.upper(), uri)
 
         # parse Accept header and get the content type
+        # Acceptable types ordered with preferred one first
+        # in list.
         accept_header = parse_accept_header(headers.get('Accept'))
-        accept_type =  self.__default_accept_type
+        if not accept_header:
+            accept_type = self.__default_accept_type
+        else:
+            accept_type = None
         for part in accept_header:
+            if accept_type:
+                # we accepted the best match, stop searching for
+                # lower quality matches.
+                break
             if part[0] in self.__accepted_content_type:
                 accept_type = self.__accepted_content_type[part[0]]
                 # Version order:
@@ -1762,7 +1779,7 @@ def dispatch(self, method, uri, input):
         #            header (Accept: application/json, application/xml)
         #            default (application/json)
         ext_type = os.path.splitext(urlparse(uri).path)[1][1:]
-        data_type = ext_type or accept_type
+        data_type = ext_type or accept_type or "invalid"
 
         if ( ext_type ):
             # strip extension so uri make sense
@@ -1893,7 +1910,9 @@ def dispatch(self, method, uri, input):
             # error out before doing any work if we can't
             # display acceptable output.
             self.client.response_code = 406
-            output = "Content type is not accepted by client"
+            output = ( "Requested content type is not available.\n"
+                       "Acceptable types: %s"%(
+                           ", ".join(self.__accepted_content_type.keys())))
 
         # Make output json end in a newline to
         # separate from following text in logs etc..

test/rest_common.py

@@ -1368,6 +1368,114 @@ def testDispatch(self):
 
         del(self.headers)
 
+    def testAcceptHeaderParsing(self):
+        # TEST #1
+        # json highest priority
+        self.server.client.request.headers.get=self.get_header
+        headers={"accept": "application/json; version=1,"
+                           "application/xml; q=0.5; version=2,"
+                           "text/plain; q=0.75; version=2"
+        }
+        self.headers=headers
+        results = self.server.dispatch('GET',
+                                       "/rest/data/status/1",
+                                       self.empty_form)
+        print(results)
+        self.assertEqual(self.server.client.response_code, 200)
+        self.assertEqual(self.server.client.additional_headers['Content-Type'],
+                         "application/json")
+
+        # TEST #2
+        # text highest priority
+        headers={"accept": "application/json; q=0.5; version=1,"
+                           "application/xml; q=0.25; version=2,"
+                           "text/plain; q=1.0; version=3"
+        }
+        self.headers=headers
+        results = self.server.dispatch('GET',
+                                       "/rest/data/status/1",
+                                       self.empty_form)
+        print(results)
+        self.assertEqual(self.server.client.response_code, 200)
+        self.assertEqual(self.server.client.additional_headers['Content-Type'],
+                         "application/json")
+
+        # TEST #3
+        # no acceptable type
+        headers={"accept": "text/plain; q=1.0; version=2"
+        }
+        self.headers=headers
+        results = self.server.dispatch('GET',
+                                       "/rest/data/status/1",
+                                       self.empty_form)
+        print(results)
+        self.assertEqual(self.server.client.response_code, 406)
+        self.assertEqual(self.server.client.additional_headers['Content-Type'],
+                         "application/json")
+
+        # TEST #4
+        # no accept header, should use default json
+        headers={}
+        self.headers=headers
+        results = self.server.dispatch('GET',
+                                       "/rest/data/status/1",
+                                       self.empty_form)
+        print(results)
+        self.assertEqual(self.server.client.response_code, 200)
+        self.assertEqual(self.server.client.additional_headers['Content-Type'],
+                         "application/json")
+
+        # TEST #5
+        # wildcard accept header, should use default json
+        headers={ "accept": "*/*"}
+        self.headers=headers
+        results = self.server.dispatch('GET',
+                                       "/rest/data/status/1",
+                                       self.empty_form)
+        print(results)
+        self.assertEqual(self.server.client.response_code, 200)
+        self.assertEqual(self.server.client.additional_headers['Content-Type'],
+                         "application/json")
+
+        # TEST #6
+        # invalid q factor if not ignored/demoted
+        # application/json is selected with invalid version
+        # and errors.
+        # this ends up choosing */* which triggers json.
+        self.server.client.request.headers.get=self.get_header
+        headers={"accept": "application/json; q=1.5; version=99,"
+                           "*/*; q=0.9; version=1,"
+                           "text/plain; q=3.75; version=2"
+        }
+        self.headers=headers
+        results = self.server.dispatch('GET',
+                                       "/rest/data/status/1",
+                                       self.empty_form)
+        print(results)
+        self.assertEqual(self.server.client.response_code, 200)
+        self.assertEqual(self.server.client.additional_headers['Content-Type'],
+                         "application/json")
+
+
+        '''
+        # only works if dicttoxml.py is installed.
+        #   not installed for testing
+        # TEST #7
+        # xml wins
+        headers={"accept": "application/json; q=0.5; version=2,"
+                           "application/xml; q=0.75; version=1,"
+                           "text/plain; q=1.0; version=2"
+        }
+        self.headers=headers
+        results = self.server.dispatch('GET',
+                                       "/rest/data/status/1",
+                                       self.empty_form)
+        print(results)
+        self.assertEqual(self.server.client.response_code, 200)
+        self.assertEqual(self.server.client.additional_headers['Content-Type'],
+                         "application/xml")
+        '''
+
     def testMethodOverride(self):
         # TEST #1
         # Use GET, PUT, PATCH to tunnel DELETE expect error
@@ -1408,7 +1516,7 @@ def testMethodOverride(self):
         etag = calculate_etag(self.db.status.getnode("1"),
                               self.db.config['WEB_SECRET_KEY'])
         etagb = etag.strip ('"')
-        headers={"accept": "application/json; q=0.75, application/xml; q=1",
+        headers={"accept": "application/json; q=1.0, application/xml; q=0.75",
                  "if-match": '"%s"'%etagb,
                  "content-length": 0,
                  "x-http-method-override": "DElETE"