-
-
Notifications
You must be signed in to change notification settings - Fork 16.3k
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
Chunked uploads missing body data. #2229
Comments
✨🍰✨ |
Further digging shows that |
@Lukasa it appears that weukzeug is intentionally returning an empty stream when there's no content length: https://github.com/pallets/werkzeug/blob/109dad4ac9e0a1690666b2d4f29d07d98a3701d9/werkzeug/wsgi.py#L210 |
Yup, that looks like a correct analysis of that code. So, I'd argue that that behaviour is wrong. There's no reason to require the Content-Length field. PEP 3333 makes it pretty clear that the WSGI server should simulate EOF if the client reads past content-length, so I'm not really sure what werkzeug is trying to protect against here. |
More generally it makes it basically impossible for werkzeug-using applications to sensibly handle chunked transfer encoding without having their WSGI server buffer the entire inbound data stream, which seems like a pretty unreasonable requirement. |
|
@Lukasa I get a Traceback (most recent call last):
File "/home/david/.virtualenvs/flask/lib/python3.6/site-packages/requests/adapters.py", line 444, in send
low_conn.send(hex(len(i))[2:].encode('utf-8'))
File "/usr/lib64/python3.6/http/client.py", line 986, in send
self.sock.sendall(data)
BrokenPipeError: [Errno 32] Broken pipe
During handling of the above exception, another exception occurred:
Traceback (most recent call last):
File "/home/david/Projects/flask/example2.py", line 11, in <module>
requests.post('http://127.0.0.1:5000/', data=gen())
File "/home/david/.virtualenvs/flask/lib/python3.6/site-packages/requests/api.py", line 110, in post
return request('post', url, data=data, json=json, **kwargs)
File "/home/david/.virtualenvs/flask/lib/python3.6/site-packages/requests/api.py", line 56, in request
return session.request(method=method, url=url, **kwargs)
File "/home/david/.virtualenvs/flask/lib/python3.6/site-packages/requests/sessions.py", line 488, in request
resp = self.send(prep, **send_kwargs)
File "/home/david/.virtualenvs/flask/lib/python3.6/site-packages/requests/sessions.py", line 609, in send
r = adapter.send(request, **kwargs)
File "/home/david/.virtualenvs/flask/lib/python3.6/site-packages/requests/adapters.py", line 473, in send
raise ConnectionError(err, request=request)
requests.exceptions.ConnectionError: [Errno 32] Broken pipe |
It's certainly possible, yes. What WSGI server are you using? |
Just the built-in dev server. Actually, I forgot I changed def gen():
while True:
yield b"hello, world" With the original non-infinite generator, I get the empty output. |
Yeah, so the infinite generator is a problem because werkzeug thinks the request has no length, so it immediately sends a response and then, with the builtin server, probably closes the connection. That'll cause the broken pipe error. |
We probably don't want to allow infinite streams in all cases? Otherwise it would be trivial to tie up a server. Although maybe Werkzeug already handles this, let me try the patch and check. |
I have no particular objection to disallowing infinite streams, though I don't know that it's really Flask/werkzeug's problem to deal with: I'd say that in the WSGI abstraction the server is responsible for being sensible here. The bigger issue, ultimately, is that werkzeug seems to require the non-standard |
If we change
I'm not too concerned with the dev server since it's not expected to be used in production. I am concerned with Gunicorn, since it's a common way to deploy a Flask app. Flask provides a I'd like to hear from other maintainers too. |
@Davism Is the uWSGI stream object empty, or is werkzeug saying it's empty? As noted above, those are two very different cases. |
It's an instance of |
Opened pallets/werkzeug#1096 about obeying |
Hrm. I don't really know what's going on with uWSGI tbh. |
There will be one server developer that argues the other way around, which clearly shows in the different behavior across WSGI servers. In the end, if nobody feels responsible for security improvements beyond the spec, none practically exist. I agree that the current behavior is not optimal, so I'm not really sure what to do there. |
Also generally I'm a fan of having as little logic like this in the WSGI server as possible, since it makes app behavior dependent on deployment. |
@untitaker Sure, that's reasonable, but that means that Flask/werkzeug needs to find a way to cope with this that isn't "assume there is no body". If the answer is "servers must support the relevant werkzeug extension" then that's fine, but that needs to be documented really clearly somewhere to make it clear to server authors that if they don't implement it then Flask won't see any chunked bodies. Alternatively, Flask/werkzeug could implement some heuristics regarding headers and the first few bytes of the body to try to work out what the server is doing. I don't mind at all what direction you choose to go. 😁 |
@untitaker I think between pallets/werkzeug#1095 and pallets/werkzeug#1096, we should be good. Allows streams without content length, lets Werkzeug handle security if configured in case the WSGI server doesn't handle it. We could still document that servers can implement |
pallets/werkzeug#1126 will fix this |
Werkzeug has been fixed, not sure when a 0.13 release will happen though. You can install the latest GitHub archive if you want the fix now, it's only a couple commits off 0.12.2 right now. |
You will need to set |
I'm trying this with the cloned versions of Flask and Werkzeug, but I'm still seeing the problem with empty request body. Should I have to configure anything else besides this?
|
See pallets/werkzeug#1149, fix needs to be fixed. |
I'm still having issues with this in Flask 1.0.2 and both gunicorn 19.7.1 & uwsgi 2.0.17 Can't seem to get the request body no matter what I try. Any known workarounds? |
@cetanu are you sure Flask 1.0.2 (or newer) with gunicorn 19.7.1 (or newer) didn't work? I can believe Flask + uwsgi didn't/doesn't work, but Flask + gunicorn shall be a different story. |
I appear to be having a similar issue in Flask 1.0.2 with mod_wsgi where the request body is going missing only when it is mod_wsgi that launches flask. i have attached the debug output from my .wsgi app. Can anyone confirm that this is the same issue?
|
Summary
In basic Flask applications, the body data of chunked uploads appears to go missing: that is, they do not appear in
request.data
. I have reproduced this with two different WSGI servers (gunicorn and twisted), both of which handle chunked data appropriately. Non-chunked data does not suffer this problem. I've reproduced this problem on both Python 2.7 and Python 3.6.Reproduction
The following Flask application demonstrates the problem:
when run like this:
gunicorn -w 4 example:app
.The following test script can be run:
Expected Output
Actual Output
Environment
References
This was spotted at httpbin. See kennethreitz/httpbin#340 for more.
The text was updated successfully, but these errors were encountered: