Opened 3 years ago

Closed 3 years ago

Last modified 3 years ago

#32982 closed Cleanup/optimization (wontfix)

Tighten up the check for status codes in HttpResponse

Reported by: Abhyudai Owned by: nobody
Component: HTTP handling Version: dev
Severity: Normal Keywords: http, status code
Cc: Triage Stage: Unreviewed
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

As of now, all values in between the range 100 and 600 are allowed. https://github.com/django/django/blob/main/django/http/response.py#L123

I'm quoting the relevant section for potential ease of discussion here.

if not 100 <= self.status_code <= 599:
    raise ValueError('HTTP status code must be an integer from 100 to 599.')

Not all of them are valid, for example 111 is not a valid status code. The list of valid status code can be seen here: https://developer.mozilla.org/en-US/docs/Web/HTTP/Status

Using the HTTPStatus class from the http module should probably help us get around the problem.

>>> from http import HTTPStatus as status
>>> status(111)
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/usr/lib/python3.8/enum.py", line 339, in __call__
    return cls.__new__(cls, value)
  File "/usr/lib/python3.8/enum.py", line 663, in __new__
    raise ve_exc
ValueError: 111 is not a valid HTTPStatus
>>> status(100)
<HTTPStatus.CONTINUE: 100>

This will have a downside as to preventing a server that may probably be sending a custom status code in between 100 and 600, which may raise an error in case this is allowed. For what it is worth, we already disallow values outside this range.

I think we can solve this issue, by introducing a method, something like verfiy_status_code which does the default verification and may be overridden in case someone wants to.

Default implementation

from http import HTTPStatus as status

class HttpResponseBase:
    def verify_status_code(self):
       if self.status is None:
           return 
       try:
           self.status_code = int(status)
       except (ValueError, TypeError):
           raise TypeError('HTTP status code must be an integer.')

       try:
           status(self.status)
       except ValueError as exc:
           raise

Overriden implementation

from django.http import HttpResponseBase

class CustomHttpResponse(HttpResponseBase):
    my_custom_codes = [111, 121]

    def verify_status_code(self):
        if self.status in self.my_custom_codes:
            return 
        super().verify_status_code()

Change History (2)

comment:1 by Mariusz Felisiak, 3 years ago

Resolution: wontfix
Status: newclosed
Type: BugCleanup/optimization

Thanks for the ticket, however HTTP status codes are extensible (see RFC 7231):

   HTTP status codes are extensible.  HTTP clients are not required to
   understand the meaning of all registered status codes, though such
   understanding is obviously desirable.  However, a client MUST
   understand the class of any status code, as indicated by the first
   digit, and treat an unrecognized status code as being equivalent to
   the x00 status code of that class, with the exception that a
   recipient MUST NOT cache a response with an unrecognized status code.

   For example, if an unrecognized status code of 471 is received by a
   client, the client can assume that there was something wrong with its
   request and treat the response as if it had received a 400 (Bad
   Request) status code.  The response message will usually contain a
   representation that explains the status.

I see no reason for additional validation and complicating users code.

comment:2 by Adam Johnson, 3 years ago

A concrete use case: htmx uses the non-standard status code 286 to mean "stop polling", and I use that from within Django: https://github.com/adamchainz/django-htmx#django_htmxhttphttpresponsestoppolling-typehttpresponse .

Note: See TracTickets for help on using tickets.
Back to Top