Skip to content

Commit f536ed9

Browse files
salonichf5salonichf5
authored
Allow RegularExpression for path match be RE2/PCRE friendly. (#4450)
Problem: Users had trouble specifying the PCRE style regex expressions when migrating from nginx-ingress Solution: Use regexp2 to validate regular expression which is Perl5 library and ensure it is PCRE and RE2 friendly to avoid any breaking changes.
1 parent 932f9aa commit f536ed9

File tree

4 files changed

+54
-57
lines changed

4 files changed

+54
-57
lines changed

‎go.mod‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ module github.com/nginx/nginx-gateway-fabric/v2
33
go 1.24.2
44

55
require (
6+
github.com/dlclark/regexp2 v1.11.5
67
github.com/envoyproxy/go-control-plane/envoy v1.36.0
78
github.com/fsnotify/fsnotify v1.9.0
89
github.com/go-logr/logr v1.4.3

‎go.sum‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,8 @@ github.com/davecgh/go-spew v1.1.2-0.20180830191138-d8f796af33cc h1:U9qPSI2PIWSS1
3535
github.com/davecgh/go-spew v1.1.2-0.20180830191138-d8f796af33cc/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
3636
github.com/distribution/reference v0.6.0 h1:0IXCQ5g4/QMHHkarYzh5l+u8T3t73zM5QvfrDyIgxBk=
3737
github.com/distribution/reference v0.6.0/go.mod h1:BbU0aIcezP1/5jX/8MP0YiH4SdvB5Y4f/wlDRiLyi3E=
38+
github.com/dlclark/regexp2 v1.11.5 h1:Q/sSnsKerHeCkc/jSTNq1oCm7KiVgUMZRDUoRu0JQZQ=
39+
github.com/dlclark/regexp2 v1.11.5/go.mod h1:DHkYz0B9wPfa6wondMfaivmHpzrQ3v9q8cnmRbL6yW8=
3840
github.com/docker/docker v28.5.1+incompatible h1:Bm8DchhSD2J6PsFzxC35TZo4TLGR2PdW/E69rU45NhM=
3941
github.com/docker/docker v28.5.1+incompatible/go.mod h1:eEKB0N0r5NX/I1kEveEz05bcu8tLC/8azJZsviup8Sk=
4042
github.com/docker/go-connections v0.6.0 h1:LlMG9azAe1TqfR7sO+NJttz1gy6KO7VJBh+pMmjSD94=

‎internal/controller/nginx/config/validation/common.go‎

Lines changed: 24 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import (
66
"regexp"
77
"strings"
88

9+
"github.com/dlclark/regexp2"
910
k8svalidation "k8s.io/apimachinery/pkg/util/validation"
1011
)
1112

@@ -15,7 +16,7 @@ const (
1516
)
1617

1718
var (
18-
pathRegexp = regexp.MustCompile("^" + pathFmt + "$")
19+
pathRegexp = regexp2.MustCompile("^"+pathFmt+"$", 0)
1920
pathExamples = []string{"/", "/path", "/path/subpath-123"}
2021
)
2122

@@ -91,7 +92,9 @@ func validatePath(path string) error {
9192
return nil
9293
}
9394

94-
if !pathRegexp.MatchString(path) {
95+
if valid, err := pathRegexp.MatchString(path); err != nil {
96+
return fmt.Errorf("failed to validate path %q: %w", path, err)
97+
} else if !valid {
9598
msg := k8svalidation.RegexError(pathErrMsg, pathFmt, pathExamples...)
9699
return errors.New(msg)
97100
}
@@ -108,56 +111,39 @@ func validatePathInMatch(path string) error {
108111
if path == "" {
109112
return errors.New("cannot be empty")
110113
}
111-
112-
if !pathRegexp.MatchString(path) {
114+
if valid, err := pathRegexp.MatchString(path); err != nil {
115+
return fmt.Errorf("failed to validate path in match %q: %w", path, err)
116+
} else if !valid {
113117
msg := k8svalidation.RegexError(pathErrMsg, pathFmt, pathExamples...)
114118
return errors.New(msg)
115119
}
116120

117121
return nil
118122
}
119123

120-
// validatePathInRegexMatch a path used in a regex location directive.
121-
// 1. Must be non-empty and start with '/'
122-
// 2. Forbidden characters in NGINX location context: {}, ;, whitespace
123-
// 3. Must compile under Go's regexp (RE2)
124-
// 4. Disallow unescaped '$' (NGINX variables / PCRE backrefs)
125-
// 5. Disallow lookahead/lookbehind (unsupported in RE2)
126-
// 6. Disallow backreferences like \1, \2 (RE2 unsupported).
124+
// validatePathInRegexMatch validates a path used in a regex location directive.
125+
//
126+
// It uses Perl5 compatible regexp2 package along with RE2 compatibility.
127+
//
128+
// Checks:
129+
// 1. Non-empty.
130+
// 2. Satisfies NGINX location path shape.
131+
// 3. Compiles as a regexp2 regular expression with RE2 option to support named capturing group.
132+
// No extra bans on backrefs, lookarounds, '$'.
127133
func validatePathInRegexMatch(path string) error {
128134
if path == "" {
129135
return errors.New("cannot be empty")
130136
}
131137

132-
if !pathRegexp.MatchString(path) {
133-
return errors.New(k8svalidation.RegexError(pathErrMsg, pathFmt, pathExamples...))
134-
}
135-
136-
if _, err := regexp.Compile(path); err != nil {
137-
return fmt.Errorf("invalid RE2 regex for path '%s': %w", path, err)
138-
}
139-
140-
for i := range len(path) {
141-
if path[i] == '$' && (i == 0 || path[i-1] != '\\') {
142-
return fmt.Errorf("invalid unescaped `$` at position %d in path '%s'", i, path)
143-
}
144-
}
145-
146-
lookarounds := []string{"(?=", "(?!", "(?<=", "(?<!"}
147-
for _, la := range lookarounds {
148-
if strings.Contains(path, la) {
149-
return fmt.Errorf("lookahead/lookbehind '%s' found in path '%s' which is not supported in RE2", la, path)
150-
}
138+
if valid, err := pathRegexp.MatchString(path); err != nil {
139+
return fmt.Errorf("failed to validate path %q: %w", path, err)
140+
} else if !valid {
141+
msg := k8svalidation.RegexError(pathErrMsg, pathFmt, pathExamples...)
142+
return errors.New(msg)
151143
}
152144

153-
backref := regexp.MustCompile(`\\[0-9]+`)
154-
matches := backref.FindAllStringIndex(path, -1)
155-
if len(matches) > 0 {
156-
var positions []string
157-
for _, m := range matches {
158-
positions = append(positions, fmt.Sprintf("[%d-%d]", m[0], m[1]))
159-
}
160-
return fmt.Errorf("backreference(s) %v found in path '%s' which are not supported in RE2", positions, path)
145+
if _, err := regexp2.Compile(path, regexp2.RE2); err != nil {
146+
return fmt.Errorf("invalid regex for path %q: %w", path, err)
161147
}
162148

163149
return nil

‎internal/controller/nginx/config/validation/common_test.go‎

Lines changed: 27 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -127,29 +127,37 @@ func TestValidatePathInRegexMatch(t *testing.T) {
127127
testValidValuesForSimpleValidator(
128128
t,
129129
validator,
130-
`/api/v[0-9]+`,
131-
`/users/(?P<id>[0-9]+)`,
132-
`/foo_service/\w+`,
133-
`/foo/bar`,
134-
`/foo/\\$bar`,
130+
`/api/v[0-9]+`, // basic char class + quantifier
131+
`/users/(?P<id>[0-9]+)`, // re2-style named group
132+
`/users/(?<id>[0-9]+)`, // pcre-style named group
133+
`/foo_service/\w+`, // \w class
134+
`/foo/bar`, // plain literal path
135+
`/foo/\\$bar`, // escaped backslash + dollar
136+
`/foo/(\w+)\1$`, // numeric backreference
137+
`/foo(?=bar)/baz`, // lookahead
138+
`/(service\/(?!private/).*)`, // negative lookahead
139+
`/rest/.*/V1/order/get/.*`, // wildcard match
140+
`/users/(?P<id>[0-9]+)/\k<id>`, // named backreference
141+
`/foo(?<=/foo)\w+`, // fixed-width lookbehind
142+
`/foo(?<=\w+)bar`, // variable-width lookbehind
143+
`/foo(?=bar)`, // lookahead
144+
`/users/(?=admin|staff)\w+`, // alternation in lookahead
145+
`/api/v1(?=/)`, // lookahead for slash
135146
)
136147

137148
testInvalidValuesForSimpleValidator(
138149
t,
139150
validator,
140-
``,
141-
`(foo`,
142-
`/path with space`,
143-
`/foo;bar`,
144-
`/foo{2}`,
145-
`/foo$bar`,
146-
`/foo(?=bar)`,
147-
`/foo(?!bar)`,
148-
`/foo(?<=bar)`,
149-
`/foo(?<!bar)`,
150-
`(\w+)\1$`,
151-
`(\w+)\2$`,
152-
`/foo/(?P<bad-name>[0-9]+)`,
153-
`/foo/(?P<bad name>[0-9]+)`,
151+
``, // empty: must be non-empty
152+
`(foo`, // unbalanced parenthesis
153+
`/path with space`, // whitespace forbidden by pathFmt
154+
`/foo;bar`, // ';' forbidden by pathFmt
155+
`/foo{2}`, // '{' '}' forbidden by pathFmt
156+
`(\w+)\2$`, // invalid backref: group 2 doesn't exist
157+
`/foo/(?P<bad-name>[0-9]+)`, // invalid group name: hyphen not allowed
158+
`/foo/(?P<bad name>[0-9]+)`, // invalid group name: space not allowed
159+
`(\w+)\2$`, // invalid backref: group 2 doesn't exist
160+
`/users/\k<nonexistent>`, // invalid named backreference
161+
`^(([a-z])+)+$`, // nested quantifiers not allowed
154162
)
155163
}

0 commit comments

Comments
 (0)