feat: worker matching (php#1646)

* Adds 'match' configuration

* test

* Adds Caddy's matcher.

* Adds no-fileserver test.

* Prevents duplicate path calculations and optimizes worker access.

* trigger

* Changes worker->match to match->worker

* Adjusts tests.

* formatting

* Resets implementation to worker->match

* Provisions match path rules.

* Allows matching multiple paths

* Fixes var

* Formatting.

* refactoring.

* Adds 'match' configuration

* test

* Adds Caddy's matcher.

* Adds no-fileserver test.

* Prevents duplicate path calculations and optimizes worker access.

* trigger

* Changes worker->match to match->worker

* Adjusts tests.

* formatting

* Resets implementation to worker->match

* Provisions match path rules.

* Allows matching multiple paths

* Fixes var

* Formatting.

* refactoring.

* Update frankenphp.go

Co-authored-by: Kévin Dunglas <[email protected]>

* Update caddy/workerconfig.go

Co-authored-by: Kévin Dunglas <[email protected]>

* Update caddy/workerconfig.go

Co-authored-by: Kévin Dunglas <[email protected]>

* Update caddy/module.go

Co-authored-by: Kévin Dunglas <[email protected]>

* Update caddy/module.go

Co-authored-by: Kévin Dunglas <[email protected]>

* Fixes suggestion

* Refactoring.

* Adds 'match' configuration

* test

* Adds Caddy's matcher.

* Adds no-fileserver test.

* Prevents duplicate path calculations and optimizes worker access.

* trigger

* Changes worker->match to match->worker

* Adjusts tests.

* formatting

* Resets implementation to worker->match

* Provisions match path rules.

* Allows matching multiple paths

* Fixes var

* Formatting.

* refactoring.

* Adds docs.

* Fixes merge removal.

* Update config.md

* go fmt.

* Adds line ending to static.txt and fixes tests.

* Trigger CI

* fix Markdown CS

---------

Co-authored-by: Alliballibaba <[email protected]>
Co-authored-by: Kévin Dunglas <[email protected]>

Author: AlliBalliBaba

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")
+}

caddy/config_test.go

@@ -1,4 +1,4 @@
-package caddy
+package caddy
 
 import (
 	"testing"

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
+}

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
+}