-
Notifications
You must be signed in to change notification settings - Fork 4.9k
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
uhv: add http-parser permissive parsing for h1 codec #20872
uhv: add http-parser permissive parsing for h1 codec #20872
Conversation
Signed-off-by: Adam Meily <[email protected]>
Signed-off-by: Adam Meily <[email protected]>
Signed-off-by: Adam Meily <[email protected]>
Signed-off-by: Adam Meily <[email protected]>
Signed-off-by: Adam Meily <[email protected]>
Signed-off-by: Adam Meily <[email protected]>
@yanavlasov This should be good for review. |
@alyssawilk this is special UHV build for H/1 parser with header validation disabled, using existing #define |
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.
Yeah this has my thumbs up, assuming it fails the compile time build without that early return. Mind taking that define out and checking once?
/wait
@alyssawilk I have confirmed that removing the early
|
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.
Awesome, thanks for confirming.
cc @snowp for !Googler pass
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, thanks!
Signed-off-by: Adam Meily <[email protected]>
Commit Message: uhv: add http-parser permissive parsing for h1 codec
Additional Description: Honor the new
uhv_enabled
build flag to disable strict parsing within the H1 parsing library (http-parser). This is a reworking of the previous PR, #20528, but this time using the new compile-time option instead of making it configurable at runtime.Risk Level: Low
Testing:
Docs Changes:
Release Notes:
Platform Specific Features:
Fixes #19750