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

Keep alive is incorrect #354

Closed
Dreamsorcerer opened this issue Apr 5, 2024 · 2 comments
Closed

Keep alive is incorrect #354

Dreamsorcerer opened this issue Apr 5, 2024 · 2 comments

Comments

@Dreamsorcerer
Copy link
Contributor

Dreamsorcerer commented Apr 5, 2024

I'm just updating aiohttp to match the new default regarding obsolete line folding. But, while updating the tests I think I've found a bug with the keep-alive behaviour. This is with lenient headers for our response parser.

The body in the test is (in Python):

    value = b"A" * 8185
    text = b"HTTP/1.1 200 Ok\r\ndata: test\r\n " + value + b"\r\n\r\n"

I'd be surprised if the size is related, so it can probably be reduced to a simpler example. But, if I understand correctly, we then get the keep-alive status with llhttp_should_keep_alive() and this is returning False. Based on this input, it should be True:

If the "close" connection option is present (Section 9.6), the connection will not persist after the current response; else
If the received protocol is HTTP/1.1 (or later), the connection will persist after the current response; else,
https://www.rfc-editor.org/rfc/rfc9112#section-9.3-3.1

@ShogunPanda
Copy link
Contributor

I see your point but the problem is more subtle there unfortunately.

Your sample text does not contain a Content-Length or a Transfer-Encoding header.
According to RFC 9112 section 6.3, since neither or those headers are present (not even in invalid forms), then the response is assumed to be without a specific body length and thus the body will be considered to be ended when the connection is interrupted from the server. This also means no keep-alive is possible.

To verify this, you can see that if you parse the message above llhttp will not invoke the on_message_complete callback.

@Dreamsorcerer
Copy link
Contributor Author

Right, the aiohttp parser must be following the step before:

If this is a request message and none of the above are true, then the message body length is zero (no message body is present).

But, this should only apply to requests, not responses. I'll try and update the Python parser then.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants