Mercurial > p > roundup > code
changeset 6311:be8d5a8e090a
Fix uncaught error when parsing rest headers, document
Started this work as better docs for rest response format. But I found
406 error response was not being tested. Also there was no error for
bad Content-Type.
In rest.py fix uncaught exceptions due to invalid Accept or
Content-Type headers. If Content-type is valid but not
application/json return code 415.
Document use of accept header (was only shown in examples) and support
for q parameter. Describe using .xml and .json extensions to select
return format for testing from browser (where setting accept header is
a problem). Document 406 error code return. Document 415 error code
return and acceptable content types. Previously only doc was in
examples.
Set up tests for 406 and 415 error codes.
| author | John Rouillard <rouilj@ieee.org> |
|---|---|
| date | Fri, 01 Jan 2021 14:14:34 -0500 |
| parents | 68d83479747b |
| children | 6ef7b66774b4 |
| files | CHANGES.txt doc/rest.txt roundup/rest.py test/rest_common.py |
| diffstat | 4 files changed, 260 insertions(+), 10 deletions(-) [+] |
line wrap: on
line diff
--- a/CHANGES.txt Sun Dec 27 15:04:02 2020 -0500 +++ b/CHANGES.txt Fri Jan 01 14:14:34 2021 -0500 @@ -63,6 +63,9 @@ - fixed some issues when generating translations. Use mappings and named format parameters so translators can move substituted tokens in tranlsations. +- in rest interface, fix uncaught exceptions when parsing invalid + Content-Type and Accept headers. Document response formats more + fully in doc/rest.txt. Features: - issue2550522 - Add 'filter' command to command-line
--- a/doc/rest.txt Sun Dec 27 15:04:02 2020 -0500 +++ b/doc/rest.txt Fri Jan 01 14:14:34 2021 -0500 @@ -187,6 +187,36 @@ The server default is reported by querying the ``/rest/`` endpoint as described above. +Input Formats +============= + +Content-Type is allowed to be ``application/json`` or +``application/x-www-form-urlencoded``. Any other value returns error +code 415. + +Response Formats +================ + +The default response format is json. + +If you add the ``dicttoxml.py`` module you can request XML formatted +data using the header ``Accept: application/xml`` in your +request. Both output formats are similar in structure. + +The rest interface accepts the http accept header and can include +``q`` values to specify the preferred mechanism. This is the preferred +way to specify alternate acceptable response formats. + +To make testing from the browser easier, you can also append the +extension `.json` or `.xml` to the path component of the url. This +will force json or xml (if supported) output. If you use an extension +it takes priority over any accept headers. + +The rest interface returns status 406 if you use an unrecognized +extension. You will also get a 406 status if none of the entries in +the accept header are available or if the accept header is invalid. + + General Guidelines ================== @@ -200,12 +230,8 @@ ``@verbose`` query parameter. The various supported values and their effects are described in the following sections. -The default return format is JSON. If you add the ``dicttoxml.py`` -module you can request XML formatted data using the header ``Accept: -application/xml`` in your request. Both output formats are similar in -structure. - -All output is wrapped in an envelope called ``data``. +All output is wrapped in an envelope called ``data``. The output +format is described in `Response Formats`_ above. When using collection endpoints (think list of issues, users ...), the ``data`` envelope contains metadata (e.g. total number of items) as
--- a/roundup/rest.py Sun Dec 27 15:04:02 2020 -0500 +++ b/roundup/rest.py Fri Jan 01 14:14:34 2021 -0500 @@ -223,7 +223,10 @@ media_params = [] # convert vendor-specific content types into something useful (see # docstring) - typ, subtyp = media_type.split('/') + try: + typ, subtyp = media_type.split('/') + except ValueError: + raise UsageError("Invalid media type: %s"%media_type) # check for a + in the sub-type if '+' in subtyp: # if it exists, determine if the subtype is a vendor-specific type @@ -246,7 +249,10 @@ media_type = '{}/{}'.format(typ, extra) q = 1.0 for part in parts: - (key, value) = part.lstrip().split("=", 1) + try: + (key, value) = part.lstrip().split("=", 1) + except ValueError: + raise UsageError("Invalid param: %s"%part.lstrip()) key = key.strip() value = value.strip() if key == "q": @@ -1873,7 +1879,15 @@ # 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')) + try: + accept_header = parse_accept_header(headers.get('Accept')) + except UsageError as e: + output = self.error_obj(406, _("Unable to parse Accept Header. %(error)s. " + "Acceptable types: %(acceptable_types)s") % { + 'error': e.args[0], + 'acceptable_types': " ".join(sorted(self.__accepted_content_type.keys()))}) + accept_header = [] + if not accept_header: accept_type = self.__default_accept_type else: @@ -1913,7 +1927,12 @@ # 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 or "invalid" + + # headers.get('Accept') is never empty if called here. + # accept_type will be set to json if there is no Accept header + # accept_type wil be empty only if there is an Accept header + # with invalid values. + data_type = ext_type or accept_type or headers.get('Accept') or "invalid" if (ext_type): # strip extension so uri make sense @@ -1956,6 +1975,10 @@ input = SimulateFieldStorageFromJson(b2s(input.value)) except ValueError as msg: output = self.error_obj(400, msg) + else: + output = self.error_obj(415, + "Unable to process input of type %s" % + content_type_header) # check for pretty print try:
--- a/test/rest_common.py Sun Dec 27 15:04:02 2020 -0500 +++ b/test/rest_common.py Fri Jan 01 14:14:34 2021 -0500 @@ -1321,6 +1321,204 @@ ['assignedto']['link'], "http://tracker.example/cgi-bin/roundup.cgi/bugs/rest/data/user/2") + def testDispatchBadContent(self): + """ + runthrough rest dispatch() with bad content_type patterns. + """ + + # simulate: /rest/data/issue + body=b'{ "title": "Joe Doe has problems", \ + "nosy": [ "1", "3" ], \ + "assignedto": "2", \ + "abool": true, \ + "afloat": 2.3, \ + "anint": 567890 \ + }' + env = { "CONTENT_TYPE": "application/jzot", + "CONTENT_LENGTH": len(body), + "REQUEST_METHOD": "POST" + } + + headers={"accept": "application/json; version=1", + "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 + results = self.server.dispatch(env["REQUEST_METHOD"], + "/rest/data/issue", + form) + + print(results) + self.assertEqual(self.server.client.response_code, 415) + json_dict = json.loads(b2s(results)) + self.assertEqual(json_dict['error']['msg'], + "Unable to process input of type application/jzot") + + # Test GET as well. I am not sure if this should pass or not. + # Arguably GET doesn't use any form/json input but.... + results = self.server.dispatch('GET', + "/rest/data/issue", + form) + print(results) + self.assertEqual(self.server.client.response_code, 415) + + + + def testDispatchBadAccept(self): + # 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" + } + + 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 + results = self.server.dispatch(env["REQUEST_METHOD"], + "/rest/data/issue", + form) + + print(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) + + # simulate: /rest/data/issue works, multiple acceptable output, one + # is valid + env = { "CONTENT_TYPE": "application/json", + "CONTENT_LENGTH": len(body), + "REQUEST_METHOD": "POST" + } + + 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 + results = self.server.dispatch(env["REQUEST_METHOD"], + "/rest/data/issue", + form) + + 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") + + + # test 3 accept is empty. This triggers */* so passes + 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 + results = self.server.dispatch(env["REQUEST_METHOD"], + "/rest/data/issue", + form) + + 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") + + # 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 + results = self.server.dispatch(env["REQUEST_METHOD"], + "/rest/data/issue", + form) + + print(results) + self.assertEqual(self.server.client.response_code, 406) + 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']) + + # 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 + results = self.server.dispatch(env["REQUEST_METHOD"], + "/rest/data/issue", + form) + + print(results) + self.assertEqual(self.server.client.response_code, 406) + json_dict = json.loads(b2s(results)) + self.assertIn('Unable to parse Accept Header. Invalid param: foo. Acceptable types: */* application/json', json_dict['error']['msg']) def testStatsGen(self): # check stats being returned by put and get ops