Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

RuntimeError: Response content longer than Content-Length in Uvicorn, for status codes without body (e.g. 204, 304), in responses with Body #1764

Closed
1 task done
tiangolo opened this issue Jul 14, 2022 · 1 comment
Assignees

Comments

@tiangolo
Copy link
Member

Related discussion

I think this is worth as an issue/bug in Starlette, and I'm gonna work on it.

Description

When a Starlette application sends a response with a status code that would have no content, e.g. 204, 304, but the response actually has content, it is accepted by Starlette as valid, the code looks valid, and there's no exception raised by Starlette, it sends the response ASGI message to the server like that.

But Uvicorn takes that as an erroneous message and raises an error of:

RuntimeError: Response content longer than Content-Length

So, an application and code that would look fine in Starlette, and Starlette would accept as valid, results in an error in Uvicorn.

This means that Starlette is accepting some code from the user as valid, but sending an invalid message to Uvicorn.

Minimal Example

Here's a minimal example to reproduce the issue with Starlette:

from starlette.applications import Starlette
from starlette.routing import Route
from starlette.responses import Response
from starlette.requests import Request


def endpoint(request: Request):
    return Response("null", status_code=204)


app = Starlette(
    routes=[Route("/", endpoint=endpoint)],
)

Then call the endpoint, e.g.:

$ curl localhost:8000

The problem is even more notorious when using FastAPI, as the same response from above is generated automatically because a function implicitly always returns None if nothing else is returned, and FastAPI will use a JSONResponse by default, so it will try to send the JSON string "null":

from fastapi import FastAPI

app = FastAPI()


@app.get("/", status_code=204)
def endpoint():
    print("Oh, hi mark")

On Uvicorn

Uvicorn checks if there's a content-length header and uses it to set the expected_content_length, if there's none, it is not modified and keeps its default of 0.

Then that default of 0 for expected_content_length is checked here, and that's where the error is raised: https://github.com/encode/uvicorn/blob/fd4386fefb8fe8a4568831a7d8b2930d5fb61455/uvicorn/protocols/http/httptools_impl.py#L532-L533

The only case where that is not affected is when chunked_encoding is used. And that is used by default when the content-length is not set only if the status code is not one of the status codes without body: https://github.com/encode/uvicorn/blob/fd4386fefb8fe8a4568831a7d8b2930d5fb61455/uvicorn/protocols/http/httptools_impl.py#L498-L505

So, from Uvicorn's point of view, Starlette is sending a bogus message.

Next steps

I think this has to be solved on Starlette, not on Uvicorn.

I think the sensible thing would be to remove the body of a request with a status code that doesn't contain a body, Uvicorn won't send that body either way, it will just error out.

Another option is to raise an exception to tell the user that they can't provide a body for a status code that doesn't contain a body, but it should be done in Starlette, not in Uvicorn. And if that was the approach chosen, then it would make sense to add a lot of typing @overloads to the __init__ methods of the request classes with the Literal for each status code that would raise an error, and making the content as exclusively None in those overloads, to make those errors show up when editing and checking the code, not only at runtime (in production 😱). This second option would be a lot more code and it wouldn't solve the problem for FastAPI, I would still have to do the first option above in FastAPI.

@tiangolo
Copy link
Member Author

This is intentional behavior on the Starlette side as discussed in the PR: #1765

On the FastAPI side, it's fixed by fastapi/fastapi#5145

Given that I'll close this issue now. 🤓

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant