Skip to content

Documentation: status in response cannot be a tuple #6

Closed
@nicolasff

Description

@nicolasff

Hello,

I started using this library today and followed the documentation describing how to set the status code when responding to a request. The triple-quoted docstring for HTTPResponse.__init__ describes status as:

    :param tuple status: The HTTP status code to return, as a tuple of (int, "message").
     Common statuses are available in `HTTPStatus`.

while the type hint for the constructor argument defines it as status: tuple = HTTPStatus.OK. The name tuple is recognized by the documentation generator, which creates a link to the Python.org documentation for the tuple type.

Passing a tuple does not actually work here, and causes the response to be badly formatted. The value given to status is saved in self.status, then passed to self._send_response, which in turn calls self._send_bytes with the value produced by self._HEADERS_FORMAT.format:

def _send_response(self, conn, status, content_type, body):
self._send_bytes(
conn, self._HEADERS_FORMAT.format(status, content_type, len(body))
)
self._send_bytes(conn, body)

This format method is str.format, since self._HEADERS_FORMAT is just a string:

_HEADERS_FORMAT = (
"HTTP/1.1 {}\r\n"
"Content-Type: {}\r\n"
"Content-Length: {}\r\n"
"Connection: close\r\n"
"\r\n"
)

Passing a tuple as documented – e.g. (200, 'OK') causes it to be rendered at the position of the first {} in the format string, making the response look like this:

HTTP/1.1 (200, 'OK')
Content-Type: text/plain
Content-Length: 5
Connection: close

hello

This is not a valid HTTP response, so when curl receives it it fails with

* Unsupported HTTP version in response
* Closing connection 0

The docs do suggest to use the common values pre-defined as static fields in HTTPStatus, which aren't tuples but HTTPStatus instances. They are only provided for status code 200, 404, and 500, so it's likely that users would want to provide other values and try to use tuples like (403, 'Forbidden') for example.
HTTPStatus instances are rendered correctly in the format string because the class specifically defines its string representation with a __str__ method:

def __str__(self):
return f"{self.value} {self.phrase}"

I'm not sure whether this should be best handled as purely a documentation change – no longer suggesting tuple – or as a code change, actually supporting tuple and rendering it correctly, maybe even raising an exception if a tuple is provided that is not made of an int and a string. I'm far from an authority on the matter, but it seems more Pythonic to me to actually support tuples rather than require an HTTPStatus object or a protocol-formatted string to directly inject into the response.

Thanks for this library!

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions