From 79bde21fec43855ca7eda098401d0220fa40dab3 Mon Sep 17 00:00:00 2001 From: "a.bolonin" Date: Fri, 23 Oct 2026 14:04:55 +0300 Subject: [PATCH 1/3] perf: skip impossible prefixes in splitPos --- cgi.go | 13 +++- cgi_splitpos_test.go | 144 +++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 156 insertions(+), 1 deletion(-) create mode 100644 cgi_splitpos_test.go diff --git a/cgi.go b/cgi.go index 391174ceb4..a9c38bf74d 100644 --- a/cgi.go +++ b/cgi.go @@ -253,7 +253,18 @@ func splitPos(path string, splitPath []string) int { continue } - for i := 0; i <= pathLen-splitLen; i++ { + // Skip the prefix before the first possible match. Lowercase letter + // prefixes still need the case-insensitive scan. Search only once so + // paths with many partial matches keep the original linear scan. + start := 0 + if first := split[0]; first < 'a' || first > 'z' { + start = strings.IndexByte(path[:pathLen-splitLen+1], first) + if start < 0 { + continue + } + } + + for i := start; i <= pathLen-splitLen; i++ { match := true for j := range splitLen { c := path[i+j] diff --git a/cgi_splitpos_test.go b/cgi_splitpos_test.go new file mode 100644 index 0000000000..9ecf52b301 --- /dev/null +++ b/cgi_splitpos_test.go @@ -0,0 +1,144 @@ +package frankenphp + +import ( + "strings" + "testing" + "unicode/utf8" +) + +func TestSplitPosPrefixSearch(t *testing.T) { + t.Parallel() + tests := []struct { + name string + path string + markers []string + want int + }{ + {"letter prefix folds case", "/INDEX.PHP/info", []string{"php"}, 10}, + {"single letter folds case", "/P/info", []string{"p"}, 2}, + {"single punctuation", "/a./info", []string{"."}, 3}, + {"nonmatching prefix before match", "/a.phx/b.PHP/info", []string{".php"}, 12}, + {"last possible start", "/a.php", []string{".php"}, 6}, + {"marker longer than path", "/a", []string{".php"}, -1}, + {"invalid first occurrence stops marker", "/a.php.txt/b.php", []string{".php"}, -1}, + {"later marker after invalid occurrence", "/a.php.txt/b.phtml", []string{".php", ".phtml"}, 18}, + {"marker order wins over path order", "/a.php/b.phtml", []string{".phtml", ".php"}, 14}, + {"empty marker skipped", "/a.php", []string{"", ".php"}, 6}, + {"nil markers", "/a.php", nil, 0}, + {"invalid UTF8 before extension", "/\xff.PHP/info", []string{".php"}, 6}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := splitPos(tt.path, tt.markers); got != tt.want { + t.Fatalf("splitPos(%q, %q) = %d, want %d", tt.path, tt.markers, got, tt.want) + } + }) + } +} + +// Keep the original scan as an independent oracle for fuzzing the optimized +// prefix search, including unvalidated marker bytes and marker ordering. +func splitPosReference(path string, splitPath []string) int { + if len(splitPath) == 0 { + return 0 + } + + pathLen := len(path) + + for _, split := range splitPath { + splitLen := len(split) + if splitLen == 0 || splitLen > pathLen { + continue + } + + for i := 0; i <= pathLen-splitLen; i++ { + match := true + for j := range splitLen { + c := path[i+j] + if c >= utf8.RuneSelf { + match = false + + break + } + + if 'A' <= c && c <= 'Z' { + c += 'a' - 'A' + } + + if c != split[j] { + match = false + + break + } + } + + if match { + pos := i + splitLen + // the split must end the path or a whole path segment: without + // this, ".php" embedded in a filename ("/a.php.txt/b.php") + // would split inside that filename and run a script the router + // never matched. Give up on this split string entirely rather + // than looking for a later occurrence of it, so the result + // stays identical to Caddy's file matcher (firstSplit). + if pos != pathLen && path[pos] != '/' { + break + } + + return pos + } + } + } + + return -1 +} + +func FuzzSplitPosEquivalent(f *testing.F) { + for _, seed := range []struct{ path, first, second string }{ + {"/index.php/info", ".php", ".phtml"}, + {"/a.php.txt/b.phtml", ".php", ".phtml"}, + {"/INDEX.PHP/info", "php", ".php"}, + {"/a.php/b.phtml", ".phtml", ".php"}, + {"/shell.ｐｈｐ", ".php", ""}, + {"/\xff.PHP", ".php", "\xff"}, + {"", "", ""}, + } { + f.Add(seed.path, seed.first, seed.second) + } + f.Fuzz(func(t *testing.T, path, first, second string) { + markers := []string{first, second} + got, want := splitPos(path, markers), splitPosReference(path, markers) + if got != want { + t.Fatalf("splitPos(%q, %q) = %d, reference = %d", path, markers, got, want) + } + }) +} + +func BenchmarkSplitPos(b *testing.B) { + cases := []struct { + name, path string + markers []string + }{ + {"short", "/index.php", []string{".php"}}, + {"long", "/some/long/application/public/directory/routes/controller/index.php/path/info", []string{".php"}}, + {"no_match", "/some/long/application/public/directory/assets/styles/main.css", []string{".php"}}, + {"uppercase", "/INDEX.PHP/path", []string{".php"}}, + {"many_dots", "/" + strings.Repeat(".", 64) + "/index.php", []string{".php"}}, + {"partial_matches", "/" + strings.Repeat(".phx/", 16) + "index.php", []string{".php"}}, + {"invalid_first", "/a.php.txt/b.php", []string{".php"}}, + {"second_marker", "/index.phtml/info", []string{".php", ".phtml"}}, + {"letter_prefix", "/INDEX.PHP/path", []string{"php"}}, + {"unicode_path", "/приложение/публичный/index.PHP/info", []string{".php"}}, + {"empty_path", "", []string{".php"}}, + {"empty_markers", "/index.php", nil}, + } + for _, tt := range cases { + b.Run(tt.name, func(b *testing.B) { + b.ReportAllocs() + for b.Loop() { + benchmarkSplitPosResult = splitPos(tt.path, tt.markers) + } + }) + } +} + +var benchmarkSplitPosResult int From aeb13b46e5fe1df91604866c8512ffc13e986241 Mon Sep 17 00:00:00 2001 From: "a.bolonin" Date: Fri, 23 Oct 2026 17:10:01 +0300 Subject: [PATCH 2/3] perf: address splitPos review feedback --- cgi.go | 85 ++++++++++--------------- cgi_splitpos_test.go | 144 ------------------------------------------- cgi_test.go | 144 +++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 175 insertions(+), 198 deletions(-) delete mode 100644 cgi_splitpos_test.go diff --git a/cgi.go b/cgi.go index a9c38bf74d..072db25ad7 100644 --- a/cgi.go +++ b/cgi.go @@ -22,7 +22,6 @@ import ( "path/filepath" "runtime" "strings" - "unicode/utf8" "unsafe" "github.com/dunglas/frankenphp/internal/phpheaders" @@ -234,77 +233,55 @@ func splitCgiPath(fc *frankenPHPContext) { // example: if splitPath is [".php"] // "/path/to/script.php/some/path": ("/path/to/script.php", "/some/path") // -// Matching is strictly ASCII case-insensitive. Bytes >= utf8.RuneSelf in path -// never match any split entry: split strings are validated ASCII-only and -// lower-cased in WithRequestSplitPath, so any Unicode equivalence (e.g. -// fullwidth or mathematical letters folding to ASCII) would let an attacker -// upload a file whose name contains such code points and have it served as -// PHP. See GHSA-3g8v-8r37-cgjm and GHSA-v4h7-cj44-8fc8. +// Matching is ASCII case-insensitive and requires lowercase ASCII markers from normalizeSplitPath. func splitPos(path string, splitPath []string) int { if len(splitPath) == 0 { return 0 } - - pathLen := len(path) - for _, split := range splitPath { - splitLen := len(split) - if splitLen == 0 || splitLen > pathLen { + n := len(split) + if n == 0 || n > len(path) { continue } - - // Skip the prefix before the first possible match. Lowercase letter - // prefixes still need the case-insensitive scan. Search only once so - // paths with many partial matches keep the original linear scan. - start := 0 - if first := split[0]; first < 'a' || first > 'z' { - start = strings.IndexByte(path[:pathLen-splitLen+1], first) - if start < 0 { + first, mask := split[0], foldMask(split[0]) + last := len(path) - n + i := 0 + if mask == 0 { + if i = strings.IndexByte(path[:last+1], first); i < 0 { continue } } - - for i := start; i <= pathLen-splitLen; i++ { - match := true - for j := range splitLen { - c := path[i+j] - if c >= utf8.RuneSelf { - match = false - - break - } - - if 'A' <= c && c <= 'Z' { - c += 'a' - 'A' - } - - if c != split[j] { - match = false - - break - } + for ; i <= last; i++ { + if path[i]|mask != first || !hasFoldedPrefix(path[i+1:i+n], split[1:]) { + continue } - - if match { - pos := i + splitLen - // the split must end the path or a whole path segment: without - // this, ".php" embedded in a filename ("/a.php.txt/b.php") - // would split inside that filename and run a script the router - // never matched. Give up on this split string entirely rather - // than looking for a later occurrence of it, so the result - // stays identical to Caddy's file matcher (firstSplit). - if pos != pathLen && path[pos] != '/' { - break - } - + if pos := i + n; pos == len(path) || path[pos] == '/' { return pos } + // Stop at the first occurrence to match Caddy's file matcher. + break } } - return -1 } +// foldMask lets c|mask == s match both cases of a lowercase ASCII letter. +func foldMask(s byte) byte { + if 'a' <= s && s <= 'z' { + return 0x20 + } + return 0 +} + +func hasFoldedPrefix(p, split string) bool { + for j := 0; j < len(split); j++ { + if s := split[j]; p[j]|foldMask(s) != s { + return false + } + } + return true +} + // go_update_request_info updates the sapi_request_info struct // See: https://github.com/php/php-src/blob/345e04b619c3bc11ea17ee02cdecad6ae8ce5891/main/SAPI.h#L72 // diff --git a/cgi_splitpos_test.go b/cgi_splitpos_test.go deleted file mode 100644 index 9ecf52b301..0000000000 --- a/cgi_splitpos_test.go +++ /dev/null @@ -1,144 +0,0 @@ -package frankenphp - -import ( - "strings" - "testing" - "unicode/utf8" -) - -func TestSplitPosPrefixSearch(t *testing.T) { - t.Parallel() - tests := []struct { - name string - path string - markers []string - want int - }{ - {"letter prefix folds case", "/INDEX.PHP/info", []string{"php"}, 10}, - {"single letter folds case", "/P/info", []string{"p"}, 2}, - {"single punctuation", "/a./info", []string{"."}, 3}, - {"nonmatching prefix before match", "/a.phx/b.PHP/info", []string{".php"}, 12}, - {"last possible start", "/a.php", []string{".php"}, 6}, - {"marker longer than path", "/a", []string{".php"}, -1}, - {"invalid first occurrence stops marker", "/a.php.txt/b.php", []string{".php"}, -1}, - {"later marker after invalid occurrence", "/a.php.txt/b.phtml", []string{".php", ".phtml"}, 18}, - {"marker order wins over path order", "/a.php/b.phtml", []string{".phtml", ".php"}, 14}, - {"empty marker skipped", "/a.php", []string{"", ".php"}, 6}, - {"nil markers", "/a.php", nil, 0}, - {"invalid UTF8 before extension", "/\xff.PHP/info", []string{".php"}, 6}, - } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - if got := splitPos(tt.path, tt.markers); got != tt.want { - t.Fatalf("splitPos(%q, %q) = %d, want %d", tt.path, tt.markers, got, tt.want) - } - }) - } -} - -// Keep the original scan as an independent oracle for fuzzing the optimized -// prefix search, including unvalidated marker bytes and marker ordering. -func splitPosReference(path string, splitPath []string) int { - if len(splitPath) == 0 { - return 0 - } - - pathLen := len(path) - - for _, split := range splitPath { - splitLen := len(split) - if splitLen == 0 || splitLen > pathLen { - continue - } - - for i := 0; i <= pathLen-splitLen; i++ { - match := true - for j := range splitLen { - c := path[i+j] - if c >= utf8.RuneSelf { - match = false - - break - } - - if 'A' <= c && c <= 'Z' { - c += 'a' - 'A' - } - - if c != split[j] { - match = false - - break - } - } - - if match { - pos := i + splitLen - // the split must end the path or a whole path segment: without - // this, ".php" embedded in a filename ("/a.php.txt/b.php") - // would split inside that filename and run a script the router - // never matched. Give up on this split string entirely rather - // than looking for a later occurrence of it, so the result - // stays identical to Caddy's file matcher (firstSplit). - if pos != pathLen && path[pos] != '/' { - break - } - - return pos - } - } - } - - return -1 -} - -func FuzzSplitPosEquivalent(f *testing.F) { - for _, seed := range []struct{ path, first, second string }{ - {"/index.php/info", ".php", ".phtml"}, - {"/a.php.txt/b.phtml", ".php", ".phtml"}, - {"/INDEX.PHP/info", "php", ".php"}, - {"/a.php/b.phtml", ".phtml", ".php"}, - {"/shell.ｐｈｐ", ".php", ""}, - {"/\xff.PHP", ".php", "\xff"}, - {"", "", ""}, - } { - f.Add(seed.path, seed.first, seed.second) - } - f.Fuzz(func(t *testing.T, path, first, second string) { - markers := []string{first, second} - got, want := splitPos(path, markers), splitPosReference(path, markers) - if got != want { - t.Fatalf("splitPos(%q, %q) = %d, reference = %d", path, markers, got, want) - } - }) -} - -func BenchmarkSplitPos(b *testing.B) { - cases := []struct { - name, path string - markers []string - }{ - {"short", "/index.php", []string{".php"}}, - {"long", "/some/long/application/public/directory/routes/controller/index.php/path/info", []string{".php"}}, - {"no_match", "/some/long/application/public/directory/assets/styles/main.css", []string{".php"}}, - {"uppercase", "/INDEX.PHP/path", []string{".php"}}, - {"many_dots", "/" + strings.Repeat(".", 64) + "/index.php", []string{".php"}}, - {"partial_matches", "/" + strings.Repeat(".phx/", 16) + "index.php", []string{".php"}}, - {"invalid_first", "/a.php.txt/b.php", []string{".php"}}, - {"second_marker", "/index.phtml/info", []string{".php", ".phtml"}}, - {"letter_prefix", "/INDEX.PHP/path", []string{"php"}}, - {"unicode_path", "/приложение/публичный/index.PHP/info", []string{".php"}}, - {"empty_path", "", []string{".php"}}, - {"empty_markers", "/index.php", nil}, - } - for _, tt := range cases { - b.Run(tt.name, func(b *testing.B) { - b.ReportAllocs() - for b.Loop() { - benchmarkSplitPosResult = splitPos(tt.path, tt.markers) - } - }) - } -} - -var benchmarkSplitPosResult int diff --git a/cgi_test.go b/cgi_test.go index 4a3a1c41f6..37cb77bfed 100644 --- a/cgi_test.go +++ b/cgi_test.go @@ -5,6 +5,7 @@ import ( "runtime" "strings" "testing" + "unicode/utf8" "github.com/stretchr/testify/assert" ) @@ -519,3 +520,146 @@ func FuzzSplitRemoteAddr(f *testing.F) { splitRemoteAddr(remoteAddr) }) } + +func TestSplitPosPrefixSearch(t *testing.T) { + t.Parallel() + tests := []struct { + name string + path string + markers []string + want int + }{ + {"letter prefix folds case", "/INDEX.PHP/info", []string{"php"}, 10}, + {"single letter folds case", "/P/info", []string{"p"}, 2}, + {"single punctuation", "/a./info", []string{"."}, 3}, + {"nonmatching prefix before match", "/a.phx/b.PHP/info", []string{".php"}, 12}, + {"last possible start", "/a.php", []string{".php"}, 6}, + {"marker longer than path", "/a", []string{".php"}, -1}, + {"invalid first occurrence stops marker", "/a.php.txt/b.php", []string{".php"}, -1}, + {"later marker after invalid occurrence", "/a.php.txt/b.phtml", []string{".php", ".phtml"}, 18}, + {"marker order wins over path order", "/a.php/b.phtml", []string{".phtml", ".php"}, 14}, + {"empty marker skipped", "/a.php", []string{"", ".php"}, 6}, + {"nil markers", "/a.php", nil, 0}, + {"invalid UTF8 before extension", "/\xff.PHP/info", []string{".php"}, 6}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := splitPos(tt.path, tt.markers); got != tt.want { + t.Fatalf("splitPos(%q, %q) = %d, want %d", tt.path, tt.markers, got, tt.want) + } + }) + } +} + +// Keep the original scan as an independent fuzzing oracle. +func splitPosReference(path string, splitPath []string) int { + if len(splitPath) == 0 { + return 0 + } + + pathLen := len(path) + + for _, split := range splitPath { + splitLen := len(split) + if splitLen == 0 || splitLen > pathLen { + continue + } + + for i := 0; i <= pathLen-splitLen; i++ { + match := true + for j := range splitLen { + c := path[i+j] + if c >= utf8.RuneSelf { + match = false + + break + } + + if 'A' <= c && c <= 'Z' { + c += 'a' - 'A' + } + + if c != split[j] { + match = false + + break + } + } + + if match { + pos := i + splitLen + // the split must end the path or a whole path segment: without + // this, ".php" embedded in a filename ("/a.php.txt/b.php") + // would split inside that filename and run a script the router + // never matched. Give up on this split string entirely rather + // than looking for a later occurrence of it, so the result + // stays identical to Caddy's file matcher (firstSplit). + if pos != pathLen && path[pos] != '/' { + break + } + + return pos + } + } + } + + return -1 +} + +func FuzzSplitPosEquivalent(f *testing.F) { + for _, seed := range []struct{ path, first, second string }{ + {"/index.php/info", ".php", ".phtml"}, + {"/a.php.txt/b.phtml", ".php", ".phtml"}, + {"/INDEX.PHP/info", "php", ".php"}, + {"/a.php/b.phtml", ".phtml", ".php"}, + {"/shell.ｐｈｐ", ".php", ""}, + {"/\xff.PHP", ".php", "\xff"}, + {"", "", ""}, + } { + f.Add(seed.path, seed.first, seed.second) + } + f.Fuzz(func(t *testing.T, path, first, second string) { + markers := []string{strings.ToLower(first), strings.ToLower(second)} + for _, marker := range markers { + for i := range len(marker) { + if marker[i] >= utf8.RuneSelf { + t.Skip() + } + } + } + got, want := splitPos(path, markers), splitPosReference(path, markers) + if got != want { + t.Fatalf("splitPos(%q, %q) = %d, reference = %d", path, markers, got, want) + } + }) +} + +func BenchmarkSplitPos(b *testing.B) { + cases := []struct { + name, path string + markers []string + }{ + {"short", "/index.php", []string{".php"}}, + {"long", "/some/long/application/public/directory/routes/controller/index.php/path/info", []string{".php"}}, + {"no_match", "/some/long/application/public/directory/assets/styles/main.css", []string{".php"}}, + {"uppercase", "/INDEX.PHP/path", []string{".php"}}, + {"many_dots", "/" + strings.Repeat(".", 64) + "/index.php", []string{".php"}}, + {"partial_matches", "/" + strings.Repeat(".phx/", 16) + "index.php", []string{".php"}}, + {"invalid_first", "/a.php.txt/b.php", []string{".php"}}, + {"second_marker", "/index.phtml/info", []string{".php", ".phtml"}}, + {"letter_prefix", "/INDEX.PHP/path", []string{"php"}}, + {"unicode_path", "/приложение/публичный/index.PHP/info", []string{".php"}}, + {"empty_path", "", []string{".php"}}, + {"empty_markers", "/index.php", nil}, + } + for _, tt := range cases { + b.Run(tt.name, func(b *testing.B) { + b.ReportAllocs() + for b.Loop() { + benchmarkSplitPosResult = splitPos(tt.path, tt.markers) + } + }) + } +} + +var benchmarkSplitPosResult int From 738dd921557f1013ec3059cc4d0d47710d51dc04 Mon Sep 17 00:00:00 2001 From: "a.bolonin" Date: Sun, 25 Oct 2026 19:46:18 +0300 Subject: [PATCH 3/3] perf: address splitPos naming and benchmark review --- cgi.go | 12 ++++++------ cgi_test.go | 30 ------------------------------ 2 files changed, 6 insertions(+), 36 deletions(-) diff --git a/cgi.go b/cgi.go index 072db25ad7..8ee900628c 100644 --- a/cgi.go +++ b/cgi.go @@ -239,12 +239,12 @@ func splitPos(path string, splitPath []string) int { return 0 } for _, split := range splitPath { - n := len(split) - if n == 0 || n > len(path) { + splitLen := len(split) + if splitLen == 0 || splitLen > len(path) { continue } first, mask := split[0], foldMask(split[0]) - last := len(path) - n + last := len(path) - splitLen i := 0 if mask == 0 { if i = strings.IndexByte(path[:last+1], first); i < 0 { @@ -252,13 +252,13 @@ func splitPos(path string, splitPath []string) int { } } for ; i <= last; i++ { - if path[i]|mask != first || !hasFoldedPrefix(path[i+1:i+n], split[1:]) { + if path[i]|mask != first || !hasFoldedPrefix(path[i+1:i+splitLen], split[1:]) { continue } - if pos := i + n; pos == len(path) || path[pos] == '/' { + if pos := i + splitLen; pos == len(path) || path[pos] == '/' { return pos } - // Stop at the first occurrence to match Caddy's file matcher. + // Reject an embedded match (e.g. /a.php.txt/b.php) without trying later occurrences. break } } diff --git a/cgi_test.go b/cgi_test.go index 37cb77bfed..de83a4df53 100644 --- a/cgi_test.go +++ b/cgi_test.go @@ -633,33 +633,3 @@ func FuzzSplitPosEquivalent(f *testing.F) { } }) } - -func BenchmarkSplitPos(b *testing.B) { - cases := []struct { - name, path string - markers []string - }{ - {"short", "/index.php", []string{".php"}}, - {"long", "/some/long/application/public/directory/routes/controller/index.php/path/info", []string{".php"}}, - {"no_match", "/some/long/application/public/directory/assets/styles/main.css", []string{".php"}}, - {"uppercase", "/INDEX.PHP/path", []string{".php"}}, - {"many_dots", "/" + strings.Repeat(".", 64) + "/index.php", []string{".php"}}, - {"partial_matches", "/" + strings.Repeat(".phx/", 16) + "index.php", []string{".php"}}, - {"invalid_first", "/a.php.txt/b.php", []string{".php"}}, - {"second_marker", "/index.phtml/info", []string{".php", ".phtml"}}, - {"letter_prefix", "/INDEX.PHP/path", []string{"php"}}, - {"unicode_path", "/приложение/публичный/index.PHP/info", []string{".php"}}, - {"empty_path", "", []string{".php"}}, - {"empty_markers", "/index.php", nil}, - } - for _, tt := range cases { - b.Run(tt.name, func(b *testing.B) { - b.ReportAllocs() - for b.Loop() { - benchmarkSplitPosResult = splitPos(tt.path, tt.markers) - } - }) - } -} - -var benchmarkSplitPosResult int