-
Notifications
You must be signed in to change notification settings - Fork 83
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
feed_data
is not safe for http header fragmentation
#21
Comments
@youknowone do you have a test case at hand to reproduce the issue? |
A request: b'''GET /ping/ HTTP/1.1\r\nHost: github.com\r\nConnection: keep-alive\r\nCache-Control: max-age=0\r\nUpgrade-Insecure-Requests: 1\r\nUser-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_12_5) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/59.0.3071.115 Safari/537.36\r\nAccept: text/html,application/xhtml+xml,application/xml;q=0.9,image/webp,image/apng,*/*;q=0.8\r\nAccept-Encoding: gzip, deflate\r\nAccept-Language: ko-KR,ko;q=0.8,en-US;q=0.6,en;q=0.4\r\n\r\n''' I called feed_data each byte by byte and the headers are broken. Related issue in sanic which uses httptools: sanic-org/sanic#755 They are implmenting their own header fragment buffer. |
I'll take a look as soon as I finish working on the next uvloop release. |
@yohanboniface feel free to work on this if you have time |
@youknowone to make sure I understand the issue: it arises when a request is chunked in a middle of a header field?
And this as second chunk:
And then you'd have an incomplete value for Is that correct or am I missing something? edit bah, no, indeed a chunked request is only about the body. |
@yohanboniface Yes, your description is correct. User-Agent will be Mozzle/5.0 in that case. As I know, the chunked body is a spec about logical chunk, not about TCP packet fragment. A question here: does httptools expect to feed the whole HTTP body (at least "a chunk") at same time? Then it can be a user fault - but still weird. Because httptools is the parser, I think basically the users can't determine which part of http request is going to httptools or not. For the point of view of user, "end of chunk" of http body and any fragmented packet in http header is not recognizable before putting it into the parser. |
@youknowone made a quick unittest to reproduce what I've understood of the issue, but… it passes ;) See #26 Can you please check the unittest and tell me what I'm missing to properly reproduce the issue? thanks :) |
Thanks, your test is really helpful. |
I changed your test a little and it now starts to be broken: #27 |
feed_data
is not safe for http headerfeed_data
is not safe for http header fragmentation
I also added a patch to #27. Thanks @yohanboniface, I would never looked into it without your test. |
Cool! |
if any broken data chunk is fed to feed_data, it will be parsed in incomplete form.
The text was updated successfully, but these errors were encountered: