From 02313c063270e3497b5b35debb31a526a54fde2b Mon Sep 17 00:00:00 2001 From: "a.bolonin" Date: Fri, 23 Oct 2026 14:33:42 +0300 Subject: [PATCH 1/3] perf: avoid allocating the assigned worker path prefix --- cgi.go | 7 ++- cgi_workerpath_test.go | 125 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 129 insertions(+), 3 deletions(-) create mode 100644 cgi_workerpath_test.go diff --git a/cgi.go b/cgi.go index 391174ceb4..9f32b9d7a7 100644 --- a/cgi.go +++ b/cgi.go @@ -204,9 +204,10 @@ func splitCgiPath(fc *frankenPHPContext) { // If a worker is already assigned explicitly, derive SCRIPT_NAME from its filename if fc.worker != nil { fc.scriptFilename = fc.worker.fileName - docRootWithSep := fc.documentRoot + string(filepath.Separator) - if strings.HasPrefix(fc.worker.fileName, docRootWithSep) { - fc.scriptName = filepath.ToSlash(strings.TrimPrefix(fc.worker.fileName, fc.documentRoot)) + if len(fc.worker.fileName) > len(fc.documentRoot) && + fc.worker.fileName[len(fc.documentRoot)] == byte(filepath.Separator) && + strings.HasPrefix(fc.worker.fileName, fc.documentRoot) { + fc.scriptName = filepath.ToSlash(fc.worker.fileName[len(fc.documentRoot):]) } else { fc.docURI = "" fc.pathInfo = "" diff --git a/cgi_workerpath_test.go b/cgi_workerpath_test.go new file mode 100644 index 0000000000..d888b3178e --- /dev/null +++ b/cgi_workerpath_test.go @@ -0,0 +1,125 @@ +package frankenphp + +import ( + "net/http" + "net/url" + "path/filepath" + "reflect" + "strings" + "testing" +) + +func TestAssignedWorkerScriptPath(t *testing.T) { + t.Parallel() + sep := string(filepath.Separator) + root := filepath.Join(sep, "var", "www") + cases := []struct{ name, root, file string }{ + {"inside", root, root + sep + "index.php"}, + {"neighbor", root, root + "-other" + sep + "index.php"}, + {"exact", root, root}, + {"empty filename", root, ""}, + {"empty root", "", sep + "index.php"}, + {"empty paths", "", ""}, + {"trailing separator", root + sep, root + sep + sep + "index.php"}, + {"missing second separator", root + sep, root + sep + "index.php"}, + {"filesystem root", sep, sep + sep + "index.php"}, + {"unicode", root + sep + "приложение", root + sep + "приложение" + sep + "index.php"}, + {"binary", root + "\x00\xff", root + "\x00\xff" + sep + "index.php"}, + {"long root", strings.Repeat(root, 16), strings.Repeat(root, 16) + sep + "index.php"}, + } + for _, tt := range cases { + t.Run(tt.name, func(t *testing.T) { checkAssignedWorkerScriptPath(t, tt.root, tt.file, "/index.php/info") }) + } +} + +func checkAssignedWorkerScriptPath(t *testing.T, root, file, path string) { + t.Helper() + actual := frankenPHPContext{ + documentRoot: root, + request: &http.Request{URL: &url.URL{Path: path}}, + worker: &worker{fileName: file}, + } + expected := actual + splitCgiPathBeforePrefixSearch(&expected) + splitCgiPath(&actual) + if !reflect.DeepEqual(actual, expected) { + t.Fatalf("root=%q file=%q path=%q: got script=%q docURI=%q pathInfo=%q, want script=%q docURI=%q pathInfo=%q", root, file, path, actual.scriptName, actual.docURI, actual.pathInfo, expected.scriptName, expected.docURI, expected.pathInfo) + } +} + +func FuzzAssignedWorkerScriptPath(f *testing.F) { + sep := string(filepath.Separator) + f.Add(sep+"var"+sep+"www", sep+"var"+sep+"www"+sep+"index.php", "/index.php/info") + f.Add("", "", "/index.php") + f.Add(sep, sep+sep+"index.php", "/a.php.txt/b.php") + f.Add("\x00\xff", "\x00\xff"+sep+"index.php", "/shell.ｐｈｐ") + f.Fuzz(func(t *testing.T, root, file, path string) { checkAssignedWorkerScriptPath(t, root, file, path) }) +} + +// The previous implementation is retained as the differential fuzzing oracle. +func splitCgiPathBeforePrefixSearch(fc *frankenPHPContext) { + path := fc.request.URL.Path + splitPath := fc.splitPath + + if splitPath == nil { + splitPath = []string{".php"} + } + + if splitPos := splitPos(path, splitPath); splitPos > -1 { + fc.docURI = path[:splitPos] + fc.pathInfo = path[splitPos:] + } + + // If a worker is already assigned explicitly, derive SCRIPT_NAME from its filename + if fc.worker != nil { + fc.scriptFilename = fc.worker.fileName + docRootWithSep := fc.documentRoot + string(filepath.Separator) + if strings.HasPrefix(fc.worker.fileName, docRootWithSep) { + fc.scriptName = filepath.ToSlash(strings.TrimPrefix(fc.worker.fileName, fc.documentRoot)) + } else { + fc.docURI = "" + fc.pathInfo = "" + } + return + } + + // Strip PATH_INFO from SCRIPT_NAME + // Ensure the SCRIPT_NAME has a leading slash for compliance with RFC3875 + // Info: https://tools.ietf.org/html/rfc3875#section-4.1.13 + fc.scriptName = ensureLeadingSlash(strings.TrimSuffix(path, fc.pathInfo)) + + // TODO: is it possible to delay this and avoid saving everything in the context? + // SCRIPT_FILENAME is the absolute path of SCRIPT_NAME + fc.scriptFilename = sanitizedPathJoin(fc.documentRoot, fc.scriptName) + + // see if a php_server worker or global worker matches the request path + // aka: root + request path == worker.filename + if fc.worker = fc.server.workersByPath[fc.scriptFilename]; fc.worker == nil { + fc.worker = globalWorkersByPath[fc.scriptFilename] + } +} + +func BenchmarkAssignedWorkerScriptPath(b *testing.B) { + sep := string(filepath.Separator) + root := filepath.Join(sep, "var", "www") + longRoot := filepath.Join(sep, "srv", "apps", "organization", "production", "releases", "2026", "public") + for _, tt := range []struct{ name, root, file string }{ + {"short_root", root, root + sep + "index.php"}, + {"long_root", longRoot, longRoot + sep + "index.php"}, + {"outside_root", longRoot, filepath.Join(sep, "srv", "elsewhere", "index.php")}, + {"empty_root", "", sep + "index.php"}, + {"trailing_separator", root + sep, root + sep + sep + "index.php"}, + {"exact_root", root, root}, + } { + b.Run(tt.name, func(b *testing.B) { + fc := &frankenPHPContext{documentRoot: tt.root, request: &http.Request{URL: &url.URL{Path: "/index.php/info"}}, worker: &worker{fileName: tt.file}} + b.ReportAllocs() + for b.Loop() { + splitCgiPath(fc) + } + benchmarkAssignedWorkerScriptName = fc.scriptName + }) + } +} + +var benchmarkAssignedWorkerScriptName string From 3182117f236ea2e84abdf2727d1af8fd38f36a9a Mon Sep 17 00:00:00 2001 From: "a.bolonin" Date: Fri, 23 Oct 2026 17:10:02 +0300 Subject: [PATCH 2/3] perf: simplify worker path prefix handling after review --- cgi.go | 6 +- cgi_test.go | 67 ++++++++++++++++++++++ cgi_workerpath_test.go | 125 ----------------------------------------- 3 files changed, 69 insertions(+), 129 deletions(-) delete mode 100644 cgi_workerpath_test.go diff --git a/cgi.go b/cgi.go index 9f32b9d7a7..9fcbd8fe46 100644 --- a/cgi.go +++ b/cgi.go @@ -204,10 +204,8 @@ func splitCgiPath(fc *frankenPHPContext) { // If a worker is already assigned explicitly, derive SCRIPT_NAME from its filename if fc.worker != nil { fc.scriptFilename = fc.worker.fileName - if len(fc.worker.fileName) > len(fc.documentRoot) && - fc.worker.fileName[len(fc.documentRoot)] == byte(filepath.Separator) && - strings.HasPrefix(fc.worker.fileName, fc.documentRoot) { - fc.scriptName = filepath.ToSlash(fc.worker.fileName[len(fc.documentRoot):]) + if rest, ok := strings.CutPrefix(fc.worker.fileName, fc.documentRoot); ok && rest != "" && rest[0] == filepath.Separator { + fc.scriptName = filepath.ToSlash(rest) } else { fc.docURI = "" fc.pathInfo = "" diff --git a/cgi_test.go b/cgi_test.go index 4a3a1c41f6..4bc579aeaf 100644 --- a/cgi_test.go +++ b/cgi_test.go @@ -1,6 +1,8 @@ package frankenphp import ( + "net/http" + "net/url" "path/filepath" "runtime" "strings" @@ -519,3 +521,68 @@ func FuzzSplitRemoteAddr(f *testing.F) { splitRemoteAddr(remoteAddr) }) } + +func TestAssignedWorkerScriptPath(t *testing.T) { + t.Parallel() + sep := string(filepath.Separator) + root := filepath.Join(sep, "var", "www") + cases := []struct{ name, root, file string }{ + {"inside", root, root + sep + "index.php"}, + {"neighbor", root, root + "-other" + sep + "index.php"}, + {"exact", root, root}, + {"empty filename", root, ""}, + {"empty root", "", sep + "index.php"}, + {"empty paths", "", ""}, + {"trailing separator", root + sep, root + sep + sep + "index.php"}, + {"missing second separator", root + sep, root + sep + "index.php"}, + {"filesystem root", sep, sep + sep + "index.php"}, + {"unicode", root + sep + "приложение", root + sep + "приложение" + sep + "index.php"}, + {"binary", root + "\x00\xff", root + "\x00\xff" + sep + "index.php"}, + {"long root", strings.Repeat(root, 16), strings.Repeat(root, 16) + sep + "index.php"}, + } + for _, tt := range cases { + t.Run(tt.name, func(t *testing.T) { + fc := frankenPHPContext{ + documentRoot: tt.root, + request: &http.Request{URL: &url.URL{Path: "/index.php/info"}}, + worker: &worker{fileName: tt.file}, + } + splitCgiPath(&fc) + assert.Equal(t, tt.file, fc.scriptFilename) + if strings.HasPrefix(tt.file, tt.root+sep) { + assert.Equal(t, filepath.ToSlash(strings.TrimPrefix(tt.file, tt.root)), fc.scriptName) + assert.Equal(t, "/index.php", fc.docURI) + assert.Equal(t, "/info", fc.pathInfo) + } else { + assert.Empty(t, fc.scriptName) + assert.Empty(t, fc.docURI) + assert.Empty(t, fc.pathInfo) + } + }) + } +} + +func BenchmarkAssignedWorkerScriptPath(b *testing.B) { + sep := string(filepath.Separator) + root := filepath.Join(sep, "var", "www") + longRoot := filepath.Join(sep, "srv", "apps", "organization", "production", "releases", "2026", "public") + for _, tt := range []struct{ name, root, file string }{ + {"short_root", root, root + sep + "index.php"}, + {"long_root", longRoot, longRoot + sep + "index.php"}, + {"outside_root", longRoot, filepath.Join(sep, "srv", "elsewhere", "index.php")}, + {"empty_root", "", sep + "index.php"}, + {"trailing_separator", root + sep, root + sep + sep + "index.php"}, + {"exact_root", root, root}, + } { + b.Run(tt.name, func(b *testing.B) { + fc := &frankenPHPContext{documentRoot: tt.root, request: &http.Request{URL: &url.URL{Path: "/index.php/info"}}, worker: &worker{fileName: tt.file}} + b.ReportAllocs() + for b.Loop() { + splitCgiPath(fc) + } + benchmarkAssignedWorkerScriptName = fc.scriptName + }) + } +} + +var benchmarkAssignedWorkerScriptName string diff --git a/cgi_workerpath_test.go b/cgi_workerpath_test.go deleted file mode 100644 index d888b3178e..0000000000 --- a/cgi_workerpath_test.go +++ /dev/null @@ -1,125 +0,0 @@ -package frankenphp - -import ( - "net/http" - "net/url" - "path/filepath" - "reflect" - "strings" - "testing" -) - -func TestAssignedWorkerScriptPath(t *testing.T) { - t.Parallel() - sep := string(filepath.Separator) - root := filepath.Join(sep, "var", "www") - cases := []struct{ name, root, file string }{ - {"inside", root, root + sep + "index.php"}, - {"neighbor", root, root + "-other" + sep + "index.php"}, - {"exact", root, root}, - {"empty filename", root, ""}, - {"empty root", "", sep + "index.php"}, - {"empty paths", "", ""}, - {"trailing separator", root + sep, root + sep + sep + "index.php"}, - {"missing second separator", root + sep, root + sep + "index.php"}, - {"filesystem root", sep, sep + sep + "index.php"}, - {"unicode", root + sep + "приложение", root + sep + "приложение" + sep + "index.php"}, - {"binary", root + "\x00\xff", root + "\x00\xff" + sep + "index.php"}, - {"long root", strings.Repeat(root, 16), strings.Repeat(root, 16) + sep + "index.php"}, - } - for _, tt := range cases { - t.Run(tt.name, func(t *testing.T) { checkAssignedWorkerScriptPath(t, tt.root, tt.file, "/index.php/info") }) - } -} - -func checkAssignedWorkerScriptPath(t *testing.T, root, file, path string) { - t.Helper() - actual := frankenPHPContext{ - documentRoot: root, - request: &http.Request{URL: &url.URL{Path: path}}, - worker: &worker{fileName: file}, - } - expected := actual - splitCgiPathBeforePrefixSearch(&expected) - splitCgiPath(&actual) - if !reflect.DeepEqual(actual, expected) { - t.Fatalf("root=%q file=%q path=%q: got script=%q docURI=%q pathInfo=%q, want script=%q docURI=%q pathInfo=%q", root, file, path, actual.scriptName, actual.docURI, actual.pathInfo, expected.scriptName, expected.docURI, expected.pathInfo) - } -} - -func FuzzAssignedWorkerScriptPath(f *testing.F) { - sep := string(filepath.Separator) - f.Add(sep+"var"+sep+"www", sep+"var"+sep+"www"+sep+"index.php", "/index.php/info") - f.Add("", "", "/index.php") - f.Add(sep, sep+sep+"index.php", "/a.php.txt/b.php") - f.Add("\x00\xff", "\x00\xff"+sep+"index.php", "/shell.ｐｈｐ") - f.Fuzz(func(t *testing.T, root, file, path string) { checkAssignedWorkerScriptPath(t, root, file, path) }) -} - -// The previous implementation is retained as the differential fuzzing oracle. -func splitCgiPathBeforePrefixSearch(fc *frankenPHPContext) { - path := fc.request.URL.Path - splitPath := fc.splitPath - - if splitPath == nil { - splitPath = []string{".php"} - } - - if splitPos := splitPos(path, splitPath); splitPos > -1 { - fc.docURI = path[:splitPos] - fc.pathInfo = path[splitPos:] - } - - // If a worker is already assigned explicitly, derive SCRIPT_NAME from its filename - if fc.worker != nil { - fc.scriptFilename = fc.worker.fileName - docRootWithSep := fc.documentRoot + string(filepath.Separator) - if strings.HasPrefix(fc.worker.fileName, docRootWithSep) { - fc.scriptName = filepath.ToSlash(strings.TrimPrefix(fc.worker.fileName, fc.documentRoot)) - } else { - fc.docURI = "" - fc.pathInfo = "" - } - return - } - - // Strip PATH_INFO from SCRIPT_NAME - // Ensure the SCRIPT_NAME has a leading slash for compliance with RFC3875 - // Info: https://tools.ietf.org/html/rfc3875#section-4.1.13 - fc.scriptName = ensureLeadingSlash(strings.TrimSuffix(path, fc.pathInfo)) - - // TODO: is it possible to delay this and avoid saving everything in the context? - // SCRIPT_FILENAME is the absolute path of SCRIPT_NAME - fc.scriptFilename = sanitizedPathJoin(fc.documentRoot, fc.scriptName) - - // see if a php_server worker or global worker matches the request path - // aka: root + request path == worker.filename - if fc.worker = fc.server.workersByPath[fc.scriptFilename]; fc.worker == nil { - fc.worker = globalWorkersByPath[fc.scriptFilename] - } -} - -func BenchmarkAssignedWorkerScriptPath(b *testing.B) { - sep := string(filepath.Separator) - root := filepath.Join(sep, "var", "www") - longRoot := filepath.Join(sep, "srv", "apps", "organization", "production", "releases", "2026", "public") - for _, tt := range []struct{ name, root, file string }{ - {"short_root", root, root + sep + "index.php"}, - {"long_root", longRoot, longRoot + sep + "index.php"}, - {"outside_root", longRoot, filepath.Join(sep, "srv", "elsewhere", "index.php")}, - {"empty_root", "", sep + "index.php"}, - {"trailing_separator", root + sep, root + sep + sep + "index.php"}, - {"exact_root", root, root}, - } { - b.Run(tt.name, func(b *testing.B) { - fc := &frankenPHPContext{documentRoot: tt.root, request: &http.Request{URL: &url.URL{Path: "/index.php/info"}}, worker: &worker{fileName: tt.file}} - b.ReportAllocs() - for b.Loop() { - splitCgiPath(fc) - } - benchmarkAssignedWorkerScriptName = fc.scriptName - }) - } -} - -var benchmarkAssignedWorkerScriptName string From 542814280bc15648387752c112bf99afca227995 Mon Sep 17 00:00:00 2001 From: "a.bolonin" Date: Sat, 24 Oct 2026 17:58:47 +0300 Subject: [PATCH 3/3] test: remove assigned worker path benchmark after review --- cgi_test.go | 25 ------------------------- 1 file changed, 25 deletions(-) diff --git a/cgi_test.go b/cgi_test.go index 4bc579aeaf..81107c55c2 100644 --- a/cgi_test.go +++ b/cgi_test.go @@ -561,28 +561,3 @@ func TestAssignedWorkerScriptPath(t *testing.T) { }) } } - -func BenchmarkAssignedWorkerScriptPath(b *testing.B) { - sep := string(filepath.Separator) - root := filepath.Join(sep, "var", "www") - longRoot := filepath.Join(sep, "srv", "apps", "organization", "production", "releases", "2026", "public") - for _, tt := range []struct{ name, root, file string }{ - {"short_root", root, root + sep + "index.php"}, - {"long_root", longRoot, longRoot + sep + "index.php"}, - {"outside_root", longRoot, filepath.Join(sep, "srv", "elsewhere", "index.php")}, - {"empty_root", "", sep + "index.php"}, - {"trailing_separator", root + sep, root + sep + sep + "index.php"}, - {"exact_root", root, root}, - } { - b.Run(tt.name, func(b *testing.B) { - fc := &frankenPHPContext{documentRoot: tt.root, request: &http.Request{URL: &url.URL{Path: "/index.php/info"}}, worker: &worker{fileName: tt.file}} - b.ReportAllocs() - for b.Loop() { - splitCgiPath(fc) - } - benchmarkAssignedWorkerScriptName = fc.scriptName - }) - } -} - -var benchmarkAssignedWorkerScriptName string