Skip to content

Commit 8e2b2da

Browse files
committed
BUG/MINOR: fix ReplacePrefixMatch path rewrite edge cases
When ReplacePrefixMatch is "/" and matchPrefix is a non-root prefix (e.g. "/strip-prefix"), the single replace-path rule produced an empty path for requests that match the prefix exactly (no suffix), because the optional capture group \1 evaluates to "". Two replace-path rules are now emitted: one anchored to the exact prefix that hard-sets "/" and one for the suffix case. When ReplacePrefixMatch is "/" and matchPrefix is also "/", the rewrite is a no-op for every path. The set-path rule that was previously emitted used regsub(^/$,) which stripped the root path to an empty string, causing a 400. The rule is now skipped entirely. The same empty-path bug existed in requestRedirectRules and is fixed there as well.
1 parent ac412d9 commit 8e2b2da

2 files changed

Lines changed: 79 additions & 10 deletions

File tree

k8s/gate/haproxy/filters/http_filters.go

Lines changed: 35 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -278,17 +278,40 @@ func urlRewriteRules(f *gatewayv1.HTTPURLRewriteFilter, matchPrefix string) mode
278278
if f.Path.ReplacePrefixMatch != nil && matchPrefix != "" {
279279
replacement := strings.TrimRight(*f.Path.ReplacePrefixMatch, "/")
280280
if matchPrefix == "/" {
281-
rules = append(rules, &models.HTTPRequestRule{
282-
Type: "set-path",
283-
PathFmt: fmt.Sprintf("%s%%[path,regsub(^/$,)]", replacement),
284-
})
281+
// replacement=="" means ReplacePrefixMatch="/" on root match:
282+
// every path maps to itself, so no rewrite rule is needed.
283+
if replacement != "" {
284+
rules = append(rules, &models.HTTPRequestRule{
285+
Type: "set-path",
286+
PathFmt: fmt.Sprintf("%s%%[path,regsub(^/$,)]", replacement),
287+
})
288+
}
285289
} else {
286290
escapedPrefix := regexEscapePath(matchPrefix)
287-
rules = append(rules, &models.HTTPRequestRule{
288-
Type: "replace-path",
289-
PathMatch: fmt.Sprintf("^%s(/.*)?$", escapedPrefix),
290-
PathFmt: fmt.Sprintf("%s\\1", replacement),
291-
})
291+
if replacement == "" {
292+
// replacePrefixMatch is "/" (trimmed to ""): two rules are
293+
// needed because a single replace-path with \1 produces an
294+
// empty path when the request path is exactly the prefix
295+
// (no suffix), whereas the expected result is "/".
296+
rules = append(rules,
297+
&models.HTTPRequestRule{
298+
Type: "replace-path",
299+
PathMatch: fmt.Sprintf("^%s$", escapedPrefix),
300+
PathFmt: "/",
301+
},
302+
&models.HTTPRequestRule{
303+
Type: "replace-path",
304+
PathMatch: fmt.Sprintf("^%s(/.*)?$", escapedPrefix),
305+
PathFmt: "\\1",
306+
},
307+
)
308+
} else {
309+
rules = append(rules, &models.HTTPRequestRule{
310+
Type: "replace-path",
311+
PathMatch: fmt.Sprintf("^%s(/.*)?$", escapedPrefix),
312+
PathFmt: fmt.Sprintf("%s\\1", replacement),
313+
})
314+
}
292315
}
293316
}
294317
}
@@ -378,6 +401,9 @@ func requestRedirectRules(f *gatewayv1.HTTPRequestRedirectFilter, matchPrefix st
378401
}
379402
replacement := strings.TrimRight(*f.Path.ReplacePrefixMatch, "/")
380403
if matchPrefix == "/" {
404+
if replacement == "" {
405+
return models.HTTPRequestRules{redirect(pfx+"%[path]", "", "")}
406+
}
381407
return models.HTTPRequestRules{
382408
redirect(fmt.Sprintf("%s%s%%[path,regsub(^/$,)]", pfx, replacement), "", ""),
383409
}

k8s/gate/haproxy/filters/http_filters_test.go

Lines changed: 44 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -200,6 +200,20 @@ func TestRequestRedirectRules(t *testing.T) {
200200
wantType: "location",
201201
wantValue: "%[path,regsub(^/old,/new)]",
202202
},
203+
{
204+
// replacePrefixMatch "/" with root matchPrefix keeps the path unchanged.
205+
name: "replace root prefix with root preserves path",
206+
filter: gatewayv1.HTTPRequestRedirectFilter{
207+
Path: &gatewayv1.HTTPPathModifier{
208+
Type: gatewayv1.PrefixMatchHTTPPathModifier,
209+
ReplacePrefixMatch: new("/"),
210+
},
211+
},
212+
matchPrefix: "/",
213+
wantLen: 1,
214+
wantType: "location",
215+
wantValue: "%[path]",
216+
},
203217
{
204218
name: "custom status code",
205219
filter: gatewayv1.HTTPRequestRedirectFilter{
@@ -277,12 +291,41 @@ func TestURLRewriteRules(t *testing.T) {
277291
wantLen: 1,
278292
wantType: "replace-path",
279293
},
294+
{
295+
// replacePrefixMatch "/" on a non-root prefix needs two rules:
296+
// one for the exact prefix (→ "/") and one for prefix+suffix.
297+
name: "strip prefix to root",
298+
filter: gatewayv1.HTTPURLRewriteFilter{
299+
Path: &gatewayv1.HTTPPathModifier{
300+
Type: gatewayv1.PrefixMatchHTTPPathModifier,
301+
ReplacePrefixMatch: new("/"),
302+
},
303+
},
304+
matchPrefix: "/strip-prefix",
305+
wantLen: 2,
306+
wantType: "replace-path",
307+
},
308+
{
309+
// replacePrefixMatch "/" with root matchPrefix is a no-op: every path
310+
// maps to itself, so no rewrite rule should be emitted.
311+
name: "replace root prefix with root is no-op",
312+
filter: gatewayv1.HTTPURLRewriteFilter{
313+
Path: &gatewayv1.HTTPPathModifier{
314+
Type: gatewayv1.PrefixMatchHTTPPathModifier,
315+
ReplacePrefixMatch: new("/"),
316+
},
317+
},
318+
matchPrefix: "/",
319+
wantLen: 0,
320+
},
280321
}
281322
for _, tc := range tests {
282323
t.Run(tc.name, func(t *testing.T) {
283324
rules := urlRewriteRules(&tc.filter, tc.matchPrefix)
284325
require.Len(t, rules, tc.wantLen)
285-
assert.Equal(t, tc.wantType, rules[0].Type)
326+
if len(rules) > 0 {
327+
assert.Equal(t, tc.wantType, rules[0].Type)
328+
}
286329
})
287330
}
288331
}

0 commit comments

Comments
 (0)