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

Incorrect error response when content-length header is missing #2020

Open
chrismathias opened this issue Dec 17, 2024 · 5 comments
Open

Incorrect error response when content-length header is missing #2020

chrismathias opened this issue Dec 17, 2024 · 5 comments
Labels
enhancement PR welcome We would welcome and review a PR addressing this issue

Comments

@chrismathias
Copy link

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
@chrisinmtown
Copy link
Contributor

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?

@chrismathias
Copy link
Author

chrismathias commented Dec 17, 2024

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: spring-projects/spring-framework#32816 (comment)

@RobbeSneyders
Copy link
Member

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
Copy link
Member

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
Copy link
Member

I'm leaning more towards the simple solution currently.

@RobbeSneyders RobbeSneyders added enhancement PR welcome We would welcome and review a PR addressing this issue labels Dec 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement PR welcome We would welcome and review a PR addressing this issue
Projects
None yet
Development

No branches or pull requests

3 participants