From fba37f6786f0e6e884564de3a13c941507ac18a2 Mon Sep 17 00:00:00 2001 From: Alliballibaba Date: Wed, 1 Jul 2026 22:52:37 +0200 Subject: [PATCH 01/18] new phpserver --- caddy/admin_test.go | 45 +++++++++++ caddy/app.go | 91 +++------------------ caddy/config_test.go | 110 -------------------------- caddy/hotreload.go | 8 +- caddy/module.go | 171 ++++++++++++++++++---------------------- caddy/workerconfig.go | 54 ++++--------- cgi.go | 11 ++- context.go | 79 +++++++++++++------ frankenphp.go | 37 +++------ frankenphp_test.go | 18 +++++ options.go | 102 ++++++++++++++++++++++++ phpmainthread_test.go | 20 ++--- phpserver.go | 102 ++++++++++++++++++++++++ phpthread.go | 12 ++- scaling_test.go | 2 +- threadinactive.go | 6 -- threadregular.go | 43 ++++------ threadtasks_test.go | 5 -- threadworker.go | 44 +++-------- worker.go | 109 +++++++++++++++++-------- worker_internal_test.go | 32 ++++++++ workerextension.go | 8 +- 22 files changed, 611 insertions(+), 498 deletions(-) create mode 100644 phpserver.go diff --git a/caddy/admin_test.go b/caddy/admin_test.go index 27ece031f3..03bf151ed5 100644 --- a/caddy/admin_test.go +++ b/caddy/admin_test.go @@ -348,3 +348,48 @@ func TestAddModuleWorkerViaAdminApi(t *testing.T) { // Make a request to the worker to verify it's working tester.AssertGetResponse("http://localhost:"+testPort+"/worker-with-counter.php", http.StatusOK, "requests:1") } + +func TestRegisteredModuleWorkerPoolsMustBeCorrect(t *testing.T) { + tester := caddytest.NewTester(t) + initServer(t, tester, ` + { + skip_install_trust + admin localhost:2999 + + frankenphp { + num_threads 4 + worker ../testdata/worker-with-env.php 1 + } + } + + http://localhost:`+testPort+` { + route { + php { + root ../testdata + worker worker-with-counter.php 1 { + match /matched* + } + } + php { + root ../testdata + worker worker.php 1 + } + } + } + `, "caddyfile") + + debugState := getDebugState(t, tester) + + worker1Path, _ := fastabs.FastAbs("../testdata/worker-with-env.php") + worker2Path, _ := fastabs.FastAbs("../testdata/worker-with-counter.php") + worker3Path, _ := fastabs.FastAbs("../testdata/worker.php") + receivedThreadNames := make([]string, 0) + for _, thread := range debugState.ThreadDebugStates { + receivedThreadNames = append(receivedThreadNames, thread.Name) + } + + assert.Contains(t, receivedThreadNames, "Regular PHP Thread", "expected a regular thread to be present") + assert.Contains(t, receivedThreadNames, "Worker PHP Thread - "+worker1Path, "expected worker 1 to be present") + assert.Contains(t, receivedThreadNames, "Worker PHP Thread - "+worker2Path, "expected worker 2 to be present") + assert.Contains(t, receivedThreadNames, "Worker PHP Thread - "+worker3Path, "expected worker 3 to be present") +} diff --git a/caddy/app.go b/caddy/app.go index e4593acc49..bd3a91f967 100644 --- a/caddy/app.go +++ b/caddy/app.go @@ -17,7 +17,6 @@ import ( "github.com/caddyserver/caddy/v2/caddyconfig/httpcaddyfile" "github.com/caddyserver/caddy/v2/modules/caddyhttp" "github.com/dunglas/frankenphp" - "github.com/dunglas/frankenphp/internal/fastabs" ) var ( @@ -60,12 +59,15 @@ type FrankenPHPApp struct { // EXPERIMENTAL: MaxRequests sets the maximum number of requests a PHP thread handles before restarting (0 = unlimited) MaxRequests int `json:"max_requests,omitempty"` - opts []frankenphp.Option - metrics frankenphp.Metrics - ctx context.Context - logger *slog.Logger + opts []frankenphp.Option + metrics frankenphp.Metrics + ctx context.Context + logger *slog.Logger + phpServerCount int } +var phpServers = make(map[int]*frankenphp.PhpServer) + var errIni = errors.New(`"php_ini" must be in the format: php_ini "" ""`) // CaddyModule returns the Caddy module information. @@ -101,74 +103,6 @@ func (f *FrankenPHPApp) Provision(ctx caddy.Context) error { return nil } -func (f *FrankenPHPApp) generateUniqueModuleWorkerName(filepath string) string { - var i uint - filepath, _ = fastabs.FastAbs(filepath) - name := "m#" + filepath - -retry: - for _, wc := range f.Workers { - if wc.Name == name { - name = fmt.Sprintf("m#%s_%d", filepath, i) - i++ - - goto retry - } - } - - return name -} - -func (f *FrankenPHPApp) addModuleWorkers(workers ...workerConfig) ([]workerConfig, error) { - for i := range workers { - w := &workers[i] - - if frankenphp.EmbeddedAppPath != "" && filepath.IsLocal(w.FileName) { - w.FileName = filepath.Join(frankenphp.EmbeddedAppPath, w.FileName) - } - } - - // A php_server directive is provisioned once per route it's embedded in. Only the first embed - // registers its pools; later embeds reuse them by position, never touching other directives (#2477). - var registered []workerConfig - if len(workers) > 0 && workers[0].routeGroup != "" { - registered = f.moduleWorkersInRouteGroup(workers[0].routeGroup) - } - - for i := range workers { - if i < len(registered) { - workers[i].Name = registered[i].Name - continue - } - - f.registerModuleWorker(&workers[i]) - } - - return workers, nil -} - -func (f *FrankenPHPApp) registerModuleWorker(w *workerConfig) { - if w.Name == "" { - w.Name = f.generateUniqueModuleWorkerName(w.FileName) - } else if !strings.HasPrefix(w.Name, "m#") { - w.Name = "m#" + w.Name - } - - f.Workers = append(f.Workers, *w) -} - -// moduleWorkersInRouteGroup returns the registered workers of one directive, in registration order. -func (f *FrankenPHPApp) moduleWorkersInRouteGroup(routeGroup string) []workerConfig { - var group []workerConfig - for _, w := range f.Workers { - if w.routeGroup == routeGroup { - group = append(group, w) - } - } - - return group -} - func (f *FrankenPHPApp) Start() error { repl := caddy.NewReplacer() @@ -189,15 +123,8 @@ func (f *FrankenPHPApp) Start() error { ) for _, w := range f.Workers { - w.options = append(w.options, - frankenphp.WithWorkerEnv(w.Env), - frankenphp.WithWorkerWatchMode(w.Watch), - frankenphp.WithWorkerMaxFailures(w.MaxConsecutiveFailures), - frankenphp.WithWorkerMaxThreads(w.MaxThreads), - frankenphp.WithWorkerRequestOptions(w.requestOptions...), - ) - - f.opts = append(f.opts, frankenphp.WithWorkers(w.Name, repl.ReplaceKnown(w.FileName, ""), w.Num, w.options...)) + w.FileName = repl.ReplaceKnown(w.FileName, "") + f.opts = append(f.opts, frankenphp.WithWorkers(w.Name, w.FileName, w.Num, w.toWorkerOptions()...)) } frankenphp.Shutdown() diff --git a/caddy/config_test.go b/caddy/config_test.go index c0da09c565..49a6739e8c 100644 --- a/caddy/config_test.go +++ b/caddy/config_test.go @@ -1,104 +1,12 @@ package caddy import ( - "strings" "testing" "github.com/caddyserver/caddy/v2/caddyconfig/caddyfile" "github.com/stretchr/testify/require" ) -func TestPhpServerWorkerMatchNoDuplicatePools(t *testing.T) { - const config = ` - { - php { - worker { - file ../testdata/worker-with-env.php - num 2 - match /index.php/* - } - worker { - file ../testdata/worker-with-counter.php - num 1 - } - } - }` - - // a fresh copy per call mirrors the two module instances Caddy provisions for one directive - parseWorkers := func(routeGroup string) []workerConfig { - d := caddyfile.NewTestDispenser(config) - module := &FrankenPHPModule{} - require.NoError(t, module.UnmarshalCaddyfile(d)) - require.Len(t, module.Workers, 2, "block should parse to two workers") - for i := range module.Workers { - module.Workers[i].routeGroup = routeGroup - } - - return module.Workers - } - - app := &FrankenPHPApp{} - - first, err := app.addModuleWorkers(parseWorkers("g1")...) - require.NoError(t, err) - second, err := app.addModuleWorkers(parseWorkers("g1")...) - require.NoError(t, err) - - require.Len(t, app.Workers, 2, "each worker must be registered exactly once") - - for _, w := range app.Workers { - require.False(t, strings.HasSuffix(w.Name, "_0"), - "no _0-suffixed duplicate pool may exist, got %q", w.Name) - } - - // both embeds must resolve to the same pools for serve-time matching - require.Len(t, first, 2) - require.Len(t, second, 2) - for i := range first { - require.Equal(t, first[i].Name, second[i].Name, - "both routes must reference the same pool name for worker %d", i) - } -} - -func TestPhpServerSeparateDirectivesKeepDistinctPools(t *testing.T) { - worker := func(routeGroup string) workerConfig { - return workerConfig{FileName: "../testdata/worker-with-env.php", Num: 2, routeGroup: routeGroup} - } - - app := &FrankenPHPApp{} - site1, err := app.addModuleWorkers(worker("g1")) - require.NoError(t, err) - site2, err := app.addModuleWorkers(worker("g2")) - require.NoError(t, err) - - require.Len(t, app.Workers, 2, "identical workers from separate directives must stay separate pools") - require.NotEqual(t, site1[0].Name, site2[0].Name, "separate directives must not share a pool name") - require.True(t, strings.HasSuffix(site2[0].Name, "_0"), - "the second directive's pool must take a unique _0 name, got %q", site2[0].Name) -} - -func TestPhpServerEmbedReuseIsPositional(t *testing.T) { - embed := func() []workerConfig { - return []workerConfig{ - {FileName: "../testdata/worker-with-env.php", Num: 1, MatchPath: []string{"/x/*"}, routeGroup: "g1"}, - {FileName: "../testdata/worker-with-env.php", Num: 1, MatchPath: []string{"/x/*"}, routeGroup: "g1"}, - } - } - - app := &FrankenPHPApp{} - first, err := app.addModuleWorkers(embed()...) - require.NoError(t, err) - second, err := app.addModuleWorkers(embed()...) - require.NoError(t, err) - - require.Len(t, app.Workers, 2, "two identical workers in one block stay two pools, just as without the duplicate embed") - require.NotEqual(t, first[0].Name, first[1].Name, "the second identical worker must take its own _0 pool") - require.True(t, strings.HasSuffix(first[1].Name, "_0"), "got %q", first[1].Name) - for i := range first { - require.Equal(t, first[i].Name, second[i].Name, "the duplicate embed must reuse pools by position for worker %d", i) - } -} - func TestModuleWorkerDuplicateFilenamesFail(t *testing.T) { // Create a test configuration with duplicate worker filenames configWithDuplicateFilenames := ` @@ -168,7 +76,6 @@ func TestModuleWorkersDifferentNamesSucceed(t *testing.T) { // Parse the first configuration d1 := caddyfile.NewTestDispenser(configWithWorkerName1) - app := &FrankenPHPApp{} module1 := &FrankenPHPModule{} // Unmarshal the first configuration @@ -196,16 +103,6 @@ func TestModuleWorkersDifferentNamesSucceed(t *testing.T) { // Verify that no error was returned require.NoError(t, err, "Expected no error when two workers have different names") - - _, err = app.addModuleWorkers(module1.Workers...) - require.NoError(t, err, "Expected no error when adding the first module workers") - _, err = app.addModuleWorkers(module2.Workers...) - require.NoError(t, err, "Expected no error when adding the second module workers") - - // Verify that both workers were added - require.Len(t, app.Workers, 2, "Expected two workers in the app") - require.Equal(t, "m#test-worker-1", app.Workers[0].Name, "First worker should have the correct name") - require.Equal(t, "m#test-worker-2", app.Workers[1].Name, "Second worker should have the correct name") } func TestModuleWorkerWithEnvironmentVariables(t *testing.T) { @@ -294,7 +191,6 @@ func TestModuleWorkerWithCustomName(t *testing.T) { // Parse the configuration d := caddyfile.NewTestDispenser(configWithCustomName) module := &FrankenPHPModule{} - app := &FrankenPHPApp{} // Unmarshal the configuration err := module.UnmarshalCaddyfile(d) @@ -305,10 +201,4 @@ func TestModuleWorkerWithCustomName(t *testing.T) { // Verify that the worker was added to the module require.Len(t, module.Workers, 1, "Expected one worker to be added to the module") require.Equal(t, "../testdata/worker-with-env.php", module.Workers[0].FileName, "Worker should have the correct filename") - - // Verify that the worker was added to app.Workers with the m# prefix - module.Workers, err = app.addModuleWorkers(module.Workers...) - require.NoError(t, err, "Expected no error when adding the worker to the app") - require.Equal(t, "m#custom-worker-name", module.Workers[0].Name, "Worker should have the custom name, prefixed with m#") - require.Equal(t, "m#custom-worker-name", app.Workers[0].Name, "Worker should have the custom name, prefixed with m#") } diff --git a/caddy/hotreload.go b/caddy/hotreload.go index 4b9c0ebe69..bcab369372 100644 --- a/caddy/hotreload.go +++ b/caddy/hotreload.go @@ -26,6 +26,7 @@ type hotReloadConfig struct { Watch []string `json:"watch"` } +// TODO: this should be scoped to the php_server to avoid duplicate hot reloads func (f *FrankenPHPModule) configureHotReload(app *FrankenPHPApp) error { if f.HotReload == nil { return nil @@ -49,7 +50,12 @@ func (f *FrankenPHPModule) configureHotReload(app *FrankenPHPApp) error { } app.opts = append(app.opts, frankenphp.WithHotReload(f.HotReload.Topic, f.mercureHub, f.HotReload.Watch)) - f.preparedEnv["FRANKENPHP_HOT_RELOAD\x00"] = "/.well-known/mercure?topic=" + url.QueryEscape(f.HotReload.Topic) + + // add the hot reload to the env variables + if f.Env == nil { + f.Env = make(map[string]string) + } + f.Env["FRANKENPHP_HOT_RELOAD"] = "/.well-known/mercure?topic=" + url.QueryEscape(f.HotReload.Topic) return nil } diff --git a/caddy/module.go b/caddy/module.go index bf57c0efa0..3c92d256ca 100644 --- a/caddy/module.go +++ b/caddy/module.go @@ -4,7 +4,6 @@ import ( "encoding/json" "errors" "fmt" - "log/slog" "net/http" "path/filepath" "runtime" @@ -45,14 +44,13 @@ type FrankenPHPModule struct { Env map[string]string `json:"env,omitempty"` // Workers configures the worker scripts to start. Workers []workerConfig `json:"workers,omitempty"` - // RouteGroup is set automatically to pair the route embeds of one php_server directive (#2477). Do not set it manually. - RouteGroup string `json:"route_group,omitempty"` - - resolvedDocumentRoot string - preparedEnv frankenphp.PreparedEnv - preparedEnvNeedsReplacement bool - logger *slog.Logger - requestOptions []frankenphp.RequestOption + // PhpServerIdx is the index of the php server to use, do not set manually + PhpServerIdx int `json:"php_server_idx,omitempty"` + + resolvedDocumentRoot string + preparedEnv frankenphp.PreparedEnv + requestOptions []frankenphp.RequestOption + phpServer *frankenphp.PhpServer } // CaddyModule returns the Caddy module information. @@ -65,7 +63,6 @@ func (FrankenPHPModule) CaddyModule() caddy.ModuleInfo { // Provision sets up the module. func (f *FrankenPHPModule) Provision(ctx caddy.Context) error { - f.logger = ctx.Slogger() app, err := ctx.App("frankenphp") if err != nil { return err @@ -80,7 +77,6 @@ func (f *FrankenPHPModule) Provision(ctx caddy.Context) error { f.assignMercureHub(ctx) - loggerOpt := frankenphp.WithRequestLogger(f.logger) for i, wc := range f.Workers { // make the file path absolute from the public directory // this can only be done if the root is defined inside php_server @@ -88,21 +84,23 @@ func (f *FrankenPHPModule) Provision(ctx caddy.Context) error { 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) - } - - wc.requestOptions = append(wc.requestOptions, loggerOpt) - wc.routeGroup = f.RouteGroup f.Workers[i] = wc } - workers, err := fapp.addModuleWorkers(f.Workers...) - if err != nil { - return err + for i, wc := range f.Workers { + if frankenphp.EmbeddedAppPath != "" && filepath.IsLocal(wc.FileName) { + wc.FileName = filepath.Join(frankenphp.EmbeddedAppPath, wc.FileName) + } + + phpServerPrefix := fmt.Sprintf("m%d#", f.PhpServerIdx) + if wc.Name == "" { + wc.Name = f.generateUniqueModuleWorkerName(wc.FileName, phpServerPrefix) + } else if !strings.HasPrefix(wc.Name, phpServerPrefix) { + wc.Name = phpServerPrefix + wc.Name + } + + f.Workers[i] = wc } - f.Workers = workers if f.Root == "" { if frankenphp.EmbeddedAppPath == "" { @@ -116,25 +114,10 @@ func (f *FrankenPHPModule) Provision(ctx caddy.Context) error { f.Root = filepath.Join(frankenphp.EmbeddedAppPath, f.Root) } - if len(f.SplitPath) == 0 { - f.SplitPath = []string{".php"} - } - - opt, err := frankenphp.WithRequestSplitPath(f.SplitPath) - if err != nil { - return fmt.Errorf("invalid split_path: %w", err) - } - f.requestOptions = append(f.requestOptions, opt) - if f.ResolveRootSymlink == nil { f.ResolveRootSymlink = new(true) } - // Always pre-compute absolute file names for fallback matching - for i := range f.Workers { - f.Workers[i].absFileName, _ = fastabs.FastAbs(f.Workers[i].FileName) - } - if !needReplacement(f.Root) { root, err := fastabs.FastAbs(f.Root) if err != nil { @@ -155,45 +138,65 @@ func (f *FrankenPHPModule) Provision(ctx caddy.Context) error { if filepath.IsAbs(wc.FileName) { resolvedPath, _ := filepath.EvalSymlinks(wc.FileName) f.Workers[i].FileName = resolvedPath - f.Workers[i].absFileName = resolvedPath } } } - // Pre-compute relative match paths for all workers (requires resolved document root) - docRootWithSep := f.resolvedDocumentRoot + string(filepath.Separator) - for i := range f.Workers { - if strings.HasPrefix(f.Workers[i].absFileName, docRootWithSep) { - f.Workers[i].matchRelPath = filepath.ToSlash(f.Workers[i].absFileName[len(f.resolvedDocumentRoot):]) - } - } - - f.requestOptions = append(f.requestOptions, frankenphp.WithRequestResolvedDocumentRoot(f.resolvedDocumentRoot)) } - if f.preparedEnv == nil { - f.preparedEnv = frankenphp.PrepareEnv(f.Env) + if err := f.configureHotReload(fapp); err != nil { + return err + } - for _, e := range f.preparedEnv { - if needReplacement(e) { - f.preparedEnvNeedsReplacement = true + unchangingEnv := make(map[string]string) // env variables that do not need replacement + requestEnv := make(map[string]string) // env variables that need replacement, e.g. {http.vars.root} - break - } + for k, e := range f.Env { + if needReplacement(e) { + requestEnv[k] = e + } else { + unchangingEnv[k] = e } } - if !f.preparedEnvNeedsReplacement { - f.requestOptions = append(f.requestOptions, frankenphp.WithRequestPreparedEnv(f.preparedEnv)) + f.preparedEnv = frankenphp.PrepareEnv(requestEnv) + + // note: duplicate PhpServerIdx registration will be ignored, only the first one will be used + // this is necessary since caddy drops the module instance between parsing and provisioning + phpServerOptions := []frankenphp.PhpServerOption{ + frankenphp.WithPhpServerRoot(f.resolvedDocumentRoot), + frankenphp.WithPhpServerEnv(unchangingEnv), + frankenphp.WithPHPServerLogger(ctx.Slogger()), + frankenphp.WithPhpServerSplitPath(f.SplitPath), } - if err := f.configureHotReload(fapp); err != nil { - return err + for _, w := range f.Workers { + phpServerOptions = append(phpServerOptions, frankenphp.WithPhpServerWorker(w.Name, w.FileName, w.Num, w.toWorkerOptions()...)) } + fapp.opts = append(fapp.opts, frankenphp.WithPhpServer(f.PhpServerIdx, phpServerOptions...)) + return nil } +func (f *FrankenPHPModule) generateUniqueModuleWorkerName(filepath string, phpServerPrefix string) string { + var i uint + filepath, _ = fastabs.FastAbs(filepath) + name := phpServerPrefix + filepath + +retry: + for _, wc := range f.Workers { + if wc.Name == name { + name = phpServerPrefix + filepath + fmt.Sprintf("_%d", i) + i++ + + goto retry + } + } + + return name +} + // needReplacement checks if a string contains placeholders. func needReplacement(s string) bool { return strings.ContainsAny(s, "{}") @@ -208,6 +211,7 @@ func (f *FrankenPHPModule) ServeHTTP(w http.ResponseWriter, r *http.Request, _ c opts := make([]frankenphp.RequestOption, 0, len(f.requestOptions)+4) opts = append(opts, f.requestOptions...) + opts = append(opts, frankenphp.WithOriginalRequest(new(ctx.Value(caddyhttp.OriginalRequestCtxKey).(http.Request)))) if documentRoot == "" { documentRoot = repl.ReplaceKnown(f.Root, "") @@ -221,8 +225,8 @@ func (f *FrankenPHPModule) ServeHTTP(w http.ResponseWriter, r *http.Request, _ c opts = append(opts, frankenphp.WithRequestDocumentRoot(documentRoot, false)) } - if f.preparedEnvNeedsReplacement { - env := make(frankenphp.PreparedEnv, len(f.Env)) + if len(f.preparedEnv) > 0 { + env := make(frankenphp.PreparedEnv, len(f.preparedEnv)) for k, v := range f.preparedEnv { env[k] = repl.ReplaceKnown(v, "") } @@ -230,28 +234,9 @@ func (f *FrankenPHPModule) ServeHTTP(w http.ResponseWriter, r *http.Request, _ c opts = append(opts, frankenphp.WithRequestPreparedEnv(env)) } - workerName := "" - for _, w := range f.Workers { - if w.matchesPath(r, documentRoot) { - workerName = w.Name - break - } - } + err := frankenphp.PhpServers[f.PhpServerIdx].ServeHTTP(w, r, opts...) - fr, err := frankenphp.NewRequestWithContext( - r, - append( - opts, - frankenphp.WithOriginalRequest(new(ctx.Value(caddyhttp.OriginalRequestCtxKey).(http.Request))), - frankenphp.WithWorkerName(workerName), - )..., - ) - - if err != nil { - return caddyhttp.Error(http.StatusInternalServerError, err) - } - - if err = frankenphp.ServeHTTP(w, fr); err != nil && !errors.As(err, &frankenphp.ErrRejected{}) { + if err != nil && !errors.As(err, &frankenphp.ErrRejected{}) { return caddyhttp.Error(http.StatusInternalServerError, err) } @@ -282,10 +267,8 @@ func (f *FrankenPHPModule) UnmarshalCaddyfile(d *caddyfile.Dispenser) error { } if f.Env == nil { f.Env = make(map[string]string) - f.preparedEnv = make(frankenphp.PreparedEnv) } f.Env[args[0]] = args[1] - f.preparedEnv[args[0]+"\x00"] = args[1] case "resolve_root_symlink": if !d.NextArg() { @@ -334,16 +317,23 @@ func (f *FrankenPHPModule) UnmarshalCaddyfile(d *caddyfile.Dispenser) error { return nil } +func (f *FrankenPHPModule) assignPhpServerIdx(h httpcaddyfile.Helper) error { + counter, _ := h.State["php_server_count"].(int) + f.PhpServerIdx = counter + h.State["php_server_count"] = counter + 1 + return nil +} + // parseCaddyfile unmarshals tokens from h into a new Middleware. func parseCaddyfile(h httpcaddyfile.Helper) (caddyhttp.MiddlewareHandler, error) { m := &FrankenPHPModule{} err := m.UnmarshalCaddyfile(h.Dispenser) + m.assignPhpServerIdx(h) + return m, err } -const routeGroupStateKey = "frankenphp.worker_route_group_seq" - // parsePhpServer parses the php_server directive, which has a similar syntax // to the php_fastcgi directive. A line such as this: // @@ -376,12 +366,6 @@ func parsePhpServer(h httpcaddyfile.Helper) ([]httpcaddyfile.ConfigValue, error) return nil, h.ArgErr() } - // per-adaptation counter: identical for both embeds of this directive, distinct for every other - // (including separate snippet imports), and stable across re-adaptation since State resets each time - seq, _ := h.State[routeGroupStateKey].(int) - h.State[routeGroupStateKey] = seq + 1 - routeGroup := strconv.Itoa(seq) - // set up FrankenPHP phpsrv := FrankenPHPModule{} @@ -481,6 +465,9 @@ func parsePhpServer(h httpcaddyfile.Helper) ([]httpcaddyfile.ConfigValue, error) return nil, err } + // assign a unique index to the php server + phpsrv.assignPhpServerIdx(h) + if frankenphp.EmbeddedAppPath != "" { if phpsrv.Root == "" { phpsrv.Root = filepath.Join(frankenphp.EmbeddedAppPath, defaultDocumentRoot) @@ -492,8 +479,6 @@ func parsePhpServer(h httpcaddyfile.Helper) ([]httpcaddyfile.ConfigValue, error) } } - phpsrv.RouteGroup = routeGroup - // set up a route list that we'll append to routes := caddyhttp.RouteList{} diff --git a/caddy/workerconfig.go b/caddy/workerconfig.go index 0ade09fee3..2464306a17 100644 --- a/caddy/workerconfig.go +++ b/caddy/workerconfig.go @@ -1,8 +1,6 @@ package caddy import ( - "net/http" - "path" "path/filepath" "strconv" @@ -10,7 +8,6 @@ import ( "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 @@ -44,9 +41,6 @@ type workerConfig struct { options []frankenphp.WorkerOption requestOptions []frankenphp.RequestOption - absFileName string - matchRelPath string // pre-computed relative URL path for fast matching - routeGroup string // identifies the php_server directive whose route embeds share this pool } func unmarshalWorker(d *caddyfile.Dispenser) (workerConfig, error) { @@ -162,41 +156,21 @@ func unmarshalWorker(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)) +func (wc *workerConfig) toWorkerOptions() []frankenphp.WorkerOption { + opts := []frankenphp.WorkerOption{ + frankenphp.WithWorkerEnv(wc.Env), + frankenphp.WithWorkerWatchMode(wc.Watch), + frankenphp.WithWorkerMaxFailures(wc.MaxConsecutiveFailures), + frankenphp.WithWorkerMaxThreads(wc.MaxThreads), + frankenphp.WithWorkerRequestOptions(wc.requestOptions...), } - 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) + // copy the caddy match logic and create a unique matcher function for this worker + // inject the matcher into frankenphp + if len(wc.MatchPath) > 0 { + matchFunc := caddyhttp.MatchPath(append([]string(nil), wc.MatchPath...)) + _ = matchFunc.Provision(caddy.Context{}) + opts = append(opts, frankenphp.WithWorkerMatchOn(matchFunc.Match)) } - - // fast path: compare the request URL path against the pre-computed relative path - if wc.matchRelPath != "" { - reqPath := r.URL.Path - if reqPath == wc.matchRelPath { - return true - } - - // ensure leading slash for relative paths (see #2166) - if reqPath == "" || reqPath[0] != '/' { - reqPath = "/" + reqPath - } - - return path.Clean(reqPath) == wc.matchRelPath - } - - // fallback when documentRoot is dynamic (contains placeholders) - fullPath, _ := fastabs.FastAbs(filepath.Join(documentRoot, r.URL.Path)) - - return fullPath == wc.absFileName + return opts } diff --git a/cgi.go b/cgi.go index 26312b7d15..55ddba50af 100644 --- a/cgi.go +++ b/cgi.go @@ -163,6 +163,10 @@ func addHeadersToServer(ctx context.Context, request *http.Request, trackVarsArr } func addPreparedEnvToServer(fc *frankenPHPContext, trackVarsArray *C.zval) { + for k, v := range fc.phpServer.env { + C.frankenphp_register_variable_safe(toUnsafeChar(k), toUnsafeChar(v), C.size_t(len(v)), trackVarsArray) + } + for k, v := range fc.env { C.frankenphp_register_variable_safe(toUnsafeChar(k), toUnsafeChar(v), C.size_t(len(v)), trackVarsArray) } @@ -218,7 +222,12 @@ func splitCgiPath(fc *frankenPHPContext) { // TODO: is it possible to delay this and avoid saving everything in the context? // SCRIPT_FILENAME is the absolute path of SCRIPT_NAME fc.scriptFilename = sanitizedPathJoin(fc.documentRoot, fc.scriptName) - fc.worker = workersByPath[fc.scriptFilename] + + // see if a phpServer or global worker matches the request + fc.worker = fc.phpServer.workersByPath[fc.scriptFilename] + if fc.worker == nil { + fc.worker = globalWorkersByPath[fc.scriptFilename] + } } // splitPos returns the index where path should be split based on splitPath. diff --git a/context.go b/context.go index c8f75b98a4..b279f16943 100644 --- a/context.go +++ b/context.go @@ -7,6 +7,7 @@ import ( "log/slog" "net/http" "os" + "path/filepath" "strconv" "strings" "time" @@ -16,6 +17,7 @@ import ( type frankenPHPContext struct { mercureContext + ctx context.Context documentRoot string splitPath []string env PreparedEnv @@ -29,6 +31,7 @@ type frankenPHPContext struct { scriptName string scriptFilename string requestURI string + phpServer *PhpServer // Whether the request is already closed by us isDone bool @@ -42,17 +45,6 @@ type frankenPHPContext struct { startedAt time.Time } -type contextHolder struct { - ctx context.Context - frankenPHPContext *frankenPHPContext -} - -// fromContext extracts the frankenPHPContext from a context. -func fromContext(ctx context.Context) (fctx *frankenPHPContext, ok bool) { - fctx, ok = ctx.Value(contextKey).(*frankenPHPContext) - return -} - func newFrankenPHPContext() *frankenPHPContext { return &frankenPHPContext{ done: make(chan any), @@ -74,12 +66,41 @@ func NewRequestWithContext(r *http.Request, opts ...RequestOption) (*http.Reques fc := newFrankenPHPContext() fc.request = r + c := context.WithValue(r.Context(), contextKey, opts) + + return r.WithContext(c), nil +} + +func newContextFromRequest(request *http.Request, responseWriter http.ResponseWriter, s *PhpServer, opts ...RequestOption) (*frankenPHPContext, error) { + fc := &frankenPHPContext{ + ctx: request.Context(), + done: make(chan any), + startedAt: time.Now(), + phpServer: s, + splitPath: s.splitPath, + logger: s.logger, + request: request, + documentRoot: s.root, + responseWriter: responseWriter, + requestURI: request.URL.RequestURI(), + } + for _, o := range opts { if err := o(fc); err != nil { return nil, err } } + // see if a worker matches the request + if fc.worker == nil { + for _, w := range s.workers { + if w.matchesRequest(request, s.root) { + fc.worker = w + break + } + } + } + if fc.logger == nil { fc.logger = globalLogger } @@ -97,26 +118,36 @@ func NewRequestWithContext(r *http.Request, opts ...RequestOption) (*http.Reques splitCgiPath(fc) - fc.requestURI = r.URL.RequestURI() - - c := context.WithValue(r.Context(), contextKey, fc) - - return r.WithContext(c), nil + return fc, nil } // newDummyContext creates a fake context from a request path -func newDummyContext(requestPath string, opts ...RequestOption) (*frankenPHPContext, error) { - r, err := http.NewRequestWithContext(globalCtx, http.MethodGet, requestPath, nil) +func newDummyContext(w *worker) (*frankenPHPContext, error) { + r, err := http.NewRequestWithContext(globalCtx, http.MethodGet, filepath.Base(w.fileName), nil) if err != nil { return nil, err } - fr, err := NewRequestWithContext(r, opts...) - if err != nil { - return nil, err + fc := &frankenPHPContext{ + ctx: r.Context(), + phpServer: w.phpServer, + request: r, + startedAt: time.Now(), + logger: globalLogger, + worker: w, + } + + for _, o := range w.requestOptions { + if err := o(fc); err != nil { + return nil, err + } } - fc, _ := fromContext(fr.Context()) + if fc.phpServer == nil { + fc.phpServer = newDummyPhpServer() + } + + splitCgiPath(fc) return fc, nil } @@ -154,12 +185,12 @@ func (fc *frankenPHPContext) validate() error { } func (fc *frankenPHPContext) clientHasClosed() bool { - if fc.request == nil { + if fc.ctx == nil { return false } select { - case <-fc.request.Context().Done(): + case <-fc.ctx.Done(): return true default: return false diff --git a/frankenphp.go b/frankenphp.go index 319f00a30c..5cce426ef2 100644 --- a/frankenphp.go +++ b/frankenphp.go @@ -284,6 +284,11 @@ func Init(options ...Option) error { maxIdleTime = opt.maxIdleTime } + // append all module workers to the global workers for registration + for _, phpServer := range PhpServers { + opt.workers = append(opt.workers, phpServer.workerOpts...) + } + workerThreadCount, err := calculateMaxThreads(opt) if err != nil { Shutdown() @@ -321,7 +326,7 @@ func Init(options ...Option) error { // reused across reloads so queued requests aren't orphaned on a stale channel if regularRequestChan == nil { - regularRequestChan = make(chan contextHolder) + regularRequestChan = make(chan *frankenPHPContext) } regularThreads = make([]*phpThread, 0, opt.numThreads-workerThreadCount) for i := 0; i < opt.numThreads-workerThreadCount; i++ { @@ -376,6 +381,7 @@ func Shutdown() { drainWatchers() drainPHPThreads() + drainPhpServers() metrics.Shutdown() @@ -394,37 +400,16 @@ func Shutdown() { // ServeHTTP executes a PHP script according to the given context. func ServeHTTP(responseWriter http.ResponseWriter, request *http.Request) error { - h := responseWriter.Header() - if h["Server"] == nil { - h["Server"] = serverHeader - } - - if !isRunning { - return ErrNotRunning - } - ctx := request.Context() - fc, ok := fromContext(ctx) - - ch := contextHolder{ctx, fc} + opts, ok := ctx.Value(contextKey).([]RequestOption) if !ok { return ErrInvalidRequest } - fc.responseWriter = responseWriter - - if err := fc.validate(); err != nil { - return err - } - - // Detect if a worker is available to handle this request - if fc.worker != nil { - return fc.worker.handleRequest(ch) - } + phpServer := newDummyPhpServer() - // If no worker was available, send the request to non-worker threads - return handleRequestWithRegularPHPThreads(ch) + return phpServer.ServeHTTP(responseWriter, request, opts...) } //export go_ub_write @@ -796,7 +781,7 @@ func resetGlobals() { globalLogger = slog.Default() workers = nil workersByName = nil - workersByPath = nil + globalWorkersByPath = nil watcherIsEnabled = false maxIdleTime = defaultMaxIdleTime maxRequestsPerThread = 0 diff --git a/frankenphp_test.go b/frankenphp_test.go index 4872893cf2..ec4700eb37 100644 --- a/frankenphp_test.go +++ b/frankenphp_test.go @@ -1349,3 +1349,21 @@ func testOpcachePreload(t *testing.T, opts *testOptions) { assert.Equal(t, "I am preloaded", body) }, opts) } + +func TestPhpServerLib(t *testing.T) { + cwd, _ := os.Getwd() + testDataDir := cwd + "/testdata/" + defer frankenphp.Shutdown() + frankenphp.Init( + frankenphp.WithPhpServer( + 1, + frankenphp.WithPhpServerRoot(testDataDir), + ), + ) + request := httptest.NewRequest("GET", "http://example.com/index.php", nil) + response := httptest.NewRecorder() + + frankenphp.PhpServers[1].ServeHTTP(response, request) + + assert.Equal(t, "I am by birth a Genevese (i not set)", response.Body.String()) +} diff --git a/options.go b/options.go index a9cd2a2630..58b4a88d20 100644 --- a/options.go +++ b/options.go @@ -4,7 +4,10 @@ import ( "context" "fmt" "log/slog" + "net/http" + "strings" "time" + "unicode/utf8" ) // defaultMaxConsecutiveFailures is the default maximum number of consecutive failures before panicking @@ -44,8 +47,10 @@ type workerOpt struct { env PreparedEnv requestOptions []RequestOption watch []string + matchRequest func(*http.Request) bool maxConsecutiveFailures int extensionWorkers *extensionWorkers + phpServer *PhpServer onThreadReady func(int) onThreadShutdown func(int) onServerStartup func() @@ -149,6 +154,17 @@ func WithPhpIni(overrides map[string]string) Option { } } +// WithPhpServer configures a PHP server. +func WithPhpServer(idx int, opts ...PhpServerOption) Option { + return func(o *opt) error { + _, err := newPhpServer(idx, opts...) + if err != nil { + return err + } + return nil + } +} + // WithMaxWaitTime configures the max time a request may be stalled waiting for a thread. func WithMaxWaitTime(maxWaitTime time.Duration) Option { return func(o *opt) error { @@ -212,6 +228,15 @@ func WithWorkerWatchMode(watch []string) WorkerOption { } } +// WithWorkerMatchOn sets a request matcher for this worker (for example from Caddy's MatchPath). +// if no request matcher is set, matching happens explicitly by path +func WithWorkerMatchOn(matchOn func(*http.Request) bool) WorkerOption { + return func(w *workerOpt) error { + w.matchRequest = matchOn + return nil + } +} + // WithWorkerMaxFailures sets the maximum number of consecutive failures before panicking func WithWorkerMaxFailures(maxFailures int) WorkerOption { return func(w *workerOpt) error { @@ -265,3 +290,80 @@ func withExtensionWorkers(w *extensionWorkers) WorkerOption { return nil } } + +func WithPhpServerRoot(root string) PhpServerOption { + return func(s *PhpServer) error { + s.root = root + return nil + } +} + +func WithPhpServerSplitPath(splitPath []string) PhpServerOption { + return func(s *PhpServer) error { + var b strings.Builder + + for i, split := range splitPath { + b.Grow(len(split)) + + for j := 0; j < len(split); j++ { + c := split[j] + if c >= utf8.RuneSelf { + return ErrInvalidSplitPath + } + + if 'A' <= c && c <= 'Z' { + b.WriteByte(c + 'a' - 'A') + } else { + b.WriteByte(c) + } + } + + splitPath[i] = b.String() + b.Reset() + } + s.splitPath = splitPath + + return nil + } +} + +func WithPhpServerEnv(env map[string]string) PhpServerOption { + return func(s *PhpServer) error { + s.env = PrepareEnv(env) + + return nil + } +} + +// WithPhpServerWorker configures the PHP workers to start for a specific php server +func WithPhpServerWorker(name, fileName string, num int, options ...WorkerOption) PhpServerOption { + return func(s *PhpServer) error { + workerOpt := workerOpt{ + name: name, + fileName: fileName, + num: num, + env: PrepareEnv(nil), + watch: []string{}, + maxConsecutiveFailures: defaultMaxConsecutiveFailures, + phpServer: s, + } + + for _, option := range options { + if err := option(&workerOpt); err != nil { + return err + } + } + + s.workerOpts = append(s.workerOpts, workerOpt) + + return nil + } +} + +func WithPHPServerLogger(logger *slog.Logger) PhpServerOption { + return func(s *PhpServer) error { + s.logger = logger + + return nil + } +} diff --git a/phpmainthread_test.go b/phpmainthread_test.go index 8ee5746505..0158425e31 100644 --- a/phpmainthread_test.go +++ b/phpmainthread_test.go @@ -92,7 +92,7 @@ func TestTransitionAThreadBetween2DifferentWorkers(t *testing.T) { // try all possible handler transitions // takes around 200ms and is supposed to force race conditions func TestTransitionThreadsWhileDoingRequests(t *testing.T) { - t.Cleanup(Shutdown) + setupGlobals(t) var ( isDone atomic.Bool @@ -236,8 +236,10 @@ func TestFinishBootingAWorkerScript(t *testing.T) { convertToWorkerThread(phpThreads[0], worker) phpThreads[0].state.WaitFor(state.Ready) - assert.NotNil(t, phpThreads[0].handler.(*workerThread).dummyContext) - assert.Nil(t, phpThreads[0].handler.(*workerThread).workerContext) + dummyFC := phpThreads[0].handler.(*workerThread).dummyFrankenPHPContext + assert.NotNil(t, dummyFC) + assert.NotNil(t, dummyFC.ctx) + assert.Nil(t, phpThreads[0].handler.(*workerThread).workerFrankenPHPContext) assert.False( t, phpThreads[0].handler.(*workerThread).isBootingScript, @@ -249,27 +251,27 @@ func TestFinishBootingAWorkerScript(t *testing.T) { } func TestReturnAnErrorIf2WorkersHaveTheSameFileName(t *testing.T) { + resetGlobals() workers = []*worker{} workersByName = map[string]*worker{} - workersByPath = map[string]*worker{} + globalWorkersByPath = map[string]*worker{} w, err1 := newWorker(workerOpt{fileName: testDataPath + "/index.php"}) assert.NoError(t, err1) workers = append(workers, w) workersByName[w.name] = w - workersByPath[w.fileName] = w + globalWorkersByPath[w.fileName] = w _, err2 := newWorker(workerOpt{fileName: testDataPath + "/index.php"}) assert.Error(t, err2, "two workers cannot have the same filename") } func TestReturnAnErrorIf2ModuleWorkersHaveTheSameName(t *testing.T) { + resetGlobals() workers = []*worker{} workersByName = map[string]*worker{} - workersByPath = map[string]*worker{} w, err1 := newWorker(workerOpt{fileName: testDataPath + "/index.php", name: "workername"}) assert.NoError(t, err1) workers = append(workers, w) workersByName[w.name] = w - workersByPath[w.fileName] = w _, err2 := newWorker(workerOpt{fileName: testDataPath + "/hello.php", name: "workername"}) assert.Error(t, err2, "two workers cannot have the same name") } @@ -313,9 +315,9 @@ func allPossibleTransitions(worker1Path string, worker2Path string) []func(*phpT thread.boot() } }, - func(thread *phpThread) { convertToWorkerThread(thread, workersByPath[worker1Path]) }, + func(thread *phpThread) { convertToWorkerThread(thread, globalWorkersByPath[worker1Path]) }, convertToInactiveThread, - func(thread *phpThread) { convertToWorkerThread(thread, workersByPath[worker2Path]) }, + func(thread *phpThread) { convertToWorkerThread(thread, globalWorkersByPath[worker2Path]) }, convertToInactiveThread, } } diff --git a/phpserver.go b/phpserver.go new file mode 100644 index 0000000000..7ee92f9c18 --- /dev/null +++ b/phpserver.go @@ -0,0 +1,102 @@ +package frankenphp + +import ( + "log/slog" + "net/http" + "sync" +) + +// PhpServer represents a PHP server instance. +// it helps to scope a request to a specific set of configurations. +// useful to represent a php_server or php block in the caddyfile. +type PhpServer struct { + idx int + root string + splitPath []string + env PreparedEnv + workers []*worker + workersByPath map[string]*worker + workerOpts []workerOpt + logger *slog.Logger + mainThread *phpMainThread +} + +// PhpServerOption instances allow to configure a PhpServer. +type PhpServerOption func(*PhpServer) error + +var ( + PhpServers = make(map[int]*PhpServer) + phpServersMu sync.Mutex +) + +func drainPhpServers() { + phpServersMu.Lock() + defer phpServersMu.Unlock() + PhpServers = make(map[int]*PhpServer) +} + +func newPhpServer(idx int, opts ...PhpServerOption) (*PhpServer, error) { + phpServersMu.Lock() + defer phpServersMu.Unlock() + + existingPhpServer, ok := PhpServers[idx] + if ok { + globalLogger.Debug("php server already registered, ignoring duplicate registration", "idx", idx) + return existingPhpServer, nil + } + + phpServer := &PhpServer{ + idx: idx, + env: make(map[string]string), + workersByPath: make(map[string]*worker), + workerOpts: make([]workerOpt, 0), + } + + for _, option := range opts { + if err := option(phpServer); err != nil { + return phpServer, err + } + } + + PhpServers[phpServer.idx] = phpServer + + return phpServer, nil +} + +// fallback PHP server if none could be associated with a request +func newDummyPhpServer() *PhpServer { + return &PhpServer{ + idx: -1, + workersByPath: make(map[string]*worker), + env: make(map[string]string), + } +} + +// ServeHTTP executes a PHP script according to the given context. +func (s *PhpServer) ServeHTTP(responseWriter http.ResponseWriter, request *http.Request, opts ...RequestOption) error { + h := responseWriter.Header() + if h["Server"] == nil { + h["Server"] = serverHeader + } + + if !isRunning { + return ErrNotRunning + } + + fc, err := newContextFromRequest(request, responseWriter, s, opts...) + if err != nil { + return err + } + + if err := fc.validate(); err != nil { + return err + } + + // Detect if a worker is available to handle this request + if fc.worker != nil { + return fc.worker.handleRequest(fc) + } + + // If no worker was available, send the request to non-worker threads + return handleRequestWithRegularPHPThreads(fc) +} diff --git a/phpthread.go b/phpthread.go index 106ab2101b..2d053b6d89 100644 --- a/phpthread.go +++ b/phpthread.go @@ -19,7 +19,7 @@ import ( type phpThread struct { runtime.Pinner threadIndex int - requestChan chan contextHolder + requestChan chan *frankenPHPContext drainChan chan struct{} handlerMu sync.RWMutex handler threadHandler @@ -39,7 +39,6 @@ type threadHandler interface { name() string beforeScriptExecution() string afterScriptExecution(exitStatus int) - context() context.Context frankenPHPContext() *frankenPHPContext // drain is a hook called by drainWorkerThreads right before drainChan is // closed. Handlers that need to wake up a thread parked in a blocking C @@ -52,7 +51,7 @@ type threadHandler interface { func newPHPThread(threadIndex int) *phpThread { return &phpThread{ threadIndex: threadIndex, - requestChan: make(chan contextHolder), + requestChan: make(chan *frankenPHPContext), state: state.NewThreadState(), } } @@ -182,12 +181,11 @@ func (thread *phpThread) frankenPHPContext() *frankenPHPContext { } func (thread *phpThread) context() context.Context { - if thread.handler == nil { - // handler can be nil when using opcache.preload - return globalCtx + if fc := thread.frankenPHPContext(); fc != nil && fc.ctx != nil { + return fc.ctx } - return thread.handler.context() + return globalCtx } func (thread *phpThread) name() string { diff --git a/scaling_test.go b/scaling_test.go index f899e7679e..d2784a8eeb 100644 --- a/scaling_test.go +++ b/scaling_test.go @@ -48,7 +48,7 @@ func TestScaleAWorkerThreadUpAndDown(t *testing.T) { autoScaledThread := phpThreads[2] // scale up - scaleWorkerThread(workersByPath[workerPath], mainThread.done, mainThread.state) + scaleWorkerThread(globalWorkersByPath[workerPath], mainThread.done, mainThread.state) assert.Equal(t, state.Ready, autoScaledThread.state.Get()) // on down-scale, the thread will be marked as inactive diff --git a/threadinactive.go b/threadinactive.go index 98fba6f234..77e313467f 100644 --- a/threadinactive.go +++ b/threadinactive.go @@ -1,8 +1,6 @@ package frankenphp import ( - "context" - "github.com/dunglas/frankenphp/internal/state" ) @@ -56,10 +54,6 @@ func (handler *inactiveThread) frankenPHPContext() *frankenPHPContext { return nil } -func (handler *inactiveThread) context() context.Context { - return globalCtx -} - func (handler *inactiveThread) name() string { return "Inactive PHP Thread" } diff --git a/threadregular.go b/threadregular.go index 283627334f..9b5eaa6655 100644 --- a/threadregular.go +++ b/threadregular.go @@ -3,7 +3,6 @@ package frankenphp // #include "frankenphp.h" import "C" import ( - "context" "log/slog" "runtime" "sync" @@ -17,8 +16,7 @@ import ( // executes PHP scripts in a web context // implements the threadHandler interface type regularThread struct { - contextHolder - + fc *frankenPHPContext state *state.ThreadState thread *phpThread requestCount int @@ -27,7 +25,7 @@ type regularThread struct { var ( regularThreads []*phpThread regularThreadMu = &sync.RWMutex{} - regularRequestChan chan contextHolder + regularRequestChan chan *frankenPHPContext queuedRegularThreads = atomic.Int32{} ) @@ -75,11 +73,7 @@ func (handler *regularThread) afterScriptExecution(_ int) { } func (handler *regularThread) frankenPHPContext() *frankenPHPContext { - return handler.contextHolder.frankenPHPContext -} - -func (handler *regularThread) context() context.Context { - return handler.ctx + return handler.fc } func (handler *regularThread) name() string { @@ -105,36 +99,33 @@ func (handler *regularThread) waitForRequest() string { handler.state.MarkAsWaiting(true) - var ch contextHolder + var fc *frankenPHPContext select { case <-handler.thread.drainChan: // go back to beforeScriptExecution return handler.beforeScriptExecution() - case ch = <-regularRequestChan: - case ch = <-handler.thread.requestChan: + case fc = <-regularRequestChan: + case fc = <-handler.thread.requestChan: } handler.requestCount++ handler.thread.contextMu.Lock() - handler.ctx = ch.ctx - handler.contextHolder.frankenPHPContext = ch.frankenPHPContext + handler.fc = fc handler.thread.contextMu.Unlock() handler.state.MarkAsWaiting(false) - // set the scriptFilename that should be executed - return handler.contextHolder.frankenPHPContext.scriptFilename + return fc.scriptFilename } func (handler *regularThread) afterRequest() { - handler.contextHolder.frankenPHPContext.closeContext() + handler.fc.closeContext() handler.thread.contextMu.Lock() - handler.contextHolder.frankenPHPContext = nil - handler.ctx = nil + handler.fc = nil handler.thread.contextMu.Unlock() } -func handleRequestWithRegularPHPThreads(ch contextHolder) error { +func handleRequestWithRegularPHPThreads(fc *frankenPHPContext) error { metrics.StartRequest() runtime.Gosched() @@ -143,9 +134,9 @@ func handleRequestWithRegularPHPThreads(ch contextHolder) error { regularThreadMu.RLock() for _, thread := range regularThreads { select { - case thread.requestChan <- ch: + case thread.requestChan <- fc: regularThreadMu.RUnlock() - <-ch.frankenPHPContext.done + <-fc.done metrics.StopRequest() return nil @@ -162,15 +153,15 @@ func handleRequestWithRegularPHPThreads(ch contextHolder) error { for { select { - case regularRequestChan <- ch: + case regularRequestChan <- fc: queuedRegularThreads.Add(-1) metrics.DequeuedRequest() - <-ch.frankenPHPContext.done + <-fc.done metrics.StopRequest() return nil - case scaleChan <- ch.frankenPHPContext: + case scaleChan <- fc: // the request has triggered scaling, continue to wait for a thread case <-timeoutChan(time.Duration(maxWaitTime.Load())): // the request has timed out stalling @@ -178,7 +169,7 @@ func handleRequestWithRegularPHPThreads(ch contextHolder) error { metrics.DequeuedRequest() metrics.StopRequest() - ch.frankenPHPContext.reject(ErrMaxWaitTimeExceeded) + fc.reject(ErrMaxWaitTimeExceeded) return ErrMaxWaitTimeExceeded } diff --git a/threadtasks_test.go b/threadtasks_test.go index d6954d161c..4e986a8805 100644 --- a/threadtasks_test.go +++ b/threadtasks_test.go @@ -1,7 +1,6 @@ package frankenphp import ( - "context" "sync" "github.com/dunglas/frankenphp/internal/state" @@ -71,10 +70,6 @@ func (handler *taskThread) frankenPHPContext() *frankenPHPContext { return nil } -func (handler *taskThread) context() context.Context { - return nil -} - func (handler *taskThread) name() string { return "Task PHP Thread" } diff --git a/threadworker.go b/threadworker.go index 843e1dd331..6d00a212f0 100644 --- a/threadworker.go +++ b/threadworker.go @@ -3,10 +3,8 @@ package frankenphp // #include "frankenphp.h" import "C" import ( - "context" "fmt" "log/slog" - "path/filepath" "time" "unsafe" @@ -21,9 +19,7 @@ type workerThread struct { thread *phpThread worker *worker dummyFrankenPHPContext *frankenPHPContext - dummyContext context.Context workerFrankenPHPContext *frankenPHPContext - workerContext context.Context isBootingScript bool // true if the worker has not reached frankenphp_handle_request yet failureCount int // number of consecutive startup failures requestCount int // number of requests handled since last restart @@ -86,13 +82,6 @@ func (handler *workerThread) frankenPHPContext() *frankenPHPContext { return handler.dummyFrankenPHPContext } -func (handler *workerThread) context() context.Context { - if handler.workerContext != nil { - return handler.workerContext - } - - return handler.dummyContext -} func (handler *workerThread) name() string { return "Worker PHP Thread - " + handler.worker.fileName @@ -104,31 +93,23 @@ func setupWorkerScript(handler *workerThread, worker *worker) { metrics.StartWorker(worker.name) // Create a dummy request to set up the worker - fc, err := newDummyContext( - filepath.Base(worker.fileName), - worker.requestOptions..., - ) + fc, err := newDummyContext(worker) if err != nil { panic(err) } - ctx := context.WithValue(globalCtx, contextKey, fc) - - fc.worker = worker handler.dummyFrankenPHPContext = fc - handler.dummyContext = ctx handler.isBootingScript = true handler.requestCount = 0 - if globalLogger.Enabled(ctx, slog.LevelDebug) { - globalLogger.LogAttrs(ctx, slog.LevelDebug, "starting", slog.String("worker", worker.name), slog.Int("thread", handler.thread.threadIndex)) + if globalLogger.Enabled(fc.ctx, slog.LevelDebug) { + globalLogger.LogAttrs(fc.ctx, slog.LevelDebug, "starting", slog.String("worker", worker.name), slog.Int("thread", handler.thread.threadIndex)) } } func tearDownWorkerScript(handler *workerThread, exitStatus int) { worker := handler.worker handler.dummyFrankenPHPContext = nil - handler.dummyContext = nil // if the worker request is not nil, the script might have crashed // make sure to close the worker request context @@ -136,7 +117,6 @@ func tearDownWorkerScript(handler *workerThread, exitStatus int) { handler.workerFrankenPHPContext.closeContext() handler.thread.contextMu.Lock() handler.workerFrankenPHPContext = nil - handler.workerContext = nil handler.thread.contextMu.Unlock() } @@ -237,7 +217,7 @@ func (handler *workerThread) waitForWorkerRequest() (bool, any) { handler.state.MarkAsWaiting(true) - var requestCH contextHolder + var fc *frankenPHPContext select { case <-handler.thread.drainChan: if globalLogger.Enabled(globalCtx, slog.LevelDebug) { @@ -245,22 +225,21 @@ func (handler *workerThread) waitForWorkerRequest() (bool, any) { } return false, nil - case requestCH = <-handler.thread.requestChan: - case requestCH = <-handler.worker.requestChan: + case fc = <-handler.thread.requestChan: + case fc = <-handler.worker.requestChan: } handler.requestCount++ handler.thread.contextMu.Lock() - handler.workerContext = requestCH.ctx - handler.workerFrankenPHPContext = requestCH.frankenPHPContext + handler.workerFrankenPHPContext = fc handler.thread.contextMu.Unlock() handler.state.MarkAsWaiting(false) - if globalLogger.Enabled(requestCH.ctx, slog.LevelDebug) { + if globalLogger.Enabled(fc.ctx, slog.LevelDebug) { if handler.workerFrankenPHPContext.request == nil { - globalLogger.LogAttrs(requestCH.ctx, slog.LevelDebug, "request handling started", slog.String("worker", handler.worker.name), slog.Int("thread", handler.thread.threadIndex)) + globalLogger.LogAttrs(fc.ctx, slog.LevelDebug, "request handling started", slog.String("worker", handler.worker.name), slog.Int("thread", handler.thread.threadIndex)) } else { - globalLogger.LogAttrs(requestCH.ctx, slog.LevelDebug, "request handling started", slog.String("worker", handler.worker.name), slog.Int("thread", handler.thread.threadIndex), slog.String("url", handler.workerFrankenPHPContext.request.RequestURI)) + globalLogger.LogAttrs(fc.ctx, slog.LevelDebug, "request handling started", slog.String("worker", handler.worker.name), slog.Int("thread", handler.thread.threadIndex), slog.String("url", handler.workerFrankenPHPContext.request.RequestURI)) } } @@ -298,7 +277,7 @@ func go_frankenphp_worker_handle_request_start(threadIndex C.uintptr_t) (C.bool, func go_frankenphp_finish_worker_request(threadIndex C.uintptr_t, retval *C.zval) { thread := phpThreads[threadIndex] ctx := thread.context() - fc := ctx.Value(contextKey).(*frankenPHPContext) + fc := thread.frankenPHPContext() if retval != nil { r, err := GoValue[any](unsafe.Pointer(retval)) @@ -314,7 +293,6 @@ func go_frankenphp_finish_worker_request(threadIndex C.uintptr_t, retval *C.zval fc.closeContext() thread.contextMu.Lock() thread.handler.(*workerThread).workerFrankenPHPContext = nil - thread.handler.(*workerThread).workerContext = nil thread.contextMu.Unlock() if globalLogger.Enabled(ctx, slog.LevelDebug) { diff --git a/worker.go b/worker.go index 2ddd70f2f4..35dbe47846 100644 --- a/worker.go +++ b/worker.go @@ -4,7 +4,9 @@ package frankenphp import "C" import ( "fmt" + "net/http" "os" + "path" "path/filepath" "runtime" "strings" @@ -22,29 +24,31 @@ type worker struct { name string fileName string + matchRequest func(*http.Request) bool + matchRelPath string num int maxThreads int requestOptions []RequestOption - requestChan chan contextHolder + requestChan chan *frankenPHPContext threads []*phpThread threadMutex sync.RWMutex - allowPathMatching bool maxConsecutiveFailures int onThreadReady func(int) onThreadShutdown func(int) queuedRequests atomic.Int32 + phpServer *PhpServer } var ( - workers []*worker - workersByName map[string]*worker - workersByPath map[string]*worker - watcherIsEnabled bool - startupFailChan chan error + workers []*worker + workersByName map[string]*worker + globalWorkersByPath map[string]*worker + watcherIsEnabled bool + startupFailChan chan error ) -func initWorkers(opt []workerOpt) error { - if len(opt) == 0 { +func initWorkers(opts []workerOpt) error { + if len(opts) == 0 { return nil } @@ -53,11 +57,11 @@ func initWorkers(opt []workerOpt) error { totalThreadsToStart int ) - workers = make([]*worker, 0, len(opt)) - workersByName = make(map[string]*worker, len(opt)) - workersByPath = make(map[string]*worker, len(opt)) + workers = make([]*worker, 0, len(opts)) + workersByName = make(map[string]*worker, len(opts)) + globalWorkersByPath = make(map[string]*worker, len(opts)) - for _, o := range opt { + for _, o := range opts { w, err := newWorker(o) if err != nil { return err @@ -66,8 +70,11 @@ func initWorkers(opt []workerOpt) error { totalThreadsToStart += w.num workers = append(workers, w) workersByName[w.name] = w - if w.allowPathMatching { - workersByPath[w.fileName] = w + if w.phpServer == nil { + globalWorkersByPath[w.fileName] = w + } else { + w.phpServer.workers = append(w.phpServer.workers, w) + w.phpServer.workersByPath[w.fileName] = w } } @@ -120,34 +127,43 @@ func newWorker(o workerOpt) (*worker, error) { o.name = absFileName } - // 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 := workersByPath[absFileName]; w != nil && allowPathMatching { - return w, fmt.Errorf("two workers cannot have the same filename: %q", absFileName) + if o.phpServer == nil { + if w := globalWorkersByPath[absFileName]; w != nil { + return w, fmt.Errorf("two workers cannot have the same filename: %q", absFileName) + } } if w := workersByName[o.name]; w != nil { return w, fmt.Errorf("two workers cannot have the same name: %q", o.name) } + // env should always contain FRANKENPHP_WORKER and the parent php_server env if o.env == nil { o.env = make(PreparedEnv, 1) } o.env["FRANKENPHP_WORKER\x00"] = "1" + + if o.phpServer != nil && len(o.phpServer.env) > 0 { + for k, v := range o.phpServer.env { + if _, exists := o.env[k]; !exists { + o.env[k] = v + } + } + } + w := &worker{ name: o.name, fileName: absFileName, + matchRequest: o.matchRequest, requestOptions: o.requestOptions, num: o.num, maxThreads: o.maxThreads, - requestChan: make(chan contextHolder), + requestChan: make(chan *frankenPHPContext), threads: make([]*phpThread, 0, o.num), - allowPathMatching: allowPathMatching, maxConsecutiveFailures: o.maxConsecutiveFailures, onThreadReady: o.onThreadReady, onThreadShutdown: o.onThreadShutdown, + phpServer: o.phpServer, } w.configureMercure(&o) @@ -162,9 +178,36 @@ func newWorker(o workerOpt) (*worker, error) { o.extensionWorkers.internalWorker = w } + if o.matchRequest == nil && o.phpServer != nil && o.phpServer.root != "" { + docRootWithSep := o.phpServer.root + string(filepath.Separator) + if strings.HasPrefix(absFileName, docRootWithSep) { + w.matchRelPath = filepath.ToSlash(absFileName[len(o.phpServer.root):]) + } + } + return w, nil } +func (w *worker) matchesRequest(r *http.Request, documentRoot string) bool { + if w.matchRequest != nil { + return w.matchRequest(r) + } + + if w.matchRelPath != "" { + reqPath := r.URL.Path + if reqPath == w.matchRelPath { + return true + } + if reqPath == "" || reqPath[0] != '/' { + reqPath = "/" + reqPath + } + return path.Clean(reqPath) == w.matchRelPath + } + + fullPath, _ := fastabs.FastAbs(filepath.Join(documentRoot, r.URL.Path)) + return fullPath == w.fileName +} + // EXPERIMENTAL: DrainWorkers initiates a graceful drain of all php threads. // Blocks until every drained thread yields. Force-kill is armed after a // grace period to wake threads parked in blocking syscalls (sleep, I/O). @@ -222,7 +265,7 @@ func (worker *worker) isAtThreadLimit() bool { return atMaxThreads } -func (worker *worker) handleRequest(ch contextHolder) error { +func (worker *worker) handleRequest(fc *frankenPHPContext) error { metrics.StartWorkerRequest(worker.name) runtime.Gosched() @@ -232,10 +275,10 @@ func (worker *worker) handleRequest(ch contextHolder) error { worker.threadMutex.RLock() for _, thread := range worker.threads { select { - case thread.requestChan <- ch: + case thread.requestChan <- fc: worker.threadMutex.RUnlock() - <-ch.frankenPHPContext.done - metrics.StopWorkerRequest(worker.name, time.Since(ch.frankenPHPContext.startedAt)) + <-fc.done + metrics.StopWorkerRequest(worker.name, time.Since(fc.startedAt)) return nil default: @@ -256,22 +299,22 @@ func (worker *worker) handleRequest(ch contextHolder) error { } select { - case worker.requestChan <- ch: + case worker.requestChan <- fc: worker.queuedRequests.Add(-1) metrics.DequeuedWorkerRequest(worker.name) - <-ch.frankenPHPContext.done - metrics.StopWorkerRequest(worker.name, time.Since(ch.frankenPHPContext.startedAt)) + <-fc.done + metrics.StopWorkerRequest(worker.name, time.Since(fc.startedAt)) return nil - case workerScaleChan <- ch.frankenPHPContext: + case workerScaleChan <- fc: // the request has triggered scaling, continue to wait for a thread case <-timeoutChan(time.Duration(maxWaitTime.Load())): // the request has timed out stalling worker.queuedRequests.Add(-1) metrics.DequeuedWorkerRequest(worker.name) - metrics.StopWorkerRequest(worker.name, time.Since(ch.frankenPHPContext.startedAt)) + metrics.StopWorkerRequest(worker.name, time.Since(fc.startedAt)) - ch.frankenPHPContext.reject(ErrMaxWaitTimeExceeded) + fc.reject(ErrMaxWaitTimeExceeded) return ErrMaxWaitTimeExceeded } diff --git a/worker_internal_test.go b/worker_internal_test.go index c0e405b701..7e089b7774 100644 --- a/worker_internal_test.go +++ b/worker_internal_test.go @@ -1,6 +1,7 @@ package frankenphp import ( + "net/http" "net/http/httptest" "os" "path/filepath" @@ -76,3 +77,34 @@ func TestRestartWorkersForceKillsStuckThread(t *testing.T) { assert.NotContains(t, recorder.Body.String(), "should not reach", "VM interrupt was never observed; sleep returned naturally") } + +func TestWorkerMatchesRequestByScriptPath(t *testing.T) { + t.Parallel() + + w := &worker{ + fileName: "/srv/app/index.php", + matchRelPath: "/index.php", + } + + req := httptest.NewRequest("GET", "/index.php", nil) + require.True(t, w.matchesRequest(req, "/srv/app")) + + req = httptest.NewRequest("GET", "/other", nil) + require.False(t, w.matchesRequest(req, "/srv/app")) +} + +func TestWorkerMatchesRequestWithInjectedMatcher(t *testing.T) { + t.Parallel() + + w := &worker{ + matchRequest: func(r *http.Request) bool { + return r.URL.Path == "/matched" + }, + } + + req := httptest.NewRequest("GET", "/matched", nil) + require.True(t, w.matchesRequest(req, "")) + + req = httptest.NewRequest("GET", "/other", nil) + require.False(t, w.matchesRequest(req, "")) +} diff --git a/workerextension.go b/workerextension.go index 82b74631a7..4ddfc78988 100644 --- a/workerextension.go +++ b/workerextension.go @@ -46,12 +46,18 @@ func (w *extensionWorkers) NumThreads() int { // EXPERIMENTAL: SendMessage sends a message to the worker and waits for a response. func (w *extensionWorkers) SendMessage(ctx context.Context, message any, rw http.ResponseWriter) (any, error) { fc := newFrankenPHPContext() + fc.phpServer = w.internalWorker.phpServer fc.logger = globalLogger fc.worker = w.internalWorker fc.responseWriter = rw fc.handlerParameters = message + fc.ctx = ctx - err := w.internalWorker.handleRequest(contextHolder{context.WithValue(ctx, contextKey, fc), fc}) + if fc.phpServer == nil { + fc.phpServer = newDummyPhpServer() + } + + err := w.internalWorker.handleRequest(fc) return fc.handlerReturn, err } From 39db699f4a4afe8c064694776e61cda8522275f7 Mon Sep 17 00:00:00 2001 From: Alliballibaba Date: Wed, 1 Jul 2026 22:56:50 +0200 Subject: [PATCH 02/18] more cleanup --- options.go | 3 +++ phpserver.go | 11 +++++------ 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/options.go b/options.go index 58b4a88d20..85367d797e 100644 --- a/options.go +++ b/options.go @@ -19,6 +19,9 @@ type Option func(h *opt) error // WorkerOption instances allow configuring FrankenPHP worker. type WorkerOption func(*workerOpt) error +// PhpServerOption instances allow to configure a PhpServer. +type PhpServerOption func(*PhpServer) error + // opt contains the available options. // // If you change this, also update the Caddy module and the documentation. diff --git a/phpserver.go b/phpserver.go index 7ee92f9c18..7f3a2f1964 100644 --- a/phpserver.go +++ b/phpserver.go @@ -6,9 +6,8 @@ import ( "sync" ) -// PhpServer represents a PHP server instance. -// it helps to scope a request to a specific set of configurations. -// useful to represent a php_server or php block in the caddyfile. +// PhpServer represents a php_server block in the caddyfile. +// can also be used to scope workers to a specific set of configurations. type PhpServer struct { idx int root string @@ -21,10 +20,9 @@ type PhpServer struct { mainThread *phpMainThread } -// PhpServerOption instances allow to configure a PhpServer. -type PhpServerOption func(*PhpServer) error - var ( + // PhpServers is a map of all registered PhpServer instances. + // instances will be accessible after frankenphp.Init() has been called. PhpServers = make(map[int]*PhpServer) phpServersMu sync.Mutex ) @@ -73,6 +71,7 @@ func newDummyPhpServer() *PhpServer { } // ServeHTTP executes a PHP script according to the given context. +// the request will be scoped to the PhpServer instance. func (s *PhpServer) ServeHTTP(responseWriter http.ResponseWriter, request *http.Request, opts ...RequestOption) error { h := responseWriter.Header() if h["Server"] == nil { From 8e12812f3f0d1c1df9c10c6f931131a46804e4e0 Mon Sep 17 00:00:00 2001 From: Alliballibaba Date: Wed, 1 Jul 2026 23:02:37 +0200 Subject: [PATCH 03/18] linting --- frankenphp_test.go | 6 +++--- phpserver.go | 1 - 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/frankenphp_test.go b/frankenphp_test.go index ec4700eb37..60553ea8f3 100644 --- a/frankenphp_test.go +++ b/frankenphp_test.go @@ -1354,16 +1354,16 @@ func TestPhpServerLib(t *testing.T) { cwd, _ := os.Getwd() testDataDir := cwd + "/testdata/" defer frankenphp.Shutdown() - frankenphp.Init( + assert.NoError(t, frankenphp.Init( frankenphp.WithPhpServer( 1, frankenphp.WithPhpServerRoot(testDataDir), ), - ) + )) request := httptest.NewRequest("GET", "http://example.com/index.php", nil) response := httptest.NewRecorder() - frankenphp.PhpServers[1].ServeHTTP(response, request) + assert.NoError(t, frankenphp.PhpServers[1].ServeHTTP(response, request)) assert.Equal(t, "I am by birth a Genevese (i not set)", response.Body.String()) } diff --git a/phpserver.go b/phpserver.go index 7f3a2f1964..afc2583f93 100644 --- a/phpserver.go +++ b/phpserver.go @@ -17,7 +17,6 @@ type PhpServer struct { workersByPath map[string]*worker workerOpts []workerOpt logger *slog.Logger - mainThread *phpMainThread } var ( From 7e1ddb81c5a851b8dcccf5d5ed81a243b0de1241 Mon Sep 17 00:00:00 2001 From: Alliballibaba Date: Wed, 1 Jul 2026 23:08:02 +0200 Subject: [PATCH 04/18] removes unnecessary code. --- context.go | 29 +++++++++++++++++++---------- workerextension.go | 13 +------------ 2 files changed, 20 insertions(+), 22 deletions(-) diff --git a/context.go b/context.go index b279f16943..57bc306b52 100644 --- a/context.go +++ b/context.go @@ -45,13 +45,6 @@ type frankenPHPContext struct { startedAt time.Time } -func newFrankenPHPContext() *frankenPHPContext { - return &frankenPHPContext{ - done: make(chan any), - startedAt: time.Now(), - } -} - // NewRequestWithContext creates a new FrankenPHP request context. // // FrankenPHP does not strip request headers whose name contains an underscore. @@ -63,9 +56,6 @@ func newFrankenPHPContext() *frankenPHPContext { // you explicitly need (and whitelist) them. The Caddy-based server and reverse // proxies such as nginx (underscores_in_headers off) already do this. func NewRequestWithContext(r *http.Request, opts ...RequestOption) (*http.Request, error) { - fc := newFrankenPHPContext() - fc.request = r - c := context.WithValue(r.Context(), contextKey, opts) return r.WithContext(c), nil @@ -152,6 +142,25 @@ func newDummyContext(w *worker) (*frankenPHPContext, error) { return fc, nil } +func newContextFromMessage(message any, rw http.ResponseWriter, ctx context.Context, w *worker) *frankenPHPContext { + fc := &frankenPHPContext{ + done: make(chan any), + startedAt: time.Now(), + phpServer: w.phpServer, + worker: w, + logger: globalLogger, + responseWriter: rw, + handlerParameters: message, + ctx: ctx, + } + + if fc.phpServer == nil { + fc.phpServer = newDummyPhpServer() + } + + return fc +} + // closeContext sends the response to the client func (fc *frankenPHPContext) closeContext() { if fc.isDone { diff --git a/workerextension.go b/workerextension.go index 4ddfc78988..814eea1565 100644 --- a/workerextension.go +++ b/workerextension.go @@ -45,18 +45,7 @@ func (w *extensionWorkers) NumThreads() int { // EXPERIMENTAL: SendMessage sends a message to the worker and waits for a response. func (w *extensionWorkers) SendMessage(ctx context.Context, message any, rw http.ResponseWriter) (any, error) { - fc := newFrankenPHPContext() - fc.phpServer = w.internalWorker.phpServer - fc.logger = globalLogger - fc.worker = w.internalWorker - fc.responseWriter = rw - fc.handlerParameters = message - fc.ctx = ctx - - if fc.phpServer == nil { - fc.phpServer = newDummyPhpServer() - } - + fc := newContextFromMessage(message, rw, ctx, w.internalWorker) err := w.internalWorker.handleRequest(fc) return fc.handlerReturn, err From 346887e24ef8d8212bd2a59ed3ef0752ce1d6d68 Mon Sep 17 00:00:00 2001 From: Alliballibaba Date: Wed, 1 Jul 2026 23:10:06 +0200 Subject: [PATCH 05/18] naming. --- context.go | 5 +++-- threadworker.go | 2 +- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/context.go b/context.go index 57bc306b52..40e7f79978 100644 --- a/context.go +++ b/context.go @@ -111,8 +111,8 @@ func newContextFromRequest(request *http.Request, responseWriter http.ResponseWr return fc, nil } -// newDummyContext creates a fake context from a request path -func newDummyContext(w *worker) (*frankenPHPContext, error) { +// newWorkerDummyContext creates a context for worker startup +func newWorkerDummyContext(w *worker) (*frankenPHPContext, error) { r, err := http.NewRequestWithContext(globalCtx, http.MethodGet, filepath.Base(w.fileName), nil) if err != nil { return nil, err @@ -142,6 +142,7 @@ func newDummyContext(w *worker) (*frankenPHPContext, error) { return fc, nil } +// newContextFromMessage creates a context from a message (external workers) func newContextFromMessage(message any, rw http.ResponseWriter, ctx context.Context, w *worker) *frankenPHPContext { fc := &frankenPHPContext{ done: make(chan any), diff --git a/threadworker.go b/threadworker.go index 6d00a212f0..0730809564 100644 --- a/threadworker.go +++ b/threadworker.go @@ -93,7 +93,7 @@ func setupWorkerScript(handler *workerThread, worker *worker) { metrics.StartWorker(worker.name) // Create a dummy request to set up the worker - fc, err := newDummyContext(worker) + fc, err := newWorkerDummyContext(worker) if err != nil { panic(err) } From 2ab47727fdeef0e426d020ce4ae3176e46a92886 Mon Sep 17 00:00:00 2001 From: Alliballibaba Date: Wed, 1 Jul 2026 23:24:43 +0200 Subject: [PATCH 06/18] more unnecessary code --- caddy/app.go | 3 --- caddy/module.go | 11 +++++++---- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/caddy/app.go b/caddy/app.go index bd3a91f967..2bd86e039e 100644 --- a/caddy/app.go +++ b/caddy/app.go @@ -63,11 +63,8 @@ type FrankenPHPApp struct { metrics frankenphp.Metrics ctx context.Context logger *slog.Logger - phpServerCount int } -var phpServers = make(map[int]*frankenphp.PhpServer) - var errIni = errors.New(`"php_ini" must be in the format: php_ini "" ""`) // CaddyModule returns the Caddy module information. diff --git a/caddy/module.go b/caddy/module.go index 3c92d256ca..02d048df5a 100644 --- a/caddy/module.go +++ b/caddy/module.go @@ -50,7 +50,6 @@ type FrankenPHPModule struct { resolvedDocumentRoot string preparedEnv frankenphp.PreparedEnv requestOptions []frankenphp.RequestOption - phpServer *frankenphp.PhpServer } // CaddyModule returns the Caddy module information. @@ -234,7 +233,12 @@ func (f *FrankenPHPModule) ServeHTTP(w http.ResponseWriter, r *http.Request, _ c opts = append(opts, frankenphp.WithRequestPreparedEnv(env)) } - err := frankenphp.PhpServers[f.PhpServerIdx].ServeHTTP(w, r, opts...) + phpServer := frankenphp.PhpServers[f.PhpServerIdx] + if phpServer == nil { + return fmt.Errorf("php server with idx %d was not correctly provisioned", f.PhpServerIdx) + } + + err := phpServer.ServeHTTP(w, r, opts...) if err != nil && !errors.As(err, &frankenphp.ErrRejected{}) { return caddyhttp.Error(http.StatusInternalServerError, err) @@ -317,11 +321,10 @@ func (f *FrankenPHPModule) UnmarshalCaddyfile(d *caddyfile.Dispenser) error { return nil } -func (f *FrankenPHPModule) assignPhpServerIdx(h httpcaddyfile.Helper) error { +func (f *FrankenPHPModule) assignPhpServerIdx(h httpcaddyfile.Helper) { counter, _ := h.State["php_server_count"].(int) f.PhpServerIdx = counter h.State["php_server_count"] = counter + 1 - return nil } // parseCaddyfile unmarshals tokens from h into a new Middleware. From 300e2e79b3d1267965be0f5b8134e4ddc22313c5 Mon Sep 17 00:00:00 2001 From: Alliballibaba Date: Wed, 1 Jul 2026 23:24:56 +0200 Subject: [PATCH 07/18] corerctly orders autoscaling chan registration --- scaling.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/scaling.go b/scaling.go index 82f2b6c2d7..e4edc0c846 100644 --- a/scaling.go +++ b/scaling.go @@ -35,15 +35,15 @@ var ( ) func initAutoScaling(mainThread *phpMainThread) { + if mainThread.maxThreads <= mainThread.numThreads { + return + } + // reused across reloads so queued requests aren't orphaned on a stale channel if scaleChan == nil { scaleChan = make(chan *frankenPHPContext) } - if mainThread.maxThreads <= mainThread.numThreads { - return - } - done := mainThread.done mstate := mainThread.state From 2c4f8187fcee3c26a72330a3396f9fff66a9a227 Mon Sep 17 00:00:00 2001 From: Alliballibaba Date: Wed, 1 Jul 2026 23:25:09 +0200 Subject: [PATCH 08/18] go fmt --- caddy/app.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/caddy/app.go b/caddy/app.go index 2bd86e039e..8a0f968824 100644 --- a/caddy/app.go +++ b/caddy/app.go @@ -59,10 +59,10 @@ type FrankenPHPApp struct { // EXPERIMENTAL: MaxRequests sets the maximum number of requests a PHP thread handles before restarting (0 = unlimited) MaxRequests int `json:"max_requests,omitempty"` - opts []frankenphp.Option - metrics frankenphp.Metrics - ctx context.Context - logger *slog.Logger + opts []frankenphp.Option + metrics frankenphp.Metrics + ctx context.Context + logger *slog.Logger } var errIni = errors.New(`"php_ini" must be in the format: php_ini "" ""`) From ddd6245eab21c6f9316de55f6ab8c3d3ccca595f Mon Sep 17 00:00:00 2001 From: Alliballibaba Date: Wed, 1 Jul 2026 23:40:55 +0200 Subject: [PATCH 09/18] reuses split-path normalization logic. --- options.go | 33 +++++++-------------------------- requestoptions.go | 21 +++++++++++++++------ worker.go | 15 +-------------- worker_internal_test.go | 15 --------------- 4 files changed, 23 insertions(+), 61 deletions(-) diff --git a/options.go b/options.go index 85367d797e..1693038085 100644 --- a/options.go +++ b/options.go @@ -5,9 +5,7 @@ import ( "fmt" "log/slog" "net/http" - "strings" "time" - "unicode/utf8" ) // defaultMaxConsecutiveFailures is the default maximum number of consecutive failures before panicking @@ -231,11 +229,12 @@ func WithWorkerWatchMode(watch []string) WorkerOption { } } -// WithWorkerMatchOn sets a request matcher for this worker (for example from Caddy's MatchPath). -// if no request matcher is set, matching happens explicitly by path -func WithWorkerMatchOn(matchOn func(*http.Request) bool) WorkerOption { +// WithWorkerMatchOn sets a request matcher for this worker +// if the matcher returns true, the worker will be used to handle the request +// if no request matcher is set, matching happens only by path (filename == root + request path) +func WithWorkerMatchOn(matcherFunc func(*http.Request) bool) WorkerOption { return func(w *workerOpt) error { - w.matchRequest = matchOn + w.matchRequest = matcherFunc return nil } } @@ -303,26 +302,8 @@ func WithPhpServerRoot(root string) PhpServerOption { func WithPhpServerSplitPath(splitPath []string) PhpServerOption { return func(s *PhpServer) error { - var b strings.Builder - - for i, split := range splitPath { - b.Grow(len(split)) - - for j := 0; j < len(split); j++ { - c := split[j] - if c >= utf8.RuneSelf { - return ErrInvalidSplitPath - } - - if 'A' <= c && c <= 'Z' { - b.WriteByte(c + 'a' - 'A') - } else { - b.WriteByte(c) - } - } - - splitPath[i] = b.String() - b.Reset() + if err := normalizeSplitPath(splitPath); err != nil { + return err } s.splitPath = splitPath diff --git a/requestoptions.go b/requestoptions.go index 42cc3cf7c0..ea6863ada6 100644 --- a/requestoptions.go +++ b/requestoptions.go @@ -83,6 +83,19 @@ func WithRequestResolvedDocumentRoot(documentRoot string) RequestOption { // which can be mitigated with use of a try_files-like behavior // that 404s if the FastCGI path info is not found. func WithRequestSplitPath(splitPath []string) (RequestOption, error) { + if err := normalizeSplitPath(splitPath); err != nil { + return nil, err + } + + return func(o *frankenPHPContext) error { + o.splitPath = splitPath + + return nil + }, nil +} + +// normalize split path in-place to lowercase ASCII characters +func normalizeSplitPath(splitPath []string) error { var b strings.Builder for i, split := range splitPath { @@ -91,7 +104,7 @@ func WithRequestSplitPath(splitPath []string) (RequestOption, error) { for j := 0; j < len(split); j++ { c := split[j] if c >= utf8.RuneSelf { - return nil, ErrInvalidSplitPath + return ErrInvalidSplitPath } if 'A' <= c && c <= 'Z' { @@ -105,11 +118,7 @@ func WithRequestSplitPath(splitPath []string) (RequestOption, error) { b.Reset() } - return func(o *frankenPHPContext) error { - o.splitPath = splitPath - - return nil - }, nil + return nil } type PreparedEnv = map[string]string diff --git a/worker.go b/worker.go index 35dbe47846..2e969f606b 100644 --- a/worker.go +++ b/worker.go @@ -6,7 +6,6 @@ import ( "fmt" "net/http" "os" - "path" "path/filepath" "runtime" "strings" @@ -193,19 +192,7 @@ func (w *worker) matchesRequest(r *http.Request, documentRoot string) bool { return w.matchRequest(r) } - if w.matchRelPath != "" { - reqPath := r.URL.Path - if reqPath == w.matchRelPath { - return true - } - if reqPath == "" || reqPath[0] != '/' { - reqPath = "/" + reqPath - } - return path.Clean(reqPath) == w.matchRelPath - } - - fullPath, _ := fastabs.FastAbs(filepath.Join(documentRoot, r.URL.Path)) - return fullPath == w.fileName + return false } // EXPERIMENTAL: DrainWorkers initiates a graceful drain of all php threads. diff --git a/worker_internal_test.go b/worker_internal_test.go index 7e089b7774..0b2fec7526 100644 --- a/worker_internal_test.go +++ b/worker_internal_test.go @@ -78,21 +78,6 @@ func TestRestartWorkersForceKillsStuckThread(t *testing.T) { "VM interrupt was never observed; sleep returned naturally") } -func TestWorkerMatchesRequestByScriptPath(t *testing.T) { - t.Parallel() - - w := &worker{ - fileName: "/srv/app/index.php", - matchRelPath: "/index.php", - } - - req := httptest.NewRequest("GET", "/index.php", nil) - require.True(t, w.matchesRequest(req, "/srv/app")) - - req = httptest.NewRequest("GET", "/other", nil) - require.False(t, w.matchesRequest(req, "/srv/app")) -} - func TestWorkerMatchesRequestWithInjectedMatcher(t *testing.T) { t.Parallel() From 19069bc49f89b86fee4393352434c6564a1f646f Mon Sep 17 00:00:00 2001 From: Alliballibaba Date: Wed, 1 Jul 2026 23:52:11 +0200 Subject: [PATCH 10/18] removes unnecessary locking. --- cgi.go | 3 ++- phpserver.go | 15 +++------------ 2 files changed, 5 insertions(+), 13 deletions(-) diff --git a/cgi.go b/cgi.go index 55ddba50af..4ca7c413f8 100644 --- a/cgi.go +++ b/cgi.go @@ -223,7 +223,8 @@ func splitCgiPath(fc *frankenPHPContext) { // SCRIPT_FILENAME is the absolute path of SCRIPT_NAME fc.scriptFilename = sanitizedPathJoin(fc.documentRoot, fc.scriptName) - // see if a phpServer or global worker matches the request + // see if a phpServer or global worker matches the request path + // aka: root + request path == worker.filename fc.worker = fc.phpServer.workersByPath[fc.scriptFilename] if fc.worker == nil { fc.worker = globalWorkersByPath[fc.scriptFilename] diff --git a/phpserver.go b/phpserver.go index afc2583f93..974454c251 100644 --- a/phpserver.go +++ b/phpserver.go @@ -3,7 +3,6 @@ package frankenphp import ( "log/slog" "net/http" - "sync" ) // PhpServer represents a php_server block in the caddyfile. @@ -19,23 +18,15 @@ type PhpServer struct { logger *slog.Logger } -var ( - // PhpServers is a map of all registered PhpServer instances. - // instances will be accessible after frankenphp.Init() has been called. - PhpServers = make(map[int]*PhpServer) - phpServersMu sync.Mutex -) +// PhpServers is a map of all registered PhpServer instances. +// instances will be accessible after frankenphp.Init() has been called. +var PhpServers = make(map[int]*PhpServer) func drainPhpServers() { - phpServersMu.Lock() - defer phpServersMu.Unlock() PhpServers = make(map[int]*PhpServer) } func newPhpServer(idx int, opts ...PhpServerOption) (*PhpServer, error) { - phpServersMu.Lock() - defer phpServersMu.Unlock() - existingPhpServer, ok := PhpServers[idx] if ok { globalLogger.Debug("php server already registered, ignoring duplicate registration", "idx", idx) From 53e30a8d8d372a25121999af59155e68b4c9bc68 Mon Sep 17 00:00:00 2001 From: Alliballibaba Date: Wed, 1 Jul 2026 23:59:06 +0200 Subject: [PATCH 11/18] improves assertions. --- caddy/admin_test.go | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/caddy/admin_test.go b/caddy/admin_test.go index 03bf151ed5..5ec45fa7c4 100644 --- a/caddy/admin_test.go +++ b/caddy/admin_test.go @@ -388,8 +388,9 @@ func TestRegisteredModuleWorkerPoolsMustBeCorrect(t *testing.T) { receivedThreadNames = append(receivedThreadNames, thread.Name) } + assert.Len(t, receivedThreadNames, 4, "expected 4 threads to be present") assert.Contains(t, receivedThreadNames, "Regular PHP Thread", "expected a regular thread to be present") - assert.Contains(t, receivedThreadNames, "Worker PHP Thread - "+worker1Path, "expected worker 1 to be present") - assert.Contains(t, receivedThreadNames, "Worker PHP Thread - "+worker2Path, "expected worker 2 to be present") - assert.Contains(t, receivedThreadNames, "Worker PHP Thread - "+worker3Path, "expected worker 3 to be present") + assert.Contains(t, receivedThreadNames, "Worker PHP Thread - "+worker1Path, "expected global worker to be present") + assert.Contains(t, receivedThreadNames, "Worker PHP Thread - "+worker2Path, "expected module worker with \"match\" directive to be present") + assert.Contains(t, receivedThreadNames, "Worker PHP Thread - "+worker3Path, "expected module worker without \"match\" directive to be present") } From 9b38f1dc230b2a43d8417173cd7552efc16ff64c Mon Sep 17 00:00:00 2001 From: Alliballibaba Date: Thu, 2 Jul 2026 00:11:57 +0200 Subject: [PATCH 12/18] pre-computes worker matches. --- context.go | 4 ++-- phpserver.go | 17 +++++++++-------- worker.go | 11 +++-------- worker_internal_test.go | 17 ----------------- 4 files changed, 14 insertions(+), 35 deletions(-) diff --git a/context.go b/context.go index 40e7f79978..9327b6c7e9 100644 --- a/context.go +++ b/context.go @@ -83,8 +83,8 @@ func newContextFromRequest(request *http.Request, responseWriter http.ResponseWr // see if a worker matches the request if fc.worker == nil { - for _, w := range s.workers { - if w.matchesRequest(request, s.root) { + for _, w := range s.workersWithRequestMatcher { + if w.matchRequest(request) { fc.worker = w break } diff --git a/phpserver.go b/phpserver.go index 974454c251..6c16376c57 100644 --- a/phpserver.go +++ b/phpserver.go @@ -8,14 +8,15 @@ import ( // PhpServer represents a php_server block in the caddyfile. // can also be used to scope workers to a specific set of configurations. type PhpServer struct { - idx int - root string - splitPath []string - env PreparedEnv - workers []*worker - workersByPath map[string]*worker - workerOpts []workerOpt - logger *slog.Logger + idx int + root string + splitPath []string + env PreparedEnv + workers []*worker + workersByPath map[string]*worker + workersWithRequestMatcher []*worker + workerOpts []workerOpt + logger *slog.Logger } // PhpServers is a map of all registered PhpServer instances. diff --git a/worker.go b/worker.go index 2e969f606b..9d76698e05 100644 --- a/worker.go +++ b/worker.go @@ -74,6 +74,9 @@ func initWorkers(opts []workerOpt) error { } else { w.phpServer.workers = append(w.phpServer.workers, w) w.phpServer.workersByPath[w.fileName] = w + if w.matchRequest != nil { + w.phpServer.workersWithRequestMatcher = append(w.phpServer.workersWithRequestMatcher, w) + } } } @@ -187,14 +190,6 @@ func newWorker(o workerOpt) (*worker, error) { return w, nil } -func (w *worker) matchesRequest(r *http.Request, documentRoot string) bool { - if w.matchRequest != nil { - return w.matchRequest(r) - } - - return false -} - // EXPERIMENTAL: DrainWorkers initiates a graceful drain of all php threads. // Blocks until every drained thread yields. Force-kill is armed after a // grace period to wake threads parked in blocking syscalls (sleep, I/O). diff --git a/worker_internal_test.go b/worker_internal_test.go index 0b2fec7526..c0e405b701 100644 --- a/worker_internal_test.go +++ b/worker_internal_test.go @@ -1,7 +1,6 @@ package frankenphp import ( - "net/http" "net/http/httptest" "os" "path/filepath" @@ -77,19 +76,3 @@ func TestRestartWorkersForceKillsStuckThread(t *testing.T) { assert.NotContains(t, recorder.Body.String(), "should not reach", "VM interrupt was never observed; sleep returned naturally") } - -func TestWorkerMatchesRequestWithInjectedMatcher(t *testing.T) { - t.Parallel() - - w := &worker{ - matchRequest: func(r *http.Request) bool { - return r.URL.Path == "/matched" - }, - } - - req := httptest.NewRequest("GET", "/matched", nil) - require.True(t, w.matchesRequest(req, "")) - - req = httptest.NewRequest("GET", "/other", nil) - require.False(t, w.matchesRequest(req, "")) -} From 178df077aefdbc70fbbd349e297cec38ba58c91c Mon Sep 17 00:00:00 2001 From: Alliballibaba Date: Thu, 2 Jul 2026 10:18:40 +0200 Subject: [PATCH 13/18] naming. --- caddy/module.go | 2 +- cgi.go | 2 +- frankenphp.go | 2 +- phpserver.go | 4 ++-- 4 files changed, 5 insertions(+), 5 deletions(-) diff --git a/caddy/module.go b/caddy/module.go index 02d048df5a..c3f26ea0b6 100644 --- a/caddy/module.go +++ b/caddy/module.go @@ -44,7 +44,7 @@ type FrankenPHPModule struct { Env map[string]string `json:"env,omitempty"` // Workers configures the worker scripts to start. Workers []workerConfig `json:"workers,omitempty"` - // PhpServerIdx is the index of the php server to use, do not set manually + // PhpServerIdx is the idx of the php_server this module belongs to PhpServerIdx int `json:"php_server_idx,omitempty"` resolvedDocumentRoot string diff --git a/cgi.go b/cgi.go index 4ca7c413f8..92b8e36440 100644 --- a/cgi.go +++ b/cgi.go @@ -223,7 +223,7 @@ func splitCgiPath(fc *frankenPHPContext) { // SCRIPT_FILENAME is the absolute path of SCRIPT_NAME fc.scriptFilename = sanitizedPathJoin(fc.documentRoot, fc.scriptName) - // see if a phpServer or global worker matches the request path + // see if a php_server worker or global worker matches the request path // aka: root + request path == worker.filename fc.worker = fc.phpServer.workersByPath[fc.scriptFilename] if fc.worker == nil { diff --git a/frankenphp.go b/frankenphp.go index 5cce426ef2..90fcca3e67 100644 --- a/frankenphp.go +++ b/frankenphp.go @@ -381,7 +381,7 @@ func Shutdown() { drainWatchers() drainPHPThreads() - drainPhpServers() + resetPhpServers() metrics.Shutdown() diff --git a/phpserver.go b/phpserver.go index 6c16376c57..0ab60d7219 100644 --- a/phpserver.go +++ b/phpserver.go @@ -20,10 +20,10 @@ type PhpServer struct { } // PhpServers is a map of all registered PhpServer instances. -// instances will be accessible after frankenphp.Init() has been called. +// access this map only between frankenphp.Init() and frankenphp.Shutdown() calls (so it can be kept lock-free) var PhpServers = make(map[int]*PhpServer) -func drainPhpServers() { +func resetPhpServers() { PhpServers = make(map[int]*PhpServer) } From c3a340fb1b5c00b9da6802ad1506e57806b2549f Mon Sep 17 00:00:00 2001 From: Alliballibaba Date: Thu, 2 Jul 2026 10:27:20 +0200 Subject: [PATCH 14/18] removes unnecessary matchrelpath --- worker.go | 8 -------- 1 file changed, 8 deletions(-) diff --git a/worker.go b/worker.go index 9d76698e05..40ac4dbdd8 100644 --- a/worker.go +++ b/worker.go @@ -24,7 +24,6 @@ type worker struct { name string fileName string matchRequest func(*http.Request) bool - matchRelPath string num int maxThreads int requestOptions []RequestOption @@ -180,13 +179,6 @@ func newWorker(o workerOpt) (*worker, error) { o.extensionWorkers.internalWorker = w } - if o.matchRequest == nil && o.phpServer != nil && o.phpServer.root != "" { - docRootWithSep := o.phpServer.root + string(filepath.Separator) - if strings.HasPrefix(absFileName, docRootWithSep) { - w.matchRelPath = filepath.ToSlash(absFileName[len(o.phpServer.root):]) - } - } - return w, nil } From 8066a30913454564b947348c99f80b0473a53f5e Mon Sep 17 00:00:00 2001 From: Alliballibaba Date: Thu, 2 Jul 2026 13:00:38 +0200 Subject: [PATCH 15/18] adds dedicated phpserver tests. --- caddy/module.go | 2 +- frankenphp_test.go | 25 ++---- options.go | 16 +++- phpserver_test.go | 209 +++++++++++++++++++++++++++++++++++++++++++++ threadworker.go | 6 +- worker.go | 1 - 6 files changed, 233 insertions(+), 26 deletions(-) create mode 100644 phpserver_test.go diff --git a/caddy/module.go b/caddy/module.go index c3f26ea0b6..763d5476d3 100644 --- a/caddy/module.go +++ b/caddy/module.go @@ -163,7 +163,7 @@ func (f *FrankenPHPModule) Provision(ctx caddy.Context) error { // note: duplicate PhpServerIdx registration will be ignored, only the first one will be used // this is necessary since caddy drops the module instance between parsing and provisioning phpServerOptions := []frankenphp.PhpServerOption{ - frankenphp.WithPhpServerRoot(f.resolvedDocumentRoot), + frankenphp.WithPhpServerRoot(f.resolvedDocumentRoot, *f.ResolveRootSymlink), frankenphp.WithPhpServerEnv(unchangingEnv), frankenphp.WithPHPServerLogger(ctx.Slogger()), frankenphp.WithPhpServerSplitPath(f.SplitPath), diff --git a/frankenphp_test.go b/frankenphp_test.go index 60553ea8f3..b7a7ff1f91 100644 --- a/frankenphp_test.go +++ b/frankenphp_test.go @@ -37,6 +37,8 @@ import ( "github.com/stretchr/testify/require" ) +var testDataDir = "" + type testOptions struct { workerScript string watch []string @@ -148,6 +150,9 @@ func TestMain(m *testing.M) { os.Exit(1) } + cwd, _ := os.Getwd() + testDataDir = cwd + strings.Clone("/testdata/") + os.Exit(m.Run()) } @@ -230,8 +235,6 @@ func TestPathInfo_worker(t *testing.T) { testPathInfo(t, &testOptions{workerScript: "server-variable.php"}) } func testPathInfo(t *testing.T, opts *testOptions) { - cwd, _ := os.Getwd() - testDataDir := cwd + strings.Clone("/testdata/") path := strings.Clone("/server-variable.php/pathinfo") runTest(t, func(_ func(http.ResponseWriter, *http.Request), _ *httptest.Server, i int) { @@ -1349,21 +1352,3 @@ func testOpcachePreload(t *testing.T, opts *testOptions) { assert.Equal(t, "I am preloaded", body) }, opts) } - -func TestPhpServerLib(t *testing.T) { - cwd, _ := os.Getwd() - testDataDir := cwd + "/testdata/" - defer frankenphp.Shutdown() - assert.NoError(t, frankenphp.Init( - frankenphp.WithPhpServer( - 1, - frankenphp.WithPhpServerRoot(testDataDir), - ), - )) - request := httptest.NewRequest("GET", "http://example.com/index.php", nil) - response := httptest.NewRecorder() - - assert.NoError(t, frankenphp.PhpServers[1].ServeHTTP(response, request)) - - assert.Equal(t, "I am by birth a Genevese (i not set)", response.Body.String()) -} diff --git a/options.go b/options.go index 1693038085..9ad9351b2d 100644 --- a/options.go +++ b/options.go @@ -5,7 +5,10 @@ import ( "fmt" "log/slog" "net/http" + "path/filepath" "time" + + "github.com/dunglas/frankenphp/internal/fastabs" ) // defaultMaxConsecutiveFailures is the default maximum number of consecutive failures before panicking @@ -293,8 +296,19 @@ func withExtensionWorkers(w *extensionWorkers) WorkerOption { } } -func WithPhpServerRoot(root string) PhpServerOption { +func WithPhpServerRoot(root string, resolveSymlink bool) PhpServerOption { return func(s *PhpServer) error { + root, err := fastabs.FastAbs(root) + if err != nil { + return err + } + + if resolveSymlink { + if root, err = filepath.EvalSymlinks(root); err != nil { + return err + } + } + s.root = root return nil } diff --git a/phpserver_test.go b/phpserver_test.go new file mode 100644 index 0000000000..6fac3a950a --- /dev/null +++ b/phpserver_test.go @@ -0,0 +1,209 @@ +package frankenphp_test + +import ( + "errors" + "io" + "net/http" + "net/http/httptest" + "path/filepath" + "strings" + "testing" + + "github.com/dunglas/frankenphp" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func initPhpServer(t *testing.T, opts ...frankenphp.Option) { + t.Helper() + t.Cleanup(frankenphp.Shutdown) + require.NoError(t, frankenphp.Init(opts...)) +} + +func initPhpServerWithOptions(t *testing.T, idx int, serverOpts ...frankenphp.PhpServerOption) *frankenphp.PhpServer { + t.Helper() + + opts := append([]frankenphp.PhpServerOption{ + frankenphp.WithPhpServerRoot(testDataDir, false), + }, serverOpts...) + + initPhpServer(t, frankenphp.WithPhpServer(idx, opts...)) + + return frankenphp.PhpServers[idx] +} + +func phpServerProbeURL(pathInfo string) string { + return "http://example.com/server-variable.php" + pathInfo +} + +func phpServerRequest(t *testing.T, server *frankenphp.PhpServer, req *http.Request) (string, *http.Response) { + t.Helper() + + w := httptest.NewRecorder() + err := server.ServeHTTP(w, req) + if err != nil { + var rejected frankenphp.ErrRejected + if !errors.As(err, &rejected) { + require.NoError(t, err) + } + } + + resp := w.Result() + body, err := io.ReadAll(resp.Body) + require.NoError(t, err) + + return string(body), resp +} + +func phpServerGet(t *testing.T, server *frankenphp.PhpServer, url string) (string, *http.Response) { + t.Helper() + + return phpServerRequest(t, server, httptest.NewRequest(http.MethodGet, url, nil)) +} + +func TestPhpServer(t *testing.T) { + t.Run("idx", func(t *testing.T) { + initPhpServer(t, + frankenphp.WithPhpServer(1, + frankenphp.WithPhpServerRoot(testDataDir, false), + frankenphp.WithPhpServerEnv(map[string]string{"PHP_SERVER_IDX": "1"}), + ), + frankenphp.WithPhpServer(2, + frankenphp.WithPhpServerRoot(testDataDir, false), + frankenphp.WithPhpServerEnv(map[string]string{"PHP_SERVER_IDX": "2"}), + ), + ) + + body1, _ := phpServerGet(t, frankenphp.PhpServers[1], phpServerProbeURL("")) + body2, _ := phpServerGet(t, frankenphp.PhpServers[2], phpServerProbeURL("")) + + assert.Contains(t, body1, "[PHP_SERVER_IDX] => 1") + assert.Contains(t, body2, "[PHP_SERVER_IDX] => 2") + assert.NotContains(t, body1, "[PHP_SERVER_IDX] => 2") + assert.NotContains(t, body2, "[PHP_SERVER_IDX] => 1") + }) + + t.Run("root", func(t *testing.T) { + server := initPhpServerWithOptions(t, 1) + + body, _ := phpServerGet(t, server, "http://example.com/server-globals.php") + + expectedRoot := filepath.Clean(strings.TrimSuffix(testDataDir, string(filepath.Separator))) + assert.Contains(t, body, "DOCUMENT_ROOT: "+expectedRoot+"\n") + }) + + t.Run("env", func(t *testing.T) { + server := initPhpServerWithOptions(t, 1, frankenphp.WithPhpServerEnv(map[string]string{ + "PHP_SERVER_TEST_KEY": "from_php_server", + })) + + body, _ := phpServerGet(t, server, phpServerProbeURL("")) + + assert.Contains(t, body, "[PHP_SERVER_TEST_KEY] => from_php_server") + }) + + t.Run("split_path", func(t *testing.T) { + server := initPhpServerWithOptions(t, 1, frankenphp.WithPhpServerSplitPath([]string{".php"})) + + body, _ := phpServerGet(t, server, "http://example.com/server-globals.php/pathinfo") + + assert.Contains(t, body, "PATH_INFO: /pathinfo\n") + assert.Contains(t, body, "SCRIPT_NAME: /server-globals.php\n") + assert.Contains(t, body, "PHP_SELF: /server-globals.php/pathinfo\n") + }) + + t.Run("logger", func(t *testing.T) { + logger, buf := newTestLogger(t) + + initPhpServer(t, frankenphp.WithPhpServer(1, + frankenphp.WithPhpServerRoot(testDataDir, false), + frankenphp.WithPHPServerLogger(logger), + frankenphp.WithPhpServerWorker("test", testDataDir+"/index.php", 1), + )) + + _, _ = phpServerGet(t, frankenphp.PhpServers[1], "http://example.com/index.php") + + assert.Contains(t, buf.String(), "request handling started", "should contain the debug message from worker start") + }) + + t.Run("workers_by_path", func(t *testing.T) { + initPhpServer(t, frankenphp.WithPhpServer(1, + frankenphp.WithPhpServerRoot(testDataDir, false), + frankenphp.WithPhpServerWorker("counter", testDataDir+"worker-with-counter.php", 1), + )) + + body1, _ := phpServerGet(t, frankenphp.PhpServers[1], "http://example.com/worker-with-counter.php") + body2, _ := phpServerGet(t, frankenphp.PhpServers[1], "http://example.com/worker-with-counter.php") + + assert.Equal(t, "requests:1", body1) + assert.Equal(t, "requests:2", body2) + }) + + t.Run("workers_with_request_matcher", func(t *testing.T) { + initPhpServer(t, frankenphp.WithPhpServer(1, + frankenphp.WithPhpServerRoot(testDataDir, false), + frankenphp.WithPhpServerWorker("match", testDataDir+"dedup-match-worker.php", 1, + frankenphp.WithWorkerMatchOn(func(r *http.Request) bool { + return strings.HasPrefix(r.URL.Path, "/match/") + }), + ), + )) + + matched, _ := phpServerGet(t, frankenphp.PhpServers[1], "http://example.com/match/anything") + regular, _ := phpServerGet(t, frankenphp.PhpServers[1], "http://example.com/server-globals.php") + + assert.Equal(t, "dedup-match-worker", matched) + assert.Contains(t, regular, "SCRIPT_NAME: /server-globals.php\n") + }) + + t.Run("worker_inherits_env", func(t *testing.T) { + initPhpServer(t, frankenphp.WithPhpServer(1, + frankenphp.WithPhpServerRoot(testDataDir, false), + frankenphp.WithPhpServerEnv(map[string]string{"APP_ENV": "staging"}), + frankenphp.WithPhpServerWorker("env", testDataDir+"worker-with-env.php", 1), + )) + + body, _ := phpServerGet(t, frankenphp.PhpServers[1], "http://example.com/worker-with-env.php") + + assert.Equal(t, "Worker has APP_ENV=staging", body) + }) + + t.Run("duplicate_registration", func(t *testing.T) { + initPhpServer(t, + frankenphp.WithPhpServer(1, + frankenphp.WithPhpServerRoot(testDataDir, false), + frankenphp.WithPhpServerEnv(map[string]string{"PHP_SERVER_IDX": "first"}), + ), + frankenphp.WithPhpServer(1, + frankenphp.WithPhpServerRoot(testDataDir+"/other/", false), + frankenphp.WithPhpServerEnv(map[string]string{"PHP_SERVER_IDX": "second"}), + ), + ) + + body, _ := phpServerGet(t, frankenphp.PhpServers[1], phpServerProbeURL("")) + + assert.Contains(t, body, "[PHP_SERVER_IDX] => first") + assert.NotContains(t, body, "[PHP_SERVER_IDX] => second") + }) + + t.Run("serve_http_validation", func(t *testing.T) { + server := initPhpServerWithOptions(t, 1) + + req := httptest.NewRequest(http.MethodGet, phpServerProbeURL(""), nil) + req.Header.Add("Content-Length", "-1") + body, resp := phpServerRequest(t, server, req) + + assert.Equal(t, 400, resp.StatusCode) + assert.Contains(t, body, "invalid") + }) + + t.Run("not_running", func(t *testing.T) { + server := &frankenphp.PhpServer{} + req := httptest.NewRequest(http.MethodGet, phpServerProbeURL(""), nil) + w := httptest.NewRecorder() + + err := server.ServeHTTP(w, req) + + assert.ErrorIs(t, err, frankenphp.ErrNotRunning) + }) +} diff --git a/threadworker.go b/threadworker.go index 0730809564..01285fef95 100644 --- a/threadworker.go +++ b/threadworker.go @@ -235,11 +235,11 @@ func (handler *workerThread) waitForWorkerRequest() (bool, any) { handler.thread.contextMu.Unlock() handler.state.MarkAsWaiting(false) - if globalLogger.Enabled(fc.ctx, slog.LevelDebug) { + if fc.logger.Enabled(fc.ctx, slog.LevelDebug) { if handler.workerFrankenPHPContext.request == nil { - globalLogger.LogAttrs(fc.ctx, slog.LevelDebug, "request handling started", slog.String("worker", handler.worker.name), slog.Int("thread", handler.thread.threadIndex)) + fc.logger.LogAttrs(fc.ctx, slog.LevelDebug, "request handling started", slog.String("worker", handler.worker.name), slog.Int("thread", handler.thread.threadIndex)) } else { - globalLogger.LogAttrs(fc.ctx, slog.LevelDebug, "request handling started", slog.String("worker", handler.worker.name), slog.Int("thread", handler.thread.threadIndex), slog.String("url", handler.workerFrankenPHPContext.request.RequestURI)) + fc.logger.LogAttrs(fc.ctx, slog.LevelDebug, "request handling started", slog.String("worker", handler.worker.name), slog.Int("thread", handler.thread.threadIndex), slog.String("url", handler.workerFrankenPHPContext.request.RequestURI)) } } diff --git a/worker.go b/worker.go index 40ac4dbdd8..ea5170fe7a 100644 --- a/worker.go +++ b/worker.go @@ -8,7 +8,6 @@ import ( "os" "path/filepath" "runtime" - "strings" "sync" "sync/atomic" "time" From adf2feda440bec70365e5ccb46f634cdd86aa6b2 Mon Sep 17 00:00:00 2001 From: Alliballibaba Date: Thu, 2 Jul 2026 13:15:52 +0200 Subject: [PATCH 16/18] refines tests. --- phpserver_test.go | 74 ++++++++++++++++++++++------------------------- 1 file changed, 35 insertions(+), 39 deletions(-) diff --git a/phpserver_test.go b/phpserver_test.go index 6fac3a950a..381f79e6a9 100644 --- a/phpserver_test.go +++ b/phpserver_test.go @@ -32,10 +32,6 @@ func initPhpServerWithOptions(t *testing.T, idx int, serverOpts ...frankenphp.Ph return frankenphp.PhpServers[idx] } -func phpServerProbeURL(pathInfo string) string { - return "http://example.com/server-variable.php" + pathInfo -} - func phpServerRequest(t *testing.T, server *frankenphp.PhpServer, req *http.Request) (string, *http.Response) { t.Helper() @@ -74,8 +70,8 @@ func TestPhpServer(t *testing.T) { ), ) - body1, _ := phpServerGet(t, frankenphp.PhpServers[1], phpServerProbeURL("")) - body2, _ := phpServerGet(t, frankenphp.PhpServers[2], phpServerProbeURL("")) + body1, _ := phpServerGet(t, frankenphp.PhpServers[1], "http://example.com/server-variable.php") + body2, _ := phpServerGet(t, frankenphp.PhpServers[2], "http://example.com/server-variable.php") assert.Contains(t, body1, "[PHP_SERVER_IDX] => 1") assert.Contains(t, body2, "[PHP_SERVER_IDX] => 2") @@ -97,19 +93,19 @@ func TestPhpServer(t *testing.T) { "PHP_SERVER_TEST_KEY": "from_php_server", })) - body, _ := phpServerGet(t, server, phpServerProbeURL("")) + body, _ := phpServerGet(t, server, "http://example.com/server-variable.php") assert.Contains(t, body, "[PHP_SERVER_TEST_KEY] => from_php_server") }) t.Run("split_path", func(t *testing.T) { - server := initPhpServerWithOptions(t, 1, frankenphp.WithPhpServerSplitPath([]string{".php"})) + server := initPhpServerWithOptions(t, 1, frankenphp.WithPhpServerSplitPath([]string{".custom"})) - body, _ := phpServerGet(t, server, "http://example.com/server-globals.php/pathinfo") + body, _ := phpServerGet(t, server, "http://example.com/split-path.custom/pathinfo") assert.Contains(t, body, "PATH_INFO: /pathinfo\n") - assert.Contains(t, body, "SCRIPT_NAME: /server-globals.php\n") - assert.Contains(t, body, "PHP_SELF: /server-globals.php/pathinfo\n") + assert.Contains(t, body, "SCRIPT_NAME: /split-path.custom\n") + assert.Contains(t, body, "PHP_SELF: /split-path.custom/pathinfo\n") }) t.Run("logger", func(t *testing.T) { @@ -126,34 +122,34 @@ func TestPhpServer(t *testing.T) { assert.Contains(t, buf.String(), "request handling started", "should contain the debug message from worker start") }) - t.Run("workers_by_path", func(t *testing.T) { - initPhpServer(t, frankenphp.WithPhpServer(1, - frankenphp.WithPhpServerRoot(testDataDir, false), - frankenphp.WithPhpServerWorker("counter", testDataDir+"worker-with-counter.php", 1), - )) + t.Run("workers_by_path_and_request_matcher", func(t *testing.T) { + initPhpServer( + t, + frankenphp.WithPhpServer(1, + frankenphp.WithPhpServerRoot(testDataDir, false), + frankenphp.WithPhpServerWorker("counter", testDataDir+"worker-with-counter.php", 1), + ), + frankenphp.WithPhpServer(2, + frankenphp.WithPhpServerRoot(testDataDir, false), + frankenphp.WithPhpServerWorker("match", testDataDir+"worker-with-counter.php", 1, + frankenphp.WithWorkerMatchOn(func(r *http.Request) bool { + return strings.HasPrefix(r.URL.Path, "/match/") + }), + ), + ), + ) body1, _ := phpServerGet(t, frankenphp.PhpServers[1], "http://example.com/worker-with-counter.php") body2, _ := phpServerGet(t, frankenphp.PhpServers[1], "http://example.com/worker-with-counter.php") - - assert.Equal(t, "requests:1", body1) - assert.Equal(t, "requests:2", body2) - }) - - t.Run("workers_with_request_matcher", func(t *testing.T) { - initPhpServer(t, frankenphp.WithPhpServer(1, - frankenphp.WithPhpServerRoot(testDataDir, false), - frankenphp.WithPhpServerWorker("match", testDataDir+"dedup-match-worker.php", 1, - frankenphp.WithWorkerMatchOn(func(r *http.Request) bool { - return strings.HasPrefix(r.URL.Path, "/match/") - }), - ), - )) - - matched, _ := phpServerGet(t, frankenphp.PhpServers[1], "http://example.com/match/anything") - regular, _ := phpServerGet(t, frankenphp.PhpServers[1], "http://example.com/server-globals.php") - - assert.Equal(t, "dedup-match-worker", matched) - assert.Contains(t, regular, "SCRIPT_NAME: /server-globals.php\n") + body3, _ := phpServerGet(t, frankenphp.PhpServers[2], "http://example.com/match/anything") + body4, _ := phpServerGet(t, frankenphp.PhpServers[2], "http://example.com/match/anything") + body5, _ := phpServerGet(t, frankenphp.PhpServers[2], "http://example.com/index.php") + + assert.Equal(t, "requests:1", body1, "should contain the counter for the first worker") + assert.Equal(t, "requests:2", body2, "should contain the counter for the first worker") + assert.Equal(t, "requests:1", body3, "should contain the counter for the second worker") + assert.Equal(t, "requests:2", body4, "should contain the counter for the second worker") + assert.Contains(t, body5, "I am by birth a Genevese (i not set)", "should fall back to (non-worker) index.php") }) t.Run("worker_inherits_env", func(t *testing.T) { @@ -180,7 +176,7 @@ func TestPhpServer(t *testing.T) { ), ) - body, _ := phpServerGet(t, frankenphp.PhpServers[1], phpServerProbeURL("")) + body, _ := phpServerGet(t, frankenphp.PhpServers[1], "http://example.com/server-variable.php") assert.Contains(t, body, "[PHP_SERVER_IDX] => first") assert.NotContains(t, body, "[PHP_SERVER_IDX] => second") @@ -189,7 +185,7 @@ func TestPhpServer(t *testing.T) { t.Run("serve_http_validation", func(t *testing.T) { server := initPhpServerWithOptions(t, 1) - req := httptest.NewRequest(http.MethodGet, phpServerProbeURL(""), nil) + req := httptest.NewRequest(http.MethodGet, "http://example.com/server-variable.php", nil) req.Header.Add("Content-Length", "-1") body, resp := phpServerRequest(t, server, req) @@ -199,7 +195,7 @@ func TestPhpServer(t *testing.T) { t.Run("not_running", func(t *testing.T) { server := &frankenphp.PhpServer{} - req := httptest.NewRequest(http.MethodGet, phpServerProbeURL(""), nil) + req := httptest.NewRequest(http.MethodGet, "http://example.com/server-variable.php", nil) w := httptest.NewRecorder() err := server.ServeHTTP(w, req) From 48bfab4a1457153d5e0243e3aa37191ae693df32 Mon Sep 17 00:00:00 2001 From: Alliballibaba Date: Thu, 2 Jul 2026 13:55:33 +0200 Subject: [PATCH 17/18] adds missing test file. --- testdata/split-path.custom | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) create mode 100644 testdata/split-path.custom diff --git a/testdata/split-path.custom b/testdata/split-path.custom new file mode 100644 index 0000000000..836a9a4512 --- /dev/null +++ b/testdata/split-path.custom @@ -0,0 +1,26 @@ + Date: Thu, 2 Jul 2026 14:17:14 +0200 Subject: [PATCH 18/18] fixes worker match logic and disallows duplicates again --- phpserver.go | 16 ++++++++++++++++ phpserver_test.go | 17 ++++++++++++++++- worker.go | 8 ++------ 3 files changed, 34 insertions(+), 7 deletions(-) diff --git a/phpserver.go b/phpserver.go index 0ab60d7219..71d3a8215b 100644 --- a/phpserver.go +++ b/phpserver.go @@ -1,6 +1,7 @@ package frankenphp import ( + "fmt" "log/slog" "net/http" ) @@ -61,6 +62,21 @@ func newDummyPhpServer() *PhpServer { } } +func (s *PhpServer) addWorker(w *worker) error { + w.phpServer.workers = append(w.phpServer.workers, w) + if w.matchRequest != nil { + w.phpServer.workersWithRequestMatcher = append(w.phpServer.workersWithRequestMatcher, w) + return nil + } + + if _, exists := w.phpServer.workersByPath[w.fileName]; exists { + return fmt.Errorf("two workers in a php_server cannot have the same filename: %q", w.fileName) + } + w.phpServer.workersByPath[w.fileName] = w + + return nil +} + // ServeHTTP executes a PHP script according to the given context. // the request will be scoped to the PhpServer instance. func (s *PhpServer) ServeHTTP(responseWriter http.ResponseWriter, request *http.Request, opts ...RequestOption) error { diff --git a/phpserver_test.go b/phpserver_test.go index 381f79e6a9..ab089256a9 100644 --- a/phpserver_test.go +++ b/phpserver_test.go @@ -152,7 +152,7 @@ func TestPhpServer(t *testing.T) { assert.Contains(t, body5, "I am by birth a Genevese (i not set)", "should fall back to (non-worker) index.php") }) - t.Run("worker_inherits_env", func(t *testing.T) { + t.Run("disallow_duplicate_worker_filenames_in_php_server", func(t *testing.T) { initPhpServer(t, frankenphp.WithPhpServer(1, frankenphp.WithPhpServerRoot(testDataDir, false), frankenphp.WithPhpServerEnv(map[string]string{"APP_ENV": "staging"}), @@ -164,6 +164,21 @@ func TestPhpServer(t *testing.T) { assert.Equal(t, "Worker has APP_ENV=staging", body) }) + t.Run("duplicate_worker_filenames_in_php_server", func(t *testing.T) { + t.Cleanup(frankenphp.Shutdown) + + err := frankenphp.Init( + frankenphp.WithPhpServer(1, + frankenphp.WithPhpServerRoot(testDataDir, false), + frankenphp.WithPhpServerWorker("worker1", testDataDir+"worker-with-counter.php", 1), + frankenphp.WithPhpServerWorker("worker2", testDataDir+"worker-with-counter.php", 1), + ), + ) + + assert.Error(t, err) + assert.Contains(t, err.Error(), "two workers in a php_server cannot have the same filename") + }) + t.Run("duplicate_registration", func(t *testing.T) { initPhpServer(t, frankenphp.WithPhpServer(1, diff --git a/worker.go b/worker.go index ea5170fe7a..e11c129c19 100644 --- a/worker.go +++ b/worker.go @@ -69,12 +69,8 @@ func initWorkers(opts []workerOpt) error { workersByName[w.name] = w if w.phpServer == nil { globalWorkersByPath[w.fileName] = w - } else { - w.phpServer.workers = append(w.phpServer.workers, w) - w.phpServer.workersByPath[w.fileName] = w - if w.matchRequest != nil { - w.phpServer.workersWithRequestMatcher = append(w.phpServer.workersWithRequestMatcher, w) - } + } else if err := w.phpServer.addWorker(w); err != nil { + return err } }