From 84cd9887460de1578a94ba04ac6eceb0e9e61129 Mon Sep 17 00:00:00 2001 From: Alliballibaba Date: Mon, 9 Jun 2025 21:01:45 +0200 Subject: [PATCH 01/59] Adds 'match' configuration --- caddy/config_test.go | 2 +- caddy/module.go | 5 ++--- caddy/workerconfig.go | 20 +++++++++++++++++++- 3 files changed, 22 insertions(+), 5 deletions(-) diff --git a/caddy/config_test.go b/caddy/config_test.go index 7bcbbfc27..29fee1d24 100644 --- a/caddy/config_test.go +++ b/caddy/config_test.go @@ -1,4 +1,4 @@ -package caddy +package caddy import ( "testing" diff --git a/caddy/module.go b/caddy/module.go index aef54df48..519643df7 100644 --- a/caddy/module.go +++ b/caddy/module.go @@ -162,12 +162,11 @@ func (f *FrankenPHPModule) ServeHTTP(w http.ResponseWriter, r *http.Request, _ c } } - fullScriptPath, _ := fastabs.FastAbs(documentRoot + "/" + r.URL.Path) - workerName := "" for _, w := range f.Workers { - if p, _ := fastabs.FastAbs(w.FileName); p == fullScriptPath { + if w.matchesPath(r.URL.Path, documentRoot) { workerName = w.Name + break } } diff --git a/caddy/workerconfig.go b/caddy/workerconfig.go index a6a797c77..47db675e7 100644 --- a/caddy/workerconfig.go +++ b/caddy/workerconfig.go @@ -7,6 +7,7 @@ import ( "github.com/caddyserver/caddy/v2/caddyconfig/caddyfile" "github.com/dunglas/frankenphp" + "github.com/dunglas/frankenphp/internal/fastabs" ) // workerConfig represents the "worker" directive in the Caddyfile @@ -29,6 +30,8 @@ type workerConfig struct { Env map[string]string `json:"env,omitempty"` // Directories to watch for file changes Watch []string `json:"watch,omitempty"` + // The path to match against the worker + Match string `json:"match,omitempty"` } func parseWorkerConfig(d *caddyfile.Dispenser) (workerConfig, error) { @@ -94,8 +97,13 @@ func parseWorkerConfig(d *caddyfile.Dispenser) (workerConfig, error) { } else { wc.Watch = append(wc.Watch, d.Val()) } + case "match": + if !d.NextArg() { + return wc, d.ArgErr() + } + wc.Match = d.Val() default: - allowedDirectives := "name, file, num, env, watch" + allowedDirectives := "name, file, num, env, watch, match" return wc, wrongSubDirectiveError("worker", allowedDirectives, v) } } @@ -110,3 +118,13 @@ func parseWorkerConfig(d *caddyfile.Dispenser) (workerConfig, error) { return wc, nil } + +func (wc workerConfig) matchesPath(path string, documentRoot string) bool { + if wc.Match != "" { + return wc.Match == "*" || wc.Match == path + } + + fullScriptPath, _ := fastabs.FastAbs(documentRoot + "/" + path) + absFileName, _ := fastabs.FastAbs(wc.FileName) + return fullScriptPath == absFileName +} From dbd66ecc0955f59d70a1db6e451e81bd0621a53c Mon Sep 17 00:00:00 2001 From: Alliballibaba Date: Mon, 9 Jun 2025 23:20:02 +0200 Subject: [PATCH 02/59] test --- caddy/caddy_test.go | 26 ++++++++ caddy/module.go | 157 +++++++++++++++++++++++++++++--------------- 2 files changed, 129 insertions(+), 54 deletions(-) diff --git a/caddy/caddy_test.go b/caddy/caddy_test.go index 9d3a9a013..a5bba66f0 100644 --- a/caddy/caddy_test.go +++ b/caddy/caddy_test.go @@ -1216,6 +1216,32 @@ func TestDisabledMetrics(t *testing.T) { require.Zero(t, count, "metrics should be missing") } +func TestWorkerMatchDirective(t *testing.T) { + tester := caddytest.NewTester(t) + tester.InitServer(` + { + skip_install_trust + admin localhost:2999 + } + + http://localhost:`+testPort+` { + php_server { + root ../testdata/files + try_files {path} index.php + worker { + match index.php + file ../worker-with-counter.php + num 1 + } + } + } + `, "caddyfile") + + tester.AssertGetResponse("http://localhost:"+testPort+"/any-path", http.StatusOK, "requests:1") + tester.AssertGetResponse("http://localhost:"+testPort+"/static.txt", http.StatusOK, "Hello from file") + tester.AssertGetResponse("http://localhost:"+testPort+"/worker-path", http.StatusOK, "requests:1") +} + func TestWorkerRestart(t *testing.T) { var wg sync.WaitGroup tester := caddytest.NewTester(t) diff --git a/caddy/module.go b/caddy/module.go index 519643df7..caf49d8a4 100644 --- a/caddy/module.go +++ b/caddy/module.go @@ -69,6 +69,34 @@ func (f *FrankenPHPModule) Provision(ctx caddy.Context) error { return fmt.Errorf(`expected ctx.App("frankenphp") to return *FrankenPHPApp, got nil`) } + for i, wc := range f.Workers { + // Inherit environment variables from the parent php_server directive + if !filepath.IsAbs(wc.FileName) && f.Root != "" { + wc.FileName = filepath.Join(f.Root, wc.FileName) + } + + if f.Env != nil { + if wc.Env == nil { + wc.Env = make(map[string]string) + } + for k, v := range f.Env { + // Only set if not already defined in the worker + if _, exists := wc.Env[k]; !exists { + wc.Env[k] = v + } + } + } + + f.Workers[i] = wc + + // Check if a worker with this filename already exists in this module + for j, existingWorker := range f.Workers { + if i != j && existingWorker.FileName == wc.FileName { + return fmt.Errorf(`workers in a single "php_server" block must not have duplicate filenames: %q`, wc.FileName) + } + } + } + workers, err := fapp.addModuleWorkers(f.Workers...) if err != nil { return err @@ -230,12 +258,11 @@ func (f *FrankenPHPModule) UnmarshalCaddyfile(d *caddyfile.Dispenser) error { f.ResolveRootSymlink = &v case "worker": - for d.NextBlock(1) { - } - for d.NextArg() { - } - // Skip "worker" blocks in the first pass - continue + wc, err := parseWorkerConfig(d) + if err != nil { + return err + } + f.Workers = append(f.Workers, wc) default: allowedDirectives := "root, split, env, resolve_root_symlink, worker" @@ -244,45 +271,6 @@ func (f *FrankenPHPModule) UnmarshalCaddyfile(d *caddyfile.Dispenser) error { } } - // Second pass: Parse only "worker" blocks - d.Reset() - for d.Next() { - for d.NextBlock(0) { - if d.Val() == "worker" { - wc, err := parseWorkerConfig(d) - if err != nil { - return err - } - - // Inherit environment variables from the parent php_server directive - if !filepath.IsAbs(wc.FileName) && f.Root != "" { - wc.FileName = filepath.Join(f.Root, wc.FileName) - } - - if f.Env != nil { - if wc.Env == nil { - wc.Env = make(map[string]string) - } - for k, v := range f.Env { - // Only set if not already defined in the worker - if _, exists := wc.Env[k]; !exists { - wc.Env[k] = v - } - } - } - - // Check if a worker with this filename already exists in this module - for _, existingWorker := range f.Workers { - if existingWorker.FileName == wc.FileName { - return fmt.Errorf(`workers in a single "php_server" block must not have duplicate filenames: %q`, wc.FileName) - } - } - - f.Workers = append(f.Workers, wc) - } - } - } - return nil } @@ -418,6 +406,16 @@ func parsePhpServer(h httpcaddyfile.Helper) ([]httpcaddyfile.ConfigValue, error) // unmarshaler can read it from the start dispenser.Reset() + // the rest of the config is specified by the user + // using the php directive syntax + dispenser.Next() // consume the directive name + err = phpsrv.UnmarshalCaddyfile(dispenser) + if err != nil { + return nil, err + } + + return routesForWorkerServer(h, phpsrv, fsrv) + if frankenphp.EmbeddedAppPath != "" { if phpsrv.Root == "" { phpsrv.Root = filepath.Join(frankenphp.EmbeddedAppPath, defaultDocumentRoot) @@ -521,15 +519,6 @@ func parsePhpServer(h httpcaddyfile.Helper) ([]httpcaddyfile.ConfigValue, error) "path": h.JSON(pathList), } - // the rest of the config is specified by the user - // using the php directive syntax - dispenser.Next() // consume the directive name - err = phpsrv.UnmarshalCaddyfile(dispenser) - - if err != nil { - return nil, err - } - // create the PHP route which is // conditional on matching PHP files phpRoute := caddyhttp.Route{ @@ -576,3 +565,63 @@ func parsePhpServer(h httpcaddyfile.Helper) ([]httpcaddyfile.ConfigValue, error) }, }, nil } + +func routesForWorkerServer(h httpcaddyfile.Helper, frankenphpModule FrankenPHPModule, fsrv caddy.Module) ([]httpcaddyfile.ConfigValue, error) { + return []httpcaddyfile.ConfigValue{ + { + Class: "route", + Value: workerSubroute(h, frankenphpModule, fsrv), + }, + }, nil +} + +// get the routing for php_worker +// serve non-php file or fallback to index worker +func workerSubroute(h httpcaddyfile.Helper, frankenphpModule FrankenPHPModule, fsrv caddy.Module) caddyhttp.Subroute { + matchers := caddyhttp.MatchPath{"*.php"} + for _, w := range frankenphpModule.Workers { + if w.Match != "" { + matchers = append(matchers, w.Match) + } + } + + subRoute := caddyhttp.Subroute{ + Routes: caddyhttp.RouteList{ + caddyhttp.Route{ + MatcherSetsRaw: []caddy.ModuleMap{ + caddy.ModuleMap{ + "file": h.JSON(fileserver.MatchFile{ + TryFiles: []string{"{http.request.uri.path}"}, + Root: frankenphpModule.Root, + }), + "not": h.JSON(caddyhttp.MatchNot{ + MatcherSetsRaw: []caddy.ModuleMap{ + { + "path": h.JSON(matchers), + }, + }, + }), + }, + }, + HandlersRaw: []json.RawMessage{ + caddyconfig.JSONModuleObject(fsrv, "handler", "file_server", nil), + }, + }, + }, + } + + for _, pattern := range matchers { + subRoute.Routes = append(subRoute.Routes, caddyhttp.Route{ + MatcherSetsRaw: []caddy.ModuleMap{ + caddy.ModuleMap{ + "path": h.JSON(caddyhttp.MatchPath{ pattern }), + }, + }, + HandlersRaw: []json.RawMessage{ + caddyconfig.JSONModuleObject(frankenphpModule, "handler", "php", nil), + }, + }) + } + + return subRoute +} From 1e8bef174e1264423a82d94b60d681547dfbf950 Mon Sep 17 00:00:00 2001 From: Alliballibaba Date: Wed, 11 Jun 2025 00:13:19 +0200 Subject: [PATCH 03/59] Adds Caddy's matcher. --- caddy/caddy_test.go | 46 ++++++++++++++++-- caddy/module.go | 107 ++++++++++++++++++++++-------------------- caddy/workerconfig.go | 23 +++++++-- 3 files changed, 119 insertions(+), 57 deletions(-) diff --git a/caddy/caddy_test.go b/caddy/caddy_test.go index a5bba66f0..b0c33d8e7 100644 --- a/caddy/caddy_test.go +++ b/caddy/caddy_test.go @@ -1218,6 +1218,7 @@ func TestDisabledMetrics(t *testing.T) { func TestWorkerMatchDirective(t *testing.T) { tester := caddytest.NewTester(t) + testport2 := "3000" tester.InitServer(` { skip_install_trust @@ -1227,19 +1228,56 @@ func TestWorkerMatchDirective(t *testing.T) { http://localhost:`+testPort+` { php_server { root ../testdata/files - try_files {path} index.php worker { - match index.php + match /matched-path* file ../worker-with-counter.php num 1 } } } + http://localhost:`+testport2+` { + php_server { + root ../testdata + worker { + match /counter* + file worker-with-counter.php + num 1 + } + worker { + match /index* + file index.php + num 1 + } + } + } `, "caddyfile") - tester.AssertGetResponse("http://localhost:"+testPort+"/any-path", http.StatusOK, "requests:1") + // the first php_server with root in testdata/files + // Forward request that match * to the worker + tester.AssertGetResponse("http://localhost:"+testPort+"/matched-path", http.StatusOK, "requests:1") + tester.AssertGetResponse("http://localhost:"+testPort+"/matched-path/anywhere", http.StatusOK, "requests:2") + + // 404 on not matching paths + r, _ := http.NewRequest("GET", "http://localhost:"+testPort+"/not-matched-path", nil) + tester.AssertResponseCode(r, http.StatusNotFound) + + // forward files to fileserver + tester.AssertGetResponse("http://localhost:"+testPort+"/static.txt", http.StatusOK, "Hello from file") + + // the second php_server has root in testdata and has 2 different workers + tester.AssertGetResponse("http://localhost:"+testport2+"/counter/sub-path", http.StatusOK, "requests:1") + tester.AssertGetResponse("http://localhost:"+testport2+"/index/sub-path", http.StatusOK, "I am by birth a Genevese (i not set)") + + // 404 on not matching path + r, _ = http.NewRequest("GET", "http://localhost:"+testPort+"/other", nil) + tester.AssertResponseCode(r, http.StatusNotFound) + + // forward files to fileserver tester.AssertGetResponse("http://localhost:"+testPort+"/static.txt", http.StatusOK, "Hello from file") - tester.AssertGetResponse("http://localhost:"+testPort+"/worker-path", http.StatusOK, "requests:1") + + // never serve PHP files directly + r, _ = http.NewRequest("GET", "http://localhost:"+testPort+"/index.php", nil) + tester.AssertResponseCode(r, http.StatusNotFound) } func TestWorkerRestart(t *testing.T) { diff --git a/caddy/module.go b/caddy/module.go index caf49d8a4..d0743bc51 100644 --- a/caddy/module.go +++ b/caddy/module.go @@ -71,30 +71,30 @@ func (f *FrankenPHPModule) Provision(ctx caddy.Context) error { for i, wc := range f.Workers { // Inherit environment variables from the parent php_server directive - if !filepath.IsAbs(wc.FileName) && f.Root != "" { - wc.FileName = filepath.Join(f.Root, wc.FileName) - } - - if f.Env != nil { - if wc.Env == nil { - wc.Env = make(map[string]string) - } - for k, v := range f.Env { - // Only set if not already defined in the worker - if _, exists := wc.Env[k]; !exists { - wc.Env[k] = v - } - } - } + if !filepath.IsAbs(wc.FileName) && f.Root != "" { + wc.FileName = filepath.Join(f.Root, wc.FileName) + } + + if f.Env != nil { + if wc.Env == nil { + wc.Env = make(map[string]string) + } + for k, v := range f.Env { + // Only set if not already defined in the worker + if _, exists := wc.Env[k]; !exists { + wc.Env[k] = v + } + } + } f.Workers[i] = wc - // Check if a worker with this filename already exists in this module - for j, existingWorker := range f.Workers { - if i != j && existingWorker.FileName == wc.FileName { - return fmt.Errorf(`workers in a single "php_server" block must not have duplicate filenames: %q`, wc.FileName) - } - } + // Check if a worker with this filename already exists in this module + for j, existingWorker := range f.Workers { + if i != j && existingWorker.FileName == wc.FileName { + return fmt.Errorf(`workers in a single "php_server" block must not have duplicate filenames: %q`, wc.FileName) + } + } } workers, err := fapp.addModuleWorkers(f.Workers...) @@ -192,7 +192,7 @@ func (f *FrankenPHPModule) ServeHTTP(w http.ResponseWriter, r *http.Request, _ c workerName := "" for _, w := range f.Workers { - if w.matchesPath(r.URL.Path, documentRoot) { + if w.matchesPath(r, documentRoot) { workerName = w.Name break } @@ -259,9 +259,9 @@ func (f *FrankenPHPModule) UnmarshalCaddyfile(d *caddyfile.Dispenser) error { case "worker": wc, err := parseWorkerConfig(d) - if err != nil { - return err - } + if err != nil { + return err + } f.Workers = append(f.Workers, wc) default: @@ -407,12 +407,12 @@ func parsePhpServer(h httpcaddyfile.Helper) ([]httpcaddyfile.ConfigValue, error) dispenser.Reset() // the rest of the config is specified by the user - // using the php directive syntax - dispenser.Next() // consume the directive name - err = phpsrv.UnmarshalCaddyfile(dispenser) - if err != nil { - return nil, err - } + // using the php directive syntax + dispenser.Next() // consume the directive name + err = phpsrv.UnmarshalCaddyfile(dispenser) + if err != nil { + return nil, err + } return routesForWorkerServer(h, phpsrv, fsrv) @@ -568,17 +568,17 @@ func parsePhpServer(h httpcaddyfile.Helper) ([]httpcaddyfile.ConfigValue, error) func routesForWorkerServer(h httpcaddyfile.Helper, frankenphpModule FrankenPHPModule, fsrv caddy.Module) ([]httpcaddyfile.ConfigValue, error) { return []httpcaddyfile.ConfigValue{ - { - Class: "route", - Value: workerSubroute(h, frankenphpModule, fsrv), - }, - }, nil + { + Class: "route", + Value: workerSubroute(h, frankenphpModule, fsrv), + }, + }, nil } // get the routing for php_worker // serve non-php file or fallback to index worker func workerSubroute(h httpcaddyfile.Helper, frankenphpModule FrankenPHPModule, fsrv caddy.Module) caddyhttp.Subroute { - matchers := caddyhttp.MatchPath{"*.php"} + matchers := caddyhttp.MatchPath{} for _, w := range frankenphpModule.Workers { if w.Match != "" { matchers = append(matchers, w.Match) @@ -597,7 +597,7 @@ func workerSubroute(h httpcaddyfile.Helper, frankenphpModule FrankenPHPModule, f "not": h.JSON(caddyhttp.MatchNot{ MatcherSetsRaw: []caddy.ModuleMap{ { - "path": h.JSON(matchers), + "path": h.JSON(caddyhttp.MatchPath{"*.php"}), }, }, }), @@ -610,18 +610,25 @@ func workerSubroute(h httpcaddyfile.Helper, frankenphpModule FrankenPHPModule, f }, } - for _, pattern := range matchers { - subRoute.Routes = append(subRoute.Routes, caddyhttp.Route{ - MatcherSetsRaw: []caddy.ModuleMap{ - caddy.ModuleMap{ - "path": h.JSON(caddyhttp.MatchPath{ pattern }), - }, - }, - HandlersRaw: []json.RawMessage{ - caddyconfig.JSONModuleObject(frankenphpModule, "handler", "php", nil), - }, - }) - } + for _, pattern := range matchers { + subRoute.Routes = append(subRoute.Routes, caddyhttp.Route{ + MatcherSetsRaw: []caddy.ModuleMap{ + caddy.ModuleMap{ + "path": h.JSON(caddyhttp.MatchPath{pattern}), + }, + }, + HandlersRaw: []json.RawMessage{ + caddyconfig.JSONModuleObject(frankenphpModule, "handler", "php", nil), + }, + }) + } + + subRoute.Routes = append(subRoute.Routes, caddyhttp.Route{ + MatcherSetsRaw: []caddy.ModuleMap{}, + HandlersRaw: []json.RawMessage{ + caddyconfig.JSONModuleObject(fsrv, "handler", "file_server", nil), + }, + }) return subRoute } diff --git a/caddy/workerconfig.go b/caddy/workerconfig.go index 47db675e7..6140bb210 100644 --- a/caddy/workerconfig.go +++ b/caddy/workerconfig.go @@ -2,10 +2,12 @@ package caddy import ( "errors" + "net/http" "path/filepath" "strconv" "github.com/caddyserver/caddy/v2/caddyconfig/caddyfile" + "github.com/caddyserver/caddy/v2/modules/caddyhttp" "github.com/dunglas/frankenphp" "github.com/dunglas/frankenphp/internal/fastabs" ) @@ -32,8 +34,21 @@ type workerConfig struct { Watch []string `json:"watch,omitempty"` // The path to match against the worker Match string `json:"match,omitempty"` + + matchStrategy matchStrategy } +type matchStrategy int + +const ( + matchNone matchStrategy = iota + matchAll + matchPrefix + matchSuffix + matchExactly + matchWildcard +) + func parseWorkerConfig(d *caddyfile.Dispenser) (workerConfig, error) { wc := workerConfig{} if d.NextArg() { @@ -102,6 +117,7 @@ func parseWorkerConfig(d *caddyfile.Dispenser) (workerConfig, error) { return wc, d.ArgErr() } wc.Match = d.Val() + default: allowedDirectives := "name, file, num, env, watch, match" return wc, wrongSubDirectiveError("worker", allowedDirectives, v) @@ -119,12 +135,13 @@ func parseWorkerConfig(d *caddyfile.Dispenser) (workerConfig, error) { return wc, nil } -func (wc workerConfig) matchesPath(path string, documentRoot string) bool { +func (wc workerConfig) matchesPath(r *http.Request, documentRoot string) bool { if wc.Match != "" { - return wc.Match == "*" || wc.Match == path + matches, _ := caddyhttp.MatchPath{wc.Match}.MatchWithError(r) + return matches } - fullScriptPath, _ := fastabs.FastAbs(documentRoot + "/" + path) + fullScriptPath, _ := fastabs.FastAbs(documentRoot + "/" + r.URL.Path) absFileName, _ := fastabs.FastAbs(wc.FileName) return fullScriptPath == absFileName } From fdbee0f3c9ceb6b53a043e12df11d2715d1bff61 Mon Sep 17 00:00:00 2001 From: Alliballibaba Date: Wed, 11 Jun 2025 19:20:32 +0200 Subject: [PATCH 04/59] Adds no-fileserver test. --- caddy/caddy_test.go | 157 ++++++++++++++++++++++--------------- caddy/module.go | 104 ++++++++++++------------ testdata/files/static.txt2 | 1 + 3 files changed, 148 insertions(+), 114 deletions(-) create mode 100644 testdata/files/static.txt2 diff --git a/caddy/caddy_test.go b/caddy/caddy_test.go index b0c33d8e7..cbe2ddcf8 100644 --- a/caddy/caddy_test.go +++ b/caddy/caddy_test.go @@ -1216,70 +1216,6 @@ func TestDisabledMetrics(t *testing.T) { require.Zero(t, count, "metrics should be missing") } -func TestWorkerMatchDirective(t *testing.T) { - tester := caddytest.NewTester(t) - testport2 := "3000" - tester.InitServer(` - { - skip_install_trust - admin localhost:2999 - } - - http://localhost:`+testPort+` { - php_server { - root ../testdata/files - worker { - match /matched-path* - file ../worker-with-counter.php - num 1 - } - } - } - http://localhost:`+testport2+` { - php_server { - root ../testdata - worker { - match /counter* - file worker-with-counter.php - num 1 - } - worker { - match /index* - file index.php - num 1 - } - } - } - `, "caddyfile") - - // the first php_server with root in testdata/files - // Forward request that match * to the worker - tester.AssertGetResponse("http://localhost:"+testPort+"/matched-path", http.StatusOK, "requests:1") - tester.AssertGetResponse("http://localhost:"+testPort+"/matched-path/anywhere", http.StatusOK, "requests:2") - - // 404 on not matching paths - r, _ := http.NewRequest("GET", "http://localhost:"+testPort+"/not-matched-path", nil) - tester.AssertResponseCode(r, http.StatusNotFound) - - // forward files to fileserver - tester.AssertGetResponse("http://localhost:"+testPort+"/static.txt", http.StatusOK, "Hello from file") - - // the second php_server has root in testdata and has 2 different workers - tester.AssertGetResponse("http://localhost:"+testport2+"/counter/sub-path", http.StatusOK, "requests:1") - tester.AssertGetResponse("http://localhost:"+testport2+"/index/sub-path", http.StatusOK, "I am by birth a Genevese (i not set)") - - // 404 on not matching path - r, _ = http.NewRequest("GET", "http://localhost:"+testPort+"/other", nil) - tester.AssertResponseCode(r, http.StatusNotFound) - - // forward files to fileserver - tester.AssertGetResponse("http://localhost:"+testPort+"/static.txt", http.StatusOK, "Hello from file") - - // never serve PHP files directly - r, _ = http.NewRequest("GET", "http://localhost:"+testPort+"/index.php", nil) - tester.AssertResponseCode(r, http.StatusNotFound) -} - func TestWorkerRestart(t *testing.T) { var wg sync.WaitGroup tester := caddytest.NewTester(t) @@ -1380,3 +1316,96 @@ func TestWorkerRestart(t *testing.T) { "frankenphp_worker_restarts", )) } + +func TestWorkerMatchDirectiveWithFileServer(t *testing.T) { + tester := caddytest.NewTester(t) + testport2 := "3000" + tester.InitServer(` + { + skip_install_trust + admin localhost:2999 + } + + http://localhost:`+testPort+` { + php_server { + root ../testdata/files + worker { + match /matched-path* + file ../worker-with-counter.php + num 1 + } + } + } + http://localhost:`+testport2+` { + php_server { + root ../testdata + worker { + match /counter* + file worker-with-counter.php + num 1 + } + worker { + match /index* + file index.php + num 1 + } + } + } + `, "caddyfile") + + // the first php_server with root in testdata/files + // Forward request that match * to the worker + tester.AssertGetResponse("http://localhost:"+testPort+"/matched-path", http.StatusOK, "requests:1") + tester.AssertGetResponse("http://localhost:"+testPort+"/matched-path/anywhere", http.StatusOK, "requests:2") + + // 404 on not matching paths + r, _ := http.NewRequest("GET", "http://localhost:"+testPort+"/not-matched-path", nil) + tester.AssertResponseCode(r, http.StatusNotFound) + + // forward files to fileserver + tester.AssertGetResponse("http://localhost:"+testPort+"/static.txt2", http.StatusOK, "Hello from file") + + // the second php_server has root in testdata and has 2 different workers + tester.AssertGetResponse("http://localhost:"+testport2+"/counter/sub-path", http.StatusOK, "requests:1") + tester.AssertGetResponse("http://localhost:"+testport2+"/index/sub-path", http.StatusOK, "I am by birth a Genevese (i not set)") + + // 404 on not matching path + r, _ = http.NewRequest("GET", "http://localhost:"+testPort+"/other", nil) + tester.AssertResponseCode(r, http.StatusNotFound) + + // forward files to fileserver + tester.AssertGetResponse("http://localhost:"+testPort+"/static.txt2", http.StatusOK, "Hello from file") + + // never serve PHP files directly + r, _ = http.NewRequest("GET", "http://localhost:"+testPort+"/index.php", nil) + tester.AssertResponseCode(r, http.StatusNotFound) +} + +func TestWorkerMatchDirectiveWithoutFileServer(t *testing.T) { + tester := caddytest.NewTester(t) + tester.InitServer(` + { + skip_install_trust + admin localhost:2999 + } + + http://localhost:`+testPort+` { + php_server { + file_server off + root ../testdata/files + worker { + match /some-path + file ../worker-with-counter.php + num 1 + } + } + } + `, "caddyfile") + + // find the worker at some-path + tester.AssertGetResponse("http://localhost:"+testPort+"/some-path", http.StatusOK, "requests:1") + + // do not find the file at static.txt2 + r, _ := http.NewRequest("GET", "http://localhost:"+testPort+"/static.txt2", nil) + tester.AssertResponseCode(r, http.StatusNotFound) +} diff --git a/caddy/module.go b/caddy/module.go index d0743bc51..817c02071 100644 --- a/caddy/module.go +++ b/caddy/module.go @@ -86,15 +86,7 @@ func (f *FrankenPHPModule) Provision(ctx caddy.Context) error { } } } - f.Workers[i] = wc - - // Check if a worker with this filename already exists in this module - for j, existingWorker := range f.Workers { - if i != j && existingWorker.FileName == wc.FileName { - return fmt.Errorf(`workers in a single "php_server" block must not have duplicate filenames: %q`, wc.FileName) - } - } } workers, err := fapp.addModuleWorkers(f.Workers...) @@ -271,6 +263,15 @@ func (f *FrankenPHPModule) UnmarshalCaddyfile(d *caddyfile.Dispenser) error { } } + // Check if a worker with this filename already exists in this module + for j, w1 := range f.Workers { + for i, w2 := range f.Workers { + if i != j && w1.FileName == w2.FileName { + return fmt.Errorf(`workers in a single "php_server" block must not have duplicate filenames: %q`, w1.FileName) + } + } + } + return nil } @@ -414,8 +415,6 @@ func parsePhpServer(h httpcaddyfile.Helper) ([]httpcaddyfile.ConfigValue, error) return nil, err } - return routesForWorkerServer(h, phpsrv, fsrv) - if frankenphp.EmbeddedAppPath != "" { if phpsrv.Root == "" { phpsrv.Root = filepath.Join(frankenphp.EmbeddedAppPath, defaultDocumentRoot) @@ -428,6 +427,13 @@ func parsePhpServer(h httpcaddyfile.Helper) ([]httpcaddyfile.ConfigValue, error) } } + // if any worker has 'match' specified, use an alternate routing approach + for _, w := range phpsrv.Workers { + if w.Match != "" { + return pureWorkerRoute(h, phpsrv, fsrv, disableFsrv), nil + } + } + // set up a route list that we'll append to routes := caddyhttp.RouteList{} @@ -566,69 +572,67 @@ func parsePhpServer(h httpcaddyfile.Helper) ([]httpcaddyfile.ConfigValue, error) }, nil } -func routesForWorkerServer(h httpcaddyfile.Helper, frankenphpModule FrankenPHPModule, fsrv caddy.Module) ([]httpcaddyfile.ConfigValue, error) { - return []httpcaddyfile.ConfigValue{ - { - Class: "route", - Value: workerSubroute(h, frankenphpModule, fsrv), - }, - }, nil -} - -// get the routing for php_worker -// serve non-php file or fallback to index worker -func workerSubroute(h httpcaddyfile.Helper, frankenphpModule FrankenPHPModule, fsrv caddy.Module) caddyhttp.Subroute { +// if any worker under php_server has a match pattern, +// routes have to be created differently +// first, we will try to match against the file system, +// then we will try to match aganist each worker +func pureWorkerRoute(h httpcaddyfile.Helper, f FrankenPHPModule, fsrv caddy.Module, disableFsrv bool) []httpcaddyfile.ConfigValue { + f.SplitPath = []string{} matchers := caddyhttp.MatchPath{} - for _, w := range frankenphpModule.Workers { + for _, w := range f.Workers { if w.Match != "" { matchers = append(matchers, w.Match) } } - subRoute := caddyhttp.Subroute{ - Routes: caddyhttp.RouteList{ - caddyhttp.Route{ - MatcherSetsRaw: []caddy.ModuleMap{ - caddy.ModuleMap{ - "file": h.JSON(fileserver.MatchFile{ - TryFiles: []string{"{http.request.uri.path}"}, - Root: frankenphpModule.Root, - }), - "not": h.JSON(caddyhttp.MatchNot{ - MatcherSetsRaw: []caddy.ModuleMap{ - { - "path": h.JSON(caddyhttp.MatchPath{"*.php"}), - }, - }, - }), - }, - }, - HandlersRaw: []json.RawMessage{ - caddyconfig.JSONModuleObject(fsrv, "handler", "file_server", nil), + subRoute := caddyhttp.Subroute{Routes: caddyhttp.RouteList{}} + + if !disableFsrv { + subRoute.Routes = append(subRoute.Routes, caddyhttp.Route{ + MatcherSetsRaw: []caddy.ModuleMap{ + caddy.ModuleMap{ + "file": h.JSON(fileserver.MatchFile{ + TryFiles: []string{"{http.request.uri.path}"}, + Root: f.Root, + }), + "not": h.JSON(caddyhttp.MatchNot{ + MatcherSetsRaw: []caddy.ModuleMap{ + {"path": h.JSON(caddyhttp.MatchPath{"*.php"})}, + }, + }), }, }, - }, + HandlersRaw: []json.RawMessage{ + caddyconfig.JSONModuleObject(fsrv, "handler", "file_server", nil), + }, + }) } for _, pattern := range matchers { subRoute.Routes = append(subRoute.Routes, caddyhttp.Route{ MatcherSetsRaw: []caddy.ModuleMap{ - caddy.ModuleMap{ - "path": h.JSON(caddyhttp.MatchPath{pattern}), - }, + caddy.ModuleMap{"path": h.JSON(caddyhttp.MatchPath{pattern})}, }, HandlersRaw: []json.RawMessage{ - caddyconfig.JSONModuleObject(frankenphpModule, "handler", "php", nil), + caddyconfig.JSONModuleObject(f, "handler", "php", nil), }, }) } + notFoundHandler := caddyhttp.StaticResponse{ + StatusCode: caddyhttp.WeakString(strconv.Itoa(http.StatusNotFound)), + } subRoute.Routes = append(subRoute.Routes, caddyhttp.Route{ MatcherSetsRaw: []caddy.ModuleMap{}, HandlersRaw: []json.RawMessage{ - caddyconfig.JSONModuleObject(fsrv, "handler", "file_server", nil), + caddyconfig.JSONModuleObject(notFoundHandler, "handler", "static_response", nil), }, }) - return subRoute + return []httpcaddyfile.ConfigValue{ + { + Class: "route", + Value: subRoute, + }, + } } diff --git a/testdata/files/static.txt2 b/testdata/files/static.txt2 new file mode 100644 index 000000000..09be5ef4e --- /dev/null +++ b/testdata/files/static.txt2 @@ -0,0 +1 @@ +Hello from file \ No newline at end of file From b383af9a758fb1951e6355000f52c08c5048a131 Mon Sep 17 00:00:00 2001 From: Alliballibaba Date: Wed, 11 Jun 2025 19:41:01 +0200 Subject: [PATCH 05/59] Prevents duplicate path calculations and optimizes worker access. --- context.go | 14 ++++++-- frankenphp.go | 4 +-- phpmainthread_test.go | 10 +++--- request_options.go | 4 ++- scaling.go | 4 +-- scaling_test.go | 2 +- worker.go | 84 +++++++++++++++++++++++++------------------ 7 files changed, 73 insertions(+), 49 deletions(-) diff --git a/context.go b/context.go index 29ed40af3..6de140da7 100644 --- a/context.go +++ b/context.go @@ -17,12 +17,12 @@ type frankenPHPContext struct { logger *slog.Logger request *http.Request originalRequest *http.Request + worker *worker docURI string pathInfo string scriptName string scriptFilename string - workerName string // Whether the request is already closed by us isDone bool @@ -89,8 +89,16 @@ func NewRequestWithContext(r *http.Request, opts ...RequestOption) (*http.Reques } } - // SCRIPT_FILENAME is the absolute path of SCRIPT_NAME - fc.scriptFilename = sanitizedPathJoin(fc.documentRoot, fc.scriptName) + // if a worker is assigned explicitly, use its filename + // determine if the filename belongs to a worker otherwise + if fc.worker != nil { + fc.scriptFilename = fc.worker.fileName + } else { + // SCRIPT_FILENAME is the absolute path of SCRIPT_NAME + fc.scriptFilename = sanitizedPathJoin(fc.documentRoot, fc.scriptName) + fc.worker = getWorkerByPath(fc.scriptFilename) + } + c := context.WithValue(r.Context(), contextKey, fc) return r.WithContext(c), nil diff --git a/frankenphp.go b/frankenphp.go index afb4b77ac..fd236fd76 100644 --- a/frankenphp.go +++ b/frankenphp.go @@ -401,8 +401,8 @@ func ServeHTTP(responseWriter http.ResponseWriter, request *http.Request) error } // Detect if a worker is available to handle this request - if worker, ok := workers[getWorkerKey(fc.workerName, fc.scriptFilename)]; ok { - worker.handleRequest(fc) + if fc.worker != nil { + fc.worker.handleRequest(fc) return nil } diff --git a/phpmainthread_test.go b/phpmainthread_test.go index 08020ea62..1a4d5ba25 100644 --- a/phpmainthread_test.go +++ b/phpmainthread_test.go @@ -181,7 +181,7 @@ func TestFinishBootingAWorkerScript(t *testing.T) { } func TestReturnAnErrorIf2WorkersHaveTheSameFileName(t *testing.T) { - workers = make(map[string]*worker) + workers = []*worker{} _, err1 := newWorker(workerOpt{fileName: "filename.php"}) _, err2 := newWorker(workerOpt{fileName: "filename.php"}) @@ -190,7 +190,7 @@ func TestReturnAnErrorIf2WorkersHaveTheSameFileName(t *testing.T) { } func TestReturnAnErrorIf2ModuleWorkersHaveTheSameName(t *testing.T) { - workers = make(map[string]*worker) + workers = []*worker{} _, err1 := newWorker(workerOpt{fileName: "filename.php", name: "workername"}) _, err2 := newWorker(workerOpt{fileName: "filename2.php", name: "workername"}) @@ -200,7 +200,7 @@ func TestReturnAnErrorIf2ModuleWorkersHaveTheSameName(t *testing.T) { func getDummyWorker(fileName string) *worker { if workers == nil { - workers = make(map[string]*worker) + workers = []*worker{} } worker, _ := newWorker(workerOpt{ fileName: testDataPath + "/" + fileName, @@ -232,9 +232,9 @@ func allPossibleTransitions(worker1Path string, worker2Path string) []func(*phpT thread.boot() } }, - func(thread *phpThread) { convertToWorkerThread(thread, workers[worker1Path]) }, + func(thread *phpThread) { convertToWorkerThread(thread, getWorkerByPath(worker1Path)) }, convertToInactiveThread, - func(thread *phpThread) { convertToWorkerThread(thread, workers[worker2Path]) }, + func(thread *phpThread) { convertToWorkerThread(thread, getWorkerByPath(worker2Path)) }, convertToInactiveThread, } } diff --git a/request_options.go b/request_options.go index 6c1cb5949..2227b06ee 100644 --- a/request_options.go +++ b/request_options.go @@ -127,7 +127,9 @@ func WithRequestLogger(logger *slog.Logger) RequestOption { // WithWorkerName sets the worker that should handle the request func WithWorkerName(name string) RequestOption { return func(o *frankenPHPContext) error { - o.workerName = name + if name != "" { + o.worker = getWorkerByName(name) + } return nil } diff --git a/scaling.go b/scaling.go index ca319a177..d462b4a54 100644 --- a/scaling.go +++ b/scaling.go @@ -154,8 +154,8 @@ func startUpscalingThreads(maxScaledThreads int, scale chan *frankenPHPContext, } // if the request has been stalled long enough, scale - if worker, ok := workers[getWorkerKey(fc.workerName, fc.scriptFilename)]; ok { - scaleWorkerThread(worker) + if fc.worker != nil { + scaleWorkerThread(fc.worker) } else { scaleRegularThread() } diff --git a/scaling_test.go b/scaling_test.go index 85adc5863..eeca51ca2 100644 --- a/scaling_test.go +++ b/scaling_test.go @@ -44,7 +44,7 @@ func TestScaleAWorkerThreadUpAndDown(t *testing.T) { autoScaledThread := phpThreads[2] // scale up - scaleWorkerThread(workers[workerPath]) + scaleWorkerThread(getWorkerByPath(workerPath)) assert.Equal(t, stateReady, autoScaledThread.state.get()) // on down-scale, the thread will be marked as inactive diff --git a/worker.go b/worker.go index 3c9455b77..e36458d9a 100644 --- a/worker.go +++ b/worker.go @@ -14,33 +14,36 @@ import ( // represents a worker script and can have many threads assigned to it type worker struct { - name string - fileName string - num int - env PreparedEnv - requestChan chan *frankenPHPContext - threads []*phpThread - threadMutex sync.RWMutex + name string + fileName string + num int + env PreparedEnv + requestChan chan *frankenPHPContext + threads []*phpThread + threadMutex sync.RWMutex + allowPathMatching bool // allow matching a request to a worker via path } var ( - workers map[string]*worker + workers []*worker watcherIsEnabled bool ) func initWorkers(opt []workerOpt) error { - workers = make(map[string]*worker, len(opt)) + workers = make([]*worker, 0, len(opt)) workersReady := sync.WaitGroup{} directoriesToWatch := getDirectoriesToWatch(opt) watcherIsEnabled = len(directoriesToWatch) > 0 for _, o := range opt { - worker, err := newWorker(o) + _, err := newWorker(o) if err != nil { return err } + } - workersReady.Add(o.num) + for _, worker := range workers { + workersReady.Add(worker.num) for i := 0; i < worker.num; i++ { thread := getInactivePHPThread() convertToWorkerThread(thread, worker) @@ -65,48 +68,59 @@ func initWorkers(opt []workerOpt) error { return nil } -func getWorkerKey(name string, filename string) string { - key := filename - if strings.HasPrefix(name, "m#") { - key = name +func getWorkerByName(name string) *worker { + for _, w := range workers { + if w.name == name { + return w + } } - return key + + return nil +} + +func getWorkerByPath(path string) *worker { + for _, w := range workers { + if w.fileName == path && w.allowPathMatching { + return w + } + } + + return nil } func newWorker(o workerOpt) (*worker, error) { absFileName, err := fastabs.FastAbs(o.fileName) + isModuleWorker := strings.HasPrefix(o.name, "m#") // module workers may not be matched by path if err != nil { return nil, fmt.Errorf("worker filename is invalid %q: %w", o.fileName, err) } - key := getWorkerKey(o.name, absFileName) - if _, ok := workers[key]; ok { - return nil, fmt.Errorf("two workers cannot use the same key %q", key) + if o.name == "" { + o.name = absFileName } - for _, w := range workers { - if w.name == o.name { - return w, fmt.Errorf("two workers cannot have the same name: %q", o.name) - } + + if w := getWorkerByPath(absFileName); w != nil && !isModuleWorker { + return w, fmt.Errorf("two workers cannot have the same filename: %q", absFileName) + } + if w := getWorkerByName(o.name); w != nil { + return w, fmt.Errorf("two workers cannot have the same name: %q", o.name) } if o.env == nil { o.env = make(PreparedEnv, 1) } - if o.name == "" { - o.name = absFileName - } - o.env["FRANKENPHP_WORKER\x00"] = "1" w := &worker{ - name: o.name, - fileName: absFileName, - num: o.num, - env: o.env, - requestChan: make(chan *frankenPHPContext), - threads: make([]*phpThread, 0, o.num), - } - workers[key] = w + name: o.name, + fileName: absFileName, + num: o.num, + env: o.env, + requestChan: make(chan *frankenPHPContext), + threads: make([]*phpThread, 0, o.num), + allowPathMatching: !isModuleWorker, + } + workers = append(workers, w) return w, nil } From 9b7ef4da846cf18db488359984f64d5d010748bc Mon Sep 17 00:00:00 2001 From: Alliballibaba Date: Wed, 11 Jun 2025 20:16:03 +0200 Subject: [PATCH 06/59] trigger From 09d8da86a4c46c90c6ad157a0ccc88746b78fd05 Mon Sep 17 00:00:00 2001 From: Alliballibaba Date: Wed, 11 Jun 2025 20:53:54 +0200 Subject: [PATCH 07/59] Changes worker->match to match->worker --- caddy/caddy_test.go | 12 ++++-------- caddy/module.go | 19 ++++++++++++++++++- caddy/workerconfig.go | 7 +------ 3 files changed, 23 insertions(+), 15 deletions(-) diff --git a/caddy/caddy_test.go b/caddy/caddy_test.go index cbe2ddcf8..ceab1d283 100644 --- a/caddy/caddy_test.go +++ b/caddy/caddy_test.go @@ -1329,8 +1329,7 @@ func TestWorkerMatchDirectiveWithFileServer(t *testing.T) { http://localhost:`+testPort+` { php_server { root ../testdata/files - worker { - match /matched-path* + match /matched-path* worker { file ../worker-with-counter.php num 1 } @@ -1339,13 +1338,11 @@ func TestWorkerMatchDirectiveWithFileServer(t *testing.T) { http://localhost:`+testport2+` { php_server { root ../testdata - worker { - match /counter* + match /counter* worker { file worker-with-counter.php num 1 } - worker { - match /index* + match /index* worker { file index.php num 1 } @@ -1393,8 +1390,7 @@ func TestWorkerMatchDirectiveWithoutFileServer(t *testing.T) { php_server { file_server off root ../testdata/files - worker { - match /some-path + match /some-path worker { file ../worker-with-counter.php num 1 } diff --git a/caddy/module.go b/caddy/module.go index 817c02071..2ff43d9e3 100644 --- a/caddy/module.go +++ b/caddy/module.go @@ -255,9 +255,26 @@ func (f *FrankenPHPModule) UnmarshalCaddyfile(d *caddyfile.Dispenser) error { return err } f.Workers = append(f.Workers, wc) + case "match": + if !d.NextArg() { + return d.ArgErr() + } + match := "*" + if d.Val() != "worker" { + match = d.Val() + if !d.NextArg() || d.Val() != "worker" { + return d.ArgErr() + } + } + wc, err := parseWorkerConfig(d) + if err != nil { + return err + } + wc.Match = match + f.Workers = append(f.Workers, wc) default: - allowedDirectives := "root, split, env, resolve_root_symlink, worker" + allowedDirectives := "root, split, env, resolve_root_symlink, worker, match" return wrongSubDirectiveError("php or php_server", allowedDirectives, d.Val()) } } diff --git a/caddy/workerconfig.go b/caddy/workerconfig.go index 6140bb210..f52fee4ed 100644 --- a/caddy/workerconfig.go +++ b/caddy/workerconfig.go @@ -112,14 +112,9 @@ func parseWorkerConfig(d *caddyfile.Dispenser) (workerConfig, error) { } else { wc.Watch = append(wc.Watch, d.Val()) } - case "match": - if !d.NextArg() { - return wc, d.ArgErr() - } - wc.Match = d.Val() default: - allowedDirectives := "name, file, num, env, watch, match" + allowedDirectives := "name, file, num, env, watch" return wc, wrongSubDirectiveError("worker", allowedDirectives, v) } } From 6acc01e08d55047ebe2c3f91311b511cd6383f6b Mon Sep 17 00:00:00 2001 From: Alliballibaba Date: Wed, 11 Jun 2025 21:00:53 +0200 Subject: [PATCH 08/59] Adjusts tests. --- caddy/caddy_test.go | 23 +++++++++++------------ 1 file changed, 11 insertions(+), 12 deletions(-) diff --git a/caddy/caddy_test.go b/caddy/caddy_test.go index ceab1d283..629b59d39 100644 --- a/caddy/caddy_test.go +++ b/caddy/caddy_test.go @@ -1338,11 +1338,11 @@ func TestWorkerMatchDirectiveWithFileServer(t *testing.T) { http://localhost:`+testport2+` { php_server { root ../testdata - match /counter* worker { + match /counter/* worker { file worker-with-counter.php num 1 } - match /index* worker { + match /index/* worker { file index.php num 1 } @@ -1350,31 +1350,30 @@ func TestWorkerMatchDirectiveWithFileServer(t *testing.T) { } `, "caddyfile") - // the first php_server with root in testdata/files - // Forward request that match * to the worker + // worker is outside of public directory, match anyways tester.AssertGetResponse("http://localhost:"+testPort+"/matched-path", http.StatusOK, "requests:1") tester.AssertGetResponse("http://localhost:"+testPort+"/matched-path/anywhere", http.StatusOK, "requests:2") - // 404 on not matching paths + // 404 on no matching paths r, _ := http.NewRequest("GET", "http://localhost:"+testPort+"/not-matched-path", nil) tester.AssertResponseCode(r, http.StatusNotFound) - // forward files to fileserver + // static file will be served by fileserver tester.AssertGetResponse("http://localhost:"+testPort+"/static.txt2", http.StatusOK, "Hello from file") - // the second php_server has root in testdata and has 2 different workers + // the second php_server 2 different workers in the public path tester.AssertGetResponse("http://localhost:"+testport2+"/counter/sub-path", http.StatusOK, "requests:1") tester.AssertGetResponse("http://localhost:"+testport2+"/index/sub-path", http.StatusOK, "I am by birth a Genevese (i not set)") - // 404 on not matching path - r, _ = http.NewRequest("GET", "http://localhost:"+testPort+"/other", nil) + // 404 on no matching paths + r, _ = http.NewRequest("GET", "http://localhost:"+testport2+"/other", nil) tester.AssertResponseCode(r, http.StatusNotFound) - // forward files to fileserver - tester.AssertGetResponse("http://localhost:"+testPort+"/static.txt2", http.StatusOK, "Hello from file") + // static file will be served by fileserver + tester.AssertGetResponse("http://localhost:"+testport2+"/files/static.txt2", http.StatusOK, "Hello from file") // never serve PHP files directly - r, _ = http.NewRequest("GET", "http://localhost:"+testPort+"/index.php", nil) + r, _ = http.NewRequest("GET", "http://localhost:"+testport2+"/index.php", nil) tester.AssertResponseCode(r, http.StatusNotFound) } From b93f261441fa9e3bb97f3181cce6e018183eee32 Mon Sep 17 00:00:00 2001 From: Alliballibaba Date: Wed, 11 Jun 2025 21:05:07 +0200 Subject: [PATCH 09/59] formatting --- caddy/caddy_test.go | 30 +++++++++++++++--------------- 1 file changed, 15 insertions(+), 15 deletions(-) diff --git a/caddy/caddy_test.go b/caddy/caddy_test.go index 629b59d39..0941ed7f0 100644 --- a/caddy/caddy_test.go +++ b/caddy/caddy_test.go @@ -1336,18 +1336,18 @@ func TestWorkerMatchDirectiveWithFileServer(t *testing.T) { } } http://localhost:`+testport2+` { - php_server { - root ../testdata - match /counter/* worker { - file worker-with-counter.php - num 1 - } + php_server { + root ../testdata + match /counter/* worker { + file worker-with-counter.php + num 1 + } match /index/* worker { - file index.php - num 1 - } - } - } + file index.php + num 1 + } + } + } `, "caddyfile") // worker is outside of public directory, match anyways @@ -1358,10 +1358,10 @@ func TestWorkerMatchDirectiveWithFileServer(t *testing.T) { r, _ := http.NewRequest("GET", "http://localhost:"+testPort+"/not-matched-path", nil) tester.AssertResponseCode(r, http.StatusNotFound) - // static file will be served by fileserver + // static file will be served by the fileserver tester.AssertGetResponse("http://localhost:"+testPort+"/static.txt2", http.StatusOK, "Hello from file") - // the second php_server 2 different workers in the public path + // the second php_server has 2 different workers in the public path tester.AssertGetResponse("http://localhost:"+testport2+"/counter/sub-path", http.StatusOK, "requests:1") tester.AssertGetResponse("http://localhost:"+testport2+"/index/sub-path", http.StatusOK, "I am by birth a Genevese (i not set)") @@ -1369,10 +1369,10 @@ func TestWorkerMatchDirectiveWithFileServer(t *testing.T) { r, _ = http.NewRequest("GET", "http://localhost:"+testport2+"/other", nil) tester.AssertResponseCode(r, http.StatusNotFound) - // static file will be served by fileserver + // static file will be served by the fileserver tester.AssertGetResponse("http://localhost:"+testport2+"/files/static.txt2", http.StatusOK, "Hello from file") - // never serve PHP files directly + // 404 on PHP files r, _ = http.NewRequest("GET", "http://localhost:"+testport2+"/index.php", nil) tester.AssertResponseCode(r, http.StatusNotFound) } From 1393be5c028eef460e6a155f716bdc05e35d416d Mon Sep 17 00:00:00 2001 From: Alliballibaba Date: Fri, 13 Jun 2025 08:52:59 +0200 Subject: [PATCH 10/59] Resets implementation to worker->match --- caddy/caddy_test.go | 87 +++++++++++++--------- caddy/module.go | 81 ++++++-------------- caddy/workerconfig.go | 30 +++----- testdata/files/.gitignore | 2 +- testdata/files/{static.txt2 => static.txt} | 0 testdata/hello.php | 3 + 6 files changed, 90 insertions(+), 113 deletions(-) rename testdata/files/{static.txt2 => static.txt} (100%) create mode 100644 testdata/hello.php diff --git a/caddy/caddy_test.go b/caddy/caddy_test.go index 0941ed7f0..c67e363e7 100644 --- a/caddy/caddy_test.go +++ b/caddy/caddy_test.go @@ -1317,9 +1317,8 @@ func TestWorkerRestart(t *testing.T) { )) } -func TestWorkerMatchDirectiveWithFileServer(t *testing.T) { +func TestWorkerMatchDirective(t *testing.T) { tester := caddytest.NewTester(t) - testport2 := "3000" tester.InitServer(` { skip_install_trust @@ -1329,52 +1328,65 @@ func TestWorkerMatchDirectiveWithFileServer(t *testing.T) { http://localhost:`+testPort+` { php_server { root ../testdata/files - match /matched-path* worker { + worker { file ../worker-with-counter.php + match /matched-path* num 1 } } } - http://localhost:`+testport2+` { + `, "caddyfile") + + // worker is outside of public directory, match anyways + tester.AssertGetResponse("http://localhost:"+testPort+"/matched-path", http.StatusOK, "requests:1") + tester.AssertGetResponse("http://localhost:"+testPort+"/matched-path/anywhere", http.StatusOK, "requests:2") + + // 404 on unmatched paths + tester.AssertGetResponse("http://localhost:"+testPort+"/elsewhere", http.StatusNotFound, "") + + // file will be serverd by file_server + tester.AssertGetResponse("http://localhost:"+testPort+"/static.txt", http.StatusOK, "Hello from file") +} + +func TestWorkerMatchDirectiveWithMultipleWorkers(t *testing.T) { + tester := caddytest.NewTester(t) + tester.InitServer(` + { + skip_install_trust + admin localhost:2999 + } + http://localhost:`+testPort+` { php_server { root ../testdata - match /counter/* worker { + worker { file worker-with-counter.php + match /counter/* num 1 } - match /index/* worker { + worker { file index.php + match /index/* num 1 } } } `, "caddyfile") - // worker is outside of public directory, match anyways - tester.AssertGetResponse("http://localhost:"+testPort+"/matched-path", http.StatusOK, "requests:1") - tester.AssertGetResponse("http://localhost:"+testPort+"/matched-path/anywhere", http.StatusOK, "requests:2") - - // 404 on no matching paths - r, _ := http.NewRequest("GET", "http://localhost:"+testPort+"/not-matched-path", nil) - tester.AssertResponseCode(r, http.StatusNotFound) + // match 2 workers respectively (in the public directory) + tester.AssertGetResponse("http://localhost:"+testPort+"/counter/sub-path", http.StatusOK, "requests:1") + tester.AssertGetResponse("http://localhost:"+testPort+"/index/sub-path", http.StatusOK, "I am by birth a Genevese (i not set)") // static file will be served by the fileserver - tester.AssertGetResponse("http://localhost:"+testPort+"/static.txt2", http.StatusOK, "Hello from file") - - // the second php_server has 2 different workers in the public path - tester.AssertGetResponse("http://localhost:"+testport2+"/counter/sub-path", http.StatusOK, "requests:1") - tester.AssertGetResponse("http://localhost:"+testport2+"/index/sub-path", http.StatusOK, "I am by birth a Genevese (i not set)") + tester.AssertGetResponse("http://localhost:"+testPort+"/files/static.txt", http.StatusOK, "Hello from file") - // 404 on no matching paths - r, _ = http.NewRequest("GET", "http://localhost:"+testport2+"/other", nil) - tester.AssertResponseCode(r, http.StatusNotFound) + // 404 if the request falls through + tester.AssertGetResponse("http://localhost:"+testPort+"/not-matched", http.StatusNotFound, "") - // static file will be served by the fileserver - tester.AssertGetResponse("http://localhost:"+testport2+"/files/static.txt2", http.StatusOK, "Hello from file") + // serve php file directly as fallback + tester.AssertGetResponse("http://localhost:"+testPort+"/hello.php", http.StatusOK, "Hello") - // 404 on PHP files - r, _ = http.NewRequest("GET", "http://localhost:"+testport2+"/index.php", nil) - tester.AssertResponseCode(r, http.StatusNotFound) + // serve worker file directly as fallback + tester.AssertGetResponse("http://localhost:"+testPort+"/index.php", http.StatusOK, "I am by birth a Genevese (i not set)") } func TestWorkerMatchDirectiveWithoutFileServer(t *testing.T) { @@ -1386,13 +1398,18 @@ func TestWorkerMatchDirectiveWithoutFileServer(t *testing.T) { } http://localhost:`+testPort+` { - php_server { - file_server off - root ../testdata/files - match /some-path worker { - file ../worker-with-counter.php - num 1 + route { + php_server { + index off + file_server off + root ../testdata/files + worker { + file ../worker-with-counter.php + match /some-path + } } + + respond "Request falls through" 404 } } `, "caddyfile") @@ -1400,7 +1417,7 @@ func TestWorkerMatchDirectiveWithoutFileServer(t *testing.T) { // find the worker at some-path tester.AssertGetResponse("http://localhost:"+testPort+"/some-path", http.StatusOK, "requests:1") - // do not find the file at static.txt2 - r, _ := http.NewRequest("GET", "http://localhost:"+testPort+"/static.txt2", nil) - tester.AssertResponseCode(r, http.StatusNotFound) + // do not find the file at static.txt + // the request should completely fall through the php_server module + tester.AssertGetResponse("http://localhost:"+testPort+"/static.txt", http.StatusNotFound, "Request falls through") } diff --git a/caddy/module.go b/caddy/module.go index 2ff43d9e3..52c69f7c7 100644 --- a/caddy/module.go +++ b/caddy/module.go @@ -255,26 +255,9 @@ func (f *FrankenPHPModule) UnmarshalCaddyfile(d *caddyfile.Dispenser) error { return err } f.Workers = append(f.Workers, wc) - case "match": - if !d.NextArg() { - return d.ArgErr() - } - match := "*" - if d.Val() != "worker" { - match = d.Val() - if !d.NextArg() || d.Val() != "worker" { - return d.ArgErr() - } - } - wc, err := parseWorkerConfig(d) - if err != nil { - return err - } - wc.Match = match - f.Workers = append(f.Workers, wc) default: - allowedDirectives := "root, split, env, resolve_root_symlink, worker, match" + allowedDirectives := "root, split, env, resolve_root_symlink, worker" return wrongSubDirectiveError("php or php_server", allowedDirectives, d.Val()) } } @@ -444,16 +427,12 @@ func parsePhpServer(h httpcaddyfile.Helper) ([]httpcaddyfile.ConfigValue, error) } } - // if any worker has 'match' specified, use an alternate routing approach - for _, w := range phpsrv.Workers { - if w.Match != "" { - return pureWorkerRoute(h, phpsrv, fsrv, disableFsrv), nil - } - } - // set up a route list that we'll append to routes := caddyhttp.RouteList{} + // prepend routes from the 'worker match *' directives + routes = prependWorkerRoutes(routes, h, phpsrv, fsrv, disableFsrv) + // set the list of allowed path segments on which to split phpsrv.SplitPath = extensions @@ -589,23 +568,23 @@ func parsePhpServer(h httpcaddyfile.Helper) ([]httpcaddyfile.ConfigValue, error) }, nil } -// if any worker under php_server has a match pattern, -// routes have to be created differently -// first, we will try to match against the file system, -// then we will try to match aganist each worker -func pureWorkerRoute(h httpcaddyfile.Helper, f FrankenPHPModule, fsrv caddy.Module, disableFsrv bool) []httpcaddyfile.ConfigValue { - f.SplitPath = []string{} - matchers := caddyhttp.MatchPath{} +// workers can also match a path without being in the public directory +// in this case we need to prepend the worker routes to the existing routes +func prependWorkerRoutes(routes caddyhttp.RouteList, h httpcaddyfile.Helper, f FrankenPHPModule, fsrv caddy.Module, disableFsrv bool) caddyhttp.RouteList { + matchPath := caddyhttp.MatchPath{} for _, w := range f.Workers { - if w.Match != "" { - matchers = append(matchers, w.Match) + if w.MatchPath != "" { + matchPath = append(matchPath, w.MatchPath) } } - subRoute := caddyhttp.Subroute{Routes: caddyhttp.RouteList{}} + if len(matchPath) == 0 { + return routes + } + // if there are match patterns, we need to check for files beforehand if !disableFsrv { - subRoute.Routes = append(subRoute.Routes, caddyhttp.Route{ + routes = append(routes, caddyhttp.Route{ MatcherSetsRaw: []caddy.ModuleMap{ caddy.ModuleMap{ "file": h.JSON(fileserver.MatchFile{ @@ -625,31 +604,15 @@ func pureWorkerRoute(h httpcaddyfile.Helper, f FrankenPHPModule, fsrv caddy.Modu }) } - for _, pattern := range matchers { - subRoute.Routes = append(subRoute.Routes, caddyhttp.Route{ - MatcherSetsRaw: []caddy.ModuleMap{ - caddy.ModuleMap{"path": h.JSON(caddyhttp.MatchPath{pattern})}, - }, - HandlersRaw: []json.RawMessage{ - caddyconfig.JSONModuleObject(f, "handler", "php", nil), - }, - }) - } - - notFoundHandler := caddyhttp.StaticResponse{ - StatusCode: caddyhttp.WeakString(strconv.Itoa(http.StatusNotFound)), - } - subRoute.Routes = append(subRoute.Routes, caddyhttp.Route{ - MatcherSetsRaw: []caddy.ModuleMap{}, + // forward matching routes instantly to the PHP handler + routes = append(routes, caddyhttp.Route{ + MatcherSetsRaw: []caddy.ModuleMap{ + caddy.ModuleMap{"path": h.JSON(matchPath)}, + }, HandlersRaw: []json.RawMessage{ - caddyconfig.JSONModuleObject(notFoundHandler, "handler", "static_response", nil), + caddyconfig.JSONModuleObject(f, "handler", "php", nil), }, }) - return []httpcaddyfile.ConfigValue{ - { - Class: "route", - Value: subRoute, - }, - } + return routes } diff --git a/caddy/workerconfig.go b/caddy/workerconfig.go index f52fee4ed..7b882a76a 100644 --- a/caddy/workerconfig.go +++ b/caddy/workerconfig.go @@ -33,22 +33,9 @@ type workerConfig struct { // Directories to watch for file changes Watch []string `json:"watch,omitempty"` // The path to match against the worker - Match string `json:"match,omitempty"` - - matchStrategy matchStrategy + MatchPath string `json:"match_path,omitempty"` } -type matchStrategy int - -const ( - matchNone matchStrategy = iota - matchAll - matchPrefix - matchSuffix - matchExactly - matchWildcard -) - func parseWorkerConfig(d *caddyfile.Dispenser) (workerConfig, error) { wc := workerConfig{} if d.NextArg() { @@ -112,9 +99,14 @@ func parseWorkerConfig(d *caddyfile.Dispenser) (workerConfig, error) { } else { wc.Watch = append(wc.Watch, d.Val()) } + case "match": + if !d.NextArg() { + return wc, d.ArgErr() + } + wc.MatchPath = d.Val() default: - allowedDirectives := "name, file, num, env, watch" + allowedDirectives := "name, file, num, env, watch, match" return wc, wrongSubDirectiveError("worker", allowedDirectives, v) } } @@ -131,11 +123,13 @@ func parseWorkerConfig(d *caddyfile.Dispenser) (workerConfig, error) { } func (wc workerConfig) matchesPath(r *http.Request, documentRoot string) bool { - if wc.Match != "" { - matches, _ := caddyhttp.MatchPath{wc.Match}.MatchWithError(r) - return matches + + // try to match against a pattern if one is assigned + if wc.MatchPath != "" { + return caddyhttp.MatchPath{wc.MatchPath}.Match(r) } + // if there is no pattern, try to match against the actual path (in the public directory) fullScriptPath, _ := fastabs.FastAbs(documentRoot + "/" + r.URL.Path) absFileName, _ := fastabs.FastAbs(wc.FileName) return fullScriptPath == absFileName diff --git a/testdata/files/.gitignore b/testdata/files/.gitignore index 2211df63d..4871fd527 100644 --- a/testdata/files/.gitignore +++ b/testdata/files/.gitignore @@ -1 +1 @@ -*.txt +test.txt diff --git a/testdata/files/static.txt2 b/testdata/files/static.txt similarity index 100% rename from testdata/files/static.txt2 rename to testdata/files/static.txt diff --git a/testdata/hello.php b/testdata/hello.php new file mode 100644 index 000000000..e09aa5c34 --- /dev/null +++ b/testdata/hello.php @@ -0,0 +1,3 @@ + Date: Fri, 13 Jun 2025 10:29:26 +0200 Subject: [PATCH 11/59] Provisions match path rules. --- caddy/workerconfig.go | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/caddy/workerconfig.go b/caddy/workerconfig.go index 7b882a76a..9c00c3236 100644 --- a/caddy/workerconfig.go +++ b/caddy/workerconfig.go @@ -6,6 +6,7 @@ import ( "path/filepath" "strconv" + "github.com/caddyserver/caddy/v2" "github.com/caddyserver/caddy/v2/caddyconfig/caddyfile" "github.com/caddyserver/caddy/v2/modules/caddyhttp" "github.com/dunglas/frankenphp" @@ -103,7 +104,11 @@ func parseWorkerConfig(d *caddyfile.Dispenser) (workerConfig, error) { if !d.NextArg() { return wc, d.ArgErr() } - wc.MatchPath = d.Val() + // provision the path so it's identical to Caddy match rules + // see: https://github.com/caddyserver/caddy/blob/master/modules/caddyhttp/matchers.go + caddyMatchPath := caddyhttp.MatchPath{d.Val()} + caddyMatchPath.Provision(caddy.Context{}) + wc.MatchPath = caddyMatchPath[0] default: allowedDirectives := "name, file, num, env, watch, match" From 608d1754a7ebd1ba015fe15dcbd29f9bd3fd4096 Mon Sep 17 00:00:00 2001 From: Alliballibaba Date: Fri, 13 Jun 2025 11:44:16 +0200 Subject: [PATCH 12/59] Allows matching multiple paths --- caddy/module.go | 12 ++++++------ caddy/workerconfig.go | 14 +++++--------- 2 files changed, 11 insertions(+), 15 deletions(-) diff --git a/caddy/module.go b/caddy/module.go index 52c69f7c7..f40aa0738 100644 --- a/caddy/module.go +++ b/caddy/module.go @@ -571,14 +571,14 @@ func parsePhpServer(h httpcaddyfile.Helper) ([]httpcaddyfile.ConfigValue, error) // workers can also match a path without being in the public directory // in this case we need to prepend the worker routes to the existing routes func prependWorkerRoutes(routes caddyhttp.RouteList, h httpcaddyfile.Helper, f FrankenPHPModule, fsrv caddy.Module, disableFsrv bool) caddyhttp.RouteList { - matchPath := caddyhttp.MatchPath{} + allWorkerMatches := caddyhttp.MatchPath{} for _, w := range f.Workers { - if w.MatchPath != "" { - matchPath = append(matchPath, w.MatchPath) + for _, path := range w.MatchPath { + allWorkerMatches = append(matchPath, path) } } - if len(matchPath) == 0 { + if len(allWorkerMatches) == 0 { return routes } @@ -604,10 +604,10 @@ func prependWorkerRoutes(routes caddyhttp.RouteList, h httpcaddyfile.Helper, f F }) } - // forward matching routes instantly to the PHP handler + // forward matching routes to the PHP handler routes = append(routes, caddyhttp.Route{ MatcherSetsRaw: []caddy.ModuleMap{ - caddy.ModuleMap{"path": h.JSON(matchPath)}, + caddy.ModuleMap{"path": h.JSON(allWorkerMatches)}, }, HandlersRaw: []json.RawMessage{ caddyconfig.JSONModuleObject(f, "handler", "php", nil), diff --git a/caddy/workerconfig.go b/caddy/workerconfig.go index 9c00c3236..e85eb2b3d 100644 --- a/caddy/workerconfig.go +++ b/caddy/workerconfig.go @@ -34,7 +34,7 @@ type workerConfig struct { // Directories to watch for file changes Watch []string `json:"watch,omitempty"` // The path to match against the worker - MatchPath string `json:"match_path,omitempty"` + MatchPath []string `json:"match_path,omitempty"` } func parseWorkerConfig(d *caddyfile.Dispenser) (workerConfig, error) { @@ -101,15 +101,11 @@ func parseWorkerConfig(d *caddyfile.Dispenser) (workerConfig, error) { wc.Watch = append(wc.Watch, d.Val()) } case "match": - if !d.NextArg() { - return wc, d.ArgErr() - } // provision the path so it's identical to Caddy match rules // see: https://github.com/caddyserver/caddy/blob/master/modules/caddyhttp/matchers.go - caddyMatchPath := caddyhttp.MatchPath{d.Val()} + caddyMatchPath := (caddyhttp.MatchPath)(d.RemainingArgs()) caddyMatchPath.Provision(caddy.Context{}) - wc.MatchPath = caddyMatchPath[0] - + wc.MatchPath = ([]string)(caddyMatchPath) default: allowedDirectives := "name, file, num, env, watch, match" return wc, wrongSubDirectiveError("worker", allowedDirectives, v) @@ -130,8 +126,8 @@ func parseWorkerConfig(d *caddyfile.Dispenser) (workerConfig, error) { func (wc workerConfig) matchesPath(r *http.Request, documentRoot string) bool { // try to match against a pattern if one is assigned - if wc.MatchPath != "" { - return caddyhttp.MatchPath{wc.MatchPath}.Match(r) + if len(wc.MatchPath) != 0 { + return (caddyhttp.MatchPath)(wc.MatchPath).Match(r) } // if there is no pattern, try to match against the actual path (in the public directory) From 8cda5d929f972abb6891326eac257db07bac7799 Mon Sep 17 00:00:00 2001 From: Alliballibaba Date: Fri, 13 Jun 2025 11:48:40 +0200 Subject: [PATCH 13/59] Fixes var --- caddy/module.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/caddy/module.go b/caddy/module.go index f40aa0738..1ef0eb964 100644 --- a/caddy/module.go +++ b/caddy/module.go @@ -574,7 +574,7 @@ func prependWorkerRoutes(routes caddyhttp.RouteList, h httpcaddyfile.Helper, f F allWorkerMatches := caddyhttp.MatchPath{} for _, w := range f.Workers { for _, path := range w.MatchPath { - allWorkerMatches = append(matchPath, path) + allWorkerMatches = append(allWorkerMatches, path) } } From b9021fde22fabcaac85cb48a729311b29da23eb6 Mon Sep 17 00:00:00 2001 From: Alliballibaba Date: Fri, 13 Jun 2025 23:35:29 +0200 Subject: [PATCH 14/59] Formatting. --- caddy/caddy_test.go | 4 ++-- testdata/hello.php | 2 +- worker.go | 11 +++++++---- 3 files changed, 10 insertions(+), 7 deletions(-) diff --git a/caddy/caddy_test.go b/caddy/caddy_test.go index c67e363e7..720a6a171 100644 --- a/caddy/caddy_test.go +++ b/caddy/caddy_test.go @@ -1344,7 +1344,7 @@ func TestWorkerMatchDirective(t *testing.T) { // 404 on unmatched paths tester.AssertGetResponse("http://localhost:"+testPort+"/elsewhere", http.StatusNotFound, "") - // file will be serverd by file_server + // static file will be served by the fileserver tester.AssertGetResponse("http://localhost:"+testPort+"/static.txt", http.StatusOK, "Hello from file") } @@ -1383,7 +1383,7 @@ func TestWorkerMatchDirectiveWithMultipleWorkers(t *testing.T) { tester.AssertGetResponse("http://localhost:"+testPort+"/not-matched", http.StatusNotFound, "") // serve php file directly as fallback - tester.AssertGetResponse("http://localhost:"+testPort+"/hello.php", http.StatusOK, "Hello") + tester.AssertGetResponse("http://localhost:"+testPort+"/hello.php", http.StatusOK, "Hello from PHP") // serve worker file directly as fallback tester.AssertGetResponse("http://localhost:"+testPort+"/index.php", http.StatusOK, "I am by birth a Genevese (i not set)") diff --git a/testdata/hello.php b/testdata/hello.php index e09aa5c34..81a3ccfdf 100644 --- a/testdata/hello.php +++ b/testdata/hello.php @@ -1,3 +1,3 @@ Date: Sat, 14 Jun 2025 12:44:35 +0200 Subject: [PATCH 15/59] refactoring. --- caddy/module.go | 15 +++++---------- caddy/workerconfig.go | 12 ++++++++++++ 2 files changed, 17 insertions(+), 10 deletions(-) diff --git a/caddy/module.go b/caddy/module.go index 1ef0eb964..e38733ef1 100644 --- a/caddy/module.go +++ b/caddy/module.go @@ -70,21 +70,16 @@ func (f *FrankenPHPModule) Provision(ctx caddy.Context) error { } for i, wc := range f.Workers { - // Inherit environment variables from the parent php_server directive + + // make the file path absolute from the public directory + // this can only be done if the root is definied inside php_server if !filepath.IsAbs(wc.FileName) && f.Root != "" { wc.FileName = filepath.Join(f.Root, wc.FileName) } + // Inherit environment variables from the parent php_server directive if f.Env != nil { - if wc.Env == nil { - wc.Env = make(map[string]string) - } - for k, v := range f.Env { - // Only set if not already defined in the worker - if _, exists := wc.Env[k]; !exists { - wc.Env[k] = v - } - } + wc.inheritEnv(f.Env) } f.Workers[i] = wc } diff --git a/caddy/workerconfig.go b/caddy/workerconfig.go index e85eb2b3d..6d58072de 100644 --- a/caddy/workerconfig.go +++ b/caddy/workerconfig.go @@ -123,6 +123,18 @@ func parseWorkerConfig(d *caddyfile.Dispenser) (workerConfig, error) { return wc, nil } +func (wc workerConfig) inheritEnv(env map[string]string) { + if wc.Env == nil { + wc.Env = make(map[string]string) + } + for k, v := range env { + // do not overwrite existing environment variables + if _, exists := wc.Env[k]; !exists { + wc.Env[k] = v + } + } +} + func (wc workerConfig) matchesPath(r *http.Request, documentRoot string) bool { // try to match against a pattern if one is assigned From 9ec956001d4f14531e4a057a71b0b529e4329342 Mon Sep 17 00:00:00 2001 From: Alliballibaba Date: Mon, 9 Jun 2025 21:01:45 +0200 Subject: [PATCH 16/59] Adds 'match' configuration --- caddy/config_test.go | 2 +- caddy/module.go | 5 ++--- caddy/workerconfig.go | 20 +++++++++++++++++++- 3 files changed, 22 insertions(+), 5 deletions(-) diff --git a/caddy/config_test.go b/caddy/config_test.go index 7bcbbfc27..29fee1d24 100644 --- a/caddy/config_test.go +++ b/caddy/config_test.go @@ -1,4 +1,4 @@ -package caddy +package caddy import ( "testing" diff --git a/caddy/module.go b/caddy/module.go index aef54df48..519643df7 100644 --- a/caddy/module.go +++ b/caddy/module.go @@ -162,12 +162,11 @@ func (f *FrankenPHPModule) ServeHTTP(w http.ResponseWriter, r *http.Request, _ c } } - fullScriptPath, _ := fastabs.FastAbs(documentRoot + "/" + r.URL.Path) - workerName := "" for _, w := range f.Workers { - if p, _ := fastabs.FastAbs(w.FileName); p == fullScriptPath { + if w.matchesPath(r.URL.Path, documentRoot) { workerName = w.Name + break } } diff --git a/caddy/workerconfig.go b/caddy/workerconfig.go index a6a797c77..47db675e7 100644 --- a/caddy/workerconfig.go +++ b/caddy/workerconfig.go @@ -7,6 +7,7 @@ import ( "github.com/caddyserver/caddy/v2/caddyconfig/caddyfile" "github.com/dunglas/frankenphp" + "github.com/dunglas/frankenphp/internal/fastabs" ) // workerConfig represents the "worker" directive in the Caddyfile @@ -29,6 +30,8 @@ type workerConfig struct { Env map[string]string `json:"env,omitempty"` // Directories to watch for file changes Watch []string `json:"watch,omitempty"` + // The path to match against the worker + Match string `json:"match,omitempty"` } func parseWorkerConfig(d *caddyfile.Dispenser) (workerConfig, error) { @@ -94,8 +97,13 @@ func parseWorkerConfig(d *caddyfile.Dispenser) (workerConfig, error) { } else { wc.Watch = append(wc.Watch, d.Val()) } + case "match": + if !d.NextArg() { + return wc, d.ArgErr() + } + wc.Match = d.Val() default: - allowedDirectives := "name, file, num, env, watch" + allowedDirectives := "name, file, num, env, watch, match" return wc, wrongSubDirectiveError("worker", allowedDirectives, v) } } @@ -110,3 +118,13 @@ func parseWorkerConfig(d *caddyfile.Dispenser) (workerConfig, error) { return wc, nil } + +func (wc workerConfig) matchesPath(path string, documentRoot string) bool { + if wc.Match != "" { + return wc.Match == "*" || wc.Match == path + } + + fullScriptPath, _ := fastabs.FastAbs(documentRoot + "/" + path) + absFileName, _ := fastabs.FastAbs(wc.FileName) + return fullScriptPath == absFileName +} From 4047b3fb48591d8ef06d78ebf8fecfaf1fce1184 Mon Sep 17 00:00:00 2001 From: Alliballibaba Date: Mon, 9 Jun 2025 23:20:02 +0200 Subject: [PATCH 17/59] test --- caddy/caddy_test.go | 26 ++++++++ caddy/module.go | 157 +++++++++++++++++++++++++++++--------------- 2 files changed, 129 insertions(+), 54 deletions(-) diff --git a/caddy/caddy_test.go b/caddy/caddy_test.go index 9d3a9a013..a5bba66f0 100644 --- a/caddy/caddy_test.go +++ b/caddy/caddy_test.go @@ -1216,6 +1216,32 @@ func TestDisabledMetrics(t *testing.T) { require.Zero(t, count, "metrics should be missing") } +func TestWorkerMatchDirective(t *testing.T) { + tester := caddytest.NewTester(t) + tester.InitServer(` + { + skip_install_trust + admin localhost:2999 + } + + http://localhost:`+testPort+` { + php_server { + root ../testdata/files + try_files {path} index.php + worker { + match index.php + file ../worker-with-counter.php + num 1 + } + } + } + `, "caddyfile") + + tester.AssertGetResponse("http://localhost:"+testPort+"/any-path", http.StatusOK, "requests:1") + tester.AssertGetResponse("http://localhost:"+testPort+"/static.txt", http.StatusOK, "Hello from file") + tester.AssertGetResponse("http://localhost:"+testPort+"/worker-path", http.StatusOK, "requests:1") +} + func TestWorkerRestart(t *testing.T) { var wg sync.WaitGroup tester := caddytest.NewTester(t) diff --git a/caddy/module.go b/caddy/module.go index 519643df7..caf49d8a4 100644 --- a/caddy/module.go +++ b/caddy/module.go @@ -69,6 +69,34 @@ func (f *FrankenPHPModule) Provision(ctx caddy.Context) error { return fmt.Errorf(`expected ctx.App("frankenphp") to return *FrankenPHPApp, got nil`) } + for i, wc := range f.Workers { + // Inherit environment variables from the parent php_server directive + if !filepath.IsAbs(wc.FileName) && f.Root != "" { + wc.FileName = filepath.Join(f.Root, wc.FileName) + } + + if f.Env != nil { + if wc.Env == nil { + wc.Env = make(map[string]string) + } + for k, v := range f.Env { + // Only set if not already defined in the worker + if _, exists := wc.Env[k]; !exists { + wc.Env[k] = v + } + } + } + + f.Workers[i] = wc + + // Check if a worker with this filename already exists in this module + for j, existingWorker := range f.Workers { + if i != j && existingWorker.FileName == wc.FileName { + return fmt.Errorf(`workers in a single "php_server" block must not have duplicate filenames: %q`, wc.FileName) + } + } + } + workers, err := fapp.addModuleWorkers(f.Workers...) if err != nil { return err @@ -230,12 +258,11 @@ func (f *FrankenPHPModule) UnmarshalCaddyfile(d *caddyfile.Dispenser) error { f.ResolveRootSymlink = &v case "worker": - for d.NextBlock(1) { - } - for d.NextArg() { - } - // Skip "worker" blocks in the first pass - continue + wc, err := parseWorkerConfig(d) + if err != nil { + return err + } + f.Workers = append(f.Workers, wc) default: allowedDirectives := "root, split, env, resolve_root_symlink, worker" @@ -244,45 +271,6 @@ func (f *FrankenPHPModule) UnmarshalCaddyfile(d *caddyfile.Dispenser) error { } } - // Second pass: Parse only "worker" blocks - d.Reset() - for d.Next() { - for d.NextBlock(0) { - if d.Val() == "worker" { - wc, err := parseWorkerConfig(d) - if err != nil { - return err - } - - // Inherit environment variables from the parent php_server directive - if !filepath.IsAbs(wc.FileName) && f.Root != "" { - wc.FileName = filepath.Join(f.Root, wc.FileName) - } - - if f.Env != nil { - if wc.Env == nil { - wc.Env = make(map[string]string) - } - for k, v := range f.Env { - // Only set if not already defined in the worker - if _, exists := wc.Env[k]; !exists { - wc.Env[k] = v - } - } - } - - // Check if a worker with this filename already exists in this module - for _, existingWorker := range f.Workers { - if existingWorker.FileName == wc.FileName { - return fmt.Errorf(`workers in a single "php_server" block must not have duplicate filenames: %q`, wc.FileName) - } - } - - f.Workers = append(f.Workers, wc) - } - } - } - return nil } @@ -418,6 +406,16 @@ func parsePhpServer(h httpcaddyfile.Helper) ([]httpcaddyfile.ConfigValue, error) // unmarshaler can read it from the start dispenser.Reset() + // the rest of the config is specified by the user + // using the php directive syntax + dispenser.Next() // consume the directive name + err = phpsrv.UnmarshalCaddyfile(dispenser) + if err != nil { + return nil, err + } + + return routesForWorkerServer(h, phpsrv, fsrv) + if frankenphp.EmbeddedAppPath != "" { if phpsrv.Root == "" { phpsrv.Root = filepath.Join(frankenphp.EmbeddedAppPath, defaultDocumentRoot) @@ -521,15 +519,6 @@ func parsePhpServer(h httpcaddyfile.Helper) ([]httpcaddyfile.ConfigValue, error) "path": h.JSON(pathList), } - // the rest of the config is specified by the user - // using the php directive syntax - dispenser.Next() // consume the directive name - err = phpsrv.UnmarshalCaddyfile(dispenser) - - if err != nil { - return nil, err - } - // create the PHP route which is // conditional on matching PHP files phpRoute := caddyhttp.Route{ @@ -576,3 +565,63 @@ func parsePhpServer(h httpcaddyfile.Helper) ([]httpcaddyfile.ConfigValue, error) }, }, nil } + +func routesForWorkerServer(h httpcaddyfile.Helper, frankenphpModule FrankenPHPModule, fsrv caddy.Module) ([]httpcaddyfile.ConfigValue, error) { + return []httpcaddyfile.ConfigValue{ + { + Class: "route", + Value: workerSubroute(h, frankenphpModule, fsrv), + }, + }, nil +} + +// get the routing for php_worker +// serve non-php file or fallback to index worker +func workerSubroute(h httpcaddyfile.Helper, frankenphpModule FrankenPHPModule, fsrv caddy.Module) caddyhttp.Subroute { + matchers := caddyhttp.MatchPath{"*.php"} + for _, w := range frankenphpModule.Workers { + if w.Match != "" { + matchers = append(matchers, w.Match) + } + } + + subRoute := caddyhttp.Subroute{ + Routes: caddyhttp.RouteList{ + caddyhttp.Route{ + MatcherSetsRaw: []caddy.ModuleMap{ + caddy.ModuleMap{ + "file": h.JSON(fileserver.MatchFile{ + TryFiles: []string{"{http.request.uri.path}"}, + Root: frankenphpModule.Root, + }), + "not": h.JSON(caddyhttp.MatchNot{ + MatcherSetsRaw: []caddy.ModuleMap{ + { + "path": h.JSON(matchers), + }, + }, + }), + }, + }, + HandlersRaw: []json.RawMessage{ + caddyconfig.JSONModuleObject(fsrv, "handler", "file_server", nil), + }, + }, + }, + } + + for _, pattern := range matchers { + subRoute.Routes = append(subRoute.Routes, caddyhttp.Route{ + MatcherSetsRaw: []caddy.ModuleMap{ + caddy.ModuleMap{ + "path": h.JSON(caddyhttp.MatchPath{ pattern }), + }, + }, + HandlersRaw: []json.RawMessage{ + caddyconfig.JSONModuleObject(frankenphpModule, "handler", "php", nil), + }, + }) + } + + return subRoute +} From 03faa72c815658e6cf9366e44fe35342a180c9b5 Mon Sep 17 00:00:00 2001 From: Alliballibaba Date: Wed, 11 Jun 2025 00:13:19 +0200 Subject: [PATCH 18/59] Adds Caddy's matcher. --- caddy/caddy_test.go | 46 ++++++++++++++++-- caddy/module.go | 107 ++++++++++++++++++++++-------------------- caddy/workerconfig.go | 23 +++++++-- 3 files changed, 119 insertions(+), 57 deletions(-) diff --git a/caddy/caddy_test.go b/caddy/caddy_test.go index a5bba66f0..b0c33d8e7 100644 --- a/caddy/caddy_test.go +++ b/caddy/caddy_test.go @@ -1218,6 +1218,7 @@ func TestDisabledMetrics(t *testing.T) { func TestWorkerMatchDirective(t *testing.T) { tester := caddytest.NewTester(t) + testport2 := "3000" tester.InitServer(` { skip_install_trust @@ -1227,19 +1228,56 @@ func TestWorkerMatchDirective(t *testing.T) { http://localhost:`+testPort+` { php_server { root ../testdata/files - try_files {path} index.php worker { - match index.php + match /matched-path* file ../worker-with-counter.php num 1 } } } + http://localhost:`+testport2+` { + php_server { + root ../testdata + worker { + match /counter* + file worker-with-counter.php + num 1 + } + worker { + match /index* + file index.php + num 1 + } + } + } `, "caddyfile") - tester.AssertGetResponse("http://localhost:"+testPort+"/any-path", http.StatusOK, "requests:1") + // the first php_server with root in testdata/files + // Forward request that match * to the worker + tester.AssertGetResponse("http://localhost:"+testPort+"/matched-path", http.StatusOK, "requests:1") + tester.AssertGetResponse("http://localhost:"+testPort+"/matched-path/anywhere", http.StatusOK, "requests:2") + + // 404 on not matching paths + r, _ := http.NewRequest("GET", "http://localhost:"+testPort+"/not-matched-path", nil) + tester.AssertResponseCode(r, http.StatusNotFound) + + // forward files to fileserver + tester.AssertGetResponse("http://localhost:"+testPort+"/static.txt", http.StatusOK, "Hello from file") + + // the second php_server has root in testdata and has 2 different workers + tester.AssertGetResponse("http://localhost:"+testport2+"/counter/sub-path", http.StatusOK, "requests:1") + tester.AssertGetResponse("http://localhost:"+testport2+"/index/sub-path", http.StatusOK, "I am by birth a Genevese (i not set)") + + // 404 on not matching path + r, _ = http.NewRequest("GET", "http://localhost:"+testPort+"/other", nil) + tester.AssertResponseCode(r, http.StatusNotFound) + + // forward files to fileserver tester.AssertGetResponse("http://localhost:"+testPort+"/static.txt", http.StatusOK, "Hello from file") - tester.AssertGetResponse("http://localhost:"+testPort+"/worker-path", http.StatusOK, "requests:1") + + // never serve PHP files directly + r, _ = http.NewRequest("GET", "http://localhost:"+testPort+"/index.php", nil) + tester.AssertResponseCode(r, http.StatusNotFound) } func TestWorkerRestart(t *testing.T) { diff --git a/caddy/module.go b/caddy/module.go index caf49d8a4..d0743bc51 100644 --- a/caddy/module.go +++ b/caddy/module.go @@ -71,30 +71,30 @@ func (f *FrankenPHPModule) Provision(ctx caddy.Context) error { for i, wc := range f.Workers { // Inherit environment variables from the parent php_server directive - if !filepath.IsAbs(wc.FileName) && f.Root != "" { - wc.FileName = filepath.Join(f.Root, wc.FileName) - } - - if f.Env != nil { - if wc.Env == nil { - wc.Env = make(map[string]string) - } - for k, v := range f.Env { - // Only set if not already defined in the worker - if _, exists := wc.Env[k]; !exists { - wc.Env[k] = v - } - } - } + if !filepath.IsAbs(wc.FileName) && f.Root != "" { + wc.FileName = filepath.Join(f.Root, wc.FileName) + } + + if f.Env != nil { + if wc.Env == nil { + wc.Env = make(map[string]string) + } + for k, v := range f.Env { + // Only set if not already defined in the worker + if _, exists := wc.Env[k]; !exists { + wc.Env[k] = v + } + } + } f.Workers[i] = wc - // Check if a worker with this filename already exists in this module - for j, existingWorker := range f.Workers { - if i != j && existingWorker.FileName == wc.FileName { - return fmt.Errorf(`workers in a single "php_server" block must not have duplicate filenames: %q`, wc.FileName) - } - } + // Check if a worker with this filename already exists in this module + for j, existingWorker := range f.Workers { + if i != j && existingWorker.FileName == wc.FileName { + return fmt.Errorf(`workers in a single "php_server" block must not have duplicate filenames: %q`, wc.FileName) + } + } } workers, err := fapp.addModuleWorkers(f.Workers...) @@ -192,7 +192,7 @@ func (f *FrankenPHPModule) ServeHTTP(w http.ResponseWriter, r *http.Request, _ c workerName := "" for _, w := range f.Workers { - if w.matchesPath(r.URL.Path, documentRoot) { + if w.matchesPath(r, documentRoot) { workerName = w.Name break } @@ -259,9 +259,9 @@ func (f *FrankenPHPModule) UnmarshalCaddyfile(d *caddyfile.Dispenser) error { case "worker": wc, err := parseWorkerConfig(d) - if err != nil { - return err - } + if err != nil { + return err + } f.Workers = append(f.Workers, wc) default: @@ -407,12 +407,12 @@ func parsePhpServer(h httpcaddyfile.Helper) ([]httpcaddyfile.ConfigValue, error) dispenser.Reset() // the rest of the config is specified by the user - // using the php directive syntax - dispenser.Next() // consume the directive name - err = phpsrv.UnmarshalCaddyfile(dispenser) - if err != nil { - return nil, err - } + // using the php directive syntax + dispenser.Next() // consume the directive name + err = phpsrv.UnmarshalCaddyfile(dispenser) + if err != nil { + return nil, err + } return routesForWorkerServer(h, phpsrv, fsrv) @@ -568,17 +568,17 @@ func parsePhpServer(h httpcaddyfile.Helper) ([]httpcaddyfile.ConfigValue, error) func routesForWorkerServer(h httpcaddyfile.Helper, frankenphpModule FrankenPHPModule, fsrv caddy.Module) ([]httpcaddyfile.ConfigValue, error) { return []httpcaddyfile.ConfigValue{ - { - Class: "route", - Value: workerSubroute(h, frankenphpModule, fsrv), - }, - }, nil + { + Class: "route", + Value: workerSubroute(h, frankenphpModule, fsrv), + }, + }, nil } // get the routing for php_worker // serve non-php file or fallback to index worker func workerSubroute(h httpcaddyfile.Helper, frankenphpModule FrankenPHPModule, fsrv caddy.Module) caddyhttp.Subroute { - matchers := caddyhttp.MatchPath{"*.php"} + matchers := caddyhttp.MatchPath{} for _, w := range frankenphpModule.Workers { if w.Match != "" { matchers = append(matchers, w.Match) @@ -597,7 +597,7 @@ func workerSubroute(h httpcaddyfile.Helper, frankenphpModule FrankenPHPModule, f "not": h.JSON(caddyhttp.MatchNot{ MatcherSetsRaw: []caddy.ModuleMap{ { - "path": h.JSON(matchers), + "path": h.JSON(caddyhttp.MatchPath{"*.php"}), }, }, }), @@ -610,18 +610,25 @@ func workerSubroute(h httpcaddyfile.Helper, frankenphpModule FrankenPHPModule, f }, } - for _, pattern := range matchers { - subRoute.Routes = append(subRoute.Routes, caddyhttp.Route{ - MatcherSetsRaw: []caddy.ModuleMap{ - caddy.ModuleMap{ - "path": h.JSON(caddyhttp.MatchPath{ pattern }), - }, - }, - HandlersRaw: []json.RawMessage{ - caddyconfig.JSONModuleObject(frankenphpModule, "handler", "php", nil), - }, - }) - } + for _, pattern := range matchers { + subRoute.Routes = append(subRoute.Routes, caddyhttp.Route{ + MatcherSetsRaw: []caddy.ModuleMap{ + caddy.ModuleMap{ + "path": h.JSON(caddyhttp.MatchPath{pattern}), + }, + }, + HandlersRaw: []json.RawMessage{ + caddyconfig.JSONModuleObject(frankenphpModule, "handler", "php", nil), + }, + }) + } + + subRoute.Routes = append(subRoute.Routes, caddyhttp.Route{ + MatcherSetsRaw: []caddy.ModuleMap{}, + HandlersRaw: []json.RawMessage{ + caddyconfig.JSONModuleObject(fsrv, "handler", "file_server", nil), + }, + }) return subRoute } diff --git a/caddy/workerconfig.go b/caddy/workerconfig.go index 47db675e7..6140bb210 100644 --- a/caddy/workerconfig.go +++ b/caddy/workerconfig.go @@ -2,10 +2,12 @@ package caddy import ( "errors" + "net/http" "path/filepath" "strconv" "github.com/caddyserver/caddy/v2/caddyconfig/caddyfile" + "github.com/caddyserver/caddy/v2/modules/caddyhttp" "github.com/dunglas/frankenphp" "github.com/dunglas/frankenphp/internal/fastabs" ) @@ -32,8 +34,21 @@ type workerConfig struct { Watch []string `json:"watch,omitempty"` // The path to match against the worker Match string `json:"match,omitempty"` + + matchStrategy matchStrategy } +type matchStrategy int + +const ( + matchNone matchStrategy = iota + matchAll + matchPrefix + matchSuffix + matchExactly + matchWildcard +) + func parseWorkerConfig(d *caddyfile.Dispenser) (workerConfig, error) { wc := workerConfig{} if d.NextArg() { @@ -102,6 +117,7 @@ func parseWorkerConfig(d *caddyfile.Dispenser) (workerConfig, error) { return wc, d.ArgErr() } wc.Match = d.Val() + default: allowedDirectives := "name, file, num, env, watch, match" return wc, wrongSubDirectiveError("worker", allowedDirectives, v) @@ -119,12 +135,13 @@ func parseWorkerConfig(d *caddyfile.Dispenser) (workerConfig, error) { return wc, nil } -func (wc workerConfig) matchesPath(path string, documentRoot string) bool { +func (wc workerConfig) matchesPath(r *http.Request, documentRoot string) bool { if wc.Match != "" { - return wc.Match == "*" || wc.Match == path + matches, _ := caddyhttp.MatchPath{wc.Match}.MatchWithError(r) + return matches } - fullScriptPath, _ := fastabs.FastAbs(documentRoot + "/" + path) + fullScriptPath, _ := fastabs.FastAbs(documentRoot + "/" + r.URL.Path) absFileName, _ := fastabs.FastAbs(wc.FileName) return fullScriptPath == absFileName } From 081327ef537035197cc2c6a71c708505f931df39 Mon Sep 17 00:00:00 2001 From: Alliballibaba Date: Wed, 11 Jun 2025 19:20:32 +0200 Subject: [PATCH 19/59] Adds no-fileserver test. --- caddy/caddy_test.go | 157 ++++++++++++++++++++++--------------- caddy/module.go | 104 ++++++++++++------------ testdata/files/static.txt2 | 1 + 3 files changed, 148 insertions(+), 114 deletions(-) create mode 100644 testdata/files/static.txt2 diff --git a/caddy/caddy_test.go b/caddy/caddy_test.go index b0c33d8e7..cbe2ddcf8 100644 --- a/caddy/caddy_test.go +++ b/caddy/caddy_test.go @@ -1216,70 +1216,6 @@ func TestDisabledMetrics(t *testing.T) { require.Zero(t, count, "metrics should be missing") } -func TestWorkerMatchDirective(t *testing.T) { - tester := caddytest.NewTester(t) - testport2 := "3000" - tester.InitServer(` - { - skip_install_trust - admin localhost:2999 - } - - http://localhost:`+testPort+` { - php_server { - root ../testdata/files - worker { - match /matched-path* - file ../worker-with-counter.php - num 1 - } - } - } - http://localhost:`+testport2+` { - php_server { - root ../testdata - worker { - match /counter* - file worker-with-counter.php - num 1 - } - worker { - match /index* - file index.php - num 1 - } - } - } - `, "caddyfile") - - // the first php_server with root in testdata/files - // Forward request that match * to the worker - tester.AssertGetResponse("http://localhost:"+testPort+"/matched-path", http.StatusOK, "requests:1") - tester.AssertGetResponse("http://localhost:"+testPort+"/matched-path/anywhere", http.StatusOK, "requests:2") - - // 404 on not matching paths - r, _ := http.NewRequest("GET", "http://localhost:"+testPort+"/not-matched-path", nil) - tester.AssertResponseCode(r, http.StatusNotFound) - - // forward files to fileserver - tester.AssertGetResponse("http://localhost:"+testPort+"/static.txt", http.StatusOK, "Hello from file") - - // the second php_server has root in testdata and has 2 different workers - tester.AssertGetResponse("http://localhost:"+testport2+"/counter/sub-path", http.StatusOK, "requests:1") - tester.AssertGetResponse("http://localhost:"+testport2+"/index/sub-path", http.StatusOK, "I am by birth a Genevese (i not set)") - - // 404 on not matching path - r, _ = http.NewRequest("GET", "http://localhost:"+testPort+"/other", nil) - tester.AssertResponseCode(r, http.StatusNotFound) - - // forward files to fileserver - tester.AssertGetResponse("http://localhost:"+testPort+"/static.txt", http.StatusOK, "Hello from file") - - // never serve PHP files directly - r, _ = http.NewRequest("GET", "http://localhost:"+testPort+"/index.php", nil) - tester.AssertResponseCode(r, http.StatusNotFound) -} - func TestWorkerRestart(t *testing.T) { var wg sync.WaitGroup tester := caddytest.NewTester(t) @@ -1380,3 +1316,96 @@ func TestWorkerRestart(t *testing.T) { "frankenphp_worker_restarts", )) } + +func TestWorkerMatchDirectiveWithFileServer(t *testing.T) { + tester := caddytest.NewTester(t) + testport2 := "3000" + tester.InitServer(` + { + skip_install_trust + admin localhost:2999 + } + + http://localhost:`+testPort+` { + php_server { + root ../testdata/files + worker { + match /matched-path* + file ../worker-with-counter.php + num 1 + } + } + } + http://localhost:`+testport2+` { + php_server { + root ../testdata + worker { + match /counter* + file worker-with-counter.php + num 1 + } + worker { + match /index* + file index.php + num 1 + } + } + } + `, "caddyfile") + + // the first php_server with root in testdata/files + // Forward request that match * to the worker + tester.AssertGetResponse("http://localhost:"+testPort+"/matched-path", http.StatusOK, "requests:1") + tester.AssertGetResponse("http://localhost:"+testPort+"/matched-path/anywhere", http.StatusOK, "requests:2") + + // 404 on not matching paths + r, _ := http.NewRequest("GET", "http://localhost:"+testPort+"/not-matched-path", nil) + tester.AssertResponseCode(r, http.StatusNotFound) + + // forward files to fileserver + tester.AssertGetResponse("http://localhost:"+testPort+"/static.txt2", http.StatusOK, "Hello from file") + + // the second php_server has root in testdata and has 2 different workers + tester.AssertGetResponse("http://localhost:"+testport2+"/counter/sub-path", http.StatusOK, "requests:1") + tester.AssertGetResponse("http://localhost:"+testport2+"/index/sub-path", http.StatusOK, "I am by birth a Genevese (i not set)") + + // 404 on not matching path + r, _ = http.NewRequest("GET", "http://localhost:"+testPort+"/other", nil) + tester.AssertResponseCode(r, http.StatusNotFound) + + // forward files to fileserver + tester.AssertGetResponse("http://localhost:"+testPort+"/static.txt2", http.StatusOK, "Hello from file") + + // never serve PHP files directly + r, _ = http.NewRequest("GET", "http://localhost:"+testPort+"/index.php", nil) + tester.AssertResponseCode(r, http.StatusNotFound) +} + +func TestWorkerMatchDirectiveWithoutFileServer(t *testing.T) { + tester := caddytest.NewTester(t) + tester.InitServer(` + { + skip_install_trust + admin localhost:2999 + } + + http://localhost:`+testPort+` { + php_server { + file_server off + root ../testdata/files + worker { + match /some-path + file ../worker-with-counter.php + num 1 + } + } + } + `, "caddyfile") + + // find the worker at some-path + tester.AssertGetResponse("http://localhost:"+testPort+"/some-path", http.StatusOK, "requests:1") + + // do not find the file at static.txt2 + r, _ := http.NewRequest("GET", "http://localhost:"+testPort+"/static.txt2", nil) + tester.AssertResponseCode(r, http.StatusNotFound) +} diff --git a/caddy/module.go b/caddy/module.go index d0743bc51..817c02071 100644 --- a/caddy/module.go +++ b/caddy/module.go @@ -86,15 +86,7 @@ func (f *FrankenPHPModule) Provision(ctx caddy.Context) error { } } } - f.Workers[i] = wc - - // Check if a worker with this filename already exists in this module - for j, existingWorker := range f.Workers { - if i != j && existingWorker.FileName == wc.FileName { - return fmt.Errorf(`workers in a single "php_server" block must not have duplicate filenames: %q`, wc.FileName) - } - } } workers, err := fapp.addModuleWorkers(f.Workers...) @@ -271,6 +263,15 @@ func (f *FrankenPHPModule) UnmarshalCaddyfile(d *caddyfile.Dispenser) error { } } + // Check if a worker with this filename already exists in this module + for j, w1 := range f.Workers { + for i, w2 := range f.Workers { + if i != j && w1.FileName == w2.FileName { + return fmt.Errorf(`workers in a single "php_server" block must not have duplicate filenames: %q`, w1.FileName) + } + } + } + return nil } @@ -414,8 +415,6 @@ func parsePhpServer(h httpcaddyfile.Helper) ([]httpcaddyfile.ConfigValue, error) return nil, err } - return routesForWorkerServer(h, phpsrv, fsrv) - if frankenphp.EmbeddedAppPath != "" { if phpsrv.Root == "" { phpsrv.Root = filepath.Join(frankenphp.EmbeddedAppPath, defaultDocumentRoot) @@ -428,6 +427,13 @@ func parsePhpServer(h httpcaddyfile.Helper) ([]httpcaddyfile.ConfigValue, error) } } + // if any worker has 'match' specified, use an alternate routing approach + for _, w := range phpsrv.Workers { + if w.Match != "" { + return pureWorkerRoute(h, phpsrv, fsrv, disableFsrv), nil + } + } + // set up a route list that we'll append to routes := caddyhttp.RouteList{} @@ -566,69 +572,67 @@ func parsePhpServer(h httpcaddyfile.Helper) ([]httpcaddyfile.ConfigValue, error) }, nil } -func routesForWorkerServer(h httpcaddyfile.Helper, frankenphpModule FrankenPHPModule, fsrv caddy.Module) ([]httpcaddyfile.ConfigValue, error) { - return []httpcaddyfile.ConfigValue{ - { - Class: "route", - Value: workerSubroute(h, frankenphpModule, fsrv), - }, - }, nil -} - -// get the routing for php_worker -// serve non-php file or fallback to index worker -func workerSubroute(h httpcaddyfile.Helper, frankenphpModule FrankenPHPModule, fsrv caddy.Module) caddyhttp.Subroute { +// if any worker under php_server has a match pattern, +// routes have to be created differently +// first, we will try to match against the file system, +// then we will try to match aganist each worker +func pureWorkerRoute(h httpcaddyfile.Helper, f FrankenPHPModule, fsrv caddy.Module, disableFsrv bool) []httpcaddyfile.ConfigValue { + f.SplitPath = []string{} matchers := caddyhttp.MatchPath{} - for _, w := range frankenphpModule.Workers { + for _, w := range f.Workers { if w.Match != "" { matchers = append(matchers, w.Match) } } - subRoute := caddyhttp.Subroute{ - Routes: caddyhttp.RouteList{ - caddyhttp.Route{ - MatcherSetsRaw: []caddy.ModuleMap{ - caddy.ModuleMap{ - "file": h.JSON(fileserver.MatchFile{ - TryFiles: []string{"{http.request.uri.path}"}, - Root: frankenphpModule.Root, - }), - "not": h.JSON(caddyhttp.MatchNot{ - MatcherSetsRaw: []caddy.ModuleMap{ - { - "path": h.JSON(caddyhttp.MatchPath{"*.php"}), - }, - }, - }), - }, - }, - HandlersRaw: []json.RawMessage{ - caddyconfig.JSONModuleObject(fsrv, "handler", "file_server", nil), + subRoute := caddyhttp.Subroute{Routes: caddyhttp.RouteList{}} + + if !disableFsrv { + subRoute.Routes = append(subRoute.Routes, caddyhttp.Route{ + MatcherSetsRaw: []caddy.ModuleMap{ + caddy.ModuleMap{ + "file": h.JSON(fileserver.MatchFile{ + TryFiles: []string{"{http.request.uri.path}"}, + Root: f.Root, + }), + "not": h.JSON(caddyhttp.MatchNot{ + MatcherSetsRaw: []caddy.ModuleMap{ + {"path": h.JSON(caddyhttp.MatchPath{"*.php"})}, + }, + }), }, }, - }, + HandlersRaw: []json.RawMessage{ + caddyconfig.JSONModuleObject(fsrv, "handler", "file_server", nil), + }, + }) } for _, pattern := range matchers { subRoute.Routes = append(subRoute.Routes, caddyhttp.Route{ MatcherSetsRaw: []caddy.ModuleMap{ - caddy.ModuleMap{ - "path": h.JSON(caddyhttp.MatchPath{pattern}), - }, + caddy.ModuleMap{"path": h.JSON(caddyhttp.MatchPath{pattern})}, }, HandlersRaw: []json.RawMessage{ - caddyconfig.JSONModuleObject(frankenphpModule, "handler", "php", nil), + caddyconfig.JSONModuleObject(f, "handler", "php", nil), }, }) } + notFoundHandler := caddyhttp.StaticResponse{ + StatusCode: caddyhttp.WeakString(strconv.Itoa(http.StatusNotFound)), + } subRoute.Routes = append(subRoute.Routes, caddyhttp.Route{ MatcherSetsRaw: []caddy.ModuleMap{}, HandlersRaw: []json.RawMessage{ - caddyconfig.JSONModuleObject(fsrv, "handler", "file_server", nil), + caddyconfig.JSONModuleObject(notFoundHandler, "handler", "static_response", nil), }, }) - return subRoute + return []httpcaddyfile.ConfigValue{ + { + Class: "route", + Value: subRoute, + }, + } } diff --git a/testdata/files/static.txt2 b/testdata/files/static.txt2 new file mode 100644 index 000000000..09be5ef4e --- /dev/null +++ b/testdata/files/static.txt2 @@ -0,0 +1 @@ +Hello from file \ No newline at end of file From b5a1f76b8a399b0804562f787dcdcaaf35a40b92 Mon Sep 17 00:00:00 2001 From: Alliballibaba Date: Wed, 11 Jun 2025 19:41:01 +0200 Subject: [PATCH 20/59] Prevents duplicate path calculations and optimizes worker access. --- context.go | 14 ++++++-- frankenphp.go | 4 +-- phpmainthread_test.go | 10 +++--- request_options.go | 4 ++- scaling.go | 4 +-- scaling_test.go | 2 +- worker.go | 84 +++++++++++++++++++++++++------------------ 7 files changed, 73 insertions(+), 49 deletions(-) diff --git a/context.go b/context.go index 29ed40af3..6de140da7 100644 --- a/context.go +++ b/context.go @@ -17,12 +17,12 @@ type frankenPHPContext struct { logger *slog.Logger request *http.Request originalRequest *http.Request + worker *worker docURI string pathInfo string scriptName string scriptFilename string - workerName string // Whether the request is already closed by us isDone bool @@ -89,8 +89,16 @@ func NewRequestWithContext(r *http.Request, opts ...RequestOption) (*http.Reques } } - // SCRIPT_FILENAME is the absolute path of SCRIPT_NAME - fc.scriptFilename = sanitizedPathJoin(fc.documentRoot, fc.scriptName) + // if a worker is assigned explicitly, use its filename + // determine if the filename belongs to a worker otherwise + if fc.worker != nil { + fc.scriptFilename = fc.worker.fileName + } else { + // SCRIPT_FILENAME is the absolute path of SCRIPT_NAME + fc.scriptFilename = sanitizedPathJoin(fc.documentRoot, fc.scriptName) + fc.worker = getWorkerByPath(fc.scriptFilename) + } + c := context.WithValue(r.Context(), contextKey, fc) return r.WithContext(c), nil diff --git a/frankenphp.go b/frankenphp.go index 37fb23678..fb30838a1 100644 --- a/frankenphp.go +++ b/frankenphp.go @@ -403,8 +403,8 @@ func ServeHTTP(responseWriter http.ResponseWriter, request *http.Request) error } // Detect if a worker is available to handle this request - if worker, ok := workers[getWorkerKey(fc.workerName, fc.scriptFilename)]; ok { - worker.handleRequest(fc) + if fc.worker != nil { + fc.worker.handleRequest(fc) return nil } diff --git a/phpmainthread_test.go b/phpmainthread_test.go index 08020ea62..1a4d5ba25 100644 --- a/phpmainthread_test.go +++ b/phpmainthread_test.go @@ -181,7 +181,7 @@ func TestFinishBootingAWorkerScript(t *testing.T) { } func TestReturnAnErrorIf2WorkersHaveTheSameFileName(t *testing.T) { - workers = make(map[string]*worker) + workers = []*worker{} _, err1 := newWorker(workerOpt{fileName: "filename.php"}) _, err2 := newWorker(workerOpt{fileName: "filename.php"}) @@ -190,7 +190,7 @@ func TestReturnAnErrorIf2WorkersHaveTheSameFileName(t *testing.T) { } func TestReturnAnErrorIf2ModuleWorkersHaveTheSameName(t *testing.T) { - workers = make(map[string]*worker) + workers = []*worker{} _, err1 := newWorker(workerOpt{fileName: "filename.php", name: "workername"}) _, err2 := newWorker(workerOpt{fileName: "filename2.php", name: "workername"}) @@ -200,7 +200,7 @@ func TestReturnAnErrorIf2ModuleWorkersHaveTheSameName(t *testing.T) { func getDummyWorker(fileName string) *worker { if workers == nil { - workers = make(map[string]*worker) + workers = []*worker{} } worker, _ := newWorker(workerOpt{ fileName: testDataPath + "/" + fileName, @@ -232,9 +232,9 @@ func allPossibleTransitions(worker1Path string, worker2Path string) []func(*phpT thread.boot() } }, - func(thread *phpThread) { convertToWorkerThread(thread, workers[worker1Path]) }, + func(thread *phpThread) { convertToWorkerThread(thread, getWorkerByPath(worker1Path)) }, convertToInactiveThread, - func(thread *phpThread) { convertToWorkerThread(thread, workers[worker2Path]) }, + func(thread *phpThread) { convertToWorkerThread(thread, getWorkerByPath(worker2Path)) }, convertToInactiveThread, } } diff --git a/request_options.go b/request_options.go index 6c1cb5949..2227b06ee 100644 --- a/request_options.go +++ b/request_options.go @@ -127,7 +127,9 @@ func WithRequestLogger(logger *slog.Logger) RequestOption { // WithWorkerName sets the worker that should handle the request func WithWorkerName(name string) RequestOption { return func(o *frankenPHPContext) error { - o.workerName = name + if name != "" { + o.worker = getWorkerByName(name) + } return nil } diff --git a/scaling.go b/scaling.go index ca319a177..d462b4a54 100644 --- a/scaling.go +++ b/scaling.go @@ -154,8 +154,8 @@ func startUpscalingThreads(maxScaledThreads int, scale chan *frankenPHPContext, } // if the request has been stalled long enough, scale - if worker, ok := workers[getWorkerKey(fc.workerName, fc.scriptFilename)]; ok { - scaleWorkerThread(worker) + if fc.worker != nil { + scaleWorkerThread(fc.worker) } else { scaleRegularThread() } diff --git a/scaling_test.go b/scaling_test.go index 85adc5863..eeca51ca2 100644 --- a/scaling_test.go +++ b/scaling_test.go @@ -44,7 +44,7 @@ func TestScaleAWorkerThreadUpAndDown(t *testing.T) { autoScaledThread := phpThreads[2] // scale up - scaleWorkerThread(workers[workerPath]) + scaleWorkerThread(getWorkerByPath(workerPath)) assert.Equal(t, stateReady, autoScaledThread.state.get()) // on down-scale, the thread will be marked as inactive diff --git a/worker.go b/worker.go index 3c9455b77..e36458d9a 100644 --- a/worker.go +++ b/worker.go @@ -14,33 +14,36 @@ import ( // represents a worker script and can have many threads assigned to it type worker struct { - name string - fileName string - num int - env PreparedEnv - requestChan chan *frankenPHPContext - threads []*phpThread - threadMutex sync.RWMutex + name string + fileName string + num int + env PreparedEnv + requestChan chan *frankenPHPContext + threads []*phpThread + threadMutex sync.RWMutex + allowPathMatching bool // allow matching a request to a worker via path } var ( - workers map[string]*worker + workers []*worker watcherIsEnabled bool ) func initWorkers(opt []workerOpt) error { - workers = make(map[string]*worker, len(opt)) + workers = make([]*worker, 0, len(opt)) workersReady := sync.WaitGroup{} directoriesToWatch := getDirectoriesToWatch(opt) watcherIsEnabled = len(directoriesToWatch) > 0 for _, o := range opt { - worker, err := newWorker(o) + _, err := newWorker(o) if err != nil { return err } + } - workersReady.Add(o.num) + for _, worker := range workers { + workersReady.Add(worker.num) for i := 0; i < worker.num; i++ { thread := getInactivePHPThread() convertToWorkerThread(thread, worker) @@ -65,48 +68,59 @@ func initWorkers(opt []workerOpt) error { return nil } -func getWorkerKey(name string, filename string) string { - key := filename - if strings.HasPrefix(name, "m#") { - key = name +func getWorkerByName(name string) *worker { + for _, w := range workers { + if w.name == name { + return w + } } - return key + + return nil +} + +func getWorkerByPath(path string) *worker { + for _, w := range workers { + if w.fileName == path && w.allowPathMatching { + return w + } + } + + return nil } func newWorker(o workerOpt) (*worker, error) { absFileName, err := fastabs.FastAbs(o.fileName) + isModuleWorker := strings.HasPrefix(o.name, "m#") // module workers may not be matched by path if err != nil { return nil, fmt.Errorf("worker filename is invalid %q: %w", o.fileName, err) } - key := getWorkerKey(o.name, absFileName) - if _, ok := workers[key]; ok { - return nil, fmt.Errorf("two workers cannot use the same key %q", key) + if o.name == "" { + o.name = absFileName } - for _, w := range workers { - if w.name == o.name { - return w, fmt.Errorf("two workers cannot have the same name: %q", o.name) - } + + if w := getWorkerByPath(absFileName); w != nil && !isModuleWorker { + return w, fmt.Errorf("two workers cannot have the same filename: %q", absFileName) + } + if w := getWorkerByName(o.name); w != nil { + return w, fmt.Errorf("two workers cannot have the same name: %q", o.name) } if o.env == nil { o.env = make(PreparedEnv, 1) } - if o.name == "" { - o.name = absFileName - } - o.env["FRANKENPHP_WORKER\x00"] = "1" w := &worker{ - name: o.name, - fileName: absFileName, - num: o.num, - env: o.env, - requestChan: make(chan *frankenPHPContext), - threads: make([]*phpThread, 0, o.num), - } - workers[key] = w + name: o.name, + fileName: absFileName, + num: o.num, + env: o.env, + requestChan: make(chan *frankenPHPContext), + threads: make([]*phpThread, 0, o.num), + allowPathMatching: !isModuleWorker, + } + workers = append(workers, w) return w, nil } From 6de13d41bae450ddb8e36d094ae0f9bc381c1ab8 Mon Sep 17 00:00:00 2001 From: Alliballibaba Date: Wed, 11 Jun 2025 20:16:03 +0200 Subject: [PATCH 21/59] trigger From b9adf6323303fa6e06676afe90764c1c6284f641 Mon Sep 17 00:00:00 2001 From: Alliballibaba Date: Wed, 11 Jun 2025 20:53:54 +0200 Subject: [PATCH 22/59] Changes worker->match to match->worker --- caddy/caddy_test.go | 12 ++++-------- caddy/module.go | 19 ++++++++++++++++++- caddy/workerconfig.go | 7 +------ 3 files changed, 23 insertions(+), 15 deletions(-) diff --git a/caddy/caddy_test.go b/caddy/caddy_test.go index cbe2ddcf8..ceab1d283 100644 --- a/caddy/caddy_test.go +++ b/caddy/caddy_test.go @@ -1329,8 +1329,7 @@ func TestWorkerMatchDirectiveWithFileServer(t *testing.T) { http://localhost:`+testPort+` { php_server { root ../testdata/files - worker { - match /matched-path* + match /matched-path* worker { file ../worker-with-counter.php num 1 } @@ -1339,13 +1338,11 @@ func TestWorkerMatchDirectiveWithFileServer(t *testing.T) { http://localhost:`+testport2+` { php_server { root ../testdata - worker { - match /counter* + match /counter* worker { file worker-with-counter.php num 1 } - worker { - match /index* + match /index* worker { file index.php num 1 } @@ -1393,8 +1390,7 @@ func TestWorkerMatchDirectiveWithoutFileServer(t *testing.T) { php_server { file_server off root ../testdata/files - worker { - match /some-path + match /some-path worker { file ../worker-with-counter.php num 1 } diff --git a/caddy/module.go b/caddy/module.go index 817c02071..2ff43d9e3 100644 --- a/caddy/module.go +++ b/caddy/module.go @@ -255,9 +255,26 @@ func (f *FrankenPHPModule) UnmarshalCaddyfile(d *caddyfile.Dispenser) error { return err } f.Workers = append(f.Workers, wc) + case "match": + if !d.NextArg() { + return d.ArgErr() + } + match := "*" + if d.Val() != "worker" { + match = d.Val() + if !d.NextArg() || d.Val() != "worker" { + return d.ArgErr() + } + } + wc, err := parseWorkerConfig(d) + if err != nil { + return err + } + wc.Match = match + f.Workers = append(f.Workers, wc) default: - allowedDirectives := "root, split, env, resolve_root_symlink, worker" + allowedDirectives := "root, split, env, resolve_root_symlink, worker, match" return wrongSubDirectiveError("php or php_server", allowedDirectives, d.Val()) } } diff --git a/caddy/workerconfig.go b/caddy/workerconfig.go index 6140bb210..f52fee4ed 100644 --- a/caddy/workerconfig.go +++ b/caddy/workerconfig.go @@ -112,14 +112,9 @@ func parseWorkerConfig(d *caddyfile.Dispenser) (workerConfig, error) { } else { wc.Watch = append(wc.Watch, d.Val()) } - case "match": - if !d.NextArg() { - return wc, d.ArgErr() - } - wc.Match = d.Val() default: - allowedDirectives := "name, file, num, env, watch, match" + allowedDirectives := "name, file, num, env, watch" return wc, wrongSubDirectiveError("worker", allowedDirectives, v) } } From 847f4d55931349b92cb38c05bbd8ddafa9c62447 Mon Sep 17 00:00:00 2001 From: Alliballibaba Date: Wed, 11 Jun 2025 21:00:53 +0200 Subject: [PATCH 23/59] Adjusts tests. --- caddy/caddy_test.go | 23 +++++++++++------------ 1 file changed, 11 insertions(+), 12 deletions(-) diff --git a/caddy/caddy_test.go b/caddy/caddy_test.go index ceab1d283..629b59d39 100644 --- a/caddy/caddy_test.go +++ b/caddy/caddy_test.go @@ -1338,11 +1338,11 @@ func TestWorkerMatchDirectiveWithFileServer(t *testing.T) { http://localhost:`+testport2+` { php_server { root ../testdata - match /counter* worker { + match /counter/* worker { file worker-with-counter.php num 1 } - match /index* worker { + match /index/* worker { file index.php num 1 } @@ -1350,31 +1350,30 @@ func TestWorkerMatchDirectiveWithFileServer(t *testing.T) { } `, "caddyfile") - // the first php_server with root in testdata/files - // Forward request that match * to the worker + // worker is outside of public directory, match anyways tester.AssertGetResponse("http://localhost:"+testPort+"/matched-path", http.StatusOK, "requests:1") tester.AssertGetResponse("http://localhost:"+testPort+"/matched-path/anywhere", http.StatusOK, "requests:2") - // 404 on not matching paths + // 404 on no matching paths r, _ := http.NewRequest("GET", "http://localhost:"+testPort+"/not-matched-path", nil) tester.AssertResponseCode(r, http.StatusNotFound) - // forward files to fileserver + // static file will be served by fileserver tester.AssertGetResponse("http://localhost:"+testPort+"/static.txt2", http.StatusOK, "Hello from file") - // the second php_server has root in testdata and has 2 different workers + // the second php_server 2 different workers in the public path tester.AssertGetResponse("http://localhost:"+testport2+"/counter/sub-path", http.StatusOK, "requests:1") tester.AssertGetResponse("http://localhost:"+testport2+"/index/sub-path", http.StatusOK, "I am by birth a Genevese (i not set)") - // 404 on not matching path - r, _ = http.NewRequest("GET", "http://localhost:"+testPort+"/other", nil) + // 404 on no matching paths + r, _ = http.NewRequest("GET", "http://localhost:"+testport2+"/other", nil) tester.AssertResponseCode(r, http.StatusNotFound) - // forward files to fileserver - tester.AssertGetResponse("http://localhost:"+testPort+"/static.txt2", http.StatusOK, "Hello from file") + // static file will be served by fileserver + tester.AssertGetResponse("http://localhost:"+testport2+"/files/static.txt2", http.StatusOK, "Hello from file") // never serve PHP files directly - r, _ = http.NewRequest("GET", "http://localhost:"+testPort+"/index.php", nil) + r, _ = http.NewRequest("GET", "http://localhost:"+testport2+"/index.php", nil) tester.AssertResponseCode(r, http.StatusNotFound) } From 9b927251f0f203525f79eef5b28fafa6c9c8bf53 Mon Sep 17 00:00:00 2001 From: Alliballibaba Date: Wed, 11 Jun 2025 21:05:07 +0200 Subject: [PATCH 24/59] formatting --- caddy/caddy_test.go | 30 +++++++++++++++--------------- 1 file changed, 15 insertions(+), 15 deletions(-) diff --git a/caddy/caddy_test.go b/caddy/caddy_test.go index 629b59d39..0941ed7f0 100644 --- a/caddy/caddy_test.go +++ b/caddy/caddy_test.go @@ -1336,18 +1336,18 @@ func TestWorkerMatchDirectiveWithFileServer(t *testing.T) { } } http://localhost:`+testport2+` { - php_server { - root ../testdata - match /counter/* worker { - file worker-with-counter.php - num 1 - } + php_server { + root ../testdata + match /counter/* worker { + file worker-with-counter.php + num 1 + } match /index/* worker { - file index.php - num 1 - } - } - } + file index.php + num 1 + } + } + } `, "caddyfile") // worker is outside of public directory, match anyways @@ -1358,10 +1358,10 @@ func TestWorkerMatchDirectiveWithFileServer(t *testing.T) { r, _ := http.NewRequest("GET", "http://localhost:"+testPort+"/not-matched-path", nil) tester.AssertResponseCode(r, http.StatusNotFound) - // static file will be served by fileserver + // static file will be served by the fileserver tester.AssertGetResponse("http://localhost:"+testPort+"/static.txt2", http.StatusOK, "Hello from file") - // the second php_server 2 different workers in the public path + // the second php_server has 2 different workers in the public path tester.AssertGetResponse("http://localhost:"+testport2+"/counter/sub-path", http.StatusOK, "requests:1") tester.AssertGetResponse("http://localhost:"+testport2+"/index/sub-path", http.StatusOK, "I am by birth a Genevese (i not set)") @@ -1369,10 +1369,10 @@ func TestWorkerMatchDirectiveWithFileServer(t *testing.T) { r, _ = http.NewRequest("GET", "http://localhost:"+testport2+"/other", nil) tester.AssertResponseCode(r, http.StatusNotFound) - // static file will be served by fileserver + // static file will be served by the fileserver tester.AssertGetResponse("http://localhost:"+testport2+"/files/static.txt2", http.StatusOK, "Hello from file") - // never serve PHP files directly + // 404 on PHP files r, _ = http.NewRequest("GET", "http://localhost:"+testport2+"/index.php", nil) tester.AssertResponseCode(r, http.StatusNotFound) } From 570069bfb033627126726f7d0e4f53fb31009e66 Mon Sep 17 00:00:00 2001 From: Alliballibaba Date: Fri, 13 Jun 2025 08:52:59 +0200 Subject: [PATCH 25/59] Resets implementation to worker->match --- caddy/caddy_test.go | 87 +++++++++++++--------- caddy/module.go | 81 ++++++-------------- caddy/workerconfig.go | 30 +++----- testdata/files/.gitignore | 2 +- testdata/files/{static.txt2 => static.txt} | 0 testdata/hello.php | 3 + 6 files changed, 90 insertions(+), 113 deletions(-) rename testdata/files/{static.txt2 => static.txt} (100%) create mode 100644 testdata/hello.php diff --git a/caddy/caddy_test.go b/caddy/caddy_test.go index 0941ed7f0..c67e363e7 100644 --- a/caddy/caddy_test.go +++ b/caddy/caddy_test.go @@ -1317,9 +1317,8 @@ func TestWorkerRestart(t *testing.T) { )) } -func TestWorkerMatchDirectiveWithFileServer(t *testing.T) { +func TestWorkerMatchDirective(t *testing.T) { tester := caddytest.NewTester(t) - testport2 := "3000" tester.InitServer(` { skip_install_trust @@ -1329,52 +1328,65 @@ func TestWorkerMatchDirectiveWithFileServer(t *testing.T) { http://localhost:`+testPort+` { php_server { root ../testdata/files - match /matched-path* worker { + worker { file ../worker-with-counter.php + match /matched-path* num 1 } } } - http://localhost:`+testport2+` { + `, "caddyfile") + + // worker is outside of public directory, match anyways + tester.AssertGetResponse("http://localhost:"+testPort+"/matched-path", http.StatusOK, "requests:1") + tester.AssertGetResponse("http://localhost:"+testPort+"/matched-path/anywhere", http.StatusOK, "requests:2") + + // 404 on unmatched paths + tester.AssertGetResponse("http://localhost:"+testPort+"/elsewhere", http.StatusNotFound, "") + + // file will be serverd by file_server + tester.AssertGetResponse("http://localhost:"+testPort+"/static.txt", http.StatusOK, "Hello from file") +} + +func TestWorkerMatchDirectiveWithMultipleWorkers(t *testing.T) { + tester := caddytest.NewTester(t) + tester.InitServer(` + { + skip_install_trust + admin localhost:2999 + } + http://localhost:`+testPort+` { php_server { root ../testdata - match /counter/* worker { + worker { file worker-with-counter.php + match /counter/* num 1 } - match /index/* worker { + worker { file index.php + match /index/* num 1 } } } `, "caddyfile") - // worker is outside of public directory, match anyways - tester.AssertGetResponse("http://localhost:"+testPort+"/matched-path", http.StatusOK, "requests:1") - tester.AssertGetResponse("http://localhost:"+testPort+"/matched-path/anywhere", http.StatusOK, "requests:2") - - // 404 on no matching paths - r, _ := http.NewRequest("GET", "http://localhost:"+testPort+"/not-matched-path", nil) - tester.AssertResponseCode(r, http.StatusNotFound) + // match 2 workers respectively (in the public directory) + tester.AssertGetResponse("http://localhost:"+testPort+"/counter/sub-path", http.StatusOK, "requests:1") + tester.AssertGetResponse("http://localhost:"+testPort+"/index/sub-path", http.StatusOK, "I am by birth a Genevese (i not set)") // static file will be served by the fileserver - tester.AssertGetResponse("http://localhost:"+testPort+"/static.txt2", http.StatusOK, "Hello from file") - - // the second php_server has 2 different workers in the public path - tester.AssertGetResponse("http://localhost:"+testport2+"/counter/sub-path", http.StatusOK, "requests:1") - tester.AssertGetResponse("http://localhost:"+testport2+"/index/sub-path", http.StatusOK, "I am by birth a Genevese (i not set)") + tester.AssertGetResponse("http://localhost:"+testPort+"/files/static.txt", http.StatusOK, "Hello from file") - // 404 on no matching paths - r, _ = http.NewRequest("GET", "http://localhost:"+testport2+"/other", nil) - tester.AssertResponseCode(r, http.StatusNotFound) + // 404 if the request falls through + tester.AssertGetResponse("http://localhost:"+testPort+"/not-matched", http.StatusNotFound, "") - // static file will be served by the fileserver - tester.AssertGetResponse("http://localhost:"+testport2+"/files/static.txt2", http.StatusOK, "Hello from file") + // serve php file directly as fallback + tester.AssertGetResponse("http://localhost:"+testPort+"/hello.php", http.StatusOK, "Hello") - // 404 on PHP files - r, _ = http.NewRequest("GET", "http://localhost:"+testport2+"/index.php", nil) - tester.AssertResponseCode(r, http.StatusNotFound) + // serve worker file directly as fallback + tester.AssertGetResponse("http://localhost:"+testPort+"/index.php", http.StatusOK, "I am by birth a Genevese (i not set)") } func TestWorkerMatchDirectiveWithoutFileServer(t *testing.T) { @@ -1386,13 +1398,18 @@ func TestWorkerMatchDirectiveWithoutFileServer(t *testing.T) { } http://localhost:`+testPort+` { - php_server { - file_server off - root ../testdata/files - match /some-path worker { - file ../worker-with-counter.php - num 1 + route { + php_server { + index off + file_server off + root ../testdata/files + worker { + file ../worker-with-counter.php + match /some-path + } } + + respond "Request falls through" 404 } } `, "caddyfile") @@ -1400,7 +1417,7 @@ func TestWorkerMatchDirectiveWithoutFileServer(t *testing.T) { // find the worker at some-path tester.AssertGetResponse("http://localhost:"+testPort+"/some-path", http.StatusOK, "requests:1") - // do not find the file at static.txt2 - r, _ := http.NewRequest("GET", "http://localhost:"+testPort+"/static.txt2", nil) - tester.AssertResponseCode(r, http.StatusNotFound) + // do not find the file at static.txt + // the request should completely fall through the php_server module + tester.AssertGetResponse("http://localhost:"+testPort+"/static.txt", http.StatusNotFound, "Request falls through") } diff --git a/caddy/module.go b/caddy/module.go index 2ff43d9e3..52c69f7c7 100644 --- a/caddy/module.go +++ b/caddy/module.go @@ -255,26 +255,9 @@ func (f *FrankenPHPModule) UnmarshalCaddyfile(d *caddyfile.Dispenser) error { return err } f.Workers = append(f.Workers, wc) - case "match": - if !d.NextArg() { - return d.ArgErr() - } - match := "*" - if d.Val() != "worker" { - match = d.Val() - if !d.NextArg() || d.Val() != "worker" { - return d.ArgErr() - } - } - wc, err := parseWorkerConfig(d) - if err != nil { - return err - } - wc.Match = match - f.Workers = append(f.Workers, wc) default: - allowedDirectives := "root, split, env, resolve_root_symlink, worker, match" + allowedDirectives := "root, split, env, resolve_root_symlink, worker" return wrongSubDirectiveError("php or php_server", allowedDirectives, d.Val()) } } @@ -444,16 +427,12 @@ func parsePhpServer(h httpcaddyfile.Helper) ([]httpcaddyfile.ConfigValue, error) } } - // if any worker has 'match' specified, use an alternate routing approach - for _, w := range phpsrv.Workers { - if w.Match != "" { - return pureWorkerRoute(h, phpsrv, fsrv, disableFsrv), nil - } - } - // set up a route list that we'll append to routes := caddyhttp.RouteList{} + // prepend routes from the 'worker match *' directives + routes = prependWorkerRoutes(routes, h, phpsrv, fsrv, disableFsrv) + // set the list of allowed path segments on which to split phpsrv.SplitPath = extensions @@ -589,23 +568,23 @@ func parsePhpServer(h httpcaddyfile.Helper) ([]httpcaddyfile.ConfigValue, error) }, nil } -// if any worker under php_server has a match pattern, -// routes have to be created differently -// first, we will try to match against the file system, -// then we will try to match aganist each worker -func pureWorkerRoute(h httpcaddyfile.Helper, f FrankenPHPModule, fsrv caddy.Module, disableFsrv bool) []httpcaddyfile.ConfigValue { - f.SplitPath = []string{} - matchers := caddyhttp.MatchPath{} +// workers can also match a path without being in the public directory +// in this case we need to prepend the worker routes to the existing routes +func prependWorkerRoutes(routes caddyhttp.RouteList, h httpcaddyfile.Helper, f FrankenPHPModule, fsrv caddy.Module, disableFsrv bool) caddyhttp.RouteList { + matchPath := caddyhttp.MatchPath{} for _, w := range f.Workers { - if w.Match != "" { - matchers = append(matchers, w.Match) + if w.MatchPath != "" { + matchPath = append(matchPath, w.MatchPath) } } - subRoute := caddyhttp.Subroute{Routes: caddyhttp.RouteList{}} + if len(matchPath) == 0 { + return routes + } + // if there are match patterns, we need to check for files beforehand if !disableFsrv { - subRoute.Routes = append(subRoute.Routes, caddyhttp.Route{ + routes = append(routes, caddyhttp.Route{ MatcherSetsRaw: []caddy.ModuleMap{ caddy.ModuleMap{ "file": h.JSON(fileserver.MatchFile{ @@ -625,31 +604,15 @@ func pureWorkerRoute(h httpcaddyfile.Helper, f FrankenPHPModule, fsrv caddy.Modu }) } - for _, pattern := range matchers { - subRoute.Routes = append(subRoute.Routes, caddyhttp.Route{ - MatcherSetsRaw: []caddy.ModuleMap{ - caddy.ModuleMap{"path": h.JSON(caddyhttp.MatchPath{pattern})}, - }, - HandlersRaw: []json.RawMessage{ - caddyconfig.JSONModuleObject(f, "handler", "php", nil), - }, - }) - } - - notFoundHandler := caddyhttp.StaticResponse{ - StatusCode: caddyhttp.WeakString(strconv.Itoa(http.StatusNotFound)), - } - subRoute.Routes = append(subRoute.Routes, caddyhttp.Route{ - MatcherSetsRaw: []caddy.ModuleMap{}, + // forward matching routes instantly to the PHP handler + routes = append(routes, caddyhttp.Route{ + MatcherSetsRaw: []caddy.ModuleMap{ + caddy.ModuleMap{"path": h.JSON(matchPath)}, + }, HandlersRaw: []json.RawMessage{ - caddyconfig.JSONModuleObject(notFoundHandler, "handler", "static_response", nil), + caddyconfig.JSONModuleObject(f, "handler", "php", nil), }, }) - return []httpcaddyfile.ConfigValue{ - { - Class: "route", - Value: subRoute, - }, - } + return routes } diff --git a/caddy/workerconfig.go b/caddy/workerconfig.go index f52fee4ed..7b882a76a 100644 --- a/caddy/workerconfig.go +++ b/caddy/workerconfig.go @@ -33,22 +33,9 @@ type workerConfig struct { // Directories to watch for file changes Watch []string `json:"watch,omitempty"` // The path to match against the worker - Match string `json:"match,omitempty"` - - matchStrategy matchStrategy + MatchPath string `json:"match_path,omitempty"` } -type matchStrategy int - -const ( - matchNone matchStrategy = iota - matchAll - matchPrefix - matchSuffix - matchExactly - matchWildcard -) - func parseWorkerConfig(d *caddyfile.Dispenser) (workerConfig, error) { wc := workerConfig{} if d.NextArg() { @@ -112,9 +99,14 @@ func parseWorkerConfig(d *caddyfile.Dispenser) (workerConfig, error) { } else { wc.Watch = append(wc.Watch, d.Val()) } + case "match": + if !d.NextArg() { + return wc, d.ArgErr() + } + wc.MatchPath = d.Val() default: - allowedDirectives := "name, file, num, env, watch" + allowedDirectives := "name, file, num, env, watch, match" return wc, wrongSubDirectiveError("worker", allowedDirectives, v) } } @@ -131,11 +123,13 @@ func parseWorkerConfig(d *caddyfile.Dispenser) (workerConfig, error) { } func (wc workerConfig) matchesPath(r *http.Request, documentRoot string) bool { - if wc.Match != "" { - matches, _ := caddyhttp.MatchPath{wc.Match}.MatchWithError(r) - return matches + + // try to match against a pattern if one is assigned + if wc.MatchPath != "" { + return caddyhttp.MatchPath{wc.MatchPath}.Match(r) } + // if there is no pattern, try to match against the actual path (in the public directory) fullScriptPath, _ := fastabs.FastAbs(documentRoot + "/" + r.URL.Path) absFileName, _ := fastabs.FastAbs(wc.FileName) return fullScriptPath == absFileName diff --git a/testdata/files/.gitignore b/testdata/files/.gitignore index 2211df63d..4871fd527 100644 --- a/testdata/files/.gitignore +++ b/testdata/files/.gitignore @@ -1 +1 @@ -*.txt +test.txt diff --git a/testdata/files/static.txt2 b/testdata/files/static.txt similarity index 100% rename from testdata/files/static.txt2 rename to testdata/files/static.txt diff --git a/testdata/hello.php b/testdata/hello.php new file mode 100644 index 000000000..e09aa5c34 --- /dev/null +++ b/testdata/hello.php @@ -0,0 +1,3 @@ + Date: Fri, 13 Jun 2025 10:29:26 +0200 Subject: [PATCH 26/59] Provisions match path rules. --- caddy/workerconfig.go | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/caddy/workerconfig.go b/caddy/workerconfig.go index 7b882a76a..9c00c3236 100644 --- a/caddy/workerconfig.go +++ b/caddy/workerconfig.go @@ -6,6 +6,7 @@ import ( "path/filepath" "strconv" + "github.com/caddyserver/caddy/v2" "github.com/caddyserver/caddy/v2/caddyconfig/caddyfile" "github.com/caddyserver/caddy/v2/modules/caddyhttp" "github.com/dunglas/frankenphp" @@ -103,7 +104,11 @@ func parseWorkerConfig(d *caddyfile.Dispenser) (workerConfig, error) { if !d.NextArg() { return wc, d.ArgErr() } - wc.MatchPath = d.Val() + // provision the path so it's identical to Caddy match rules + // see: https://github.com/caddyserver/caddy/blob/master/modules/caddyhttp/matchers.go + caddyMatchPath := caddyhttp.MatchPath{d.Val()} + caddyMatchPath.Provision(caddy.Context{}) + wc.MatchPath = caddyMatchPath[0] default: allowedDirectives := "name, file, num, env, watch, match" From 5a1ecf2ffa270978682b684e76f7dfd59a32434b Mon Sep 17 00:00:00 2001 From: Alliballibaba Date: Fri, 13 Jun 2025 11:44:16 +0200 Subject: [PATCH 27/59] Allows matching multiple paths --- caddy/module.go | 12 ++++++------ caddy/workerconfig.go | 14 +++++--------- 2 files changed, 11 insertions(+), 15 deletions(-) diff --git a/caddy/module.go b/caddy/module.go index 52c69f7c7..f40aa0738 100644 --- a/caddy/module.go +++ b/caddy/module.go @@ -571,14 +571,14 @@ func parsePhpServer(h httpcaddyfile.Helper) ([]httpcaddyfile.ConfigValue, error) // workers can also match a path without being in the public directory // in this case we need to prepend the worker routes to the existing routes func prependWorkerRoutes(routes caddyhttp.RouteList, h httpcaddyfile.Helper, f FrankenPHPModule, fsrv caddy.Module, disableFsrv bool) caddyhttp.RouteList { - matchPath := caddyhttp.MatchPath{} + allWorkerMatches := caddyhttp.MatchPath{} for _, w := range f.Workers { - if w.MatchPath != "" { - matchPath = append(matchPath, w.MatchPath) + for _, path := range w.MatchPath { + allWorkerMatches = append(matchPath, path) } } - if len(matchPath) == 0 { + if len(allWorkerMatches) == 0 { return routes } @@ -604,10 +604,10 @@ func prependWorkerRoutes(routes caddyhttp.RouteList, h httpcaddyfile.Helper, f F }) } - // forward matching routes instantly to the PHP handler + // forward matching routes to the PHP handler routes = append(routes, caddyhttp.Route{ MatcherSetsRaw: []caddy.ModuleMap{ - caddy.ModuleMap{"path": h.JSON(matchPath)}, + caddy.ModuleMap{"path": h.JSON(allWorkerMatches)}, }, HandlersRaw: []json.RawMessage{ caddyconfig.JSONModuleObject(f, "handler", "php", nil), diff --git a/caddy/workerconfig.go b/caddy/workerconfig.go index 9c00c3236..e85eb2b3d 100644 --- a/caddy/workerconfig.go +++ b/caddy/workerconfig.go @@ -34,7 +34,7 @@ type workerConfig struct { // Directories to watch for file changes Watch []string `json:"watch,omitempty"` // The path to match against the worker - MatchPath string `json:"match_path,omitempty"` + MatchPath []string `json:"match_path,omitempty"` } func parseWorkerConfig(d *caddyfile.Dispenser) (workerConfig, error) { @@ -101,15 +101,11 @@ func parseWorkerConfig(d *caddyfile.Dispenser) (workerConfig, error) { wc.Watch = append(wc.Watch, d.Val()) } case "match": - if !d.NextArg() { - return wc, d.ArgErr() - } // provision the path so it's identical to Caddy match rules // see: https://github.com/caddyserver/caddy/blob/master/modules/caddyhttp/matchers.go - caddyMatchPath := caddyhttp.MatchPath{d.Val()} + caddyMatchPath := (caddyhttp.MatchPath)(d.RemainingArgs()) caddyMatchPath.Provision(caddy.Context{}) - wc.MatchPath = caddyMatchPath[0] - + wc.MatchPath = ([]string)(caddyMatchPath) default: allowedDirectives := "name, file, num, env, watch, match" return wc, wrongSubDirectiveError("worker", allowedDirectives, v) @@ -130,8 +126,8 @@ func parseWorkerConfig(d *caddyfile.Dispenser) (workerConfig, error) { func (wc workerConfig) matchesPath(r *http.Request, documentRoot string) bool { // try to match against a pattern if one is assigned - if wc.MatchPath != "" { - return caddyhttp.MatchPath{wc.MatchPath}.Match(r) + if len(wc.MatchPath) != 0 { + return (caddyhttp.MatchPath)(wc.MatchPath).Match(r) } // if there is no pattern, try to match against the actual path (in the public directory) From ab26c9e85ad7a308e708edcb8abeb106b44537ba Mon Sep 17 00:00:00 2001 From: Alliballibaba Date: Fri, 13 Jun 2025 11:48:40 +0200 Subject: [PATCH 28/59] Fixes var --- caddy/module.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/caddy/module.go b/caddy/module.go index f40aa0738..1ef0eb964 100644 --- a/caddy/module.go +++ b/caddy/module.go @@ -574,7 +574,7 @@ func prependWorkerRoutes(routes caddyhttp.RouteList, h httpcaddyfile.Helper, f F allWorkerMatches := caddyhttp.MatchPath{} for _, w := range f.Workers { for _, path := range w.MatchPath { - allWorkerMatches = append(matchPath, path) + allWorkerMatches = append(allWorkerMatches, path) } } From cfeaf07ef552d3f0641e94706d50067b98b61438 Mon Sep 17 00:00:00 2001 From: Alliballibaba Date: Fri, 13 Jun 2025 23:35:29 +0200 Subject: [PATCH 29/59] Formatting. --- caddy/caddy_test.go | 4 ++-- testdata/hello.php | 2 +- worker.go | 11 +++++++---- 3 files changed, 10 insertions(+), 7 deletions(-) diff --git a/caddy/caddy_test.go b/caddy/caddy_test.go index c67e363e7..720a6a171 100644 --- a/caddy/caddy_test.go +++ b/caddy/caddy_test.go @@ -1344,7 +1344,7 @@ func TestWorkerMatchDirective(t *testing.T) { // 404 on unmatched paths tester.AssertGetResponse("http://localhost:"+testPort+"/elsewhere", http.StatusNotFound, "") - // file will be serverd by file_server + // static file will be served by the fileserver tester.AssertGetResponse("http://localhost:"+testPort+"/static.txt", http.StatusOK, "Hello from file") } @@ -1383,7 +1383,7 @@ func TestWorkerMatchDirectiveWithMultipleWorkers(t *testing.T) { tester.AssertGetResponse("http://localhost:"+testPort+"/not-matched", http.StatusNotFound, "") // serve php file directly as fallback - tester.AssertGetResponse("http://localhost:"+testPort+"/hello.php", http.StatusOK, "Hello") + tester.AssertGetResponse("http://localhost:"+testPort+"/hello.php", http.StatusOK, "Hello from PHP") // serve worker file directly as fallback tester.AssertGetResponse("http://localhost:"+testPort+"/index.php", http.StatusOK, "I am by birth a Genevese (i not set)") diff --git a/testdata/hello.php b/testdata/hello.php index e09aa5c34..81a3ccfdf 100644 --- a/testdata/hello.php +++ b/testdata/hello.php @@ -1,3 +1,3 @@ Date: Sat, 14 Jun 2025 12:44:35 +0200 Subject: [PATCH 30/59] refactoring. --- caddy/module.go | 15 +++++---------- caddy/workerconfig.go | 12 ++++++++++++ 2 files changed, 17 insertions(+), 10 deletions(-) diff --git a/caddy/module.go b/caddy/module.go index 1ef0eb964..e38733ef1 100644 --- a/caddy/module.go +++ b/caddy/module.go @@ -70,21 +70,16 @@ func (f *FrankenPHPModule) Provision(ctx caddy.Context) error { } for i, wc := range f.Workers { - // Inherit environment variables from the parent php_server directive + + // make the file path absolute from the public directory + // this can only be done if the root is definied inside php_server if !filepath.IsAbs(wc.FileName) && f.Root != "" { wc.FileName = filepath.Join(f.Root, wc.FileName) } + // Inherit environment variables from the parent php_server directive if f.Env != nil { - if wc.Env == nil { - wc.Env = make(map[string]string) - } - for k, v := range f.Env { - // Only set if not already defined in the worker - if _, exists := wc.Env[k]; !exists { - wc.Env[k] = v - } - } + wc.inheritEnv(f.Env) } f.Workers[i] = wc } diff --git a/caddy/workerconfig.go b/caddy/workerconfig.go index e85eb2b3d..6d58072de 100644 --- a/caddy/workerconfig.go +++ b/caddy/workerconfig.go @@ -123,6 +123,18 @@ func parseWorkerConfig(d *caddyfile.Dispenser) (workerConfig, error) { return wc, nil } +func (wc workerConfig) inheritEnv(env map[string]string) { + if wc.Env == nil { + wc.Env = make(map[string]string) + } + for k, v := range env { + // do not overwrite existing environment variables + if _, exists := wc.Env[k]; !exists { + wc.Env[k] = v + } + } +} + func (wc workerConfig) matchesPath(r *http.Request, documentRoot string) bool { // try to match against a pattern if one is assigned From c9cc0c5e7efeb49d345fe3db616fc393100b25c6 Mon Sep 17 00:00:00 2001 From: Alexander Stecher <45872305+AlliBalliBaba@users.noreply.github.com> Date: Wed, 25 Jun 2025 22:14:53 +0200 Subject: [PATCH 31/59] Update frankenphp.go MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Kévin Dunglas --- frankenphp.go | 1 + 1 file changed, 1 insertion(+) diff --git a/frankenphp.go b/frankenphp.go index fb30838a1..164df14c5 100644 --- a/frankenphp.go +++ b/frankenphp.go @@ -405,6 +405,7 @@ func ServeHTTP(responseWriter http.ResponseWriter, request *http.Request) error // Detect if a worker is available to handle this request if fc.worker != nil { fc.worker.handleRequest(fc) + return nil } From 636745677a0194ae4d0725b907fbd2f96466a045 Mon Sep 17 00:00:00 2001 From: Alexander Stecher <45872305+AlliBalliBaba@users.noreply.github.com> Date: Wed, 25 Jun 2025 22:15:30 +0200 Subject: [PATCH 32/59] Update caddy/workerconfig.go MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Kévin Dunglas --- caddy/workerconfig.go | 1 + 1 file changed, 1 insertion(+) diff --git a/caddy/workerconfig.go b/caddy/workerconfig.go index 6d58072de..bb0604339 100644 --- a/caddy/workerconfig.go +++ b/caddy/workerconfig.go @@ -145,5 +145,6 @@ func (wc workerConfig) matchesPath(r *http.Request, documentRoot string) bool { // if there is no pattern, try to match against the actual path (in the public directory) fullScriptPath, _ := fastabs.FastAbs(documentRoot + "/" + r.URL.Path) absFileName, _ := fastabs.FastAbs(wc.FileName) + return fullScriptPath == absFileName } From 771099ceff8d3b8da06f8e7a42a7acd4a9205e53 Mon Sep 17 00:00:00 2001 From: Alexander Stecher <45872305+AlliBalliBaba@users.noreply.github.com> Date: Wed, 25 Jun 2025 22:15:38 +0200 Subject: [PATCH 33/59] Update caddy/workerconfig.go MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Kévin Dunglas --- caddy/workerconfig.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/caddy/workerconfig.go b/caddy/workerconfig.go index bb0604339..ee6729dfa 100644 --- a/caddy/workerconfig.go +++ b/caddy/workerconfig.go @@ -125,7 +125,7 @@ func parseWorkerConfig(d *caddyfile.Dispenser) (workerConfig, error) { func (wc workerConfig) inheritEnv(env map[string]string) { if wc.Env == nil { - wc.Env = make(map[string]string) + wc.Env = make(map[string]string, len(env)) } for k, v := range env { // do not overwrite existing environment variables From 16d7877f1eb5e5faca24be50e4e6ed4bd8f68bf9 Mon Sep 17 00:00:00 2001 From: Alexander Stecher <45872305+AlliBalliBaba@users.noreply.github.com> Date: Wed, 25 Jun 2025 22:15:51 +0200 Subject: [PATCH 34/59] Update caddy/module.go MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Kévin Dunglas --- caddy/module.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/caddy/module.go b/caddy/module.go index e38733ef1..4c54230b0 100644 --- a/caddy/module.go +++ b/caddy/module.go @@ -405,8 +405,7 @@ func parsePhpServer(h httpcaddyfile.Helper) ([]httpcaddyfile.ConfigValue, error) // the rest of the config is specified by the user // using the php directive syntax dispenser.Next() // consume the directive name - err = phpsrv.UnmarshalCaddyfile(dispenser) - if err != nil { + if err := phpsrv.UnmarshalCaddyfile(dispenser); err != nil { return nil, err } From 94ffc393166f883d2e9e17b61f222524b080a8f1 Mon Sep 17 00:00:00 2001 From: Alexander Stecher <45872305+AlliBalliBaba@users.noreply.github.com> Date: Wed, 25 Jun 2025 22:16:05 +0200 Subject: [PATCH 35/59] Update caddy/module.go MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Kévin Dunglas --- caddy/module.go | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/caddy/module.go b/caddy/module.go index 4c54230b0..16cc4b29f 100644 --- a/caddy/module.go +++ b/caddy/module.go @@ -259,9 +259,13 @@ func (f *FrankenPHPModule) UnmarshalCaddyfile(d *caddyfile.Dispenser) error { } // Check if a worker with this filename already exists in this module - for j, w1 := range f.Workers { - for i, w2 := range f.Workers { - if i != j && w1.FileName == w2.FileName { + fileNames := make(map[string]struct{}, len(f.Workers) + for j, w := range f.Workers { + if _, ok := fileNames[w.FileName]; ok { + return fmt.Errorf(`workers in a single "php_server" block must not have duplicate filenames: %q`, w.FileName) + } + fileNames[w.FileName] := struct{}{} + } return fmt.Errorf(`workers in a single "php_server" block must not have duplicate filenames: %q`, w1.FileName) } } From 0fcb92004e042aab6a09716f2923367bb8ff4093 Mon Sep 17 00:00:00 2001 From: Alliballibaba Date: Wed, 25 Jun 2025 22:24:08 +0200 Subject: [PATCH 36/59] Fixes suggestion --- caddy/module.go | 14 +++++--------- 1 file changed, 5 insertions(+), 9 deletions(-) diff --git a/caddy/module.go b/caddy/module.go index 16cc4b29f..be03785ee 100644 --- a/caddy/module.go +++ b/caddy/module.go @@ -259,16 +259,12 @@ func (f *FrankenPHPModule) UnmarshalCaddyfile(d *caddyfile.Dispenser) error { } // Check if a worker with this filename already exists in this module - fileNames := make(map[string]struct{}, len(f.Workers) - for j, w := range f.Workers { - if _, ok := fileNames[w.FileName]; ok { - return fmt.Errorf(`workers in a single "php_server" block must not have duplicate filenames: %q`, w.FileName) - } - fileNames[w.FileName] := struct{}{} - } - return fmt.Errorf(`workers in a single "php_server" block must not have duplicate filenames: %q`, w1.FileName) - } + fileNames := make(map[string]struct{}, len(f.Workers)) + for _, w := range f.Workers { + if _, ok := fileNames[w.FileName]; ok { + return fmt.Errorf(`workers in a single "php_server" block must not have duplicate filenames: %q`, w.FileName) } + fileNames[w.FileName] = struct{}{} } return nil From d3d84050bf3d42fedb17ddd9170a967574eac4ac Mon Sep 17 00:00:00 2001 From: Alliballibaba Date: Wed, 25 Jun 2025 22:28:49 +0200 Subject: [PATCH 37/59] Refactoring. --- phpmainthread_test.go | 7 +++++-- worker.go | 4 ++-- 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/phpmainthread_test.go b/phpmainthread_test.go index 1a4d5ba25..67f492f27 100644 --- a/phpmainthread_test.go +++ b/phpmainthread_test.go @@ -182,7 +182,8 @@ func TestFinishBootingAWorkerScript(t *testing.T) { func TestReturnAnErrorIf2WorkersHaveTheSameFileName(t *testing.T) { workers = []*worker{} - _, err1 := newWorker(workerOpt{fileName: "filename.php"}) + w, err1 := newWorker(workerOpt{fileName: "filename.php"}) + workers = append(workers, w) _, err2 := newWorker(workerOpt{fileName: "filename.php"}) assert.NoError(t, err1) @@ -191,7 +192,8 @@ func TestReturnAnErrorIf2WorkersHaveTheSameFileName(t *testing.T) { func TestReturnAnErrorIf2ModuleWorkersHaveTheSameName(t *testing.T) { workers = []*worker{} - _, err1 := newWorker(workerOpt{fileName: "filename.php", name: "workername"}) + w, err1 := newWorker(workerOpt{fileName: "filename.php", name: "workername"}) + workers = append(workers, w) _, err2 := newWorker(workerOpt{fileName: "filename2.php", name: "workername"}) assert.NoError(t, err1) @@ -206,6 +208,7 @@ func getDummyWorker(fileName string) *worker { fileName: testDataPath + "/" + fileName, num: 1, }) + workers = append(workers, worker) return worker } diff --git a/worker.go b/worker.go index a62f4586b..63eceb4d9 100644 --- a/worker.go +++ b/worker.go @@ -36,10 +36,11 @@ func initWorkers(opt []workerOpt) error { watcherIsEnabled = len(directoriesToWatch) > 0 for _, o := range opt { - _, err := newWorker(o) + w, err := newWorker(o) if err != nil { return err } + workers = append(workers, w) } for _, worker := range workers { @@ -123,7 +124,6 @@ func newWorker(o workerOpt) (*worker, error) { threads: make([]*phpThread, 0, o.num), allowPathMatching: allowPathMatching, } - workers = append(workers, w) return w, nil } From 23606e616704994529c4fa036a5ea07f49c4e625 Mon Sep 17 00:00:00 2001 From: Alliballibaba Date: Mon, 9 Jun 2025 21:01:45 +0200 Subject: [PATCH 38/59] Adds 'match' configuration --- caddy/config_test.go | 2 +- caddy/module.go | 5 ++--- caddy/workerconfig.go | 20 +++++++++++++++++++- 3 files changed, 22 insertions(+), 5 deletions(-) diff --git a/caddy/config_test.go b/caddy/config_test.go index 7bcbbfc27..29fee1d24 100644 --- a/caddy/config_test.go +++ b/caddy/config_test.go @@ -1,4 +1,4 @@ -package caddy +package caddy import ( "testing" diff --git a/caddy/module.go b/caddy/module.go index 08f284ebc..34e5f87f6 100644 --- a/caddy/module.go +++ b/caddy/module.go @@ -161,12 +161,11 @@ func (f *FrankenPHPModule) ServeHTTP(w http.ResponseWriter, r *http.Request, _ c } } - fullScriptPath, _ := fastabs.FastAbs(documentRoot + "/" + r.URL.Path) - workerName := "" for _, w := range f.Workers { - if p, _ := fastabs.FastAbs(w.FileName); p == fullScriptPath { + if w.matchesPath(r.URL.Path, documentRoot) { workerName = w.Name + break } } diff --git a/caddy/workerconfig.go b/caddy/workerconfig.go index a6a797c77..47db675e7 100644 --- a/caddy/workerconfig.go +++ b/caddy/workerconfig.go @@ -7,6 +7,7 @@ import ( "github.com/caddyserver/caddy/v2/caddyconfig/caddyfile" "github.com/dunglas/frankenphp" + "github.com/dunglas/frankenphp/internal/fastabs" ) // workerConfig represents the "worker" directive in the Caddyfile @@ -29,6 +30,8 @@ type workerConfig struct { Env map[string]string `json:"env,omitempty"` // Directories to watch for file changes Watch []string `json:"watch,omitempty"` + // The path to match against the worker + Match string `json:"match,omitempty"` } func parseWorkerConfig(d *caddyfile.Dispenser) (workerConfig, error) { @@ -94,8 +97,13 @@ func parseWorkerConfig(d *caddyfile.Dispenser) (workerConfig, error) { } else { wc.Watch = append(wc.Watch, d.Val()) } + case "match": + if !d.NextArg() { + return wc, d.ArgErr() + } + wc.Match = d.Val() default: - allowedDirectives := "name, file, num, env, watch" + allowedDirectives := "name, file, num, env, watch, match" return wc, wrongSubDirectiveError("worker", allowedDirectives, v) } } @@ -110,3 +118,13 @@ func parseWorkerConfig(d *caddyfile.Dispenser) (workerConfig, error) { return wc, nil } + +func (wc workerConfig) matchesPath(path string, documentRoot string) bool { + if wc.Match != "" { + return wc.Match == "*" || wc.Match == path + } + + fullScriptPath, _ := fastabs.FastAbs(documentRoot + "/" + path) + absFileName, _ := fastabs.FastAbs(wc.FileName) + return fullScriptPath == absFileName +} From 226cafeafaee8e13b9540168f320111206ec962b Mon Sep 17 00:00:00 2001 From: Alliballibaba Date: Mon, 9 Jun 2025 23:20:02 +0200 Subject: [PATCH 39/59] test --- caddy/caddy_test.go | 26 ++++++++ caddy/module.go | 157 +++++++++++++++++++++++++++++--------------- 2 files changed, 129 insertions(+), 54 deletions(-) diff --git a/caddy/caddy_test.go b/caddy/caddy_test.go index 9d3a9a013..a5bba66f0 100644 --- a/caddy/caddy_test.go +++ b/caddy/caddy_test.go @@ -1216,6 +1216,32 @@ func TestDisabledMetrics(t *testing.T) { require.Zero(t, count, "metrics should be missing") } +func TestWorkerMatchDirective(t *testing.T) { + tester := caddytest.NewTester(t) + tester.InitServer(` + { + skip_install_trust + admin localhost:2999 + } + + http://localhost:`+testPort+` { + php_server { + root ../testdata/files + try_files {path} index.php + worker { + match index.php + file ../worker-with-counter.php + num 1 + } + } + } + `, "caddyfile") + + tester.AssertGetResponse("http://localhost:"+testPort+"/any-path", http.StatusOK, "requests:1") + tester.AssertGetResponse("http://localhost:"+testPort+"/static.txt", http.StatusOK, "Hello from file") + tester.AssertGetResponse("http://localhost:"+testPort+"/worker-path", http.StatusOK, "requests:1") +} + func TestWorkerRestart(t *testing.T) { var wg sync.WaitGroup tester := caddytest.NewTester(t) diff --git a/caddy/module.go b/caddy/module.go index 34e5f87f6..3672a1468 100644 --- a/caddy/module.go +++ b/caddy/module.go @@ -69,6 +69,34 @@ func (f *FrankenPHPModule) Provision(ctx caddy.Context) error { return fmt.Errorf(`expected ctx.App("frankenphp") to return *FrankenPHPApp, got nil`) } + for i, wc := range f.Workers { + // Inherit environment variables from the parent php_server directive + if !filepath.IsAbs(wc.FileName) && f.Root != "" { + wc.FileName = filepath.Join(f.Root, wc.FileName) + } + + if f.Env != nil { + if wc.Env == nil { + wc.Env = make(map[string]string) + } + for k, v := range f.Env { + // Only set if not already defined in the worker + if _, exists := wc.Env[k]; !exists { + wc.Env[k] = v + } + } + } + + f.Workers[i] = wc + + // Check if a worker with this filename already exists in this module + for j, existingWorker := range f.Workers { + if i != j && existingWorker.FileName == wc.FileName { + return fmt.Errorf(`workers in a single "php_server" block must not have duplicate filenames: %q`, wc.FileName) + } + } + } + workers, err := fapp.addModuleWorkers(f.Workers...) if err != nil { return err @@ -229,12 +257,11 @@ func (f *FrankenPHPModule) UnmarshalCaddyfile(d *caddyfile.Dispenser) error { f.ResolveRootSymlink = &v case "worker": - for d.NextBlock(1) { - } - for d.NextArg() { - } - // Skip "worker" blocks in the first pass - continue + wc, err := parseWorkerConfig(d) + if err != nil { + return err + } + f.Workers = append(f.Workers, wc) default: allowedDirectives := "root, split, env, resolve_root_symlink, worker" @@ -243,45 +270,6 @@ func (f *FrankenPHPModule) UnmarshalCaddyfile(d *caddyfile.Dispenser) error { } } - // Second pass: Parse only "worker" blocks - d.Reset() - for d.Next() { - for d.NextBlock(0) { - if d.Val() == "worker" { - wc, err := parseWorkerConfig(d) - if err != nil { - return err - } - - // Inherit environment variables from the parent php_server directive - if !filepath.IsAbs(wc.FileName) && f.Root != "" { - wc.FileName = filepath.Join(f.Root, wc.FileName) - } - - if f.Env != nil { - if wc.Env == nil { - wc.Env = make(map[string]string) - } - for k, v := range f.Env { - // Only set if not already defined in the worker - if _, exists := wc.Env[k]; !exists { - wc.Env[k] = v - } - } - } - - // Check if a worker with this filename already exists in this module - for _, existingWorker := range f.Workers { - if existingWorker.FileName == wc.FileName { - return fmt.Errorf(`workers in a single "php_server" block must not have duplicate filenames: %q`, wc.FileName) - } - } - - f.Workers = append(f.Workers, wc) - } - } - } - return nil } @@ -417,6 +405,16 @@ func parsePhpServer(h httpcaddyfile.Helper) ([]httpcaddyfile.ConfigValue, error) // unmarshaler can read it from the start dispenser.Reset() + // the rest of the config is specified by the user + // using the php directive syntax + dispenser.Next() // consume the directive name + err = phpsrv.UnmarshalCaddyfile(dispenser) + if err != nil { + return nil, err + } + + return routesForWorkerServer(h, phpsrv, fsrv) + if frankenphp.EmbeddedAppPath != "" { if phpsrv.Root == "" { phpsrv.Root = filepath.Join(frankenphp.EmbeddedAppPath, defaultDocumentRoot) @@ -520,15 +518,6 @@ func parsePhpServer(h httpcaddyfile.Helper) ([]httpcaddyfile.ConfigValue, error) "path": h.JSON(pathList), } - // the rest of the config is specified by the user - // using the php directive syntax - dispenser.Next() // consume the directive name - err = phpsrv.UnmarshalCaddyfile(dispenser) - - if err != nil { - return nil, err - } - // create the PHP route which is // conditional on matching PHP files phpRoute := caddyhttp.Route{ @@ -575,3 +564,63 @@ func parsePhpServer(h httpcaddyfile.Helper) ([]httpcaddyfile.ConfigValue, error) }, }, nil } + +func routesForWorkerServer(h httpcaddyfile.Helper, frankenphpModule FrankenPHPModule, fsrv caddy.Module) ([]httpcaddyfile.ConfigValue, error) { + return []httpcaddyfile.ConfigValue{ + { + Class: "route", + Value: workerSubroute(h, frankenphpModule, fsrv), + }, + }, nil +} + +// get the routing for php_worker +// serve non-php file or fallback to index worker +func workerSubroute(h httpcaddyfile.Helper, frankenphpModule FrankenPHPModule, fsrv caddy.Module) caddyhttp.Subroute { + matchers := caddyhttp.MatchPath{"*.php"} + for _, w := range frankenphpModule.Workers { + if w.Match != "" { + matchers = append(matchers, w.Match) + } + } + + subRoute := caddyhttp.Subroute{ + Routes: caddyhttp.RouteList{ + caddyhttp.Route{ + MatcherSetsRaw: []caddy.ModuleMap{ + caddy.ModuleMap{ + "file": h.JSON(fileserver.MatchFile{ + TryFiles: []string{"{http.request.uri.path}"}, + Root: frankenphpModule.Root, + }), + "not": h.JSON(caddyhttp.MatchNot{ + MatcherSetsRaw: []caddy.ModuleMap{ + { + "path": h.JSON(matchers), + }, + }, + }), + }, + }, + HandlersRaw: []json.RawMessage{ + caddyconfig.JSONModuleObject(fsrv, "handler", "file_server", nil), + }, + }, + }, + } + + for _, pattern := range matchers { + subRoute.Routes = append(subRoute.Routes, caddyhttp.Route{ + MatcherSetsRaw: []caddy.ModuleMap{ + caddy.ModuleMap{ + "path": h.JSON(caddyhttp.MatchPath{ pattern }), + }, + }, + HandlersRaw: []json.RawMessage{ + caddyconfig.JSONModuleObject(frankenphpModule, "handler", "php", nil), + }, + }) + } + + return subRoute +} From ea9a9febc4943914cdfa118786858a6d1bf93981 Mon Sep 17 00:00:00 2001 From: Alliballibaba Date: Wed, 11 Jun 2025 00:13:19 +0200 Subject: [PATCH 40/59] Adds Caddy's matcher. --- caddy/caddy_test.go | 46 ++++++++++++++++-- caddy/module.go | 107 ++++++++++++++++++++++-------------------- caddy/workerconfig.go | 23 +++++++-- 3 files changed, 119 insertions(+), 57 deletions(-) diff --git a/caddy/caddy_test.go b/caddy/caddy_test.go index a5bba66f0..b0c33d8e7 100644 --- a/caddy/caddy_test.go +++ b/caddy/caddy_test.go @@ -1218,6 +1218,7 @@ func TestDisabledMetrics(t *testing.T) { func TestWorkerMatchDirective(t *testing.T) { tester := caddytest.NewTester(t) + testport2 := "3000" tester.InitServer(` { skip_install_trust @@ -1227,19 +1228,56 @@ func TestWorkerMatchDirective(t *testing.T) { http://localhost:`+testPort+` { php_server { root ../testdata/files - try_files {path} index.php worker { - match index.php + match /matched-path* file ../worker-with-counter.php num 1 } } } + http://localhost:`+testport2+` { + php_server { + root ../testdata + worker { + match /counter* + file worker-with-counter.php + num 1 + } + worker { + match /index* + file index.php + num 1 + } + } + } `, "caddyfile") - tester.AssertGetResponse("http://localhost:"+testPort+"/any-path", http.StatusOK, "requests:1") + // the first php_server with root in testdata/files + // Forward request that match * to the worker + tester.AssertGetResponse("http://localhost:"+testPort+"/matched-path", http.StatusOK, "requests:1") + tester.AssertGetResponse("http://localhost:"+testPort+"/matched-path/anywhere", http.StatusOK, "requests:2") + + // 404 on not matching paths + r, _ := http.NewRequest("GET", "http://localhost:"+testPort+"/not-matched-path", nil) + tester.AssertResponseCode(r, http.StatusNotFound) + + // forward files to fileserver + tester.AssertGetResponse("http://localhost:"+testPort+"/static.txt", http.StatusOK, "Hello from file") + + // the second php_server has root in testdata and has 2 different workers + tester.AssertGetResponse("http://localhost:"+testport2+"/counter/sub-path", http.StatusOK, "requests:1") + tester.AssertGetResponse("http://localhost:"+testport2+"/index/sub-path", http.StatusOK, "I am by birth a Genevese (i not set)") + + // 404 on not matching path + r, _ = http.NewRequest("GET", "http://localhost:"+testPort+"/other", nil) + tester.AssertResponseCode(r, http.StatusNotFound) + + // forward files to fileserver tester.AssertGetResponse("http://localhost:"+testPort+"/static.txt", http.StatusOK, "Hello from file") - tester.AssertGetResponse("http://localhost:"+testPort+"/worker-path", http.StatusOK, "requests:1") + + // never serve PHP files directly + r, _ = http.NewRequest("GET", "http://localhost:"+testPort+"/index.php", nil) + tester.AssertResponseCode(r, http.StatusNotFound) } func TestWorkerRestart(t *testing.T) { diff --git a/caddy/module.go b/caddy/module.go index 3672a1468..ab6536bae 100644 --- a/caddy/module.go +++ b/caddy/module.go @@ -71,30 +71,30 @@ func (f *FrankenPHPModule) Provision(ctx caddy.Context) error { for i, wc := range f.Workers { // Inherit environment variables from the parent php_server directive - if !filepath.IsAbs(wc.FileName) && f.Root != "" { - wc.FileName = filepath.Join(f.Root, wc.FileName) - } - - if f.Env != nil { - if wc.Env == nil { - wc.Env = make(map[string]string) - } - for k, v := range f.Env { - // Only set if not already defined in the worker - if _, exists := wc.Env[k]; !exists { - wc.Env[k] = v - } - } - } + if !filepath.IsAbs(wc.FileName) && f.Root != "" { + wc.FileName = filepath.Join(f.Root, wc.FileName) + } + + if f.Env != nil { + if wc.Env == nil { + wc.Env = make(map[string]string) + } + for k, v := range f.Env { + // Only set if not already defined in the worker + if _, exists := wc.Env[k]; !exists { + wc.Env[k] = v + } + } + } f.Workers[i] = wc - // Check if a worker with this filename already exists in this module - for j, existingWorker := range f.Workers { - if i != j && existingWorker.FileName == wc.FileName { - return fmt.Errorf(`workers in a single "php_server" block must not have duplicate filenames: %q`, wc.FileName) - } - } + // Check if a worker with this filename already exists in this module + for j, existingWorker := range f.Workers { + if i != j && existingWorker.FileName == wc.FileName { + return fmt.Errorf(`workers in a single "php_server" block must not have duplicate filenames: %q`, wc.FileName) + } + } } workers, err := fapp.addModuleWorkers(f.Workers...) @@ -191,7 +191,7 @@ func (f *FrankenPHPModule) ServeHTTP(w http.ResponseWriter, r *http.Request, _ c workerName := "" for _, w := range f.Workers { - if w.matchesPath(r.URL.Path, documentRoot) { + if w.matchesPath(r, documentRoot) { workerName = w.Name break } @@ -258,9 +258,9 @@ func (f *FrankenPHPModule) UnmarshalCaddyfile(d *caddyfile.Dispenser) error { case "worker": wc, err := parseWorkerConfig(d) - if err != nil { - return err - } + if err != nil { + return err + } f.Workers = append(f.Workers, wc) default: @@ -406,12 +406,12 @@ func parsePhpServer(h httpcaddyfile.Helper) ([]httpcaddyfile.ConfigValue, error) dispenser.Reset() // the rest of the config is specified by the user - // using the php directive syntax - dispenser.Next() // consume the directive name - err = phpsrv.UnmarshalCaddyfile(dispenser) - if err != nil { - return nil, err - } + // using the php directive syntax + dispenser.Next() // consume the directive name + err = phpsrv.UnmarshalCaddyfile(dispenser) + if err != nil { + return nil, err + } return routesForWorkerServer(h, phpsrv, fsrv) @@ -567,17 +567,17 @@ func parsePhpServer(h httpcaddyfile.Helper) ([]httpcaddyfile.ConfigValue, error) func routesForWorkerServer(h httpcaddyfile.Helper, frankenphpModule FrankenPHPModule, fsrv caddy.Module) ([]httpcaddyfile.ConfigValue, error) { return []httpcaddyfile.ConfigValue{ - { - Class: "route", - Value: workerSubroute(h, frankenphpModule, fsrv), - }, - }, nil + { + Class: "route", + Value: workerSubroute(h, frankenphpModule, fsrv), + }, + }, nil } // get the routing for php_worker // serve non-php file or fallback to index worker func workerSubroute(h httpcaddyfile.Helper, frankenphpModule FrankenPHPModule, fsrv caddy.Module) caddyhttp.Subroute { - matchers := caddyhttp.MatchPath{"*.php"} + matchers := caddyhttp.MatchPath{} for _, w := range frankenphpModule.Workers { if w.Match != "" { matchers = append(matchers, w.Match) @@ -596,7 +596,7 @@ func workerSubroute(h httpcaddyfile.Helper, frankenphpModule FrankenPHPModule, f "not": h.JSON(caddyhttp.MatchNot{ MatcherSetsRaw: []caddy.ModuleMap{ { - "path": h.JSON(matchers), + "path": h.JSON(caddyhttp.MatchPath{"*.php"}), }, }, }), @@ -609,18 +609,25 @@ func workerSubroute(h httpcaddyfile.Helper, frankenphpModule FrankenPHPModule, f }, } - for _, pattern := range matchers { - subRoute.Routes = append(subRoute.Routes, caddyhttp.Route{ - MatcherSetsRaw: []caddy.ModuleMap{ - caddy.ModuleMap{ - "path": h.JSON(caddyhttp.MatchPath{ pattern }), - }, - }, - HandlersRaw: []json.RawMessage{ - caddyconfig.JSONModuleObject(frankenphpModule, "handler", "php", nil), - }, - }) - } + for _, pattern := range matchers { + subRoute.Routes = append(subRoute.Routes, caddyhttp.Route{ + MatcherSetsRaw: []caddy.ModuleMap{ + caddy.ModuleMap{ + "path": h.JSON(caddyhttp.MatchPath{pattern}), + }, + }, + HandlersRaw: []json.RawMessage{ + caddyconfig.JSONModuleObject(frankenphpModule, "handler", "php", nil), + }, + }) + } + + subRoute.Routes = append(subRoute.Routes, caddyhttp.Route{ + MatcherSetsRaw: []caddy.ModuleMap{}, + HandlersRaw: []json.RawMessage{ + caddyconfig.JSONModuleObject(fsrv, "handler", "file_server", nil), + }, + }) return subRoute } diff --git a/caddy/workerconfig.go b/caddy/workerconfig.go index 47db675e7..6140bb210 100644 --- a/caddy/workerconfig.go +++ b/caddy/workerconfig.go @@ -2,10 +2,12 @@ package caddy import ( "errors" + "net/http" "path/filepath" "strconv" "github.com/caddyserver/caddy/v2/caddyconfig/caddyfile" + "github.com/caddyserver/caddy/v2/modules/caddyhttp" "github.com/dunglas/frankenphp" "github.com/dunglas/frankenphp/internal/fastabs" ) @@ -32,8 +34,21 @@ type workerConfig struct { Watch []string `json:"watch,omitempty"` // The path to match against the worker Match string `json:"match,omitempty"` + + matchStrategy matchStrategy } +type matchStrategy int + +const ( + matchNone matchStrategy = iota + matchAll + matchPrefix + matchSuffix + matchExactly + matchWildcard +) + func parseWorkerConfig(d *caddyfile.Dispenser) (workerConfig, error) { wc := workerConfig{} if d.NextArg() { @@ -102,6 +117,7 @@ func parseWorkerConfig(d *caddyfile.Dispenser) (workerConfig, error) { return wc, d.ArgErr() } wc.Match = d.Val() + default: allowedDirectives := "name, file, num, env, watch, match" return wc, wrongSubDirectiveError("worker", allowedDirectives, v) @@ -119,12 +135,13 @@ func parseWorkerConfig(d *caddyfile.Dispenser) (workerConfig, error) { return wc, nil } -func (wc workerConfig) matchesPath(path string, documentRoot string) bool { +func (wc workerConfig) matchesPath(r *http.Request, documentRoot string) bool { if wc.Match != "" { - return wc.Match == "*" || wc.Match == path + matches, _ := caddyhttp.MatchPath{wc.Match}.MatchWithError(r) + return matches } - fullScriptPath, _ := fastabs.FastAbs(documentRoot + "/" + path) + fullScriptPath, _ := fastabs.FastAbs(documentRoot + "/" + r.URL.Path) absFileName, _ := fastabs.FastAbs(wc.FileName) return fullScriptPath == absFileName } From db66a76d49921a9e44205ddb9161c91cf4be030b Mon Sep 17 00:00:00 2001 From: Alliballibaba Date: Wed, 11 Jun 2025 19:20:32 +0200 Subject: [PATCH 41/59] Adds no-fileserver test. --- caddy/caddy_test.go | 157 ++++++++++++++++++++++--------------- caddy/module.go | 104 ++++++++++++------------ testdata/files/static.txt2 | 1 + 3 files changed, 148 insertions(+), 114 deletions(-) create mode 100644 testdata/files/static.txt2 diff --git a/caddy/caddy_test.go b/caddy/caddy_test.go index b0c33d8e7..cbe2ddcf8 100644 --- a/caddy/caddy_test.go +++ b/caddy/caddy_test.go @@ -1216,70 +1216,6 @@ func TestDisabledMetrics(t *testing.T) { require.Zero(t, count, "metrics should be missing") } -func TestWorkerMatchDirective(t *testing.T) { - tester := caddytest.NewTester(t) - testport2 := "3000" - tester.InitServer(` - { - skip_install_trust - admin localhost:2999 - } - - http://localhost:`+testPort+` { - php_server { - root ../testdata/files - worker { - match /matched-path* - file ../worker-with-counter.php - num 1 - } - } - } - http://localhost:`+testport2+` { - php_server { - root ../testdata - worker { - match /counter* - file worker-with-counter.php - num 1 - } - worker { - match /index* - file index.php - num 1 - } - } - } - `, "caddyfile") - - // the first php_server with root in testdata/files - // Forward request that match * to the worker - tester.AssertGetResponse("http://localhost:"+testPort+"/matched-path", http.StatusOK, "requests:1") - tester.AssertGetResponse("http://localhost:"+testPort+"/matched-path/anywhere", http.StatusOK, "requests:2") - - // 404 on not matching paths - r, _ := http.NewRequest("GET", "http://localhost:"+testPort+"/not-matched-path", nil) - tester.AssertResponseCode(r, http.StatusNotFound) - - // forward files to fileserver - tester.AssertGetResponse("http://localhost:"+testPort+"/static.txt", http.StatusOK, "Hello from file") - - // the second php_server has root in testdata and has 2 different workers - tester.AssertGetResponse("http://localhost:"+testport2+"/counter/sub-path", http.StatusOK, "requests:1") - tester.AssertGetResponse("http://localhost:"+testport2+"/index/sub-path", http.StatusOK, "I am by birth a Genevese (i not set)") - - // 404 on not matching path - r, _ = http.NewRequest("GET", "http://localhost:"+testPort+"/other", nil) - tester.AssertResponseCode(r, http.StatusNotFound) - - // forward files to fileserver - tester.AssertGetResponse("http://localhost:"+testPort+"/static.txt", http.StatusOK, "Hello from file") - - // never serve PHP files directly - r, _ = http.NewRequest("GET", "http://localhost:"+testPort+"/index.php", nil) - tester.AssertResponseCode(r, http.StatusNotFound) -} - func TestWorkerRestart(t *testing.T) { var wg sync.WaitGroup tester := caddytest.NewTester(t) @@ -1380,3 +1316,96 @@ func TestWorkerRestart(t *testing.T) { "frankenphp_worker_restarts", )) } + +func TestWorkerMatchDirectiveWithFileServer(t *testing.T) { + tester := caddytest.NewTester(t) + testport2 := "3000" + tester.InitServer(` + { + skip_install_trust + admin localhost:2999 + } + + http://localhost:`+testPort+` { + php_server { + root ../testdata/files + worker { + match /matched-path* + file ../worker-with-counter.php + num 1 + } + } + } + http://localhost:`+testport2+` { + php_server { + root ../testdata + worker { + match /counter* + file worker-with-counter.php + num 1 + } + worker { + match /index* + file index.php + num 1 + } + } + } + `, "caddyfile") + + // the first php_server with root in testdata/files + // Forward request that match * to the worker + tester.AssertGetResponse("http://localhost:"+testPort+"/matched-path", http.StatusOK, "requests:1") + tester.AssertGetResponse("http://localhost:"+testPort+"/matched-path/anywhere", http.StatusOK, "requests:2") + + // 404 on not matching paths + r, _ := http.NewRequest("GET", "http://localhost:"+testPort+"/not-matched-path", nil) + tester.AssertResponseCode(r, http.StatusNotFound) + + // forward files to fileserver + tester.AssertGetResponse("http://localhost:"+testPort+"/static.txt2", http.StatusOK, "Hello from file") + + // the second php_server has root in testdata and has 2 different workers + tester.AssertGetResponse("http://localhost:"+testport2+"/counter/sub-path", http.StatusOK, "requests:1") + tester.AssertGetResponse("http://localhost:"+testport2+"/index/sub-path", http.StatusOK, "I am by birth a Genevese (i not set)") + + // 404 on not matching path + r, _ = http.NewRequest("GET", "http://localhost:"+testPort+"/other", nil) + tester.AssertResponseCode(r, http.StatusNotFound) + + // forward files to fileserver + tester.AssertGetResponse("http://localhost:"+testPort+"/static.txt2", http.StatusOK, "Hello from file") + + // never serve PHP files directly + r, _ = http.NewRequest("GET", "http://localhost:"+testPort+"/index.php", nil) + tester.AssertResponseCode(r, http.StatusNotFound) +} + +func TestWorkerMatchDirectiveWithoutFileServer(t *testing.T) { + tester := caddytest.NewTester(t) + tester.InitServer(` + { + skip_install_trust + admin localhost:2999 + } + + http://localhost:`+testPort+` { + php_server { + file_server off + root ../testdata/files + worker { + match /some-path + file ../worker-with-counter.php + num 1 + } + } + } + `, "caddyfile") + + // find the worker at some-path + tester.AssertGetResponse("http://localhost:"+testPort+"/some-path", http.StatusOK, "requests:1") + + // do not find the file at static.txt2 + r, _ := http.NewRequest("GET", "http://localhost:"+testPort+"/static.txt2", nil) + tester.AssertResponseCode(r, http.StatusNotFound) +} diff --git a/caddy/module.go b/caddy/module.go index ab6536bae..e0990e68c 100644 --- a/caddy/module.go +++ b/caddy/module.go @@ -86,15 +86,7 @@ func (f *FrankenPHPModule) Provision(ctx caddy.Context) error { } } } - f.Workers[i] = wc - - // Check if a worker with this filename already exists in this module - for j, existingWorker := range f.Workers { - if i != j && existingWorker.FileName == wc.FileName { - return fmt.Errorf(`workers in a single "php_server" block must not have duplicate filenames: %q`, wc.FileName) - } - } } workers, err := fapp.addModuleWorkers(f.Workers...) @@ -270,6 +262,15 @@ func (f *FrankenPHPModule) UnmarshalCaddyfile(d *caddyfile.Dispenser) error { } } + // Check if a worker with this filename already exists in this module + for j, w1 := range f.Workers { + for i, w2 := range f.Workers { + if i != j && w1.FileName == w2.FileName { + return fmt.Errorf(`workers in a single "php_server" block must not have duplicate filenames: %q`, w1.FileName) + } + } + } + return nil } @@ -413,8 +414,6 @@ func parsePhpServer(h httpcaddyfile.Helper) ([]httpcaddyfile.ConfigValue, error) return nil, err } - return routesForWorkerServer(h, phpsrv, fsrv) - if frankenphp.EmbeddedAppPath != "" { if phpsrv.Root == "" { phpsrv.Root = filepath.Join(frankenphp.EmbeddedAppPath, defaultDocumentRoot) @@ -427,6 +426,13 @@ func parsePhpServer(h httpcaddyfile.Helper) ([]httpcaddyfile.ConfigValue, error) } } + // if any worker has 'match' specified, use an alternate routing approach + for _, w := range phpsrv.Workers { + if w.Match != "" { + return pureWorkerRoute(h, phpsrv, fsrv, disableFsrv), nil + } + } + // set up a route list that we'll append to routes := caddyhttp.RouteList{} @@ -565,69 +571,67 @@ func parsePhpServer(h httpcaddyfile.Helper) ([]httpcaddyfile.ConfigValue, error) }, nil } -func routesForWorkerServer(h httpcaddyfile.Helper, frankenphpModule FrankenPHPModule, fsrv caddy.Module) ([]httpcaddyfile.ConfigValue, error) { - return []httpcaddyfile.ConfigValue{ - { - Class: "route", - Value: workerSubroute(h, frankenphpModule, fsrv), - }, - }, nil -} - -// get the routing for php_worker -// serve non-php file or fallback to index worker -func workerSubroute(h httpcaddyfile.Helper, frankenphpModule FrankenPHPModule, fsrv caddy.Module) caddyhttp.Subroute { +// if any worker under php_server has a match pattern, +// routes have to be created differently +// first, we will try to match against the file system, +// then we will try to match aganist each worker +func pureWorkerRoute(h httpcaddyfile.Helper, f FrankenPHPModule, fsrv caddy.Module, disableFsrv bool) []httpcaddyfile.ConfigValue { + f.SplitPath = []string{} matchers := caddyhttp.MatchPath{} - for _, w := range frankenphpModule.Workers { + for _, w := range f.Workers { if w.Match != "" { matchers = append(matchers, w.Match) } } - subRoute := caddyhttp.Subroute{ - Routes: caddyhttp.RouteList{ - caddyhttp.Route{ - MatcherSetsRaw: []caddy.ModuleMap{ - caddy.ModuleMap{ - "file": h.JSON(fileserver.MatchFile{ - TryFiles: []string{"{http.request.uri.path}"}, - Root: frankenphpModule.Root, - }), - "not": h.JSON(caddyhttp.MatchNot{ - MatcherSetsRaw: []caddy.ModuleMap{ - { - "path": h.JSON(caddyhttp.MatchPath{"*.php"}), - }, - }, - }), - }, - }, - HandlersRaw: []json.RawMessage{ - caddyconfig.JSONModuleObject(fsrv, "handler", "file_server", nil), + subRoute := caddyhttp.Subroute{Routes: caddyhttp.RouteList{}} + + if !disableFsrv { + subRoute.Routes = append(subRoute.Routes, caddyhttp.Route{ + MatcherSetsRaw: []caddy.ModuleMap{ + caddy.ModuleMap{ + "file": h.JSON(fileserver.MatchFile{ + TryFiles: []string{"{http.request.uri.path}"}, + Root: f.Root, + }), + "not": h.JSON(caddyhttp.MatchNot{ + MatcherSetsRaw: []caddy.ModuleMap{ + {"path": h.JSON(caddyhttp.MatchPath{"*.php"})}, + }, + }), }, }, - }, + HandlersRaw: []json.RawMessage{ + caddyconfig.JSONModuleObject(fsrv, "handler", "file_server", nil), + }, + }) } for _, pattern := range matchers { subRoute.Routes = append(subRoute.Routes, caddyhttp.Route{ MatcherSetsRaw: []caddy.ModuleMap{ - caddy.ModuleMap{ - "path": h.JSON(caddyhttp.MatchPath{pattern}), - }, + caddy.ModuleMap{"path": h.JSON(caddyhttp.MatchPath{pattern})}, }, HandlersRaw: []json.RawMessage{ - caddyconfig.JSONModuleObject(frankenphpModule, "handler", "php", nil), + caddyconfig.JSONModuleObject(f, "handler", "php", nil), }, }) } + notFoundHandler := caddyhttp.StaticResponse{ + StatusCode: caddyhttp.WeakString(strconv.Itoa(http.StatusNotFound)), + } subRoute.Routes = append(subRoute.Routes, caddyhttp.Route{ MatcherSetsRaw: []caddy.ModuleMap{}, HandlersRaw: []json.RawMessage{ - caddyconfig.JSONModuleObject(fsrv, "handler", "file_server", nil), + caddyconfig.JSONModuleObject(notFoundHandler, "handler", "static_response", nil), }, }) - return subRoute + return []httpcaddyfile.ConfigValue{ + { + Class: "route", + Value: subRoute, + }, + } } diff --git a/testdata/files/static.txt2 b/testdata/files/static.txt2 new file mode 100644 index 000000000..09be5ef4e --- /dev/null +++ b/testdata/files/static.txt2 @@ -0,0 +1 @@ +Hello from file \ No newline at end of file From e634358244cc62660d1efec0d3d12205d337bdfb Mon Sep 17 00:00:00 2001 From: Alliballibaba Date: Wed, 11 Jun 2025 19:41:01 +0200 Subject: [PATCH 42/59] Prevents duplicate path calculations and optimizes worker access. --- context.go | 14 ++++++-- frankenphp.go | 4 +-- phpmainthread_test.go | 10 +++--- request_options.go | 4 ++- scaling.go | 4 +-- scaling_test.go | 2 +- worker.go | 84 +++++++++++++++++++++++++------------------ 7 files changed, 73 insertions(+), 49 deletions(-) diff --git a/context.go b/context.go index 29ed40af3..6de140da7 100644 --- a/context.go +++ b/context.go @@ -17,12 +17,12 @@ type frankenPHPContext struct { logger *slog.Logger request *http.Request originalRequest *http.Request + worker *worker docURI string pathInfo string scriptName string scriptFilename string - workerName string // Whether the request is already closed by us isDone bool @@ -89,8 +89,16 @@ func NewRequestWithContext(r *http.Request, opts ...RequestOption) (*http.Reques } } - // SCRIPT_FILENAME is the absolute path of SCRIPT_NAME - fc.scriptFilename = sanitizedPathJoin(fc.documentRoot, fc.scriptName) + // if a worker is assigned explicitly, use its filename + // determine if the filename belongs to a worker otherwise + if fc.worker != nil { + fc.scriptFilename = fc.worker.fileName + } else { + // SCRIPT_FILENAME is the absolute path of SCRIPT_NAME + fc.scriptFilename = sanitizedPathJoin(fc.documentRoot, fc.scriptName) + fc.worker = getWorkerByPath(fc.scriptFilename) + } + c := context.WithValue(r.Context(), contextKey, fc) return r.WithContext(c), nil diff --git a/frankenphp.go b/frankenphp.go index 37fb23678..fb30838a1 100644 --- a/frankenphp.go +++ b/frankenphp.go @@ -403,8 +403,8 @@ func ServeHTTP(responseWriter http.ResponseWriter, request *http.Request) error } // Detect if a worker is available to handle this request - if worker, ok := workers[getWorkerKey(fc.workerName, fc.scriptFilename)]; ok { - worker.handleRequest(fc) + if fc.worker != nil { + fc.worker.handleRequest(fc) return nil } diff --git a/phpmainthread_test.go b/phpmainthread_test.go index 08020ea62..1a4d5ba25 100644 --- a/phpmainthread_test.go +++ b/phpmainthread_test.go @@ -181,7 +181,7 @@ func TestFinishBootingAWorkerScript(t *testing.T) { } func TestReturnAnErrorIf2WorkersHaveTheSameFileName(t *testing.T) { - workers = make(map[string]*worker) + workers = []*worker{} _, err1 := newWorker(workerOpt{fileName: "filename.php"}) _, err2 := newWorker(workerOpt{fileName: "filename.php"}) @@ -190,7 +190,7 @@ func TestReturnAnErrorIf2WorkersHaveTheSameFileName(t *testing.T) { } func TestReturnAnErrorIf2ModuleWorkersHaveTheSameName(t *testing.T) { - workers = make(map[string]*worker) + workers = []*worker{} _, err1 := newWorker(workerOpt{fileName: "filename.php", name: "workername"}) _, err2 := newWorker(workerOpt{fileName: "filename2.php", name: "workername"}) @@ -200,7 +200,7 @@ func TestReturnAnErrorIf2ModuleWorkersHaveTheSameName(t *testing.T) { func getDummyWorker(fileName string) *worker { if workers == nil { - workers = make(map[string]*worker) + workers = []*worker{} } worker, _ := newWorker(workerOpt{ fileName: testDataPath + "/" + fileName, @@ -232,9 +232,9 @@ func allPossibleTransitions(worker1Path string, worker2Path string) []func(*phpT thread.boot() } }, - func(thread *phpThread) { convertToWorkerThread(thread, workers[worker1Path]) }, + func(thread *phpThread) { convertToWorkerThread(thread, getWorkerByPath(worker1Path)) }, convertToInactiveThread, - func(thread *phpThread) { convertToWorkerThread(thread, workers[worker2Path]) }, + func(thread *phpThread) { convertToWorkerThread(thread, getWorkerByPath(worker2Path)) }, convertToInactiveThread, } } diff --git a/request_options.go b/request_options.go index 6c1cb5949..2227b06ee 100644 --- a/request_options.go +++ b/request_options.go @@ -127,7 +127,9 @@ func WithRequestLogger(logger *slog.Logger) RequestOption { // WithWorkerName sets the worker that should handle the request func WithWorkerName(name string) RequestOption { return func(o *frankenPHPContext) error { - o.workerName = name + if name != "" { + o.worker = getWorkerByName(name) + } return nil } diff --git a/scaling.go b/scaling.go index ca319a177..d462b4a54 100644 --- a/scaling.go +++ b/scaling.go @@ -154,8 +154,8 @@ func startUpscalingThreads(maxScaledThreads int, scale chan *frankenPHPContext, } // if the request has been stalled long enough, scale - if worker, ok := workers[getWorkerKey(fc.workerName, fc.scriptFilename)]; ok { - scaleWorkerThread(worker) + if fc.worker != nil { + scaleWorkerThread(fc.worker) } else { scaleRegularThread() } diff --git a/scaling_test.go b/scaling_test.go index 85adc5863..eeca51ca2 100644 --- a/scaling_test.go +++ b/scaling_test.go @@ -44,7 +44,7 @@ func TestScaleAWorkerThreadUpAndDown(t *testing.T) { autoScaledThread := phpThreads[2] // scale up - scaleWorkerThread(workers[workerPath]) + scaleWorkerThread(getWorkerByPath(workerPath)) assert.Equal(t, stateReady, autoScaledThread.state.get()) // on down-scale, the thread will be marked as inactive diff --git a/worker.go b/worker.go index 3c9455b77..e36458d9a 100644 --- a/worker.go +++ b/worker.go @@ -14,33 +14,36 @@ import ( // represents a worker script and can have many threads assigned to it type worker struct { - name string - fileName string - num int - env PreparedEnv - requestChan chan *frankenPHPContext - threads []*phpThread - threadMutex sync.RWMutex + name string + fileName string + num int + env PreparedEnv + requestChan chan *frankenPHPContext + threads []*phpThread + threadMutex sync.RWMutex + allowPathMatching bool // allow matching a request to a worker via path } var ( - workers map[string]*worker + workers []*worker watcherIsEnabled bool ) func initWorkers(opt []workerOpt) error { - workers = make(map[string]*worker, len(opt)) + workers = make([]*worker, 0, len(opt)) workersReady := sync.WaitGroup{} directoriesToWatch := getDirectoriesToWatch(opt) watcherIsEnabled = len(directoriesToWatch) > 0 for _, o := range opt { - worker, err := newWorker(o) + _, err := newWorker(o) if err != nil { return err } + } - workersReady.Add(o.num) + for _, worker := range workers { + workersReady.Add(worker.num) for i := 0; i < worker.num; i++ { thread := getInactivePHPThread() convertToWorkerThread(thread, worker) @@ -65,48 +68,59 @@ func initWorkers(opt []workerOpt) error { return nil } -func getWorkerKey(name string, filename string) string { - key := filename - if strings.HasPrefix(name, "m#") { - key = name +func getWorkerByName(name string) *worker { + for _, w := range workers { + if w.name == name { + return w + } } - return key + + return nil +} + +func getWorkerByPath(path string) *worker { + for _, w := range workers { + if w.fileName == path && w.allowPathMatching { + return w + } + } + + return nil } func newWorker(o workerOpt) (*worker, error) { absFileName, err := fastabs.FastAbs(o.fileName) + isModuleWorker := strings.HasPrefix(o.name, "m#") // module workers may not be matched by path if err != nil { return nil, fmt.Errorf("worker filename is invalid %q: %w", o.fileName, err) } - key := getWorkerKey(o.name, absFileName) - if _, ok := workers[key]; ok { - return nil, fmt.Errorf("two workers cannot use the same key %q", key) + if o.name == "" { + o.name = absFileName } - for _, w := range workers { - if w.name == o.name { - return w, fmt.Errorf("two workers cannot have the same name: %q", o.name) - } + + if w := getWorkerByPath(absFileName); w != nil && !isModuleWorker { + return w, fmt.Errorf("two workers cannot have the same filename: %q", absFileName) + } + if w := getWorkerByName(o.name); w != nil { + return w, fmt.Errorf("two workers cannot have the same name: %q", o.name) } if o.env == nil { o.env = make(PreparedEnv, 1) } - if o.name == "" { - o.name = absFileName - } - o.env["FRANKENPHP_WORKER\x00"] = "1" w := &worker{ - name: o.name, - fileName: absFileName, - num: o.num, - env: o.env, - requestChan: make(chan *frankenPHPContext), - threads: make([]*phpThread, 0, o.num), - } - workers[key] = w + name: o.name, + fileName: absFileName, + num: o.num, + env: o.env, + requestChan: make(chan *frankenPHPContext), + threads: make([]*phpThread, 0, o.num), + allowPathMatching: !isModuleWorker, + } + workers = append(workers, w) return w, nil } From 30427d315bb1dd41625e6fa2bee97a5f346d29a4 Mon Sep 17 00:00:00 2001 From: Alliballibaba Date: Wed, 11 Jun 2025 20:16:03 +0200 Subject: [PATCH 43/59] trigger From 1ed4504cd3f4023d1e23d668c384c7abb4537468 Mon Sep 17 00:00:00 2001 From: Alliballibaba Date: Wed, 11 Jun 2025 20:53:54 +0200 Subject: [PATCH 44/59] Changes worker->match to match->worker --- caddy/caddy_test.go | 12 ++++-------- caddy/module.go | 19 ++++++++++++++++++- caddy/workerconfig.go | 7 +------ 3 files changed, 23 insertions(+), 15 deletions(-) diff --git a/caddy/caddy_test.go b/caddy/caddy_test.go index cbe2ddcf8..ceab1d283 100644 --- a/caddy/caddy_test.go +++ b/caddy/caddy_test.go @@ -1329,8 +1329,7 @@ func TestWorkerMatchDirectiveWithFileServer(t *testing.T) { http://localhost:`+testPort+` { php_server { root ../testdata/files - worker { - match /matched-path* + match /matched-path* worker { file ../worker-with-counter.php num 1 } @@ -1339,13 +1338,11 @@ func TestWorkerMatchDirectiveWithFileServer(t *testing.T) { http://localhost:`+testport2+` { php_server { root ../testdata - worker { - match /counter* + match /counter* worker { file worker-with-counter.php num 1 } - worker { - match /index* + match /index* worker { file index.php num 1 } @@ -1393,8 +1390,7 @@ func TestWorkerMatchDirectiveWithoutFileServer(t *testing.T) { php_server { file_server off root ../testdata/files - worker { - match /some-path + match /some-path worker { file ../worker-with-counter.php num 1 } diff --git a/caddy/module.go b/caddy/module.go index e0990e68c..9f2604cdb 100644 --- a/caddy/module.go +++ b/caddy/module.go @@ -254,9 +254,26 @@ func (f *FrankenPHPModule) UnmarshalCaddyfile(d *caddyfile.Dispenser) error { return err } f.Workers = append(f.Workers, wc) + case "match": + if !d.NextArg() { + return d.ArgErr() + } + match := "*" + if d.Val() != "worker" { + match = d.Val() + if !d.NextArg() || d.Val() != "worker" { + return d.ArgErr() + } + } + wc, err := parseWorkerConfig(d) + if err != nil { + return err + } + wc.Match = match + f.Workers = append(f.Workers, wc) default: - allowedDirectives := "root, split, env, resolve_root_symlink, worker" + allowedDirectives := "root, split, env, resolve_root_symlink, worker, match" return wrongSubDirectiveError("php or php_server", allowedDirectives, d.Val()) } } diff --git a/caddy/workerconfig.go b/caddy/workerconfig.go index 6140bb210..f52fee4ed 100644 --- a/caddy/workerconfig.go +++ b/caddy/workerconfig.go @@ -112,14 +112,9 @@ func parseWorkerConfig(d *caddyfile.Dispenser) (workerConfig, error) { } else { wc.Watch = append(wc.Watch, d.Val()) } - case "match": - if !d.NextArg() { - return wc, d.ArgErr() - } - wc.Match = d.Val() default: - allowedDirectives := "name, file, num, env, watch, match" + allowedDirectives := "name, file, num, env, watch" return wc, wrongSubDirectiveError("worker", allowedDirectives, v) } } From 36820c31c7328c9557bb5c80aeb6cfe922952e00 Mon Sep 17 00:00:00 2001 From: Alliballibaba Date: Wed, 11 Jun 2025 21:00:53 +0200 Subject: [PATCH 45/59] Adjusts tests. --- caddy/caddy_test.go | 23 +++++++++++------------ 1 file changed, 11 insertions(+), 12 deletions(-) diff --git a/caddy/caddy_test.go b/caddy/caddy_test.go index ceab1d283..629b59d39 100644 --- a/caddy/caddy_test.go +++ b/caddy/caddy_test.go @@ -1338,11 +1338,11 @@ func TestWorkerMatchDirectiveWithFileServer(t *testing.T) { http://localhost:`+testport2+` { php_server { root ../testdata - match /counter* worker { + match /counter/* worker { file worker-with-counter.php num 1 } - match /index* worker { + match /index/* worker { file index.php num 1 } @@ -1350,31 +1350,30 @@ func TestWorkerMatchDirectiveWithFileServer(t *testing.T) { } `, "caddyfile") - // the first php_server with root in testdata/files - // Forward request that match * to the worker + // worker is outside of public directory, match anyways tester.AssertGetResponse("http://localhost:"+testPort+"/matched-path", http.StatusOK, "requests:1") tester.AssertGetResponse("http://localhost:"+testPort+"/matched-path/anywhere", http.StatusOK, "requests:2") - // 404 on not matching paths + // 404 on no matching paths r, _ := http.NewRequest("GET", "http://localhost:"+testPort+"/not-matched-path", nil) tester.AssertResponseCode(r, http.StatusNotFound) - // forward files to fileserver + // static file will be served by fileserver tester.AssertGetResponse("http://localhost:"+testPort+"/static.txt2", http.StatusOK, "Hello from file") - // the second php_server has root in testdata and has 2 different workers + // the second php_server 2 different workers in the public path tester.AssertGetResponse("http://localhost:"+testport2+"/counter/sub-path", http.StatusOK, "requests:1") tester.AssertGetResponse("http://localhost:"+testport2+"/index/sub-path", http.StatusOK, "I am by birth a Genevese (i not set)") - // 404 on not matching path - r, _ = http.NewRequest("GET", "http://localhost:"+testPort+"/other", nil) + // 404 on no matching paths + r, _ = http.NewRequest("GET", "http://localhost:"+testport2+"/other", nil) tester.AssertResponseCode(r, http.StatusNotFound) - // forward files to fileserver - tester.AssertGetResponse("http://localhost:"+testPort+"/static.txt2", http.StatusOK, "Hello from file") + // static file will be served by fileserver + tester.AssertGetResponse("http://localhost:"+testport2+"/files/static.txt2", http.StatusOK, "Hello from file") // never serve PHP files directly - r, _ = http.NewRequest("GET", "http://localhost:"+testPort+"/index.php", nil) + r, _ = http.NewRequest("GET", "http://localhost:"+testport2+"/index.php", nil) tester.AssertResponseCode(r, http.StatusNotFound) } From 683393322b3e939a46f77a6d7ea67659e8a77986 Mon Sep 17 00:00:00 2001 From: Alliballibaba Date: Wed, 11 Jun 2025 21:05:07 +0200 Subject: [PATCH 46/59] formatting --- caddy/caddy_test.go | 30 +++++++++++++++--------------- 1 file changed, 15 insertions(+), 15 deletions(-) diff --git a/caddy/caddy_test.go b/caddy/caddy_test.go index 629b59d39..0941ed7f0 100644 --- a/caddy/caddy_test.go +++ b/caddy/caddy_test.go @@ -1336,18 +1336,18 @@ func TestWorkerMatchDirectiveWithFileServer(t *testing.T) { } } http://localhost:`+testport2+` { - php_server { - root ../testdata - match /counter/* worker { - file worker-with-counter.php - num 1 - } + php_server { + root ../testdata + match /counter/* worker { + file worker-with-counter.php + num 1 + } match /index/* worker { - file index.php - num 1 - } - } - } + file index.php + num 1 + } + } + } `, "caddyfile") // worker is outside of public directory, match anyways @@ -1358,10 +1358,10 @@ func TestWorkerMatchDirectiveWithFileServer(t *testing.T) { r, _ := http.NewRequest("GET", "http://localhost:"+testPort+"/not-matched-path", nil) tester.AssertResponseCode(r, http.StatusNotFound) - // static file will be served by fileserver + // static file will be served by the fileserver tester.AssertGetResponse("http://localhost:"+testPort+"/static.txt2", http.StatusOK, "Hello from file") - // the second php_server 2 different workers in the public path + // the second php_server has 2 different workers in the public path tester.AssertGetResponse("http://localhost:"+testport2+"/counter/sub-path", http.StatusOK, "requests:1") tester.AssertGetResponse("http://localhost:"+testport2+"/index/sub-path", http.StatusOK, "I am by birth a Genevese (i not set)") @@ -1369,10 +1369,10 @@ func TestWorkerMatchDirectiveWithFileServer(t *testing.T) { r, _ = http.NewRequest("GET", "http://localhost:"+testport2+"/other", nil) tester.AssertResponseCode(r, http.StatusNotFound) - // static file will be served by fileserver + // static file will be served by the fileserver tester.AssertGetResponse("http://localhost:"+testport2+"/files/static.txt2", http.StatusOK, "Hello from file") - // never serve PHP files directly + // 404 on PHP files r, _ = http.NewRequest("GET", "http://localhost:"+testport2+"/index.php", nil) tester.AssertResponseCode(r, http.StatusNotFound) } From 1ecb44cfb056ba58411788096ba68f4a9d7dc4fd Mon Sep 17 00:00:00 2001 From: Alliballibaba Date: Fri, 13 Jun 2025 08:52:59 +0200 Subject: [PATCH 47/59] Resets implementation to worker->match --- caddy/caddy_test.go | 87 +++++++++++++--------- caddy/module.go | 81 ++++++-------------- caddy/workerconfig.go | 30 +++----- testdata/files/.gitignore | 2 +- testdata/files/{static.txt2 => static.txt} | 0 testdata/hello.php | 3 + 6 files changed, 90 insertions(+), 113 deletions(-) rename testdata/files/{static.txt2 => static.txt} (100%) create mode 100644 testdata/hello.php diff --git a/caddy/caddy_test.go b/caddy/caddy_test.go index 0941ed7f0..c67e363e7 100644 --- a/caddy/caddy_test.go +++ b/caddy/caddy_test.go @@ -1317,9 +1317,8 @@ func TestWorkerRestart(t *testing.T) { )) } -func TestWorkerMatchDirectiveWithFileServer(t *testing.T) { +func TestWorkerMatchDirective(t *testing.T) { tester := caddytest.NewTester(t) - testport2 := "3000" tester.InitServer(` { skip_install_trust @@ -1329,52 +1328,65 @@ func TestWorkerMatchDirectiveWithFileServer(t *testing.T) { http://localhost:`+testPort+` { php_server { root ../testdata/files - match /matched-path* worker { + worker { file ../worker-with-counter.php + match /matched-path* num 1 } } } - http://localhost:`+testport2+` { + `, "caddyfile") + + // worker is outside of public directory, match anyways + tester.AssertGetResponse("http://localhost:"+testPort+"/matched-path", http.StatusOK, "requests:1") + tester.AssertGetResponse("http://localhost:"+testPort+"/matched-path/anywhere", http.StatusOK, "requests:2") + + // 404 on unmatched paths + tester.AssertGetResponse("http://localhost:"+testPort+"/elsewhere", http.StatusNotFound, "") + + // file will be serverd by file_server + tester.AssertGetResponse("http://localhost:"+testPort+"/static.txt", http.StatusOK, "Hello from file") +} + +func TestWorkerMatchDirectiveWithMultipleWorkers(t *testing.T) { + tester := caddytest.NewTester(t) + tester.InitServer(` + { + skip_install_trust + admin localhost:2999 + } + http://localhost:`+testPort+` { php_server { root ../testdata - match /counter/* worker { + worker { file worker-with-counter.php + match /counter/* num 1 } - match /index/* worker { + worker { file index.php + match /index/* num 1 } } } `, "caddyfile") - // worker is outside of public directory, match anyways - tester.AssertGetResponse("http://localhost:"+testPort+"/matched-path", http.StatusOK, "requests:1") - tester.AssertGetResponse("http://localhost:"+testPort+"/matched-path/anywhere", http.StatusOK, "requests:2") - - // 404 on no matching paths - r, _ := http.NewRequest("GET", "http://localhost:"+testPort+"/not-matched-path", nil) - tester.AssertResponseCode(r, http.StatusNotFound) + // match 2 workers respectively (in the public directory) + tester.AssertGetResponse("http://localhost:"+testPort+"/counter/sub-path", http.StatusOK, "requests:1") + tester.AssertGetResponse("http://localhost:"+testPort+"/index/sub-path", http.StatusOK, "I am by birth a Genevese (i not set)") // static file will be served by the fileserver - tester.AssertGetResponse("http://localhost:"+testPort+"/static.txt2", http.StatusOK, "Hello from file") - - // the second php_server has 2 different workers in the public path - tester.AssertGetResponse("http://localhost:"+testport2+"/counter/sub-path", http.StatusOK, "requests:1") - tester.AssertGetResponse("http://localhost:"+testport2+"/index/sub-path", http.StatusOK, "I am by birth a Genevese (i not set)") + tester.AssertGetResponse("http://localhost:"+testPort+"/files/static.txt", http.StatusOK, "Hello from file") - // 404 on no matching paths - r, _ = http.NewRequest("GET", "http://localhost:"+testport2+"/other", nil) - tester.AssertResponseCode(r, http.StatusNotFound) + // 404 if the request falls through + tester.AssertGetResponse("http://localhost:"+testPort+"/not-matched", http.StatusNotFound, "") - // static file will be served by the fileserver - tester.AssertGetResponse("http://localhost:"+testport2+"/files/static.txt2", http.StatusOK, "Hello from file") + // serve php file directly as fallback + tester.AssertGetResponse("http://localhost:"+testPort+"/hello.php", http.StatusOK, "Hello") - // 404 on PHP files - r, _ = http.NewRequest("GET", "http://localhost:"+testport2+"/index.php", nil) - tester.AssertResponseCode(r, http.StatusNotFound) + // serve worker file directly as fallback + tester.AssertGetResponse("http://localhost:"+testPort+"/index.php", http.StatusOK, "I am by birth a Genevese (i not set)") } func TestWorkerMatchDirectiveWithoutFileServer(t *testing.T) { @@ -1386,13 +1398,18 @@ func TestWorkerMatchDirectiveWithoutFileServer(t *testing.T) { } http://localhost:`+testPort+` { - php_server { - file_server off - root ../testdata/files - match /some-path worker { - file ../worker-with-counter.php - num 1 + route { + php_server { + index off + file_server off + root ../testdata/files + worker { + file ../worker-with-counter.php + match /some-path + } } + + respond "Request falls through" 404 } } `, "caddyfile") @@ -1400,7 +1417,7 @@ func TestWorkerMatchDirectiveWithoutFileServer(t *testing.T) { // find the worker at some-path tester.AssertGetResponse("http://localhost:"+testPort+"/some-path", http.StatusOK, "requests:1") - // do not find the file at static.txt2 - r, _ := http.NewRequest("GET", "http://localhost:"+testPort+"/static.txt2", nil) - tester.AssertResponseCode(r, http.StatusNotFound) + // do not find the file at static.txt + // the request should completely fall through the php_server module + tester.AssertGetResponse("http://localhost:"+testPort+"/static.txt", http.StatusNotFound, "Request falls through") } diff --git a/caddy/module.go b/caddy/module.go index 9f2604cdb..7b8a94768 100644 --- a/caddy/module.go +++ b/caddy/module.go @@ -254,26 +254,9 @@ func (f *FrankenPHPModule) UnmarshalCaddyfile(d *caddyfile.Dispenser) error { return err } f.Workers = append(f.Workers, wc) - case "match": - if !d.NextArg() { - return d.ArgErr() - } - match := "*" - if d.Val() != "worker" { - match = d.Val() - if !d.NextArg() || d.Val() != "worker" { - return d.ArgErr() - } - } - wc, err := parseWorkerConfig(d) - if err != nil { - return err - } - wc.Match = match - f.Workers = append(f.Workers, wc) default: - allowedDirectives := "root, split, env, resolve_root_symlink, worker, match" + allowedDirectives := "root, split, env, resolve_root_symlink, worker" return wrongSubDirectiveError("php or php_server", allowedDirectives, d.Val()) } } @@ -443,16 +426,12 @@ func parsePhpServer(h httpcaddyfile.Helper) ([]httpcaddyfile.ConfigValue, error) } } - // if any worker has 'match' specified, use an alternate routing approach - for _, w := range phpsrv.Workers { - if w.Match != "" { - return pureWorkerRoute(h, phpsrv, fsrv, disableFsrv), nil - } - } - // set up a route list that we'll append to routes := caddyhttp.RouteList{} + // prepend routes from the 'worker match *' directives + routes = prependWorkerRoutes(routes, h, phpsrv, fsrv, disableFsrv) + // set the list of allowed path segments on which to split phpsrv.SplitPath = extensions @@ -588,23 +567,23 @@ func parsePhpServer(h httpcaddyfile.Helper) ([]httpcaddyfile.ConfigValue, error) }, nil } -// if any worker under php_server has a match pattern, -// routes have to be created differently -// first, we will try to match against the file system, -// then we will try to match aganist each worker -func pureWorkerRoute(h httpcaddyfile.Helper, f FrankenPHPModule, fsrv caddy.Module, disableFsrv bool) []httpcaddyfile.ConfigValue { - f.SplitPath = []string{} - matchers := caddyhttp.MatchPath{} +// workers can also match a path without being in the public directory +// in this case we need to prepend the worker routes to the existing routes +func prependWorkerRoutes(routes caddyhttp.RouteList, h httpcaddyfile.Helper, f FrankenPHPModule, fsrv caddy.Module, disableFsrv bool) caddyhttp.RouteList { + matchPath := caddyhttp.MatchPath{} for _, w := range f.Workers { - if w.Match != "" { - matchers = append(matchers, w.Match) + if w.MatchPath != "" { + matchPath = append(matchPath, w.MatchPath) } } - subRoute := caddyhttp.Subroute{Routes: caddyhttp.RouteList{}} + if len(matchPath) == 0 { + return routes + } + // if there are match patterns, we need to check for files beforehand if !disableFsrv { - subRoute.Routes = append(subRoute.Routes, caddyhttp.Route{ + routes = append(routes, caddyhttp.Route{ MatcherSetsRaw: []caddy.ModuleMap{ caddy.ModuleMap{ "file": h.JSON(fileserver.MatchFile{ @@ -624,31 +603,15 @@ func pureWorkerRoute(h httpcaddyfile.Helper, f FrankenPHPModule, fsrv caddy.Modu }) } - for _, pattern := range matchers { - subRoute.Routes = append(subRoute.Routes, caddyhttp.Route{ - MatcherSetsRaw: []caddy.ModuleMap{ - caddy.ModuleMap{"path": h.JSON(caddyhttp.MatchPath{pattern})}, - }, - HandlersRaw: []json.RawMessage{ - caddyconfig.JSONModuleObject(f, "handler", "php", nil), - }, - }) - } - - notFoundHandler := caddyhttp.StaticResponse{ - StatusCode: caddyhttp.WeakString(strconv.Itoa(http.StatusNotFound)), - } - subRoute.Routes = append(subRoute.Routes, caddyhttp.Route{ - MatcherSetsRaw: []caddy.ModuleMap{}, + // forward matching routes instantly to the PHP handler + routes = append(routes, caddyhttp.Route{ + MatcherSetsRaw: []caddy.ModuleMap{ + caddy.ModuleMap{"path": h.JSON(matchPath)}, + }, HandlersRaw: []json.RawMessage{ - caddyconfig.JSONModuleObject(notFoundHandler, "handler", "static_response", nil), + caddyconfig.JSONModuleObject(f, "handler", "php", nil), }, }) - return []httpcaddyfile.ConfigValue{ - { - Class: "route", - Value: subRoute, - }, - } + return routes } diff --git a/caddy/workerconfig.go b/caddy/workerconfig.go index f52fee4ed..7b882a76a 100644 --- a/caddy/workerconfig.go +++ b/caddy/workerconfig.go @@ -33,22 +33,9 @@ type workerConfig struct { // Directories to watch for file changes Watch []string `json:"watch,omitempty"` // The path to match against the worker - Match string `json:"match,omitempty"` - - matchStrategy matchStrategy + MatchPath string `json:"match_path,omitempty"` } -type matchStrategy int - -const ( - matchNone matchStrategy = iota - matchAll - matchPrefix - matchSuffix - matchExactly - matchWildcard -) - func parseWorkerConfig(d *caddyfile.Dispenser) (workerConfig, error) { wc := workerConfig{} if d.NextArg() { @@ -112,9 +99,14 @@ func parseWorkerConfig(d *caddyfile.Dispenser) (workerConfig, error) { } else { wc.Watch = append(wc.Watch, d.Val()) } + case "match": + if !d.NextArg() { + return wc, d.ArgErr() + } + wc.MatchPath = d.Val() default: - allowedDirectives := "name, file, num, env, watch" + allowedDirectives := "name, file, num, env, watch, match" return wc, wrongSubDirectiveError("worker", allowedDirectives, v) } } @@ -131,11 +123,13 @@ func parseWorkerConfig(d *caddyfile.Dispenser) (workerConfig, error) { } func (wc workerConfig) matchesPath(r *http.Request, documentRoot string) bool { - if wc.Match != "" { - matches, _ := caddyhttp.MatchPath{wc.Match}.MatchWithError(r) - return matches + + // try to match against a pattern if one is assigned + if wc.MatchPath != "" { + return caddyhttp.MatchPath{wc.MatchPath}.Match(r) } + // if there is no pattern, try to match against the actual path (in the public directory) fullScriptPath, _ := fastabs.FastAbs(documentRoot + "/" + r.URL.Path) absFileName, _ := fastabs.FastAbs(wc.FileName) return fullScriptPath == absFileName diff --git a/testdata/files/.gitignore b/testdata/files/.gitignore index 2211df63d..4871fd527 100644 --- a/testdata/files/.gitignore +++ b/testdata/files/.gitignore @@ -1 +1 @@ -*.txt +test.txt diff --git a/testdata/files/static.txt2 b/testdata/files/static.txt similarity index 100% rename from testdata/files/static.txt2 rename to testdata/files/static.txt diff --git a/testdata/hello.php b/testdata/hello.php new file mode 100644 index 000000000..e09aa5c34 --- /dev/null +++ b/testdata/hello.php @@ -0,0 +1,3 @@ + Date: Fri, 13 Jun 2025 10:29:26 +0200 Subject: [PATCH 48/59] Provisions match path rules. --- caddy/workerconfig.go | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/caddy/workerconfig.go b/caddy/workerconfig.go index 7b882a76a..9c00c3236 100644 --- a/caddy/workerconfig.go +++ b/caddy/workerconfig.go @@ -6,6 +6,7 @@ import ( "path/filepath" "strconv" + "github.com/caddyserver/caddy/v2" "github.com/caddyserver/caddy/v2/caddyconfig/caddyfile" "github.com/caddyserver/caddy/v2/modules/caddyhttp" "github.com/dunglas/frankenphp" @@ -103,7 +104,11 @@ func parseWorkerConfig(d *caddyfile.Dispenser) (workerConfig, error) { if !d.NextArg() { return wc, d.ArgErr() } - wc.MatchPath = d.Val() + // provision the path so it's identical to Caddy match rules + // see: https://github.com/caddyserver/caddy/blob/master/modules/caddyhttp/matchers.go + caddyMatchPath := caddyhttp.MatchPath{d.Val()} + caddyMatchPath.Provision(caddy.Context{}) + wc.MatchPath = caddyMatchPath[0] default: allowedDirectives := "name, file, num, env, watch, match" From d3fb25d224b7ebe8dcab40919d51a9b94478be6c Mon Sep 17 00:00:00 2001 From: Alliballibaba Date: Fri, 13 Jun 2025 11:44:16 +0200 Subject: [PATCH 49/59] Allows matching multiple paths --- caddy/module.go | 12 ++++++------ caddy/workerconfig.go | 14 +++++--------- 2 files changed, 11 insertions(+), 15 deletions(-) diff --git a/caddy/module.go b/caddy/module.go index 7b8a94768..451f0bb3c 100644 --- a/caddy/module.go +++ b/caddy/module.go @@ -570,14 +570,14 @@ func parsePhpServer(h httpcaddyfile.Helper) ([]httpcaddyfile.ConfigValue, error) // workers can also match a path without being in the public directory // in this case we need to prepend the worker routes to the existing routes func prependWorkerRoutes(routes caddyhttp.RouteList, h httpcaddyfile.Helper, f FrankenPHPModule, fsrv caddy.Module, disableFsrv bool) caddyhttp.RouteList { - matchPath := caddyhttp.MatchPath{} + allWorkerMatches := caddyhttp.MatchPath{} for _, w := range f.Workers { - if w.MatchPath != "" { - matchPath = append(matchPath, w.MatchPath) + for _, path := range w.MatchPath { + allWorkerMatches = append(matchPath, path) } } - if len(matchPath) == 0 { + if len(allWorkerMatches) == 0 { return routes } @@ -603,10 +603,10 @@ func prependWorkerRoutes(routes caddyhttp.RouteList, h httpcaddyfile.Helper, f F }) } - // forward matching routes instantly to the PHP handler + // forward matching routes to the PHP handler routes = append(routes, caddyhttp.Route{ MatcherSetsRaw: []caddy.ModuleMap{ - caddy.ModuleMap{"path": h.JSON(matchPath)}, + caddy.ModuleMap{"path": h.JSON(allWorkerMatches)}, }, HandlersRaw: []json.RawMessage{ caddyconfig.JSONModuleObject(f, "handler", "php", nil), diff --git a/caddy/workerconfig.go b/caddy/workerconfig.go index 9c00c3236..e85eb2b3d 100644 --- a/caddy/workerconfig.go +++ b/caddy/workerconfig.go @@ -34,7 +34,7 @@ type workerConfig struct { // Directories to watch for file changes Watch []string `json:"watch,omitempty"` // The path to match against the worker - MatchPath string `json:"match_path,omitempty"` + MatchPath []string `json:"match_path,omitempty"` } func parseWorkerConfig(d *caddyfile.Dispenser) (workerConfig, error) { @@ -101,15 +101,11 @@ func parseWorkerConfig(d *caddyfile.Dispenser) (workerConfig, error) { wc.Watch = append(wc.Watch, d.Val()) } case "match": - if !d.NextArg() { - return wc, d.ArgErr() - } // provision the path so it's identical to Caddy match rules // see: https://github.com/caddyserver/caddy/blob/master/modules/caddyhttp/matchers.go - caddyMatchPath := caddyhttp.MatchPath{d.Val()} + caddyMatchPath := (caddyhttp.MatchPath)(d.RemainingArgs()) caddyMatchPath.Provision(caddy.Context{}) - wc.MatchPath = caddyMatchPath[0] - + wc.MatchPath = ([]string)(caddyMatchPath) default: allowedDirectives := "name, file, num, env, watch, match" return wc, wrongSubDirectiveError("worker", allowedDirectives, v) @@ -130,8 +126,8 @@ func parseWorkerConfig(d *caddyfile.Dispenser) (workerConfig, error) { func (wc workerConfig) matchesPath(r *http.Request, documentRoot string) bool { // try to match against a pattern if one is assigned - if wc.MatchPath != "" { - return caddyhttp.MatchPath{wc.MatchPath}.Match(r) + if len(wc.MatchPath) != 0 { + return (caddyhttp.MatchPath)(wc.MatchPath).Match(r) } // if there is no pattern, try to match against the actual path (in the public directory) From 6e5350e0f70063941772172245a51f1ef7e010fa Mon Sep 17 00:00:00 2001 From: Alliballibaba Date: Fri, 13 Jun 2025 11:48:40 +0200 Subject: [PATCH 50/59] Fixes var --- caddy/module.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/caddy/module.go b/caddy/module.go index 451f0bb3c..c7461c27f 100644 --- a/caddy/module.go +++ b/caddy/module.go @@ -573,7 +573,7 @@ func prependWorkerRoutes(routes caddyhttp.RouteList, h httpcaddyfile.Helper, f F allWorkerMatches := caddyhttp.MatchPath{} for _, w := range f.Workers { for _, path := range w.MatchPath { - allWorkerMatches = append(matchPath, path) + allWorkerMatches = append(allWorkerMatches, path) } } From a95faf4df02fa9776a27cee2e5d2953a863acc05 Mon Sep 17 00:00:00 2001 From: Alliballibaba Date: Fri, 13 Jun 2025 23:35:29 +0200 Subject: [PATCH 51/59] Formatting. --- caddy/caddy_test.go | 4 ++-- testdata/hello.php | 2 +- worker.go | 11 +++++++---- 3 files changed, 10 insertions(+), 7 deletions(-) diff --git a/caddy/caddy_test.go b/caddy/caddy_test.go index c67e363e7..720a6a171 100644 --- a/caddy/caddy_test.go +++ b/caddy/caddy_test.go @@ -1344,7 +1344,7 @@ func TestWorkerMatchDirective(t *testing.T) { // 404 on unmatched paths tester.AssertGetResponse("http://localhost:"+testPort+"/elsewhere", http.StatusNotFound, "") - // file will be serverd by file_server + // static file will be served by the fileserver tester.AssertGetResponse("http://localhost:"+testPort+"/static.txt", http.StatusOK, "Hello from file") } @@ -1383,7 +1383,7 @@ func TestWorkerMatchDirectiveWithMultipleWorkers(t *testing.T) { tester.AssertGetResponse("http://localhost:"+testPort+"/not-matched", http.StatusNotFound, "") // serve php file directly as fallback - tester.AssertGetResponse("http://localhost:"+testPort+"/hello.php", http.StatusOK, "Hello") + tester.AssertGetResponse("http://localhost:"+testPort+"/hello.php", http.StatusOK, "Hello from PHP") // serve worker file directly as fallback tester.AssertGetResponse("http://localhost:"+testPort+"/index.php", http.StatusOK, "I am by birth a Genevese (i not set)") diff --git a/testdata/hello.php b/testdata/hello.php index e09aa5c34..81a3ccfdf 100644 --- a/testdata/hello.php +++ b/testdata/hello.php @@ -1,3 +1,3 @@ Date: Sat, 14 Jun 2025 12:44:35 +0200 Subject: [PATCH 52/59] refactoring. --- caddy/module.go | 15 +++++---------- caddy/workerconfig.go | 12 ++++++++++++ 2 files changed, 17 insertions(+), 10 deletions(-) diff --git a/caddy/module.go b/caddy/module.go index c7461c27f..6ce1ffb2e 100644 --- a/caddy/module.go +++ b/caddy/module.go @@ -70,21 +70,16 @@ func (f *FrankenPHPModule) Provision(ctx caddy.Context) error { } for i, wc := range f.Workers { - // Inherit environment variables from the parent php_server directive + + // make the file path absolute from the public directory + // this can only be done if the root is definied inside php_server if !filepath.IsAbs(wc.FileName) && f.Root != "" { wc.FileName = filepath.Join(f.Root, wc.FileName) } + // Inherit environment variables from the parent php_server directive if f.Env != nil { - if wc.Env == nil { - wc.Env = make(map[string]string) - } - for k, v := range f.Env { - // Only set if not already defined in the worker - if _, exists := wc.Env[k]; !exists { - wc.Env[k] = v - } - } + wc.inheritEnv(f.Env) } f.Workers[i] = wc } diff --git a/caddy/workerconfig.go b/caddy/workerconfig.go index e85eb2b3d..6d58072de 100644 --- a/caddy/workerconfig.go +++ b/caddy/workerconfig.go @@ -123,6 +123,18 @@ func parseWorkerConfig(d *caddyfile.Dispenser) (workerConfig, error) { return wc, nil } +func (wc workerConfig) inheritEnv(env map[string]string) { + if wc.Env == nil { + wc.Env = make(map[string]string) + } + for k, v := range env { + // do not overwrite existing environment variables + if _, exists := wc.Env[k]; !exists { + wc.Env[k] = v + } + } +} + func (wc workerConfig) matchesPath(r *http.Request, documentRoot string) bool { // try to match against a pattern if one is assigned From eb9b02ac5aa2e9e78bd1aa48394050015a69b2bb Mon Sep 17 00:00:00 2001 From: Alliballibaba Date: Sat, 28 Jun 2025 22:59:18 +0200 Subject: [PATCH 53/59] Adds docs. --- docs/config.md | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/docs/config.md b/docs/config.md index 4bf8c989b..0fcef5714 100644 --- a/docs/config.md +++ b/docs/config.md @@ -153,6 +153,7 @@ php_server [] { name # Sets the name for the worker, used in logs and metrics. Default: absolute path of worker file. Always starts with m# when defined in a php_server block. watch # Sets the path to watch for file changes. Can be specified more than once for multiple paths. env # Sets an extra environment variable to the given value. Can be specified more than once for multiple environment variables. Environment variables for this worker are also inherited from the php_server parent, but can be overwritten here. + match # match the worker to a path pattern. Overrides try_files and can only be used in the php_server directive. } worker # Can also use the short form like in the global frankenphp block. } @@ -203,6 +204,29 @@ where the FrankenPHP process was started. You can instead also specify one or mo The file watcher is based on [e-dant/watcher](https://github.com/e-dant/watcher). +## Matching the worker to a path + +In traditional PHP applications, scripts are always placed in the public directory. +This is also true for worker scripts, which are treated like any other PHP script. +If you want to instead put the worker script outside the public directory, you can do so via the `match` directive. + +The `match` directive is an optimized alternative to `try_files` only available inside `php_server` and `php`. +The following example will always serve a file in the public directory if present +and otherwise forward the request to the worker matching the path pattern. + +```caddyfile +{ + frankenphp { + php_server { + worker { + file /path/to/worker.php + match /api/* # all requests starting with /api/ will be handled by this worker + } + } + } +} +``` + ### Full Duplex (HTTP/1) When using HTTP/1.x, it may be desirable to enable full-duplex mode to allow writing a response before the entire body From 48bb34cf5ddaa078867e078e17de767829d61cac Mon Sep 17 00:00:00 2001 From: Alliballibaba Date: Sat, 28 Jun 2025 23:04:05 +0200 Subject: [PATCH 54/59] Fixes merge removal. --- caddy/module.go | 1 - 1 file changed, 1 deletion(-) diff --git a/caddy/module.go b/caddy/module.go index be03785ee..1aae40885 100644 --- a/caddy/module.go +++ b/caddy/module.go @@ -151,7 +151,6 @@ func needReplacement(s string) bool { } // ServeHTTP implements caddyhttp.MiddlewareHandler. -// TODO: Expose TLS versions as env vars, as Apache's mod_ssl: https://github.com/caddyserver/caddy/blob/master/modules/caddyhttp/reverseproxy/fastcgi/fastcgi.go#L298 func (f *FrankenPHPModule) ServeHTTP(w http.ResponseWriter, r *http.Request, _ caddyhttp.Handler) error { origReq := r.Context().Value(caddyhttp.OriginalRequestCtxKey).(http.Request) repl := r.Context().Value(caddy.ReplacerCtxKey).(*caddy.Replacer) From 392739992e7f84c7237dda774402b0832f2cc584 Mon Sep 17 00:00:00 2001 From: Alexander Stecher <45872305+AlliBalliBaba@users.noreply.github.com> Date: Sun, 29 Jun 2025 20:50:56 +0200 Subject: [PATCH 55/59] Update config.md --- docs/config.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/config.md b/docs/config.md index 0fcef5714..b4a527545 100644 --- a/docs/config.md +++ b/docs/config.md @@ -219,7 +219,7 @@ and otherwise forward the request to the worker matching the path pattern. frankenphp { php_server { worker { - file /path/to/worker.php + file /path/to/worker.php # file can be outside of public path match /api/* # all requests starting with /api/ will be handled by this worker } } From 949295bcfeafb66a8b1dc6191f2d6baef8785356 Mon Sep 17 00:00:00 2001 From: Alliballibaba Date: Mon, 30 Jun 2025 21:08:23 +0200 Subject: [PATCH 56/59] go fmt. --- worker.go | 30 +++++++++++++++--------------- 1 file changed, 15 insertions(+), 15 deletions(-) diff --git a/worker.go b/worker.go index ade38b727..04772fa4a 100644 --- a/worker.go +++ b/worker.go @@ -14,14 +14,14 @@ import ( // represents a worker script and can have many threads assigned to it type worker struct { - name string - fileName string - num int - env PreparedEnv - requestChan chan *frankenPHPContext - threads []*phpThread - threadMutex sync.RWMutex - allowPathMatching bool + name string + fileName string + num int + env PreparedEnv + requestChan chan *frankenPHPContext + threads []*phpThread + threadMutex sync.RWMutex + allowPathMatching bool maxConsecutiveFailures int } @@ -117,13 +117,13 @@ func newWorker(o workerOpt) (*worker, error) { o.env["FRANKENPHP_WORKER\x00"] = "1" w := &worker{ - name: o.name, - fileName: absFileName, - num: o.num, - env: o.env, - requestChan: make(chan *frankenPHPContext), - threads: make([]*phpThread, 0, o.num), - allowPathMatching: allowPathMatching, + name: o.name, + fileName: absFileName, + num: o.num, + env: o.env, + requestChan: make(chan *frankenPHPContext), + threads: make([]*phpThread, 0, o.num), + allowPathMatching: allowPathMatching, maxConsecutiveFailures: o.maxConsecutiveFailures, } From fd18aa456e61c902069a84b33ddfdbd841dd02ab Mon Sep 17 00:00:00 2001 From: Alliballibaba Date: Mon, 30 Jun 2025 21:18:42 +0200 Subject: [PATCH 57/59] Adds line ending to static.txt and fixes tests. --- caddy/caddy_test.go | 8 ++++++-- testdata/files/static.txt | 2 +- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/caddy/caddy_test.go b/caddy/caddy_test.go index 720a6a171..d02cd86c5 100644 --- a/caddy/caddy_test.go +++ b/caddy/caddy_test.go @@ -1345,7 +1345,9 @@ func TestWorkerMatchDirective(t *testing.T) { tester.AssertGetResponse("http://localhost:"+testPort+"/elsewhere", http.StatusNotFound, "") // static file will be served by the fileserver - tester.AssertGetResponse("http://localhost:"+testPort+"/static.txt", http.StatusOK, "Hello from file") + expectedFileResponse, err := os.ReadFile("../testdata/files/static.txt") + require.NoError(t, err, "static.txt file must be readable for this test") + tester.AssertGetResponse("http://localhost:"+testPort+"/static.txt", http.StatusOK, string(expectedFileResponse)) } func TestWorkerMatchDirectiveWithMultipleWorkers(t *testing.T) { @@ -1377,7 +1379,9 @@ func TestWorkerMatchDirectiveWithMultipleWorkers(t *testing.T) { tester.AssertGetResponse("http://localhost:"+testPort+"/index/sub-path", http.StatusOK, "I am by birth a Genevese (i not set)") // static file will be served by the fileserver - tester.AssertGetResponse("http://localhost:"+testPort+"/files/static.txt", http.StatusOK, "Hello from file") + expectedFileResponse, err := os.ReadFile("../testdata/files/static.txt") + require.NoError(t, err, "static.txt file must be readable for this test") + tester.AssertGetResponse("http://localhost:"+testPort+"/files/static.txt", http.StatusOK, string(expectedFileResponse)) // 404 if the request falls through tester.AssertGetResponse("http://localhost:"+testPort+"/not-matched", http.StatusNotFound, "") diff --git a/testdata/files/static.txt b/testdata/files/static.txt index 09be5ef4e..6e4a53176 100644 --- a/testdata/files/static.txt +++ b/testdata/files/static.txt @@ -1 +1 @@ -Hello from file \ No newline at end of file +Hello from file From a11c17df57387c37deafbb892e0a3fcd0381f8d9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?K=C3=A9vin=20Dunglas?= Date: Mon, 30 Jun 2025 22:02:05 +0200 Subject: [PATCH 58/59] Trigger CI From a9a15899dbf378f1e97130ccfbb5f85ba95d7b2e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?K=C3=A9vin=20Dunglas?= Date: Mon, 30 Jun 2025 22:07:01 +0200 Subject: [PATCH 59/59] fix Markdown CS --- docs/config.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/config.md b/docs/config.md index 9f18c4b83..58b19e8ea 100644 --- a/docs/config.md +++ b/docs/config.md @@ -207,7 +207,7 @@ The file watcher is based on [e-dant/watcher](https://github.com/e-dant/watcher) ## Matching the worker to a path -In traditional PHP applications, scripts are always placed in the public directory. +In traditional PHP applications, scripts are always placed in the public directory. This is also true for worker scripts, which are treated like any other PHP script. If you want to instead put the worker script outside the public directory, you can do so via the `match` directive.