Mercurial > p > roundup > code

diff test/rest_common.py @ 8177:2967f37e73e4

Find changesets by keywords (author, files, the commit message), revision number or hash, or revset expression.
refactor: issue2551289. invalid REST Accept header stops request Sending a POST, PUT (maybe PATCH) with an accept header that is not application/json or xml (if enabled) used to complete the request before throwing a 406 error. This was wrong. Now it reports an error without dispatching/processing the requested transaction. This is the first of a series of refactors of the dispatch method to make it faster and more readable by using return early pattern and extracting methods from the code. changes: The following now return 406 errors not 400 errors invalid version specified with @apiver in URL. invalid version specified with @apiver in payload body invalid version specified in accept headers as application/vnd.roundup.test-vz+json or version property Parsing the accept header returns a 400 when presented with a parameter without an = sign or other parse error. They used to return a 406 which is wrong since the header is malformed rather than having a value I can't respond to. Some error messages were made clearer. Results in the case of an error are proper json error object rather than text/plain strings. New test added for testdetermine_output_formatBadAccept that test the new method using the same test cases as for testDispatchBadAccept. I intend to extend the test coverage for determine_output_format to cover more cases. This should be a faster unit test than for dispatch. Removed .lower() calls for accept_mime_type as the input values are taken from the values in the __accepted_content_type dict which only has lower case values.
author John Rouillard <rouilj@ieee.org>
date Sun, 08 Dec 2024 01:09:34 -0500
parents 8e310a7b5e09
children d02ce1d14acd
line wrap: on
line diff
--- a/test/rest_common.py	Sat Dec 07 17:34:06 2024 -0500
+++ b/test/rest_common.py	Sun Dec 08 01:09:34 2024 -0500
@@ -1837,6 +1837,140 @@
                          self.server.client.additional_headers)
         self.server.client.additional_headers = {}
 
+    def testdetermine_output_formatBadAccept(self):
+        dof = self.server.determine_output_format
+
+        # simulate: /rest/data/issue expect failure unknown accept settings
+        body=b'{ "title": "Joe Doe has problems", \
+                 "nosy": [ "1", "3" ], \
+                 "assignedto": "2", \
+                 "abool": true, \
+                 "afloat": 2.3, \
+                 "anint": 567890 \
+        }'
+        env = { "CONTENT_TYPE": "application/json",
+                "CONTENT_LENGTH": len(body),
+                "REQUEST_METHOD": "POST"
+        }
+        self.server.client.env.update(env)
+        headers={"accept": "application/zot; version=1; q=0.5",
+                 "content-type": env['CONTENT_TYPE'],
+                 "content-length": env['CONTENT_LENGTH'],
+        }
+
+        self.headers=headers
+        # we need to generate a FieldStorage the looks like
+        #  FieldStorage(None, None, 'string') rather than
+        #  FieldStorage(None, None, [])
+        body_file=BytesIO(body)  # FieldStorage needs a file
+        form = client.BinaryFieldStorage(body_file,
+                                headers=headers,
+                                environ=env)
+        self.server.client.request.headers.get=self.get_header
+
+        (output_type, uri, error) = dof("/rest/data/issue")
+
+        self.assertEqual(self.server.client.response_code, 406)
+        self.assertIn(b"Requested content type(s) 'application/zot; version=1; q=0.5' not available.\nAcceptable mime types are: */*, application/json",
+                      s2b(error['error']['msg']))
+
+        # simulate: /rest/data/issue works, multiple acceptable output, one
+        # is valid
+        self.server.client.response_code = ""
+        env = { "CONTENT_TYPE": "application/json",
+                "CONTENT_LENGTH": len(body),
+                "REQUEST_METHOD": "POST"
+        }
+        self.server.client.env.update(env)
+        headers={"accept": "application/zot; version=1; q=0.75, "
+                           "application/json; version=1; q=0.5",
+                 "content-type": env['CONTENT_TYPE'],
+                 "content-length": env['CONTENT_LENGTH'],
+        }
+
+        self.headers=headers
+        # we need to generate a FieldStorage the looks like
+        #  FieldStorage(None, None, 'string') rather than
+        #  FieldStorage(None, None, [])
+        body_file=BytesIO(body)  # FieldStorage needs a file
+        form = client.BinaryFieldStorage(body_file,
+                                headers=headers,
+                                environ=env)
+        self.server.client.request.headers.get=self.get_header
+        (output_type, uri, error) = dof("/rest/data/issue")
+
+        self.assertEqual(self.server.client.response_code, "")
+        self.assertEqual(output_type, "json")
+        self.assertEqual(uri, "/rest/data/issue")
+        self.assertEqual(error, None)
+
+        # test 3 accept is empty. This triggers */* so passes
+        self.server.client.response_code = ""
+        headers={"accept": "",
+                 "content-type": env['CONTENT_TYPE'],
+                 "content-length": env['CONTENT_LENGTH'],
+        }
+
+        self.headers=headers
+        # we need to generate a FieldStorage the looks like
+        #  FieldStorage(None, None, 'string') rather than
+        #  FieldStorage(None, None, [])
+        body_file=BytesIO(body)  # FieldStorage needs a file
+        form = client.BinaryFieldStorage(body_file,
+                                headers=headers,
+                                environ=env)
+        self.server.client.request.headers.get=self.get_header
+        (output_type, uri, error) = dof("/rest/data/issue")
+
+        self.assertEqual(self.server.client.response_code, "")
+        self.assertEqual(output_type, "json")
+        self.assertEqual(uri, "/rest/data/issue")
+        self.assertEqual(error, None)
+
+        # test 4 accept is random junk.
+        headers={"accept": "Xyzzy I am not a mime, type;",
+                 "content-type": env['CONTENT_TYPE'],
+                 "content-length": env['CONTENT_LENGTH'],
+        }
+
+        self.headers=headers
+        # we need to generate a FieldStorage the looks like
+        #  FieldStorage(None, None, 'string') rather than
+        #  FieldStorage(None, None, [])
+        body_file=BytesIO(body)  # FieldStorage needs a file
+        form = client.BinaryFieldStorage(body_file,
+                                headers=headers,
+                                environ=env)
+        self.server.client.request.headers.get=self.get_header
+        (output_type, uri, error) = dof("/rest/data/issue")
+
+        self.assertEqual(self.server.client.response_code, 400)
+        self.assertEqual(output_type, None)
+        self.assertEqual(uri, "/rest/data/issue")
+        self.assertIn('Unable to parse Accept Header. Invalid media type: Xyzzy I am not a mime. Acceptable types: */* application/json', error['error']['msg'])
+
+        # test 5 accept mimetype is ok, param is not
+        headers={"accept": "*/*; foo",
+                 "content-type": env['CONTENT_TYPE'],
+                 "content-length": env['CONTENT_LENGTH'],
+        }
+
+        self.headers=headers
+        # we need to generate a FieldStorage the looks like
+        #  FieldStorage(None, None, 'string') rather than
+        #  FieldStorage(None, None, [])
+        body_file=BytesIO(body)  # FieldStorage needs a file
+        form = client.BinaryFieldStorage(body_file,
+                                headers=headers,
+                                environ=env)
+        self.server.client.request.headers.get=self.get_header
+        (output_type, uri, error) = dof("/rest/data/issue")
+
+        self.assertEqual(self.server.client.response_code, 400)
+        self.assertEqual(output_type, None)
+        self.assertEqual(uri, "/rest/data/issue")
+        self.assertIn('Unable to parse Accept Header. Invalid param: foo. Acceptable types: */* application/json', error['error']['msg'])
+
     def testDispatchBadAccept(self):
         # simulate: /rest/data/issue expect failure unknown accept settings
         body=b'{ "title": "Joe Doe has problems", \
@@ -1868,10 +2002,11 @@
         results = self.server.dispatch(env["REQUEST_METHOD"],
                             "/rest/data/issue",
                             form)
-
         print(results)
+        json_dict = json.loads(b2s(results))
         self.assertEqual(self.server.client.response_code, 406)
-        self.assertIn(b"Requested content type 'application/zot; version=1; q=0.5' is not available.\nAcceptable types: */*, application/json", results)
+        self.assertIn("Requested content type(s) 'application/zot; version=1; q=0.5' not available.\nAcceptable mime types are: */*, application/json",
+                      json_dict['error']['msg'])
 
         # simulate: /rest/data/issue works, multiple acceptable output, one
         # is valid
@@ -1902,12 +2037,7 @@
         print(results)
         self.assertEqual(self.server.client.response_code, 201)
         json_dict = json.loads(b2s(results))
-        # ERROR this should be 1. What's happening is that the code
-        # for handling 406 error code runs through everything and creates
-        # the item. Then it throws a 406 after the work is done when it
-        # realizes it can't respond as requested. So the 406 post above
-        # creates issue 1 and this one creates issue 2.
-        self.assertEqual(json_dict['data']['id'], "2")
+        self.assertEqual(json_dict['data']['id'], "1")
 
 
         # test 3 accept is empty. This triggers */* so passes
@@ -1932,9 +2062,7 @@
         print(results)
         self.assertEqual(self.server.client.response_code, 201)
         json_dict = json.loads(b2s(results))
-        # This is one more than above. Will need to be fixed
-        # When error above is fixed.
-        self.assertEqual(json_dict['data']['id'], "3")
+        self.assertEqual(json_dict['data']['id'], "2")
 
         # test 4 accept is random junk.
         headers={"accept": "Xyzzy I am not a mime, type;",
@@ -1956,7 +2084,7 @@
                             form)
 
         print(results)
-        self.assertEqual(self.server.client.response_code, 406)
+        self.assertEqual(self.server.client.response_code, 400)
         json_dict = json.loads(b2s(results))
         self.assertIn('Unable to parse Accept Header. Invalid media type: Xyzzy I am not a mime. Acceptable types: */* application/json', json_dict['error']['msg'])
 
@@ -1980,7 +2108,7 @@
                             form)
 
         print(results)
-        self.assertEqual(self.server.client.response_code, 406)
+        self.assertEqual(self.server.client.response_code, 400)
         json_dict = json.loads(b2s(results))
         self.assertIn('Unable to parse Accept Header. Invalid param: foo. Acceptable types: */* application/json', json_dict['error']['msg'])
 
@@ -2147,7 +2275,7 @@
         results = self.server.dispatch('PUT',
                             "/rest/data/user/%s/realname"%self.joeid,
                             form)
-        self.assertEqual(self.server.client.response_code, 400)
+        self.assertEqual(self.server.client.response_code, 406)
         del(self.headers)
 
         # TEST #2
@@ -2427,13 +2555,14 @@
         print(results)
         try:  # only verify local copy not system installed copy
             from roundup.dicttoxml import dicttoxml
-            includexml = ', application/xml'
+            includexml = ', xml'
         except ImportError:
             includexml = ''
-        
-        response="Requested content type 'jon' is not available.\n" \
-                 "Acceptable types: */*, application/json%s\n"%includexml
-        self.assertEqual(b2s(results), response)
+
+        json_dict = json.loads(b2s(results))
+        response= ("Content type 'jon' requested in URL is not available.\n"
+                 "Acceptable types: json%s\n") % includexml
+        self.assertEqual(json_dict['error']['msg'], response)
 
         # TEST #9
         # GET: test that version can be set with accept:
@@ -2457,7 +2586,8 @@
                             "/rest/data/issue/1", form)
         print("9a: " + b2s(results))
         json_dict = json.loads(b2s(results))
-        self.assertEqual(json_dict['error']['status'], 400)
+        # note bad @apiver returns 400 not 406.
+        self.assertEqual(json_dict['error']['status'], 406)
         self.assertEqual(json_dict['error']['msg'],
               "Unrecognized api version: L. See /rest without "
               "specifying api version for supported versions.")
@@ -2469,7 +2599,8 @@
                             "/rest/data/issue/1", form)
         print("9b: " + b2s(results))
         json_dict = json.loads(b2s(results))
-        self.assertEqual(json_dict['error']['status'], 400)
+        self.assertEqual(self.server.client.response_code, 406)
+        self.assertEqual(json_dict['error']['status'], 406)
         self.assertEqual(json_dict['error']['msg'],
               "Unrecognized api version: z. See /rest without "
               "specifying api version for supported versions.")
@@ -2480,9 +2611,9 @@
         results = self.server.dispatch('GET',
                             "/rest/data/issue/1", self.empty_form)
         print("9c:" + b2s(results))
-        self.assertEqual(self.server.client.response_code, 400)
+        self.assertEqual(self.server.client.response_code, 406)
         json_dict = json.loads(b2s(results))
-        self.assertEqual(json_dict['error']['status'], 400)
+        self.assertEqual(json_dict['error']['status'], 406)
         self.assertEqual(json_dict['error']['msg'],
               "Unrecognized api version: z. See /rest without "
               "specifying api version for supported versions.")
@@ -2495,9 +2626,9 @@
         results = self.server.dispatch('GET',
                             "/rest/data/issue/1", self.empty_form)
         print("9d: " + b2s(results))
-        self.assertEqual(self.server.client.response_code, 400)
+        self.assertEqual(self.server.client.response_code, 406)
         json_dict = json.loads(b2s(results))
-        self.assertEqual(json_dict['error']['status'], 400)
+        self.assertEqual(json_dict['error']['status'], 406)
         self.assertEqual(json_dict['error']['msg'],
               "Unrecognized api version: a. See /rest without "
               "specifying api version for supported versions.")
@@ -2728,7 +2859,7 @@
                                        self.empty_form)
         print(results)
         json_dict = json.loads(b2s(results))
-        self.assertEqual(self.server.client.response_code, 400)
+        self.assertEqual(self.server.client.response_code, 406)
         self.assertEqual(self.server.client.additional_headers['Content-Type'],
                          "application/json")
         self.assertEqual(json_dict['error']['msg'],
Roundup Issue Tracker: http://roundup-tracker.org/