-
Notifications
You must be signed in to change notification settings - Fork 5.1k
ext_authz: Fix missing body in CheckRequest for HTTP/2 #8877
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
Conversation
This patch fixes missing body in CheckRequest when the with_request_body is enabled and the downstream protocol is HTTP/2. Signed-off-by: Dhi Aurrahman <dio@tetrate.io>
Signed-off-by: Dhi Aurrahman <dio@tetrate.io>
/azp run envoy-linux |
Azure Pipelines successfully started running 1 pipeline(s). |
Signed-off-by: Dhi Aurrahman <dio@tetrate.io>
Signed-off-by: Dhi Aurrahman <dio@tetrate.io>
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.
It looks good to me. Thanks for adding the integration test!
uint64_t request_size) { | ||
initializeWithDownstreamProtocol(downstream_protocol); | ||
initiateClientConnection(request_size); | ||
waitForExtAuthzRequest(fmt::format(R"EOF( |
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.
@dio The integration tests are great but is there a way that we can make this part a bit more readable?
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.
Sure. Updated.
@zuercher could you help to review this? Thank you! |
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.
Thanks for working out the integration tests. I have one question about how this works in cases where there are multiple decodeData calls.
Signed-off-by: Dhi Aurrahman <dio@tetrate.io>
Signed-off-by: Dhi Aurrahman <dio@tetrate.io>
Signed-off-by: Dhi Aurrahman <dio@tetrate.io>
Signed-off-by: Dhi Aurrahman <dio@tetrate.io>
@zuercher do we need more changes? |
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.
Looks good to me!
This patch fixes the missing body in `CheckRequest` when the `with_request_body` is enabled and the downstream protocol is HTTP/2. Fixes envoyproxy#8666 Signed-off-by: Dhi Aurrahman <dio@tetrate.io>
Description:
This patch fixes the missing body in
CheckRequest
when thewith_request_body
is enabled and the downstream protocol is HTTP/2.Risk Level: Low
Testing: integration-test
Docs Changes: N/A
Release Notes: N/A
Fixes #8666
Signed-off-by: Dhi Aurrahman dio@tetrate.io