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.

Sign up for GitHub

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

Jump to bottom

HeaderMatcher.invert_match should invert the operation, not the result #4896

Closed
ejona86 opened this issue Oct 21, 2021 · 1 comment · Fixed by #4902
Closed

HeaderMatcher.invert_match should invert the operation, not the result #4896

ejona86 opened this issue Oct 21, 2021 · 1 comment · Fixed by #4902
Assignees
@zasweq
Labels
Type: Bug

Comments

@ejona86
Copy link
Member

ejona86 commented Oct 21, 2021 •
edited
Loading

Go inverts the match result directly:

grpc-go/internal/xds/rbac/matchers.go

Lines 288 to 290 in 6f2a922

if headerMatcherConfig.InvertMatch {
m = internalmatcher.NewInvertMatcher(m)
}

However, the Envoy behavior is to invert the operation, such that == becomes != and both fail for a missing entry:
https://github.com/envoyproxy/envoy/blob/b7393b0bd221dab43bea0d697df0954a5b01d7d7/source/common/http/header_utility.cc#L140-L146

This was implemented appropriately in Java. I'll add some language to call this out in the gRFC.
ejona86 added a commit to ejona86/proposal that referenced this issue Oct 21, 2021
@ejona86
A41: Call out precise HeaderMatcher.invert_match semantics
23c2cf8 
The proper behavior is subtle and easy to assume incorrectly. This
caused a bug in Go (grpc/grpc-go#4896) and
would be easy for other implementations to assume incorrectly.

As determined by Envoy's behavior:
https://github.com/envoyproxy/envoy/blob/7d3e444d757696c15a4020b1d2d8ba5cb7f12501/source/common/http/header_utility.cc#L140-L146
@ejona86 ejona86 mentioned this issue Oct 21, 2021
Merged
@zasweq zasweq mentioned this issue Oct 27, 2021
Merged
@menghanl
Copy link
Contributor

fixed by #4902

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Apr 27, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees

@zasweq zasweq

Labels
Type: Bug
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

xds: Fix invert functionality for header matcher zasweq/grpc-go
3 participants
@menghanl @ejona86 @zasweq