connexion icon indicating copy to clipboard operation
connexion copied to clipboard

Incorrect error response when content-length header is missing

Open chrismathias opened this issue 1 year ago • 7 comments

Description

In the validator https://github.com/spec-first/connexion/blob/main/connexion/validators/abstract.py#L126 a http status code 400 error is raised if content-length header is missing or 0 when a request body is present. The server response is then "RequestBody is required", which is confusing, as the request body does exist.

Expected behaviour

According to https://datatracker.ietf.org/doc/html/rfc7230#section-3.3.2 the correct server response should be http status code 411 "Length Required", ideally identifying that body was present but content-length not set or 0. Ideally, the error message would indicate "content-length header missing or 0".

Actual behaviour

Server response to missing content-length header or header with value 0 is 400 "RequestBody is required".

Steps to reproduce

Send http POST with body but missing content-length or content-length 0.

Additional info:

Output of the commands:

  • python --version: Python 3.10.12
  • pip show connexion | grep "^Version\:" Version: 3.1.0

chrismathias avatar Dec 17 '24 09:12 chrismathias

Just trying to follow this, I looked at the function and see this at line 128:

            if body is None and self._required:
                raise BadRequestProblem("RequestBody is required")

Are you sure the 400/Bad Request response is raised even if the body is present? Can you show a way to reproduce this, possibly with a clever curl invocation?

chrisinmtown avatar Dec 17 '24 12:12 chrisinmtown

Curl does add the content-length by default. I can force it not to add the content-length by specifying -H 'Content-Length:' . Executed with the helloworld example:

Default CURL (will add Content-Length header): curl -X 'POST' \ 'http://localhost:8080/openapi/greeting/dave' \ -H 'accept: text/plain' \ -H 'Content-Type: application/json' \ -d '{}'

Response: Hello dave%

CURL without Content-Length header (explicit removal): curl -X 'POST' \ 'http://localhost:8080/openapi/greeting/dave' \ -H 'accept: text/plain' \ -H 'Content-Type: application/json' \ -H 'Content-Length:' \ -d '{}'

Response: Invalid HTTP request received.%

Logs of the server: WARNING: Invalid HTTP request received. BadRequestProblem(status_code=400, detail='Request body must not be empty')

For instance, the spring framework also does not add content-length headers in some versions: https://github.com/spring-projects/spring-framework/issues/32816#issuecomment-2110030063

chrismathias avatar Dec 17 '24 13:12 chrismathias

Thanks for the report @chrismathias!

This is not an easy fix though, since we use the content-length header to determine if a request body is present without consuming the stream. The RFC you linked also mentions that a user agent SHOULD add a content-length header, so I'm not sure if we want to add a lot of complexity to handle this.

Another less complex way to tackle this would be to still raise a 400, but change the message to "Request body is empty or content-length is missing".

RobbeSneyders avatar Dec 22 '24 10:12 RobbeSneyders

If we do want to go for the more correct but more complex solution, we should probably:

  1. Read the first message from the receive channel to check if there is a body
  2. Check that this matches the existence of the content-length
  3. Use the insert_messages method to wrap it in a new stream for further processing

RobbeSneyders avatar Dec 22 '24 10:12 RobbeSneyders

I'm leaning more towards the simple solution currently.

RobbeSneyders avatar Dec 22 '24 10:12 RobbeSneyders

Had the same issue by upgrading to connexion v3.
Lost a huge amount of time to understand what was the issue ahah.
But in my opinion, as the RFC is :

A user agent SHOULD send Content-Length in a request when the method defines a meaning for enclosed content and it is not sending [Transfer-Encoding](https://httpwg.org/specs/rfc9112.html#field.transfer-encoding). For example, a user agent normally sends Content-Length in a POST request even when the value is 0 (indicating empty content). A user agent SHOULD NOT send a Content-Length header field when the request message does not contain content and the method semantics do not anticipate such data.

THe issue is on the user agent side EDIT I think there is an issue in connexion side in fact, as content-length should not be set when the caller is using Transfer-encoding, which is my case. Though I had to add a middleware to set a random content-length header to bypass the check un request validation middleware.

class ContentLengthMiddleware:
    def __init__(self, app: Callable):
        self.app = app

    async def __call__(self, scope: Dict[str, Any], receive: Callable, send: Callable) -> Awaitable:
        if scope.get("type") == "http":
            headers = scope.get("headers", [])
            if not any(key.lower() == b"content-length" for key, _ in headers):
                random_value = 666
                headers.append((b"content-length", str(random_value).encode("utf-8")))
                scope["headers"] = headers

        await self.app(scope, receive, send)

jledrumics avatar Mar 31 '25 10:03 jledrumics

I hit this bug today. My HTTP client (https://ogen.dev/) is using transfer-encoding:chunked, not content-length: as expected by connexion.

I think the correct behavior is to accept the request rather than merely improve the error message.

strager avatar Jul 17 '25 19:07 strager