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

dev server doesn't read entire request body leading to client error #1313

Closed
rmorehead opened this issue May 24, 2018 · 8 comments
Closed

dev server doesn't read entire request body leading to client error #1313

rmorehead opened this issue May 24, 2018 · 8 comments

Comments

@rmorehead
Copy link

According to the HTTP spec, a server must read the entire request for HTTP 1.0 style request, or if the client supports "100 continue" via accept headers, the server must read entire request if the "100 continue" has been issued.

The issue above leads timing related CURLE_SEND_ERROR (55) with POSTS/PUTS via curl, and likely causes issues with other clients. The werkzeug server itself does not see this as an error as it has prematurely closed the socket on the client.

Since werkzeug does the socket closing and the 100 continue, it would be consistent if werkzeug ensured the entire request is read prior to closing the socket and not require the web application to add extra code/logic for every request.

To reproduce, have a werkzeug URL that accepts PUT or POST, but immediately responds with a redirect. Use curl to upload some data, 500kb should be plenty. Observe intermittent curl send errors.

@davidism
Copy link
Member

The dev server does not drain input. Use a production server, this isn't a real issue during development.

@rmorehead
Copy link
Author

If I submit an appropriate patch will you accept?

@davidism
Copy link
Member

I can't say whether I'll accept a patch until I see it, but the help would definitely be appreciated.

@rmorehead
Copy link
Author

Will do! I am embedding an httpbin server in a test script testing a CURL based embedded client, so having httpbin dev mode work smoothly is desirable. I have worked around for now.

@davidism
Copy link
Member

Discussion from Flask: pallets/flask#2188

@davidism davidism changed the title werkzeug does read entire request leading to client error dev server doesn't read entire request body leading to client error May 28, 2018
@davidism
Copy link
Member

davidism commented Jan 20, 2019

Closing. I don't want to add more stream handling to Werkzeug / the dev server. WSGI servers like Gunicorn can be used for input termination, and can be started programatically if you want to embed a production-level server in another application.

This should probably be addressed in Python itself, as we're just subclassing the built-in HTTP server.

@cipri-tom
Copy link

@davidism I understand your reluctance to add stream code to wekzeug, as it should never be used in production.

However, please consider that this is the default development server and this issue causes very subtle errors on the client, like it just did for me. I have been bitten by this before, but then I was testing with curl and could see the connection reset, which has directed me to the fix a bit sooner.

As this is the development server, I'd say it is worthwhile to have reasonable behaviour out of the box, since it is the first one that is used, especially by newcomers.

Thank you!

@mitsuhiko
Copy link
Contributor

The WSGI spec is not clear who needs to handle it. My interpretation in Werkzeug was that this is the app's responsibility which would make this either a werkzeug or app bug, not server bug.

The original idea was that one would only respond when reading of form data is wanted but for error cases that's wrong. This means the draining code in Werkzeug's parser alone is insufficient.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Nov 13, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants