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.

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

internal/envoy: switch HeaderMatch to SafeRegex #1881

Merged
merged 1 commit into from
Nov 15, 2019

Conversation

davecheney
Copy link
Contributor

Updates #1351

nb. there appear to be no internal/contour, internal/e2e, or
internal/featuretest tests for header contains matching. I've raised
#1880 to track this.

Signed-off-by: Dave Cheney dave@cheney.net

Updates projectcontour#1351

nb. there appear to be no internal/contour, internal/e2e, or
internal/featuretest tests for header contains matching. I've raised
 projectcontour#1880 to track this.

Signed-off-by: Dave Cheney <dave@cheney.net>
@davecheney davecheney added this to the 1.1.0 milestone Nov 11, 2019
@stevesloka
Copy link
Member

Fixes #1513

@jpeach
Copy link
Contributor

jpeach commented Nov 11, 2019

IIUC regexes are exposed to users through Ingress? If so, we should document the change in regex engine? I recalled that RE2 has some slightly different syntax, but maybe I'm misremembering because I can't find it now.

@davecheney
Copy link
Contributor Author

IIUC regexes are exposed to users through Ingress? If so, we should document the change in regex engine? I recalled that RE2 has some slightly different syntax, but maybe I'm misremembering because I can't find it now.

Yup. This misfeature of ingress is categorically the worse. Envoy offers Re2 but the Ingress spec says the regex engine is extended POSIX as defined by IEEE Std 1003.1. Luckily not many people seem to know that Ingress's path spec is a regex.

Now you're starting to understand why I really really really don't want to expose regex in any form via contour.

Copy link
Contributor

@jpeach jpeach left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@jpeach jpeach merged commit 01e9811 into projectcontour:master Nov 15, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants