-
Notifications
You must be signed in to change notification settings - Fork 4.8k
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
Skip http method matching in http_parser with UHV #36245
Conversation
Signed-off-by: G Vamshi Krishna Reddy <vamshikrishna.gunreddy@gmail.com>
Hi @vamshi177, welcome and thank you for your contribution. We will try to review your Pull Request as quickly as possible. In the meantime, please take a look at the contribution guidelines if you have not done so already. |
Signed-off-by: G Vamshi Krishna Reddy <vamshikrishna.gunreddy@gmail.com>
Signed-off-by: G Vamshi Krishna Reddy <vamshikrishna.gunreddy@gmail.com>
Signed-off-by: G Vamshi Krishna Reddy <vamshikrishna.gunreddy@gmail.com>
/retest failed |
/retest |
Hi, just a quick heads up that I do not work on Envoy any more. Consider contacting @yanavlasov for further assistance with custom methods. |
Hi, thanks for trying to resolve this problem. But could we just switch the parser in the http_inspector to our new parser and add an option to the http_inspector to enable the custom methods support? Considering we finally may remove the legacy parser, I think updating the http_inspector would be better than to hacking the legacy Thanks again for you contribution. :) /wait |
So you are suggesting to switch the parser in http_inspector and add a new [#not-implemented-hide:] flag like allow_custom_method similar to what we for HCM? |
Not sure if the #not-implemented-hide annotation is necessary or not. Because we basically won't provide another UHV in the http_inspector. |
Regarding updating the parser in the http_inspector, I'm planning to do this in two steps.
Does this approach seeem fine? |
SGTM. |
Created a new issue #36433 to update the parser in http_inspector. |
Commit Message: Skip http method matching in http_parser with UHV
Additional Description: When the UHV compile-time flag is enabled, strict HTTP parsing is disabled by default. However, there is currently no support for custom methods in http_parser.
There is an option available to enable runtime
http_protocol_options.allow_custom_methods
feature that allows custom methods in BalsaParser, which is the current default HTTP/1 codec. But thehttp_inspector
still useshttp_parser
which currently doesn't support custom methods.As a result, even with the allow_custom_methods runtime feature enabled, it doesn't fully resolve the issue, as calls are still rejected by the client-side Envoy at the http_inspector stage. This change introduces support for custom HTTP methods in http_parser using the existing
HTTP_PARSER_STRICT
compile-time flag.Risk Level: Low
Testing:
Docs Changes:
Release Notes:
Platform Specific Features:
[Optional Runtime guard:]
[Optional Fixes #Issue]
[Optional Fixes commit #PR or SHA]
[Optional Deprecated:]
[Optional API Considerations:]