-
Notifications
You must be signed in to change notification settings - Fork 186
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
Fix zero length chunked response regression #256
Conversation
Narrowed it down to this line (added in #245) Line 65 in 9ab2afc
Removing it here, resolves the issue. /CC @ShogunPanda Would you mind taking a look? |
Looks like that fix causes other tests to fail https://github.com/bdraco/llhttp/actions/runs/6566967226/job/17838778913?pr=1 Digging... |
Test existing test was missing the ending 0 https://github.com/bdraco/llhttp/actions/runs/6567209475?pr=1 |
@ShogunPanda Sorry for the ping, can you take a look at this PR? Thanks! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
RFC 9112 dictates that 304 response does not have message body.
and
It is allowed to send transfer-encoding header field in 304 response, but it must be ignored because the message body must be empty. I think this PR should be reverted. |
Unfortunately this means this is a breaking change between 8.x and 9.x, but I agree with how you read the rfc. |
This reverts commit 8acaf3c.
Reverted in #262. |
This test fails on 9.1.3 but passes on 8.1.1
related issues aio-libs/aiohttp#7697 home-assistant/core#101893 home-assistant/core#101913