-
-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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
aiohttp does not skip response body when a HEAD request response has a body when using C extensions #10322
Comments
Is there a redirect involved? Might be due to recoding... The traceback shows the server responding with HTTP 400. |
yes redirects in play
|
Try passing You may also disable the redirects and walk them manually, verifying that the |
@jonathon-love also note that curl goes for HTTP/2 which aiohttp won't upgrade. That's another difference. |
thanks for your help. adding
and the addition of
one thing worth noting is that this is a
if i change this to a with thanks |
Is the issue present with I'll try and reproduce in a couple of days. |
it works when |
I wonder if there's something we're missing, to tell the C parser that it is a HEAD response. I suspect it's trying to parse a full response.. |
I tried to reproduce this but I'm not seeing a failure
|
still reproducible for me.
it works if i set jonathon |
Let me try running it in another directory just in case something is leaking across in my dev setup |
I can reproduce it when I do it in another directory
|
Here is the transaction
|
so dropbox is sending a response body on a HEAD request. curl hint: |
love your work! |
https://datatracker.ietf.org/doc/html/rfc9112#section-6.3-2.1 is pretty clear that its not allowed to have a body:
aiohttp is uses llhttp for the c parser https://github.com/nodejs/llhttp It seems like curl is more forgiving. I think there could be an argument made to be that |
I'm not so sure. If one parser is reading the next request (as it should do), and another parser is reading a body, that's a request smuggling attack. There would literally be no way to distinguish between a body which looks like a request and a new request. Therefore I think that the bytes following the headers must always be treated as a new request, which is why there is a parsing error. At best, we could silence the parsing error in lax mode and just close the connection. The Python parser should probably also do the same behaviour (I assume it doesn't given your first attempt didn't error). As curl sends and receives single requests, I assume that it doesn't use keep-alive connections, in which case it's not really a security issue for curl. |
Actually, the error says "Invalid character in chunk size". Could this actually be the opposite issue? That llhttp is trying to read the body when it's not supposed to. If llhttp was processing it as a new request (as it should be), then the error message should have been about an invalid response line. |
I'm wondering if there's a mistake around here: aiohttp/aiohttp/_http_parser.pyx Lines 455 to 473 in 28832b8
It looks like it'd assign a SteamReader to self._payload, then add an EMPTY_PAYLOAD to the messages. I wonder if it should be setting to an empty payload at the start.. |
Created a test in #10587. Probably not going to look at it just yet (my guess at fixing it was wrong), but the C parser is behaving incorrectly and trying to parse the body. If that is fixed, it might help out with this issue, as it may allow the response to be received and only error on the next request (which I assume is what happens with the Python parser). Though the overall fix is obviously needed from Dropbox, who are sending invalid HTTP responses. |
Describe the bug
aiohttp barfs on a dropbox download, that other software (i.e. curl) doesn't seem to have difficulty with.
To Reproduce
Expected behavior
200 OK
Logs/tracebacks
aiohttp Version
multidict Version
propcache Version
yarl Version
OS
linux, macOS
Related component
Client
Additional context
No response
Code of Conduct
The text was updated successfully, but these errors were encountered: