Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
[http] add llhttp parser implementation #15814
[http] add llhttp parser implementation #15814
Changes from 4 commits
1e9d3be
204e519
d458ac4
591811f
9398f11
e4f6b56
397d027
f342c0e
53232a4
9365663
9289b6b
43e29dc
d243dda
dcd8752
a206ccc
2e9421b
8cba94e
f37f3e4
a080df9
f023654
526c73b
4882b6a
f5fcfaf
ddc6d3b
6592db0
0d23c11
20c8935
5fea125
6599974
b5d59d9
c77400d
8f84cf4
7e480fe
2a41f11
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
What happens if a request has both transfer-encoding: chunked and content-length?
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.
https://github.com/nodejs/llhttp/blob/aa07189d39b8d0da9d5a887d93761014a45804bf/test/request/content-length.md#error-on-simultaneous-content-length-and-transfer-encoding-identity
It's handled with an error. Actually this is just because llhttp doesn't have an initial value for unset content length.
http_parser
uses all 1 bits set to indicate there is no content length. Added comment.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.
We should have tests in envoy to cover these cases. This is related to my question regarding use of lenient parsing further down in this file. It seems that those lenient options were added for the benefit of Envoy's H1 implementation.
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.
envoy/test/common/http/http1/codec_impl_test.cc
Line 1945 in 56d3f8a
There are tests for both content length and chunked.