Mercurial > p > roundup > code
changeset 7595:26ef5054e510
refactor(api): early return if REST rate limit is exceeded
If the rate limit at the top of dispatch() is exceeded, return rather
than running to final return at end of method.
This does change a couple of things:
1) output format is always in json
2) json is alays pretty printed
Previously, the output format requested by the Accept header or format
extension in the URL was used for 1. Similarly value of @pretty was
used to control 2.
I am trying to reduce the complexities in this routine with the goal
of fixing: issue2551289 to return 406 error if the Accept
header/format extension is incorrect before executing the request.
| author | John Rouillard <rouilj@ieee.org> |
|---|---|
| date | Thu, 03 Aug 2023 17:04:34 -0400 |
| parents | c9180009a286 |
| children | e5fa31aad344 |
| files | roundup/rest.py |
| diffstat | 1 files changed, 40 insertions(+), 102 deletions(-) [+] |
line wrap: on
line diff
--- a/roundup/rest.py Mon Jul 31 18:50:07 2023 -0400 +++ b/roundup/rest.py Thu Aug 03 17:04:34 2023 -0400 @@ -2042,9 +2042,6 @@ output = None # Before we do anything has the user hit the rate limit. - # This should (but doesn't at the moment) bypass - # all other processing to minimize load of badly - # behaving client. # Get the limit here and not in the init() routine to allow # for a different rate limit per user. @@ -2076,32 +2073,40 @@ if reject: for header, value in limitStatus.items(): self.client.setHeader(header, value) - # User exceeded limits: tell humans how long to wait - # Headers above will do the right thing for api - # aware clients. - try: - retry_after = limitStatus['Retry-After'] - except KeyError: - # handle race condition. If the time between - # the call to grca.update and grca.status - # is sufficient to reload the bucket by 1 - # item, Retry-After will be missing from - # limitStatus. So report a 1 second delay back - # to the client. We treat update as sole - # source of truth for exceeded rate limits. - retry_after = 1 - self.client.setHeader('Retry-After', '1') + # User exceeded limits: tell humans how long to wait + # Headers above will do the right thing for api + # aware clients. + try: + retry_after = limitStatus['Retry-After'] + except KeyError: + # handle race condition. If the time between + # the call to grca.update and grca.status + # is sufficient to reload the bucket by 1 + # item, Retry-After will be missing from + # limitStatus. So report a 1 second delay back + # to the client. We treat update as sole + # source of truth for exceeded rate limits. + retry_after = 1 + self.client.setHeader('Retry-After', '1') - msg = _("Api rate limits exceeded. Please wait: %s seconds.") % retry_after - output = self.error_obj(429, msg, source="ApiRateLimiter") - else: - 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) + msg = _("Api rate limits exceeded. Please wait: %s seconds.") % retry_after + output = self.error_obj(429, msg, source="ApiRateLimiter") + + return self.format_dispatch_output( + self.__default_accept_type, + output, + True # pretty print for this error case as a + # human may read it + ) + + + 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 @@ -2121,76 +2126,6 @@ 'Ignoring X-HTTP-Method-Override using %s request on %s', method.upper(), uri) - # parse Accept header and get the content type - # Acceptable types ordered with preferred one first - # in list. - 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: - 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: - # 1) accept header version=X specifier - # application/vnd.x.y; version=1 - # 2) from type in accept-header type/subtype-vX - # application/vnd.x.y-v1 - # 3) from @apiver in query string to make browser - # use easy - # This code handles 1 and 2. Set api_version to none - # to trigger @apiver parsing below - # Places that need the api_version info should - # use default if version = None - try: - self.api_version = int(part[1]['version']) - except KeyError: - self.api_version = None - except (ValueError, TypeError): - # TypeError if int(None) - msg = ("Unrecognized api version: %s. " - "See /rest without specifying api version " - "for supported versions." % ( - part[1]['version'])) - output = self.error_obj(400, msg) - - # get the request format for response - # priority : extension from uri (/rest/data/issue.json), - # 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 method.upper() == 'OPTIONS': # add access-control-allow-* access-control-max-age to support # CORS preflight @@ -2349,15 +2284,18 @@ 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) + + def format_dispatch_output(self, accept_mime_type, output, pretty_print): # Format the content type - if data_type.lower() == "json": + if accept_mime_type.lower() == "json": self.client.setHeader("Content-Type", "application/json") - if pretty_output: + if pretty_print: indent = 4 else: indent = None output = RoundupJSONEncoder(indent=indent).encode(output) - elif data_type.lower() == "xml" and dicttoxml: + elif accept_mime_type.lower() == "xml" and dicttoxml: self.client.setHeader("Content-Type", "application/xml") if 'error' in output: # capture values in error with types unsupported @@ -2390,7 +2328,7 @@ # display acceptable output. self.client.response_code = 406 output = ("Requested content type '%s' is not available.\n" - "Acceptable types: %s" % (data_type, + "Acceptable types: %s" % (accept_mime_type, ", ".join(sorted(self.__accepted_content_type.keys())))) # Make output json end in a newline to