diff --git a/caddy/caddy_test.go b/caddy/caddy_test.go index 9d3a9a0132..d02cd86c57 100644 --- a/caddy/caddy_test.go +++ b/caddy/caddy_test.go @@ -1316,3 +1316,112 @@ func TestWorkerRestart(t *testing.T) { "frankenphp_worker_restarts", )) } + +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 + worker { + file ../worker-with-counter.php + match /matched-path* + 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 unmatched paths + tester.AssertGetResponse("http://localhost:"+testPort+"/elsewhere", http.StatusNotFound, "") + + // static file will be served by the fileserver + 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) { + tester := caddytest.NewTester(t) + tester.InitServer(` + { + skip_install_trust + admin localhost:2999 + } + http://localhost:`+testPort+` { + php_server { + root ../testdata + worker { + file worker-with-counter.php + match /counter/* + num 1 + } + worker { + file index.php + match /index/* + num 1 + } + } + } + `, "caddyfile") + + // 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 + 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, "") + + // serve php file directly as fallback + 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)") +} + +func TestWorkerMatchDirectiveWithoutFileServer(t *testing.T) { + tester := caddytest.NewTester(t) + tester.InitServer(` + { + skip_install_trust + admin localhost:2999 + } + + http://localhost:`+testPort+` { + 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") + + // find the worker at some-path + tester.AssertGetResponse("http://localhost:"+testPort+"/some-path", http.StatusOK, "requests:1") + + // 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/config_test.go b/caddy/config_test.go index 7bcbbfc27b..29fee1d241 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 08f284ebcd..1aae408859 100644 --- a/caddy/module.go +++ b/caddy/module.go @@ -69,6 +69,21 @@ 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 { + + // 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 { + wc.inheritEnv(f.Env) + } + f.Workers[i] = wc + } + workers, err := fapp.addModuleWorkers(f.Workers...) if err != nil { return err @@ -161,12 +176,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, documentRoot) { workerName = w.Name + break } } @@ -230,12 +244,11 @@ func (f *FrankenPHPModule) UnmarshalCaddyfile(d *caddyfile.Dispenser) error { f.ResolveRootSymlink = &v case "worker": - for d.NextBlock(1) { - } - for d.NextArg() { + wc, err := parseWorkerConfig(d) + if err != nil { + return err } - // Skip "worker" blocks in the first pass - continue + f.Workers = append(f.Workers, wc) default: allowedDirectives := "root, split, env, resolve_root_symlink, worker" @@ -244,43 +257,13 @@ 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) - } + // Check if a worker with this filename already exists in this module + 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 @@ -418,6 +401,13 @@ 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 + if err := phpsrv.UnmarshalCaddyfile(dispenser); err != nil { + return nil, err + } + if frankenphp.EmbeddedAppPath != "" { if phpsrv.Root == "" { phpsrv.Root = filepath.Join(frankenphp.EmbeddedAppPath, defaultDocumentRoot) @@ -433,6 +423,9 @@ func parsePhpServer(h httpcaddyfile.Helper) ([]httpcaddyfile.ConfigValue, error) // 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 @@ -521,15 +514,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 +560,52 @@ func parsePhpServer(h httpcaddyfile.Helper) ([]httpcaddyfile.ConfigValue, error) }, }, nil } + +// 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 { + allWorkerMatches := caddyhttp.MatchPath{} + for _, w := range f.Workers { + for _, path := range w.MatchPath { + allWorkerMatches = append(allWorkerMatches, path) + } + } + + if len(allWorkerMatches) == 0 { + return routes + } + + // if there are match patterns, we need to check for files beforehand + if !disableFsrv { + routes = append(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), + }, + }) + } + + // forward matching routes to the PHP handler + routes = append(routes, caddyhttp.Route{ + MatcherSetsRaw: []caddy.ModuleMap{ + caddy.ModuleMap{"path": h.JSON(allWorkerMatches)}, + }, + HandlersRaw: []json.RawMessage{ + caddyconfig.JSONModuleObject(f, "handler", "php", nil), + }, + }) + + return routes +} diff --git a/caddy/workerconfig.go b/caddy/workerconfig.go index dcf1afa18a..60a2c6fd33 100644 --- a/caddy/workerconfig.go +++ b/caddy/workerconfig.go @@ -2,11 +2,15 @@ package caddy import ( "errors" + "net/http" "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" + "github.com/dunglas/frankenphp/internal/fastabs" ) // workerConfig represents the "worker" directive in the Caddyfile @@ -29,6 +33,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 + MatchPath []string `json:"match_path,omitempty"` // MaxConsecutiveFailures sets the maximum number of consecutive failures before panicking (defaults to 6, set to -1 to never panick) MaxConsecutiveFailures int `json:"max_consecutive_failures,omitempty"` } @@ -96,6 +102,12 @@ func parseWorkerConfig(d *caddyfile.Dispenser) (workerConfig, error) { } else { wc.Watch = append(wc.Watch, d.Val()) } + case "match": + // 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.RemainingArgs()) + caddyMatchPath.Provision(caddy.Context{}) + wc.MatchPath = ([]string)(caddyMatchPath) case "max_consecutive_failures": if !d.NextArg() { return wc, d.ArgErr() @@ -111,7 +123,7 @@ func parseWorkerConfig(d *caddyfile.Dispenser) (workerConfig, error) { wc.MaxConsecutiveFailures = int(v) default: - allowedDirectives := "name, file, num, env, watch, max_consecutive_failures" + allowedDirectives := "name, file, num, env, watch, match, max_consecutive_failures" return wc, wrongSubDirectiveError("worker", allowedDirectives, v) } } @@ -126,3 +138,29 @@ 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, len(env)) + } + 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 + 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) + fullScriptPath, _ := fastabs.FastAbs(documentRoot + "/" + r.URL.Path) + absFileName, _ := fastabs.FastAbs(wc.FileName) + + return fullScriptPath == absFileName +} diff --git a/context.go b/context.go index 29ed40af30..6de140da7a 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/docs/config.md b/docs/config.md index 8720ac48f0..58b19e8ea1 100644 --- a/docs/config.md +++ b/docs/config.md @@ -154,6 +154,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. } @@ -204,6 +205,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 # file can be outside of public path + 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 diff --git a/frankenphp.go b/frankenphp.go index 37fb236784..164df14c5c 100644 --- a/frankenphp.go +++ b/frankenphp.go @@ -403,8 +403,9 @@ 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 57776a7e91..db348b54d0 100644 --- a/phpmainthread_test.go +++ b/phpmainthread_test.go @@ -189,8 +189,9 @@ func TestFinishBootingAWorkerScript(t *testing.T) { } func TestReturnAnErrorIf2WorkersHaveTheSameFileName(t *testing.T) { - workers = make(map[string]*worker) - _, err1 := newWorker(workerOpt{fileName: "filename.php", maxConsecutiveFailures: defaultMaxConsecutiveFailures}) + workers = []*worker{} + w, err1 := newWorker(workerOpt{fileName: "filename.php", maxConsecutiveFailures: defaultMaxConsecutiveFailures}) + workers = append(workers, w) _, err2 := newWorker(workerOpt{fileName: "filename.php", maxConsecutiveFailures: defaultMaxConsecutiveFailures}) assert.NoError(t, err1) @@ -198,8 +199,9 @@ func TestReturnAnErrorIf2WorkersHaveTheSameFileName(t *testing.T) { } func TestReturnAnErrorIf2ModuleWorkersHaveTheSameName(t *testing.T) { - workers = make(map[string]*worker) - _, err1 := newWorker(workerOpt{fileName: "filename.php", name: "workername", maxConsecutiveFailures: defaultMaxConsecutiveFailures}) + workers = []*worker{} + w, err1 := newWorker(workerOpt{fileName: "filename.php", name: "workername", maxConsecutiveFailures: defaultMaxConsecutiveFailures}) + workers = append(workers, w) _, err2 := newWorker(workerOpt{fileName: "filename2.php", name: "workername", maxConsecutiveFailures: defaultMaxConsecutiveFailures}) assert.NoError(t, err1) @@ -208,13 +210,14 @@ 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, num: 1, maxConsecutiveFailures: defaultMaxConsecutiveFailures, }) + workers = append(workers, worker) return worker } @@ -241,9 +244,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 6c1cb59491..2227b06eed 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 a75e579d63..57e6c598b9 100644 --- a/scaling.go +++ b/scaling.go @@ -158,8 +158,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 aa4c363cc4..89e04b51ee 100644 --- a/scaling_test.go +++ b/scaling_test.go @@ -48,7 +48,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/testdata/files/.gitignore b/testdata/files/.gitignore index 2211df63dd..4871fd5275 100644 --- a/testdata/files/.gitignore +++ b/testdata/files/.gitignore @@ -1 +1 @@ -*.txt +test.txt diff --git a/testdata/files/static.txt b/testdata/files/static.txt new file mode 100644 index 0000000000..6e4a53176e --- /dev/null +++ b/testdata/files/static.txt @@ -0,0 +1 @@ +Hello from file diff --git a/testdata/hello.php b/testdata/hello.php new file mode 100644 index 0000000000..81a3ccfdf2 --- /dev/null +++ b/testdata/hello.php @@ -0,0 +1,3 @@ + 0 for _, o := range opt { - worker, err := newWorker(o) + w, err := newWorker(o) if err != nil { return err } + workers = append(workers, w) + } - workersReady.Add(o.num) + for _, worker := range workers { + workersReady.Add(worker.num) for i := 0; i < worker.num; i++ { thread := getInactivePHPThread() convertToWorkerThread(thread, worker) @@ -66,12 +70,24 @@ 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 nil +} + +func getWorkerByPath(path string) *worker { + for _, w := range workers { + if w.fileName == path && w.allowPathMatching { + return w + } } - return key + + return nil } func newWorker(o workerOpt) (*worker, error) { @@ -80,24 +96,25 @@ func newWorker(o workerOpt) (*worker, error) { 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) - } + + // workers that have a name starting with "m#" are module workers + // they can only be matched by their name, not by their path + allowPathMatching := !strings.HasPrefix(o.name, "m#") + + if w := getWorkerByPath(absFileName); w != nil && allowPathMatching { + 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, @@ -106,9 +123,9 @@ func newWorker(o workerOpt) (*worker, error) { env: o.env, requestChan: make(chan *frankenPHPContext), threads: make([]*phpThread, 0, o.num), + allowPathMatching: allowPathMatching, maxConsecutiveFailures: o.maxConsecutiveFailures, } - workers[key] = w return w, nil }