Mercurial Repository: p/roundup/code: roundup/rest.py comparison

comparison roundup/rest.py @ 8177:2967f37e73e4

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	f7bd22bdef9d
children	d02ce1d14acd

comparison

equal deleted inserted replaced

-:736f769b48c8
+:2967f37e73e4
 It will actually convert the vendor and version into parameters and
 convert the content type into `application/json` so appropriate content
 negotiation decisions can be made.
 Default `q` for values that are not specified is 1.0
+If q value > 1.0, it is parsed as a very small value.
 # Based on https://gist.github.com/samuraisam/2714195
 # Also, based on a snipped found in this project:
 #   https://github.com/martinblech/mimerender
 """
 output,
 True  # pretty print for this error case as a
 # human may read it
 ), None)
-def dispatch(self, method, uri, input):
+def determine_output_format(self, uri):
-"""format and process the request"""
+"""Returns tuple of:
-output = None
+(format for returned output,
-# Before we do anything has the user hit the rate limit.
+possibly modified uri,
+output error object (if error else None))
-# Get the limit here and not in the init() routine to allow
-# for a different rate limit per user.
+Verify that client is requesting an output we can produce
-apiRateLimit = self.getRateLimit()
+with a version we support.
-if apiRateLimit:  # if None, disable rate limiting
+Only application/json and application/xml (optional)
-LimitExceeded, limitStatus = self.handle_apiRateLimitExceeded(
+are currently supported. .vcard might be useful
-apiRateLimit)
+in the future for user objects.
-if LimitExceeded:
-return LimitExceeded  # error message
+Find format for returned output:
-for header, value in limitStatus.items():
+1) Get format from url extension (.json, .xml) and return
-# Retry-After will be 0 because
+if invalid return (None, uri, 406 error)
-# user still has quota available.
+if not found continue
-# Don't put out the header.
+2) Parse Accept header obeying q values
-if header in ('Retry-After',):
+if header unparsible return 400 error object.
-continue
+3) if empty or missing Accept header
-self.client.setHeader(header, value)
+return self.__default_accept_type
+4) match and return best Accept header/version
-# if X-HTTP-Method-Override is set, follow the override method
+if version error found in matching type return 406 error
-headers = self.client.request.headers
+5) if no requested format is supported return 406
-# Never allow GET to be an unsafe operation (i.e. data changing).
+error
-# User must use POST to "tunnel" DELETE, PUT, OPTIONS etc.
-override = headers.get('X-HTTP-Method-Override')
+"""
-if override:
+# get the request format for response
-if method.upper() == 'POST':
+# priority : extension from uri (/rest/data/issue.json)
-logger.debug(
+#              only json or xml valid at this time.
-'Method overridden from %s to %s', method, override)
+#            header (Accept: application/json, application/xml)
-method = override
+#            default (application/json)
-else:
+ext_type = os.path.splitext(urlparse(uri).path)[1][1:]
-output = self.error_obj(400,
-"X-HTTP-Method-Override: %s must be used with "
+# Check to see if the extension matches a value in
-"POST method not %s." % (override, method.upper()))
+# self.__accepted_content_type. In the future other output
-logger.info(
+# such as .vcard for downloading user info can be
-'Ignoring X-HTTP-Method-Override using %s request on %s',
+# supported. This method also allow detection of mistyped
-method.upper(), uri)
+# types like jon for json. Limit extension length to less than
+# 10 characters to allow passing JWT via URL path. Could be
+# useful for magic link login method or account recovery workflow,
+# using a JWT with a short expiration time and limited rights
+# (e.g. only password change permission))
+if ext_type and (len(ext_type) < 10):
+if ext_type not in list(self.__accepted_content_type.values()):
+self.client.response_code = 406
+return (None, uri,
+self.error_obj(
+406,
+_("Content type '%s' requested in URL is "
+"not available.\nAcceptable types: %s\n") %
+(ext_type,
+", ".join(sorted(
+set(self.__accepted_content_type.values()))))))
+# strip extension so uri makes sense.
+# E.G. .../issue.json -> .../issue
+uri = uri[:-(len(ext_type) + 1)]
+return (ext_type, uri, None)
 # parse Accept header and get the content type
-# Acceptable types ordered with preferred one first
+# Acceptable types ordered with preferred one (by q value)
-# in list.
+# first in list.
 try:
-accept_header = parse_accept_header(headers.get('Accept'))
+accept_header = parse_accept_header(
+self.client.request.headers.get('Accept')
+)
 except UsageError as e:
-output = self.error_obj(406, _("Unable to parse Accept Header. %(error)s. "
+self.client.response_code = 406
-"Acceptable types: %(acceptable_types)s") % {
+return (None, uri, self.error_obj(
-'error': e.args[0],
+400, _("Unable to parse Accept Header. %(error)s. "
-'acceptable_types': " ".join(sorted(self.__accepted_content_type.keys()))})
+"Acceptable types: %(acceptable_types)s") % {
-accept_header = []
+'error': e.args[0],
+'acceptable_types': " ".join(sorted(
+self.__accepted_content_type.keys()))}))
 if not accept_header:
-accept_type = self.__default_accept_type
+# we are using the default
-else:
+return (self.__default_accept_type, uri, None)
-accept_type = None
+accept_type = ""
 for part in accept_header:
 if accept_type:
 # we accepted the best match, stop searching for
 # lower quality matches.
 break
 try:
 self.api_version = int(part[1]['version'])
 except KeyError:
 self.api_version = None
 except (ValueError, TypeError):
+self.client.response_code = 406
 # TypeError if int(None)
-msg = ("Unrecognized api version: %s. "
+msg = _("Unrecognized api version: %s. "
 "See /rest without specifying api version "
-"for supported versions." % (
+"for supported versions.") % (
-part[1]['version']))
+part[1]['version'])
-output = self.error_obj(400, msg)
+return (None, uri,
+self.error_obj(406, msg))
-# get the request format for response
-# priority : extension from uri (/rest/data/issue.json),
+# accept_type will be empty only if there is an Accept header
-#            header (Accept: application/json, application/xml)
-#            default (application/json)
-ext_type = os.path.splitext(urlparse(uri).path)[1][1:]
-# Check to see if the length of the extension is less than 6.
-# this allows use of .vcard for a future use in downloading
-# user info. It also allows passing through larger items like
-# JWT that has a final component > 6 items. This method also
-# allow detection of mistyped types like jon for json.
-if ext_type and (len(ext_type) < 6):
-# strip extension so uri make sense
-# .../issue.json -> .../issue
-uri = uri[:-(len(ext_type) + 1)]
-else:
-ext_type = None
-# 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 accept_type:
+return (accept_type, uri, None)
+self.client.response_code = 400
+return (None, uri,
+self.error_obj(
+406,
+_("Requested content type(s) '%s' not available.\n"
+"Acceptable mime types are: %s") %
+(self.client.request.headers.get('Accept'),
+", ".join(sorted(
+self.__accepted_content_type.keys())))))
+def dispatch(self, method, uri, input):
+"""format and process the request"""
+output = None
+# Before we do anything has the user hit the rate limit.
+# Get the limit here and not in the init() routine to allow
+# for a different rate limit per user.
+apiRateLimit = self.getRateLimit()
+if apiRateLimit:  # if None, disable rate limiting
+LimitExceeded, limitStatus = self.handle_apiRateLimitExceeded(
+apiRateLimit)
+if LimitExceeded:
+return LimitExceeded  # error message
+for header, value in limitStatus.items():
+# Retry-After will be 0 because
+# user still has quota available.
+# Don't put out the header.
+if header in ('Retry-After',):
+continue
+self.client.setHeader(header, value)
+# if X-HTTP-Method-Override is set, follow the override method
+headers = self.client.request.headers
+# Never allow GET to be an unsafe operation (i.e. data changing).
+# User must use POST to "tunnel" DELETE, PUT, OPTIONS etc.
+override = headers.get('X-HTTP-Method-Override')
+if override:
+if method.upper() == 'POST':
+logger.debug(
+'Method overridden from %s to %s', method, override)
+method = override
+else:
+output = self.error_obj(400,
+"X-HTTP-Method-Override: %s must be used with "
+"POST method not %s." % (override, method.upper()))
+logger.info(
+'Ignoring X-HTTP-Method-Override using %s request on %s',
+method.upper(), uri)
+# FIXME: when this method is refactored, change
+# determine_output_format to raise an exception. Catch it here
+# and return early. Also set self.client.response_code from
+# error object['error']['status'] and remove from
+# determine_output_format.
+(output_format, uri, error) = self.determine_output_format(uri)
+if error:
+output = error
 if method.upper() == 'OPTIONS':
 # add access-control-allow-* access-control-max-age to support
 # CORS preflight
 self.client.setHeader(
 "Access-Control-Allow-Headers",
 # check for @apiver in query string
 msg = _("Unrecognized api version: %s. "
 "See /rest without specifying api version "
 "for supported versions.")
 try:
+# FIXME: the version priority here is different
+# from accept header. accept mime type in url
+# takes priority over Accept header. Opposite here.
 if not self.api_version:
 self.api_version = int(input['@apiver'].value)
 # Can also return a TypeError ("not indexable")
 # In case the FieldStorage could not parse the result
 except (KeyError, TypeError):
 self.api_version = None
 except ValueError:
-output = self.error_obj(400, msg % input['@apiver'].value)
+output = self.error_obj(406, msg % input['@apiver'].value)
 # by this time the API version is set. Error if we don't
 # support it?
 if self.api_version is None:
 # FIXME: do we need to raise an error if client did not specify
 # version? This may be a good thing to require. Note that:
 # Accept: application/json; version=1 may not be legal but....
 #    Use default if not specified for now.
 self.api_version = self.__default_api_version
 elif self.api_version not in self.__supported_api_versions:
-output = self.error_obj(400, msg % self.api_version)
+output = self.error_obj(406, msg % self.api_version)
 # sadly del doesn't work on FieldStorage which can be the type of
 # input. So we have to ignore keys starting with @ at other
 # places in the code.
 # else:
 output = self.error_obj(404, msg)
 except Reject as msg:
 output = self.error_obj(405, msg.args[0])
 self.client.setHeader("Allow", msg.args[1])
-return self.format_dispatch_output(data_type, output, pretty_output)
+return self.format_dispatch_output(output_format,
+output,
+pretty_output)
 def format_dispatch_output(self, accept_mime_type, output,
 pretty_print=True):
 # Format the content type
-if accept_mime_type.lower() == "json":
+# if accept_mime_type is None, the client specified invalid
+# mime types so we default to json output.
+if accept_mime_type == "json" or accept_mime_type is None:
 self.client.setHeader("Content-Type", "application/json")
 if pretty_print:
 indent = 4
 else:
 indent = None
 output = RoundupJSONEncoder(indent=indent).encode(output)
-elif accept_mime_type.lower() == "xml" and dicttoxml:
+elif accept_mime_type == "xml" and dicttoxml:
 self.client.setHeader("Content-Type", "application/xml")
 if 'error' in output:
 # capture values in error with types unsupported
 # by dicttoxml e.g. an exception, into something it
 # can handle

Mercurial > p > roundup > code

comparison roundup/rest.py @ 8177:2967f37e73e4