@marcel91200 / caddy Mirror · read-only / commits / 7bf1b905
Fork 0

Fast and extensible multi-platform HTTP/1-2-3 web server with automatic HTTPS

Mirrored from https://github.com/Slazee/caddy.git

Synced Nov 11, 06:00 UTC

rewrite: fix strip_path_suffix ignoring percent-encoding (#7877)

StripPathSuffix is documented to behave like StripPathPrefix: the suffix is matched in normalized (unescaped) space except where the pattern uses an escape sequence. But suffix stripping was implemented as reverse(trimPathPrefix(reverse(escapedPath), reverse(suffix))) Reversing the strings moves the '%' to the *end* of each "%xx" escape, which defeats trimPathPrefix's escape detection (it expects '%' to precede the two hex digits). As a result the escape-aware, normalized comparison never happened for suffixes: a decoded pattern failed to match a percent-encoded path. Concretely, StripPathPrefix "/a/b/c" strips "/a%2Fb/c/d" to "/d", but the mirror StripPathSuffix "/b/c" left "/a/b%2Fc" untouched instead of producing "/a"; likewise StripPathSuffix "bc" did not strip "/a%62c". This has been the behavior since #4948, which introduced both the escape-aware trimPathPrefix and the reverse-based suffix trimming. Replace the reverse trick with a dedicated trimPathSuffix that iterates from the ends of both strings and applies the same escape-aware, case-insensitive comparison as trimPathPrefix. An escape in the pattern itself is still compared literally, so "%2fsuffix" continues to require the path to contain that exact escape. Co-authored-by: Claude Fable 5 <noreply@anthropic.com>

TowyTowy committed Aug 28, 2026 at 11:44 UTC 7bf1b9057b29903efef451440e4664efd8775ec5
2 files changed +73 -6
modules/caddyhttp/rewrite/rewrite.go
+52 -6
@@ -296,7 +296,7 @@ func (rewr Rewrite) Rewrite(r *http.Request, repl *caddy.Replacer) bool {
296 mergeSlashes := !strings.Contains(suffix, "//")
297 changePath(r, func(escapedPath string) string {
298 escapedPath = caddyhttp.CleanPath(escapedPath, mergeSlashes)
299 - return reverse(trimPathPrefix(reverse(escapedPath), reverse(suffix)))
299 + return trimPathSuffix(escapedPath, suffix)
300 })
301 }
302
@@ -464,12 +464,58 @@ func trimPathPrefix(escapedPath, prefix string) string {
464 return escapedPath
465 }
466
467 -func reverse(s string) string {
468 - r := []rune(s)
469 - for i, j := 0, len(r)-1; i < len(r)/2; i, j = i+1, j-1 {
470 - r[i], r[j] = r[j], r[i]
467 +// trimPathSuffix is the suffix counterpart of trimPathPrefix: it trims suffix
468 +// from the end of escapedPath using the same escape-aware, case-insensitive
469 +// comparison semantics. Both strings are iterated in lock-step from their ends,
470 +// and if escapedPath has a '%' encoding at a particular position where the
471 +// suffix pattern uses the decoded character, escapedPath's escape is decoded so
472 +// the comparison happens in normalized/unescaped space. Conversely, if the
473 +// suffix pattern itself uses an escape (`%xx`), escapedPath must literally use
474 +// the same escape at that position (the escapes are then compared byte-for-byte).
475 +//
476 +// A naive reverse-then-trimPathPrefix approach cannot be used here: reversing
477 +// the strings moves the '%' to the end of each escape sequence, which defeats
478 +// trimPathPrefix's escape detection (it expects '%' to precede the two hex
479 +// digits) and makes escaped path bytes compare unequal to their decoded form.
480 +func trimPathSuffix(escapedPath, suffix string) string {
481 + iPath, iSuffix := len(escapedPath), len(suffix)
482 + for iPath > 0 && iSuffix > 0 {
483 + suffixCh := suffix[iSuffix-1]
484 + ch := string(escapedPath[iPath-1])
485 + step := 1
486 +
487 + // if escapedPath uses a percent-encoding that ends at this position but
488 + // the suffix pattern does not encode this position, decode escapedPath's
489 + // escape so the comparison happens in normalized/unescaped space
490 + pathHasEscape := iPath >= 3 && escapedPath[iPath-3] == '%'
491 + suffixHasEscape := iSuffix >= 3 && suffix[iSuffix-3] == '%'
492 + if pathHasEscape && !suffixHasEscape {
493 + decoded, err := url.PathUnescape(escapedPath[iPath-3 : iPath])
494 + if err != nil {
495 + // should be impossible unless EscapedPath() is returning invalid values!
496 + return escapedPath
497 + }
498 + ch = decoded
499 + step = 3
500 + }
501 +
502 + // suffix comparisons are case-insensitive for consistency with
503 + // trimPathPrefix, which is case-insensitive for good reasons
504 + if !strings.EqualFold(ch, string(suffixCh)) {
505 + return escapedPath
506 + }
507 +
508 + iPath -= step
509 + iSuffix--
510 }
472 - return string(r)
511 +
512 + // if we iterated through the entire suffix, we found it, so trim it
513 + if iSuffix <= 0 {
514 + return escapedPath[:iPath]
515 + }
516 +
517 + // otherwise we did not find the suffix
518 + return escapedPath
519 }
520
521 // substrReplacer describes either a simple and fast substring replacement.
modules/caddyhttp/rewrite/rewrite_test.go
+21
@@ -335,6 +335,27 @@ func TestRewrite(t *testing.T) {
335 input: newRequest(t, "GET", "/foo/suffix/bar"),
336 expect: newRequest(t, "GET", "/foo/suffix/bar"),
337 },
338 + {
339 + // a decoded suffix pattern must match a percent-encoded path in
340 + // normalized space, mirroring StripPathPrefix (see the "/a/b/c"
341 + // vs "/a%2Fb/c/d" case above)
342 + rule: Rewrite{StripPathSuffix: "/b/c"},
343 + input: newRequest(t, "GET", "/a/b%2Fc"),
344 + expect: newRequest(t, "GET", "/a"),
345 + },
346 + {
347 + // decoded suffix char matches its percent-encoded form in the path
348 + rule: Rewrite{StripPathSuffix: "bc"},
349 + input: newRequest(t, "GET", "/a%62c"), // %62 == 'b'
350 + expect: newRequest(t, "GET", "/a"),
351 + },
352 + {
353 + // an escaped suffix pattern requires the path to use the same
354 + // escape at that position, so a decoded path must NOT be stripped
355 + rule: Rewrite{StripPathSuffix: "%2fsuffix"},
356 + input: newRequest(t, "GET", "/foo/bar/suffix"),
357 + expect: newRequest(t, "GET", "/foo/bar/suffix"),
358 + },
359
360 {
361 rule: Rewrite{URISubstring: []substrReplacer{{Find: "findme", Replace: "replaced"}}},