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

fix 100 response with headers #287

Merged
merged 3 commits into from Mar 9, 2023

Conversation

catbro666
Copy link
Contributor

The 100 reponse is not processed correctly. After receiving the 100 status line, it only reads a line here and assumes there’s no headers. Then it directly tries to receive the status line of the next response here. If there are headers after 100 statue line, it will fail there.

I checked the RFC (section 5.2.1 and 6.2.1). Unfortunately, the specification does not explicitly require or prohibit the inclusion of headers in the 100 response. I tested many sites (httpbin.org, mockbin.org, hurl.it, webhook.site, codepunker.com, typicode.com), all of them don’t include any headers. But the squid proxy includes a header of Connection: keep-alive. So it’s better to be compatible with both cases.

@pintsized
Copy link
Member

Amazing, thank you for this! This issue was first reported eight years ago :) #32

PR looks good to me at first glance, just an unused variable which is failing the luacheck test. Please fix this, and I'll do a proper review later today.

@pintsized pintsized merged commit d4eaf88 into ledgetech:master Mar 9, 2023
@catbro666 catbro666 deleted the fix-100-continue branch March 10, 2023 02:44
@catbro666
Copy link
Contributor Author

@pintsized Could you help bump the version?

@pintsized
Copy link
Member

v0.17.1 just released, to both luarocks and opm.

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

Successfully merging this pull request may close these issues.

None yet

2 participants