From fba37f6786f0e6e884564de3a13c941507ac18a2 Mon Sep 17 00:00:00 2001 From: Alliballibaba Date: Wed, 1 Jul 2026 22:52:37 +0200 Subject: [PATCH 01/39] 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/39] 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/39] 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/39] 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/39] 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/39] 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/39] 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/39] 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/39] 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/39] 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/39] 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/39] 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/39] 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/39] 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/39] 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/39] 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/39] 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/39] 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 } } From e0896d84da0022cd905c6159481440d20f47f498 Mon Sep 17 00:00:00 2001 From: Alliballibaba Date: Thu, 2 Jul 2026 16:55:19 +0200 Subject: [PATCH 19/39] prepared env merge. --- cgi.go | 14 +++++++------- frankenphp.c | 4 ++++ 2 files changed, 11 insertions(+), 7 deletions(-) diff --git a/cgi.go b/cgi.go index b8d0aa402d..3dbb16403d 100644 --- a/cgi.go +++ b/cgi.go @@ -164,14 +164,13 @@ func addHeadersToServer(ctx context.Context, request *http.Request, trackVarsArr } } -// registerPreparedEnv exposes fc.env to getenv() before any PHP code runs. -func registerPreparedEnv(fc *frankenPHPContext) { - size := C.size_t(len(fc.env) + len(fc.phpServer.env)) +// registerPreparedEnv exposes fc.env and fc.phpServer.env to getenv() before any PHP code runs. +func registerPreparedEnv(fc *frankenPHPContext, preparedEnvLen int) { for k, v := range fc.phpServer.env { - C.frankenphp_add_to_prepared_env(toUnsafeChar(k), C.size_t(len(k)-1), toUnsafeChar(v), C.size_t(len(v)), size) + C.frankenphp_add_to_prepared_env(toUnsafeChar(k), C.size_t(len(k)-1), toUnsafeChar(v), C.size_t(len(v)), C.size_t(preparedEnvLen)) } for k, v := range fc.env { - C.frankenphp_add_to_prepared_env(toUnsafeChar(k), C.size_t(len(k)-1), toUnsafeChar(v), C.size_t(len(v)), size) + C.frankenphp_add_to_prepared_env(toUnsafeChar(k), C.size_t(len(k)-1), toUnsafeChar(v), C.size_t(len(v)), C.size_t(preparedEnvLen)) } } @@ -301,8 +300,9 @@ func go_update_request_info(threadIndex C.uintptr_t, info *C.sapi_request_info) return nil } - if len(fc.env) != 0 || len(fc.phpServer.env) != 0 { - registerPreparedEnv(fc) + preparedEnvLen := len(fc.env) + len(fc.phpServer.env) + if preparedEnvLen != 0 { + registerPreparedEnv(fc, preparedEnvLen) } if m, ok := cStringHTTPMethods[request.Method]; ok { diff --git a/frankenphp.c b/frankenphp.c index f665b214f1..6198cdd7d1 100644 --- a/frankenphp.c +++ b/frankenphp.c @@ -1209,6 +1209,10 @@ void frankenphp_register_server_vars(zval *track_vars_array, zend_hash_extend(ht, vars.total_num_vars, 0); zend_hash_copy(ht, main_thread_env, NULL); + if (prepared_env != NULL) { + zend_hash_copy(ht, prepared_env, NULL); + } + // update values with variable strings #define FRANKENPHP_REGISTER_VAR(name) \ frankenphp_register_trusted_var(frankenphp_strings.name, vars.name, \ From 82eb1b42754f596e47f9f494887ad6ed08f0d784 Mon Sep 17 00:00:00 2001 From: Alliballibaba Date: Thu, 2 Jul 2026 22:03:39 +0200 Subject: [PATCH 20/39] fixes env logic --- cgi.go | 2 +- frankenphp.c | 4 ---- 2 files changed, 1 insertion(+), 5 deletions(-) diff --git a/cgi.go b/cgi.go index 3dbb16403d..0981fd55f5 100644 --- a/cgi.go +++ b/cgi.go @@ -185,7 +185,7 @@ func go_register_server_variables(threadIndex C.uintptr_t, trackVarsArray *C.zva } // The Prepared Environment is registered last and can overwrite any previous values - if len(fc.env) != 0 { + if len(fc.env) != 0 || len(fc.phpServer.env) != 0 { C.frankenphp_merge_with_prepared_env(trackVarsArray) } } diff --git a/frankenphp.c b/frankenphp.c index 6198cdd7d1..f665b214f1 100644 --- a/frankenphp.c +++ b/frankenphp.c @@ -1209,10 +1209,6 @@ void frankenphp_register_server_vars(zval *track_vars_array, zend_hash_extend(ht, vars.total_num_vars, 0); zend_hash_copy(ht, main_thread_env, NULL); - if (prepared_env != NULL) { - zend_hash_copy(ht, prepared_env, NULL); - } - // update values with variable strings #define FRANKENPHP_REGISTER_VAR(name) \ frankenphp_register_trusted_var(frankenphp_strings.name, vars.name, \ From 3b1bc731664c193486f65510c27ab1270028b152 Mon Sep 17 00:00:00 2001 From: Alliballibaba Date: Sat, 4 Jul 2026 20:32:39 +0200 Subject: [PATCH 21/39] suggestions by @dunglas --- caddy/caddy_test.go | 10 +- cgi.go | 10 +- context.go | 18 ++-- frankenphp.go | 11 ++- options.go | 115 ++++++++-------------- phpserver.go | 108 --------------------- phpserver_test.go | 220 ----------------------------------------- server.go | 126 ++++++++++++++++++++++++ server_test.go | 232 ++++++++++++++++++++++++++++++++++++++++++++ worker.go | 14 +-- 10 files changed, 432 insertions(+), 432 deletions(-) delete mode 100644 phpserver.go delete mode 100644 phpserver_test.go create mode 100644 server.go create mode 100644 server_test.go diff --git a/caddy/caddy_test.go b/caddy/caddy_test.go index 86987239c7..5467f18186 100644 --- a/caddy/caddy_test.go +++ b/caddy/caddy_test.go @@ -442,7 +442,7 @@ func TestCustomCaddyVariablesInEnv(t *testing.T) { tester.AssertGetResponse("http://localhost:"+testPort+"/worker-env.php", http.StatusOK, "hello world") } -func TestPHPServerDirective(t *testing.T) { +func TestServerDirective(t *testing.T) { tester := caddytest.NewTester(t) initServer(t, tester, ` { @@ -463,7 +463,7 @@ func TestPHPServerDirective(t *testing.T) { tester.AssertGetResponse("http://localhost:"+testPort+"/not-found.txt", http.StatusOK, "I am by birth a Genevese (i not set)") } -func TestPHPServerDirectiveDisableFileServer(t *testing.T) { +func TestServerDirectiveDisableFileServer(t *testing.T) { tester := caddytest.NewTester(t) initServer(t, tester, ` { @@ -487,7 +487,7 @@ func TestPHPServerDirectiveDisableFileServer(t *testing.T) { tester.AssertGetResponse("http://localhost:"+testPort+"/not-found.txt", http.StatusOK, "I am by birth a Genevese (i not set)") } -func TestPHPServerGlobals(t *testing.T) { +func TestServerGlobals(t *testing.T) { documentRoot, _ := filepath.Abs("../testdata") scriptFilename := filepath.Join(documentRoot, "server-globals.php") @@ -539,7 +539,7 @@ REQUEST_URI: /server-globals.php/en ) } -func TestWorkerPHPServerGlobals(t *testing.T) { +func TestWorkerServerGlobals(t *testing.T) { documentRoot, _ := filepath.Abs("../testdata") documentRoot2, _ := filepath.Abs("../caddy") scriptFilename := documentRoot + string(filepath.Separator) + "server-globals.php" @@ -859,7 +859,7 @@ func TestWorkerMetrics(t *testing.T) { } // #2477: verify one pool per worker, no "_0" duplicates. -func TestPhpServerWorkerMatchPoolCount(t *testing.T) { +func TestServerWorkerMatchPoolCount(t *testing.T) { tester := caddytest.NewTester(t) initServer(t, tester, ` { diff --git a/cgi.go b/cgi.go index 0981fd55f5..185c64dc3e 100644 --- a/cgi.go +++ b/cgi.go @@ -164,9 +164,9 @@ func addHeadersToServer(ctx context.Context, request *http.Request, trackVarsArr } } -// registerPreparedEnv exposes fc.env and fc.phpServer.env to getenv() before any PHP code runs. +// registerPreparedEnv exposes fc.env and fc.server.env to getenv() before any PHP code runs. func registerPreparedEnv(fc *frankenPHPContext, preparedEnvLen int) { - for k, v := range fc.phpServer.env { + for k, v := range fc.server.env { C.frankenphp_add_to_prepared_env(toUnsafeChar(k), C.size_t(len(k)-1), toUnsafeChar(v), C.size_t(len(v)), C.size_t(preparedEnvLen)) } for k, v := range fc.env { @@ -185,7 +185,7 @@ func go_register_server_variables(threadIndex C.uintptr_t, trackVarsArray *C.zva } // The Prepared Environment is registered last and can overwrite any previous values - if len(fc.env) != 0 || len(fc.phpServer.env) != 0 { + if len(fc.env) != 0 || len(fc.server.env) != 0 { C.frankenphp_merge_with_prepared_env(trackVarsArray) } } @@ -228,7 +228,7 @@ func splitCgiPath(fc *frankenPHPContext) { // 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] + fc.worker = fc.server.workersByPath[fc.scriptFilename] if fc.worker == nil { fc.worker = globalWorkersByPath[fc.scriptFilename] } @@ -300,7 +300,7 @@ func go_update_request_info(threadIndex C.uintptr_t, info *C.sapi_request_info) return nil } - preparedEnvLen := len(fc.env) + len(fc.phpServer.env) + preparedEnvLen := len(fc.env) + len(fc.server.env) if preparedEnvLen != 0 { registerPreparedEnv(fc, preparedEnvLen) } diff --git a/context.go b/context.go index 9327b6c7e9..897a27ee52 100644 --- a/context.go +++ b/context.go @@ -31,7 +31,7 @@ type frankenPHPContext struct { scriptName string scriptFilename string requestURI string - phpServer *PhpServer + server *server // Whether the request is already closed by us isDone bool @@ -61,12 +61,12 @@ func NewRequestWithContext(r *http.Request, opts ...RequestOption) (*http.Reques return r.WithContext(c), nil } -func newContextFromRequest(request *http.Request, responseWriter http.ResponseWriter, s *PhpServer, opts ...RequestOption) (*frankenPHPContext, error) { +func newContextFromRequest(request *http.Request, responseWriter http.ResponseWriter, s *server, opts ...RequestOption) (*frankenPHPContext, error) { fc := &frankenPHPContext{ ctx: request.Context(), done: make(chan any), startedAt: time.Now(), - phpServer: s, + server: s, splitPath: s.splitPath, logger: s.logger, request: request, @@ -120,7 +120,7 @@ func newWorkerDummyContext(w *worker) (*frankenPHPContext, error) { fc := &frankenPHPContext{ ctx: r.Context(), - phpServer: w.phpServer, + server: w.server, request: r, startedAt: time.Now(), logger: globalLogger, @@ -133,8 +133,8 @@ func newWorkerDummyContext(w *worker) (*frankenPHPContext, error) { } } - if fc.phpServer == nil { - fc.phpServer = newDummyPhpServer() + if fc.server == nil { + fc.server = newDummyServer() } splitCgiPath(fc) @@ -147,7 +147,7 @@ func newContextFromMessage(message any, rw http.ResponseWriter, ctx context.Cont fc := &frankenPHPContext{ done: make(chan any), startedAt: time.Now(), - phpServer: w.phpServer, + server: w.server, worker: w, logger: globalLogger, responseWriter: rw, @@ -155,8 +155,8 @@ func newContextFromMessage(message any, rw http.ResponseWriter, ctx context.Cont ctx: ctx, } - if fc.phpServer == nil { - fc.phpServer = newDummyPhpServer() + if fc.server == nil { + fc.server = newDummyServer() } return fc diff --git a/frankenphp.go b/frankenphp.go index 90fcca3e67..e8af11759e 100644 --- a/frankenphp.go +++ b/frankenphp.go @@ -49,6 +49,7 @@ var ( ErrMainThreadCreation = errors.New("error creating the main thread") ErrScriptExecution = errors.New("error during PHP script execution") ErrNotRunning = errors.New("FrankenPHP is not running. For proper configuration visit: https://frankenphp.dev/docs/config/#caddyfile-config") + ServerNotFoundError = errors.New("server not found") ErrInvalidRequestPath = ErrRejected{"invalid request path", http.StatusBadRequest} ErrInvalidContentLengthHeader = ErrRejected{"invalid Content-Length header", http.StatusBadRequest} @@ -285,8 +286,8 @@ func Init(options ...Option) error { } // append all module workers to the global workers for registration - for _, phpServer := range PhpServers { - opt.workers = append(opt.workers, phpServer.workerOpts...) + for _, server := range servers { + opt.workers = append(opt.workers, server.workerOpts...) } workerThreadCount, err := calculateMaxThreads(opt) @@ -381,7 +382,7 @@ func Shutdown() { drainWatchers() drainPHPThreads() - resetPhpServers() + resetServers() metrics.Shutdown() @@ -407,9 +408,9 @@ func ServeHTTP(responseWriter http.ResponseWriter, request *http.Request) error return ErrInvalidRequest } - phpServer := newDummyPhpServer() + s := newDummyServer() - return phpServer.ServeHTTP(responseWriter, request, opts...) + return s.serveHTTP(responseWriter, request, opts...) } //export go_ub_write diff --git a/options.go b/options.go index 9ad9351b2d..5425cc3752 100644 --- a/options.go +++ b/options.go @@ -5,7 +5,6 @@ import ( "fmt" "log/slog" "net/http" - "path/filepath" "time" "github.com/dunglas/frankenphp/internal/fastabs" @@ -20,8 +19,8 @@ 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 +// ServerOption instances allow to configure a server. +type ServerOption func(*server) error // opt contains the available options. // @@ -54,7 +53,7 @@ type workerOpt struct { matchRequest func(*http.Request) bool maxConsecutiveFailures int extensionWorkers *extensionWorkers - phpServer *PhpServer + server *server onThreadReady func(int) onThreadShutdown func(int) onServerStartup func() @@ -98,19 +97,9 @@ func WithMetrics(m Metrics) Option { // WithWorkers configures the PHP workers to start func WithWorkers(name, fileName string, num int, options ...WorkerOption) Option { return func(o *opt) error { - worker := workerOpt{ - name: name, - fileName: fileName, - num: num, - env: PrepareEnv(nil), - watch: []string{}, - maxConsecutiveFailures: defaultMaxConsecutiveFailures, - } - - for _, option := range options { - if err := option(&worker); err != nil { - return err - } + worker, err := newWorkerOpt(name, fileName, num, options...) + if err != nil { + return err } o.workers = append(o.workers, worker) @@ -158,17 +147,6 @@ 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 { @@ -296,60 +274,40 @@ func withExtensionWorkers(w *extensionWorkers) WorkerOption { } } -func WithPhpServerRoot(root string, resolveSymlink bool) PhpServerOption { - return func(s *PhpServer) error { - root, err := fastabs.FastAbs(root) +// WithServer configures a server. +func WithServer( + idx int, + resolvedDocumentRoot string, + splitPath []string, + env map[string]string, + opts ...ServerOption, +) Option { + return func(o *opt) error { + root, err := fastabs.FastAbs(resolvedDocumentRoot) if err != nil { return err } - if resolveSymlink { - if root, err = filepath.EvalSymlinks(root); err != nil { - return err - } - } - - s.root = root - return nil - } -} - -func WithPhpServerSplitPath(splitPath []string) PhpServerOption { - return func(s *PhpServer) error { if err := normalizeSplitPath(splitPath); err != nil { return err } - s.splitPath = splitPath - return nil - } -} - -func WithPhpServerEnv(env map[string]string) PhpServerOption { - return func(s *PhpServer) error { - s.env = PrepareEnv(env) + _, err = newServer(idx, root, splitPath, PrepareEnv(env), opts...) + if err != nil { + return err + } 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 - } +// WithServerWorker configures the PHP workers to start for a specific server +func WithServerWorker(name, fileName string, num int, options ...WorkerOption) ServerOption { + return func(s *server) error { + workerOpt, err := newWorkerOpt(name, fileName, num, options...) + workerOpt.server = s + if err != nil { + return err } s.workerOpts = append(s.workerOpts, workerOpt) @@ -358,10 +316,21 @@ func WithPhpServerWorker(name, fileName string, num int, options ...WorkerOption } } -func WithPHPServerLogger(logger *slog.Logger) PhpServerOption { - return func(s *PhpServer) error { - s.logger = logger +func newWorkerOpt(name, fileName string, num int, options ...WorkerOption) (workerOpt, error) { + workerOpt := workerOpt{ + name: name, + fileName: fileName, + num: num, + env: PrepareEnv(nil), + watch: []string{}, + maxConsecutiveFailures: defaultMaxConsecutiveFailures, + } - return nil + for _, option := range options { + if err := option(&workerOpt); err != nil { + return workerOpt, err + } } + + return workerOpt, nil } diff --git a/phpserver.go b/phpserver.go deleted file mode 100644 index 71d3a8215b..0000000000 --- a/phpserver.go +++ /dev/null @@ -1,108 +0,0 @@ -package frankenphp - -import ( - "fmt" - "log/slog" - "net/http" -) - -// 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 - workersWithRequestMatcher []*worker - workerOpts []workerOpt - logger *slog.Logger -} - -// PhpServers is a map of all registered PhpServer instances. -// 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 resetPhpServers() { - PhpServers = make(map[int]*PhpServer) -} - -func newPhpServer(idx int, opts ...PhpServerOption) (*PhpServer, error) { - 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), - } -} - -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 { - 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/phpserver_test.go b/phpserver_test.go deleted file mode 100644 index ab089256a9..0000000000 --- a/phpserver_test.go +++ /dev/null @@ -1,220 +0,0 @@ -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 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], "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") - 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, "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{".custom"})) - - 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: /split-path.custom\n") - assert.Contains(t, body, "PHP_SELF: /split-path.custom/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_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") - 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("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"}), - 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_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, - 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], "http://example.com/server-variable.php") - - 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, "http://example.com/server-variable.php", 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, "http://example.com/server-variable.php", nil) - w := httptest.NewRecorder() - - err := server.ServeHTTP(w, req) - - assert.ErrorIs(t, err, frankenphp.ErrNotRunning) - }) -} diff --git a/server.go b/server.go new file mode 100644 index 0000000000..471d901852 --- /dev/null +++ b/server.go @@ -0,0 +1,126 @@ +package frankenphp + +import ( + "errors" + "fmt" + "log/slog" + "net/http" +) + +// server represents a server block in the caddyfile. +// can also be used to scope workers to a specific set of configurations. +type server struct { + idx int + root string + splitPath []string + env PreparedEnv + workers []*worker + workersByPath map[string]*worker + workersWithRequestMatcher []*worker + workerOpts []workerOpt + logger *slog.Logger +} + +var servers = make(map[int]*server) + +func resetServers() { + servers = make(map[int]*server) +} + +func newServer(idx int, root string, splitPath []string, env map[string]string, opts ...ServerOption) (*server, error) { + existingServer, ok := servers[idx] + if ok { + globalLogger.Debug("server already registered, ignoring duplicate registration", "idx", idx) + return existingServer, nil + } + + server := &server{ + idx: idx, + root: root, + splitPath: splitPath, + env: env, + workersByPath: make(map[string]*worker), + workerOpts: make([]workerOpt, 0), + } + + for _, option := range opts { + if err := option(server); err != nil { + return server, err + } + } + + if len(server.splitPath) == 0 { + server.splitPath = []string{".php"} + } + + if env == nil { + env = PrepareEnv(nil) + } + + servers[server.idx] = server + + return server, nil +} + +// fallback PHP server if none could be associated with a request +func newDummyServer() *server { + return &server{ + idx: -1, + workersByPath: make(map[string]*worker), + env: make(map[string]string), + } +} + +func (s *server) addWorker(w *worker) error { + s.workers = append(s.workers, w) + if w.matchRequest != nil { + s.workersWithRequestMatcher = append(s.workersWithRequestMatcher, w) + return nil + } + + if _, exists := s.workersByPath[w.fileName]; exists { + return fmt.Errorf("two workers in a server cannot have the same filename: %q", w.fileName) + } + s.workersByPath[w.fileName] = w + + return nil +} + +func ServeHTTPSrv(serverIdx int, responseWriter http.ResponseWriter, request *http.Request, opts ...RequestOption) error { + s, ok := servers[serverIdx] + if !ok { + return errors.Join(ServerNotFoundError, fmt.Errorf("server with idx %d not found", serverIdx)) + } + + return s.serveHTTP(responseWriter, request, opts...) +} + +// ServeHTTP executes a PHP script according to the given context. +// the request will be scoped to the server instance. +func (s *server) 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 + } + + // Handle request with a worker if one is assigned + 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/server_test.go b/server_test.go new file mode 100644 index 0000000000..4952bdf115 --- /dev/null +++ b/server_test.go @@ -0,0 +1,232 @@ +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 initServer(t *testing.T, opts ...frankenphp.Option) { + t.Helper() + t.Cleanup(frankenphp.Shutdown) + require.NoError(t, frankenphp.Init(opts...)) +} + +func serverRequest(t *testing.T, serverIdx int, req *http.Request) (string, *http.Response) { + t.Helper() + + w := httptest.NewRecorder() + err := frankenphp.ServeHTTPSrv(serverIdx, 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 serverGet(t *testing.T, serverIdx int, url string) (string, *http.Response) { + t.Helper() + + return serverRequest(t, serverIdx, httptest.NewRequest(http.MethodGet, url, nil)) +} + +func TestServer(t *testing.T) { + t.Run("idx", func(t *testing.T) { + initServer(t, + frankenphp.WithServer(1, + testDataDir, + []string{}, + map[string]string{ + "PHP_SERVER_IDX": "1", + }, + ), + frankenphp.WithServer(2, + testDataDir, + []string{}, + map[string]string{ + "PHP_SERVER_IDX": "2", + }, + ), + ) + + body1, _ := serverGet(t, 1, "http://example.com/server-variable.php") + body2, _ := serverGet(t, 2, "http://example.com/server-variable.php") + + 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) { + initServer(t, frankenphp.WithServer(1, + testDataDir, + []string{}, + map[string]string{}, + )) + + body, _ := serverGet(t, 1, "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) { + initServer(t, frankenphp.WithServer(1, + testDataDir, + []string{}, + map[string]string{ + "PHP_SERVER_TEST_KEY": "from_php_server", + }, + )) + + body, _ := serverGet(t, 1, "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) { + initServer(t, frankenphp.WithServer( + 1, + testDataDir, + []string{".custom"}, + map[string]string{}, + )) + + body, _ := serverGet(t, 1, "http://example.com/split-path.custom/pathinfo") + + assert.Contains(t, body, "PATH_INFO: /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("workers_by_path_and_request_matcher", func(t *testing.T) { + initServer( + t, + frankenphp.WithServer(1, + testDataDir, + []string{}, + map[string]string{}, + frankenphp.WithServerWorker("counter", testDataDir+"worker-with-counter.php", 1), + ), + frankenphp.WithServer(2, + testDataDir, + []string{}, + map[string]string{}, + frankenphp.WithServerWorker("match", testDataDir+"worker-with-counter.php", 1, + frankenphp.WithWorkerMatchOn(func(r *http.Request) bool { + return strings.HasPrefix(r.URL.Path, "/match/") + }), + ), + ), + ) + + body1, _ := serverGet(t, 1, "http://example.com/worker-with-counter.php") + body2, _ := serverGet(t, 1, "http://example.com/worker-with-counter.php") + body3, _ := serverGet(t, 2, "http://example.com/match/anything") + body4, _ := serverGet(t, 2, "http://example.com/match/anything") + body5, _ := serverGet(t, 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_env_inheritance", func(t *testing.T) { + initServer(t, frankenphp.WithServer( + 1, + testDataDir, + []string{}, + map[string]string{ + "APP_ENV": "staging", + }, + frankenphp.WithServerWorker("env", testDataDir+"worker-with-env.php", 1), + )) + + body, _ := serverGet(t, 1, "http://example.com/worker-with-env.php") + + 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.WithServer(1, + testDataDir, + []string{}, + map[string]string{}, + frankenphp.WithServerWorker("worker1", testDataDir+"worker-with-counter.php", 1), + frankenphp.WithServerWorker("worker2", testDataDir+"worker-with-counter.php", 1), + ), + ) + + assert.Error(t, err) + assert.Contains(t, err.Error(), "two workers in a server cannot have the same filename") + }) + + t.Run("duplicate_registration", func(t *testing.T) { + initServer(t, + frankenphp.WithServer(1, + testDataDir, + []string{}, + map[string]string{ + "PHP_SERVER_IDX": "first", + }, + ), + frankenphp.WithServer(1, + testDataDir+"/other/", + []string{}, + map[string]string{ + "PHP_SERVER_IDX": "second", + }, + ), + ) + + body, _ := serverGet(t, 1, "http://example.com/server-variable.php") + + assert.Contains(t, body, "[PHP_SERVER_IDX] => first", "should contain the first server env variable") + assert.NotContains(t, body, "[PHP_SERVER_IDX] => second", "should not contain the duplicate server env variable") + }) + + t.Run("serve_http_validation", func(t *testing.T) { + initServer(t, frankenphp.WithServer(1, + testDataDir, + []string{}, + map[string]string{}, + )) + + req := httptest.NewRequest(http.MethodGet, "http://example.com/server-variable.php", nil) + req.Header.Add("Content-Length", "-1") + body, resp := serverRequest(t, 1, req) + + assert.Equal(t, 400, resp.StatusCode) + assert.Contains(t, body, "invalid") + }) + + t.Run("idx_not_found", func(t *testing.T) { + req := httptest.NewRequest(http.MethodGet, "http://example.com/server-variable.php", nil) + w := httptest.NewRecorder() + + err := frankenphp.ServeHTTPSrv(1, w, req) + + assert.ErrorIs(t, err, frankenphp.ServerNotFoundError) + }) +} diff --git a/worker.go b/worker.go index e11c129c19..28b1ac4463 100644 --- a/worker.go +++ b/worker.go @@ -33,7 +33,7 @@ type worker struct { onThreadReady func(int) onThreadShutdown func(int) queuedRequests atomic.Int32 - phpServer *PhpServer + server *server } var ( @@ -67,9 +67,9 @@ func initWorkers(opts []workerOpt) error { totalThreadsToStart += w.num workers = append(workers, w) workersByName[w.name] = w - if w.phpServer == nil { + if w.server == nil { globalWorkersByPath[w.fileName] = w - } else if err := w.phpServer.addWorker(w); err != nil { + } else if err := w.server.addWorker(w); err != nil { return err } } @@ -123,7 +123,7 @@ func newWorker(o workerOpt) (*worker, error) { o.name = absFileName } - if o.phpServer == nil { + if o.server == nil { if w := globalWorkersByPath[absFileName]; w != nil { return w, fmt.Errorf("two workers cannot have the same filename: %q", absFileName) } @@ -139,8 +139,8 @@ func newWorker(o workerOpt) (*worker, error) { o.env["FRANKENPHP_WORKER\x00"] = "1" - if o.phpServer != nil && len(o.phpServer.env) > 0 { - for k, v := range o.phpServer.env { + if o.server != nil && len(o.server.env) > 0 { + for k, v := range o.server.env { if _, exists := o.env[k]; !exists { o.env[k] = v } @@ -159,7 +159,7 @@ func newWorker(o workerOpt) (*worker, error) { maxConsecutiveFailures: o.maxConsecutiveFailures, onThreadReady: o.onThreadReady, onThreadShutdown: o.onThreadShutdown, - phpServer: o.phpServer, + server: o.server, } w.configureMercure(&o) From c7ea1f776ae718bd8d11d594c898f85d18ca2e5d Mon Sep 17 00:00:00 2001 From: Alliballibaba Date: Sat, 4 Jul 2026 20:33:07 +0200 Subject: [PATCH 22/39] fixes the module --- caddy/module.go | 33 ++++++++++++++------------------- 1 file changed, 14 insertions(+), 19 deletions(-) diff --git a/caddy/module.go b/caddy/module.go index 763d5476d3..78ea9638ef 100644 --- a/caddy/module.go +++ b/caddy/module.go @@ -147,33 +147,33 @@ func (f *FrankenPHPModule) Provision(ctx caddy.Context) error { return err } - 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} + resolvedEnv := 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} for k, e := range f.Env { if needReplacement(e) { requestEnv[k] = e } else { - unchangingEnv[k] = e + resolvedEnv[k] = e } } f.preparedEnv = frankenphp.PrepareEnv(requestEnv) - // note: duplicate PhpServerIdx registration will be ignored, only the first one will be used + // duplicate PhpServerIdx registrations 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, *f.ResolveRootSymlink), - frankenphp.WithPhpServerEnv(unchangingEnv), - frankenphp.WithPHPServerLogger(ctx.Slogger()), - frankenphp.WithPhpServerSplitPath(f.SplitPath), - } - + phpServerOptions := []frankenphp.ServerOption{} for _, w := range f.Workers { - phpServerOptions = append(phpServerOptions, frankenphp.WithPhpServerWorker(w.Name, w.FileName, w.Num, w.toWorkerOptions()...)) + phpServerOptions = append(phpServerOptions, frankenphp.WithServerWorker(w.Name, w.FileName, w.Num, w.toWorkerOptions()...)) } - fapp.opts = append(fapp.opts, frankenphp.WithPhpServer(f.PhpServerIdx, phpServerOptions...)) + fapp.opts = append(fapp.opts, frankenphp.WithServer( + f.PhpServerIdx, + f.resolvedDocumentRoot, + f.SplitPath, + resolvedEnv, + phpServerOptions..., + )) return nil } @@ -233,12 +233,7 @@ func (f *FrankenPHPModule) ServeHTTP(w http.ResponseWriter, r *http.Request, _ c opts = append(opts, frankenphp.WithRequestPreparedEnv(env)) } - 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...) + err := frankenphp.ServeHTTPSrv(f.PhpServerIdx, w, r, opts...) if err != nil && !errors.As(err, &frankenphp.ErrRejected{}) { return caddyhttp.Error(http.StatusInternalServerError, err) From 4871ea48fa1d15f1c98781a091cc660da5c0554a Mon Sep 17 00:00:00 2001 From: Alliballibaba Date: Sat, 4 Jul 2026 20:50:52 +0200 Subject: [PATCH 23/39] cleanup --- frankenphp.go | 16 ++++++++++++++++ server.go | 16 ---------------- 2 files changed, 16 insertions(+), 16 deletions(-) diff --git a/frankenphp.go b/frankenphp.go index e8af11759e..a517b897a9 100644 --- a/frankenphp.go +++ b/frankenphp.go @@ -401,6 +401,10 @@ func Shutdown() { // ServeHTTP executes a PHP script according to the given context. func ServeHTTP(responseWriter http.ResponseWriter, request *http.Request) error { + if !isRunning { + return ErrNotRunning + } + ctx := request.Context() opts, ok := ctx.Value(contextKey).([]RequestOption) @@ -413,6 +417,18 @@ func ServeHTTP(responseWriter http.ResponseWriter, request *http.Request) error return s.serveHTTP(responseWriter, request, opts...) } +// ServeHTTPSrv executes a PHP script with a registered server. +// this allows using a pre-configured server instance. +// otherwise, it is equivalent to calling ServeHTTP. +func ServeHTTPSrv(serverIdx int, responseWriter http.ResponseWriter, request *http.Request, opts ...RequestOption) error { + s, ok := servers[serverIdx] + if !ok { + return errors.Join(ServerNotFoundError, fmt.Errorf("server with idx %d not found or not started", serverIdx)) + } + + return s.serveHTTP(responseWriter, request, opts...) +} + //export go_ub_write func go_ub_write(threadIndex C.uintptr_t, cBuf *C.char, length C.size_t) (C.size_t, C.bool) { thread := phpThreads[threadIndex] diff --git a/server.go b/server.go index 471d901852..ea5d63b2dd 100644 --- a/server.go +++ b/server.go @@ -1,7 +1,6 @@ package frankenphp import ( - "errors" "fmt" "log/slog" "net/http" @@ -86,27 +85,12 @@ func (s *server) addWorker(w *worker) error { return nil } -func ServeHTTPSrv(serverIdx int, responseWriter http.ResponseWriter, request *http.Request, opts ...RequestOption) error { - s, ok := servers[serverIdx] - if !ok { - return errors.Join(ServerNotFoundError, fmt.Errorf("server with idx %d not found", serverIdx)) - } - - return s.serveHTTP(responseWriter, request, opts...) -} - -// ServeHTTP executes a PHP script according to the given context. -// the request will be scoped to the server instance. func (s *server) 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 From 0401c623ec57012b30e742e2d0dd7d746f9d7ee1 Mon Sep 17 00:00:00 2001 From: Alliballibaba Date: Sat, 4 Jul 2026 22:15:30 +0200 Subject: [PATCH 24/39] closes admin body immediately --- caddy/admin_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/caddy/admin_test.go b/caddy/admin_test.go index 5ec45fa7c4..d2946bb179 100644 --- a/caddy/admin_test.go +++ b/caddy/admin_test.go @@ -325,7 +325,7 @@ func TestAddModuleWorkerViaAdminApi(t *testing.T) { assert.NoError(t, err) r.Header.Set("Content-Type", "text/caddyfile") resp := tester.AssertResponseCode(r, http.StatusOK) - defer func() { require.NoError(t, resp.Body.Close()) }() + require.NoError(t, resp.Body.Close()) // Get the updated debug state to check if the worker was added updatedDebugState := getDebugState(t, tester) From 9b7967f9acd220d0d1e060a5101a35764d3fb492 Mon Sep 17 00:00:00 2001 From: Alliballibaba Date: Sat, 4 Jul 2026 23:22:11 +0200 Subject: [PATCH 25/39] removes ServerOption. --- caddy/app.go | 20 +++++++++++--- caddy/module.go | 45 ++++++++++++++++++------------- frankenphp.go | 11 ++++---- options.go | 71 +++++++++++++++++++------------------------------ server.go | 8 +----- worker.go | 23 ++++++++++------ 6 files changed, 92 insertions(+), 86 deletions(-) diff --git a/caddy/app.go b/caddy/app.go index 8a0f968824..f4d8861a2a 100644 --- a/caddy/app.go +++ b/caddy/app.go @@ -59,10 +59,11 @@ 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 + moduleWorkers map[int][]workerConfig } var errIni = errors.New(`"php_ini" must be in the format: php_ini "" ""`) @@ -119,11 +120,21 @@ func (f *FrankenPHPApp) Start() error { frankenphp.WithMaxRequests(f.MaxRequests), ) + // register global workers for _, w := range f.Workers { w.FileName = repl.ReplaceKnown(w.FileName, "") f.opts = append(f.opts, frankenphp.WithWorkers(w.Name, w.FileName, w.Num, w.toWorkerOptions()...)) } + // register module workers + for serverIdx, workers := range f.moduleWorkers { + for _, w := range workers { + w.FileName = repl.ReplaceKnown(w.FileName, "") + workerOptions := append(w.toWorkerOptions(), frankenphp.WithWorkerServerScope(serverIdx)) + f.opts = append(f.opts, frankenphp.WithWorkers(w.Name, w.FileName, w.Num, workerOptions...)) + } + } + frankenphp.Shutdown() if err := frankenphp.Init(f.opts...); err != nil { return err @@ -150,6 +161,7 @@ func (f *FrankenPHPApp) Stop() error { f.MaxWaitTime = 0 f.MaxIdleTime = 0 f.MaxRequests = 0 + f.moduleWorkers = nil optionsMU.Lock() options = nil diff --git a/caddy/module.go b/caddy/module.go index 78ea9638ef..a84eaa5a91 100644 --- a/caddy/module.go +++ b/caddy/module.go @@ -44,8 +44,8 @@ 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 idx of the php_server this module belongs to - PhpServerIdx int `json:"php_server_idx,omitempty"` + // ServerIdx is the idx of the php_server this module belongs to + ServerIdx int `json:"server_idx,omitempty"` resolvedDocumentRoot string preparedEnv frankenphp.PreparedEnv @@ -91,11 +91,11 @@ func (f *FrankenPHPModule) Provision(ctx caddy.Context) error { wc.FileName = filepath.Join(frankenphp.EmbeddedAppPath, wc.FileName) } - phpServerPrefix := fmt.Sprintf("m%d#", f.PhpServerIdx) + serverPrefix := fmt.Sprintf("m%d#", f.ServerIdx) if wc.Name == "" { - wc.Name = f.generateUniqueModuleWorkerName(wc.FileName, phpServerPrefix) - } else if !strings.HasPrefix(wc.Name, phpServerPrefix) { - wc.Name = phpServerPrefix + wc.Name + wc.Name = f.generateUniqueModuleWorkerName(wc.FileName, serverPrefix) + } else if !strings.HasPrefix(wc.Name, serverPrefix) { + wc.Name = serverPrefix + wc.Name } f.Workers[i] = wc @@ -160,19 +160,28 @@ func (f *FrankenPHPModule) Provision(ctx caddy.Context) error { f.preparedEnv = frankenphp.PrepareEnv(requestEnv) - // duplicate PhpServerIdx registrations 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.ServerOption{} - for _, w := range f.Workers { - phpServerOptions = append(phpServerOptions, frankenphp.WithServerWorker(w.Name, w.FileName, w.Num, w.toWorkerOptions()...)) + if fapp.moduleWorkers == nil { + fapp.moduleWorkers = make(map[int][]workerConfig) + } + + if f.ServerIdx <= 0 { + // when registering via JSON configuration, it's possible that no idx was yet assigned + f.ServerIdx = len(fapp.moduleWorkers) + 1 + caddy.Log().Warn("\"php\" is missing a \"server_idx\", assigning one on-the-fly") } + if _, ok := fapp.moduleWorkers[f.ServerIdx]; ok { + // server was already registered, avoid duplicate registrations + return nil + } + + fapp.moduleWorkers[f.ServerIdx] = f.Workers + fapp.opts = append(fapp.opts, frankenphp.WithServer( - f.PhpServerIdx, + f.ServerIdx, f.resolvedDocumentRoot, f.SplitPath, resolvedEnv, - phpServerOptions..., )) return nil @@ -233,7 +242,7 @@ func (f *FrankenPHPModule) ServeHTTP(w http.ResponseWriter, r *http.Request, _ c opts = append(opts, frankenphp.WithRequestPreparedEnv(env)) } - err := frankenphp.ServeHTTPSrv(f.PhpServerIdx, w, r, opts...) + err := frankenphp.ServeHTTPSrv(f.ServerIdx, w, r, opts...) if err != nil && !errors.As(err, &frankenphp.ErrRejected{}) { return caddyhttp.Error(http.StatusInternalServerError, err) @@ -316,9 +325,9 @@ func (f *FrankenPHPModule) UnmarshalCaddyfile(d *caddyfile.Dispenser) error { return nil } -func (f *FrankenPHPModule) assignPhpServerIdx(h httpcaddyfile.Helper) { +func (f *FrankenPHPModule) assignServerIdx(h httpcaddyfile.Helper) { counter, _ := h.State["php_server_count"].(int) - f.PhpServerIdx = counter + f.ServerIdx = counter + 1 h.State["php_server_count"] = counter + 1 } @@ -327,7 +336,7 @@ func parseCaddyfile(h httpcaddyfile.Helper) (caddyhttp.MiddlewareHandler, error) m := &FrankenPHPModule{} err := m.UnmarshalCaddyfile(h.Dispenser) - m.assignPhpServerIdx(h) + m.assignServerIdx(h) return m, err } @@ -464,7 +473,7 @@ func parsePhpServer(h httpcaddyfile.Helper) ([]httpcaddyfile.ConfigValue, error) } // assign a unique index to the php server - phpsrv.assignPhpServerIdx(h) + phpsrv.assignServerIdx(h) if frankenphp.EmbeddedAppPath != "" { if phpsrv.Root == "" { diff --git a/frankenphp.go b/frankenphp.go index a517b897a9..1e06356805 100644 --- a/frankenphp.go +++ b/frankenphp.go @@ -285,11 +285,6 @@ func Init(options ...Option) error { maxIdleTime = opt.maxIdleTime } - // append all module workers to the global workers for registration - for _, server := range servers { - opt.workers = append(opt.workers, server.workerOpts...) - } - workerThreadCount, err := calculateMaxThreads(opt) if err != nil { Shutdown() @@ -423,7 +418,11 @@ func ServeHTTP(responseWriter http.ResponseWriter, request *http.Request) error func ServeHTTPSrv(serverIdx int, responseWriter http.ResponseWriter, request *http.Request, opts ...RequestOption) error { s, ok := servers[serverIdx] if !ok { - return errors.Join(ServerNotFoundError, fmt.Errorf("server with idx %d not found or not started", serverIdx)) + existinggIdxs := "" + for idx := range servers { + existinggIdxs += fmt.Sprintf("%d, ", idx) + } + return errors.Join(ServerNotFoundError, fmt.Errorf("server with idx %d not found or not started, existing servers: %s", serverIdx, existinggIdxs)) } return s.serveHTTP(responseWriter, request, opts...) diff --git a/options.go b/options.go index 5425cc3752..132eddb667 100644 --- a/options.go +++ b/options.go @@ -19,9 +19,6 @@ type Option func(h *opt) error // WorkerOption instances allow configuring FrankenPHP worker. type WorkerOption func(*workerOpt) error -// ServerOption instances allow to configure a server. -type ServerOption func(*server) error - // opt contains the available options. // // If you change this, also update the Caddy module and the documentation. @@ -53,11 +50,11 @@ type workerOpt struct { matchRequest func(*http.Request) bool maxConsecutiveFailures int extensionWorkers *extensionWorkers - server *server onThreadReady func(int) onThreadShutdown func(int) onServerStartup func() onServerShutdown func() + serverIdx int } // WithContext sets the main context to use. @@ -97,9 +94,19 @@ func WithMetrics(m Metrics) Option { // WithWorkers configures the PHP workers to start func WithWorkers(name, fileName string, num int, options ...WorkerOption) Option { return func(o *opt) error { - worker, err := newWorkerOpt(name, fileName, num, options...) - if err != nil { - return err + worker := workerOpt{ + name: name, + fileName: fileName, + num: num, + env: PrepareEnv(nil), + watch: []string{}, + maxConsecutiveFailures: defaultMaxConsecutiveFailures, + } + + for _, option := range options { + if err := option(&worker); err != nil { + return err + } } o.workers = append(o.workers, worker) @@ -220,6 +227,15 @@ func WithWorkerMatchOn(matcherFunc func(*http.Request) bool) WorkerOption { } } +// WithWorkerServerScope scopes the worker to a server instance. +// Only requests that are handled by the server instance will reach the worker. +func WithWorkerServerScope(serverIdx int) WorkerOption { + return func(w *workerOpt) error { + w.serverIdx = serverIdx + return nil + } +} + // WithWorkerMaxFailures sets the maximum number of consecutive failures before panicking func WithWorkerMaxFailures(maxFailures int) WorkerOption { return func(w *workerOpt) error { @@ -280,9 +296,12 @@ func WithServer( resolvedDocumentRoot string, splitPath []string, env map[string]string, - opts ...ServerOption, ) Option { return func(o *opt) error { + if idx <= 0 { + return fmt.Errorf("server idx must be > 0, got %d", idx) + } + root, err := fastabs.FastAbs(resolvedDocumentRoot) if err != nil { return err @@ -292,45 +311,11 @@ func WithServer( return err } - _, err = newServer(idx, root, splitPath, PrepareEnv(env), opts...) - - if err != nil { - return err - } - return nil - } -} + _, err = newServer(idx, root, splitPath, PrepareEnv(env)) -// WithServerWorker configures the PHP workers to start for a specific server -func WithServerWorker(name, fileName string, num int, options ...WorkerOption) ServerOption { - return func(s *server) error { - workerOpt, err := newWorkerOpt(name, fileName, num, options...) - workerOpt.server = s if err != nil { return err } - - s.workerOpts = append(s.workerOpts, workerOpt) - return nil } } - -func newWorkerOpt(name, fileName string, num int, options ...WorkerOption) (workerOpt, error) { - workerOpt := workerOpt{ - name: name, - fileName: fileName, - num: num, - env: PrepareEnv(nil), - watch: []string{}, - maxConsecutiveFailures: defaultMaxConsecutiveFailures, - } - - for _, option := range options { - if err := option(&workerOpt); err != nil { - return workerOpt, err - } - } - - return workerOpt, nil -} diff --git a/server.go b/server.go index ea5d63b2dd..56c007e959 100644 --- a/server.go +++ b/server.go @@ -26,7 +26,7 @@ func resetServers() { servers = make(map[int]*server) } -func newServer(idx int, root string, splitPath []string, env map[string]string, opts ...ServerOption) (*server, error) { +func newServer(idx int, root string, splitPath []string, env map[string]string) (*server, error) { existingServer, ok := servers[idx] if ok { globalLogger.Debug("server already registered, ignoring duplicate registration", "idx", idx) @@ -42,12 +42,6 @@ func newServer(idx int, root string, splitPath []string, env map[string]string, workerOpts: make([]workerOpt, 0), } - for _, option := range opts { - if err := option(server); err != nil { - return server, err - } - } - if len(server.splitPath) == 0 { server.splitPath = []string{".php"} } diff --git a/worker.go b/worker.go index 28b1ac4463..b136a72fed 100644 --- a/worker.go +++ b/worker.go @@ -123,11 +123,17 @@ func newWorker(o workerOpt) (*worker, error) { o.name = absFileName } - if o.server == nil { - if w := globalWorkersByPath[absFileName]; w != nil { - return w, fmt.Errorf("two workers cannot have the same filename: %q", absFileName) + var server *server + if o.serverIdx != 0 { + server = servers[o.serverIdx] + + if server == nil { + return nil, fmt.Errorf("worker was registered with a non-existent server idx %d: %q", o.serverIdx, absFileName) } + } else if w := globalWorkersByPath[absFileName]; w != nil { + return w, fmt.Errorf("two global 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) } @@ -137,16 +143,17 @@ func newWorker(o workerOpt) (*worker, error) { o.env = make(PreparedEnv, 1) } - o.env["FRANKENPHP_WORKER\x00"] = "1" - - if o.server != nil && len(o.server.env) > 0 { - for k, v := range o.server.env { + // if the worker is scoped to a server, inherit the server env + if server != nil && len(server.env) > 0 { + for k, v := range server.env { if _, exists := o.env[k]; !exists { o.env[k] = v } } } + o.env["FRANKENPHP_WORKER\x00"] = "1" + w := &worker{ name: o.name, fileName: absFileName, @@ -159,7 +166,7 @@ func newWorker(o workerOpt) (*worker, error) { maxConsecutiveFailures: o.maxConsecutiveFailures, onThreadReady: o.onThreadReady, onThreadShutdown: o.onThreadShutdown, - server: o.server, + server: server, } w.configureMercure(&o) From 6987c1ee3f55862d15cd78fc9c3a17925794f76b Mon Sep 17 00:00:00 2001 From: Alliballibaba Date: Sat, 4 Jul 2026 23:22:26 +0200 Subject: [PATCH 26/39] fixes tests. --- server_test.go | 118 ++++++++++++++++--------------------------------- 1 file changed, 37 insertions(+), 81 deletions(-) diff --git a/server_test.go b/server_test.go index 4952bdf115..d676395391 100644 --- a/server_test.go +++ b/server_test.go @@ -48,20 +48,12 @@ func serverGet(t *testing.T, serverIdx int, url string) (string, *http.Response) func TestServer(t *testing.T) { t.Run("idx", func(t *testing.T) { initServer(t, - frankenphp.WithServer(1, - testDataDir, - []string{}, - map[string]string{ - "PHP_SERVER_IDX": "1", - }, - ), - frankenphp.WithServer(2, - testDataDir, - []string{}, - map[string]string{ - "PHP_SERVER_IDX": "2", - }, - ), + frankenphp.WithServer(1, testDataDir, nil, map[string]string{ + "PHP_SERVER_IDX": "1", + }), + frankenphp.WithServer(2, testDataDir, nil, map[string]string{ + "PHP_SERVER_IDX": "2", + }), ) body1, _ := serverGet(t, 1, "http://example.com/server-variable.php") @@ -74,11 +66,7 @@ func TestServer(t *testing.T) { }) t.Run("root", func(t *testing.T) { - initServer(t, frankenphp.WithServer(1, - testDataDir, - []string{}, - map[string]string{}, - )) + initServer(t, frankenphp.WithServer(1, testDataDir, nil, nil)) body, _ := serverGet(t, 1, "http://example.com/server-globals.php") @@ -87,13 +75,9 @@ func TestServer(t *testing.T) { }) t.Run("env", func(t *testing.T) { - initServer(t, frankenphp.WithServer(1, - testDataDir, - []string{}, - map[string]string{ - "PHP_SERVER_TEST_KEY": "from_php_server", - }, - )) + initServer(t, frankenphp.WithServer(1, testDataDir, nil, map[string]string{ + "PHP_SERVER_TEST_KEY": "from_php_server", + })) body, _ := serverGet(t, 1, "http://example.com/server-variable.php") @@ -101,12 +85,7 @@ func TestServer(t *testing.T) { }) t.Run("split_path", func(t *testing.T) { - initServer(t, frankenphp.WithServer( - 1, - testDataDir, - []string{".custom"}, - map[string]string{}, - )) + initServer(t, frankenphp.WithServer(1, testDataDir, []string{".custom"}, nil)) body, _ := serverGet(t, 1, "http://example.com/split-path.custom/pathinfo") @@ -116,23 +95,18 @@ func TestServer(t *testing.T) { }) t.Run("workers_by_path_and_request_matcher", func(t *testing.T) { + server1Idx := 1 + server2Idx := 2 initServer( t, - frankenphp.WithServer(1, - testDataDir, - []string{}, - map[string]string{}, - frankenphp.WithServerWorker("counter", testDataDir+"worker-with-counter.php", 1), - ), - frankenphp.WithServer(2, - testDataDir, - []string{}, - map[string]string{}, - frankenphp.WithServerWorker("match", testDataDir+"worker-with-counter.php", 1, - frankenphp.WithWorkerMatchOn(func(r *http.Request) bool { - return strings.HasPrefix(r.URL.Path, "/match/") - }), - ), + frankenphp.WithServer(server1Idx, testDataDir, nil, nil), + frankenphp.WithServer(server2Idx, testDataDir, nil, nil), + frankenphp.WithWorkers("counter", testDataDir+"worker-with-counter.php", 1, frankenphp.WithWorkerServerScope(server1Idx)), + frankenphp.WithWorkers("match", testDataDir+"worker-with-counter.php", 1, + frankenphp.WithWorkerServerScope(server2Idx), + frankenphp.WithWorkerMatchOn(func(r *http.Request) bool { + return strings.HasPrefix(r.URL.Path, "/match/") + }), ), ) @@ -150,15 +124,13 @@ func TestServer(t *testing.T) { }) t.Run("worker_env_inheritance", func(t *testing.T) { - initServer(t, frankenphp.WithServer( - 1, - testDataDir, - []string{}, - map[string]string{ + initServer( + t, + frankenphp.WithServer(1, testDataDir, nil, map[string]string{ "APP_ENV": "staging", - }, - frankenphp.WithServerWorker("env", testDataDir+"worker-with-env.php", 1), - )) + }), + frankenphp.WithWorkers("env", testDataDir+"worker-with-env.php", 1, frankenphp.WithWorkerServerScope(1)), + ) body, _ := serverGet(t, 1, "http://example.com/worker-with-env.php") @@ -169,13 +141,9 @@ func TestServer(t *testing.T) { t.Cleanup(frankenphp.Shutdown) err := frankenphp.Init( - frankenphp.WithServer(1, - testDataDir, - []string{}, - map[string]string{}, - frankenphp.WithServerWorker("worker1", testDataDir+"worker-with-counter.php", 1), - frankenphp.WithServerWorker("worker2", testDataDir+"worker-with-counter.php", 1), - ), + frankenphp.WithServer(1, testDataDir, nil, nil), + frankenphp.WithWorkers("worker1", testDataDir+"worker-with-counter.php", 1, frankenphp.WithWorkerServerScope(1)), + frankenphp.WithWorkers("worker2", testDataDir+"worker-with-counter.php", 1, frankenphp.WithWorkerServerScope(1)), ) assert.Error(t, err) @@ -184,20 +152,12 @@ func TestServer(t *testing.T) { t.Run("duplicate_registration", func(t *testing.T) { initServer(t, - frankenphp.WithServer(1, - testDataDir, - []string{}, - map[string]string{ - "PHP_SERVER_IDX": "first", - }, - ), - frankenphp.WithServer(1, - testDataDir+"/other/", - []string{}, - map[string]string{ - "PHP_SERVER_IDX": "second", - }, - ), + frankenphp.WithServer(1, testDataDir, nil, map[string]string{ + "PHP_SERVER_IDX": "first", + }), + frankenphp.WithServer(1, testDataDir+"/other/", nil, map[string]string{ + "PHP_SERVER_IDX": "second", + }), ) body, _ := serverGet(t, 1, "http://example.com/server-variable.php") @@ -207,11 +167,7 @@ func TestServer(t *testing.T) { }) t.Run("serve_http_validation", func(t *testing.T) { - initServer(t, frankenphp.WithServer(1, - testDataDir, - []string{}, - map[string]string{}, - )) + initServer(t, frankenphp.WithServer(1, testDataDir, nil, nil)) req := httptest.NewRequest(http.MethodGet, "http://example.com/server-variable.php", nil) req.Header.Add("Content-Length", "-1") From ea35f2aa7851d1cda9519bd0149082ac8ef781d2 Mon Sep 17 00:00:00 2001 From: Alliballibaba Date: Sat, 4 Jul 2026 23:31:18 +0200 Subject: [PATCH 27/39] preallocates the fallback server --- context.go | 8 +++----- frankenphp.go | 4 +--- server.go | 18 ++++++++---------- 3 files changed, 12 insertions(+), 18 deletions(-) diff --git a/context.go b/context.go index 897a27ee52..927f955481 100644 --- a/context.go +++ b/context.go @@ -134,7 +134,8 @@ func newWorkerDummyContext(w *worker) (*frankenPHPContext, error) { } if fc.server == nil { - fc.server = newDummyServer() + // global worker, not associated with a server + fc.server = fallbackServer } splitCgiPath(fc) @@ -153,10 +154,7 @@ func newContextFromMessage(message any, rw http.ResponseWriter, ctx context.Cont responseWriter: rw, handlerParameters: message, ctx: ctx, - } - - if fc.server == nil { - fc.server = newDummyServer() + server: fallbackServer, } return fc diff --git a/frankenphp.go b/frankenphp.go index 1e06356805..5d24d951ae 100644 --- a/frankenphp.go +++ b/frankenphp.go @@ -407,9 +407,7 @@ func ServeHTTP(responseWriter http.ResponseWriter, request *http.Request) error return ErrInvalidRequest } - s := newDummyServer() - - return s.serveHTTP(responseWriter, request, opts...) + return fallbackServer.serveHTTP(responseWriter, request, opts...) } // ServeHTTPSrv executes a PHP script with a registered server. diff --git a/server.go b/server.go index 56c007e959..215f29eb4c 100644 --- a/server.go +++ b/server.go @@ -20,7 +20,14 @@ type server struct { logger *slog.Logger } -var servers = make(map[int]*server) +var ( + servers = make(map[int]*server) + fallbackServer = &server{ + idx: -1, + workersByPath: make(map[string]*worker), + env: make(map[string]string), + } +) func resetServers() { servers = make(map[int]*server) @@ -55,15 +62,6 @@ func newServer(idx int, root string, splitPath []string, env map[string]string) return server, nil } -// fallback PHP server if none could be associated with a request -func newDummyServer() *server { - return &server{ - idx: -1, - workersByPath: make(map[string]*worker), - env: make(map[string]string), - } -} - func (s *server) addWorker(w *worker) error { s.workers = append(s.workers, w) if w.matchRequest != nil { From 47f45ce29a478e7b8e0242f7aea9df6cae94ab48 Mon Sep 17 00:00:00 2001 From: Alliballibaba Date: Sat, 4 Jul 2026 23:31:59 +0200 Subject: [PATCH 28/39] precomputes the fallback server --- context.go | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/context.go b/context.go index 927f955481..40e4bc60db 100644 --- a/context.go +++ b/context.go @@ -154,7 +154,10 @@ func newContextFromMessage(message any, rw http.ResponseWriter, ctx context.Cont responseWriter: rw, handlerParameters: message, ctx: ctx, - server: fallbackServer, + } + + if fc.server == nil { + fc.server = fallbackServer } return fc From ce095d4910b5d42b0743cbfcc2d4ccdcfaf8f2d9 Mon Sep 17 00:00:00 2001 From: Alliballibaba Date: Sat, 4 Jul 2026 23:46:35 +0200 Subject: [PATCH 29/39] error cleanup --- frankenphp.go | 9 +++------ server.go | 10 ++++------ server_test.go | 21 +++++++-------------- 3 files changed, 14 insertions(+), 26 deletions(-) diff --git a/frankenphp.go b/frankenphp.go index 5d24d951ae..4fadeb6772 100644 --- a/frankenphp.go +++ b/frankenphp.go @@ -49,7 +49,8 @@ var ( ErrMainThreadCreation = errors.New("error creating the main thread") ErrScriptExecution = errors.New("error during PHP script execution") ErrNotRunning = errors.New("FrankenPHP is not running. For proper configuration visit: https://frankenphp.dev/docs/config/#caddyfile-config") - ServerNotFoundError = errors.New("server not found") + ErrServerNotFound = errors.New("server not found") + ErrAlreadyRegistered = errors.New("server already registered") ErrInvalidRequestPath = ErrRejected{"invalid request path", http.StatusBadRequest} ErrInvalidContentLengthHeader = ErrRejected{"invalid Content-Length header", http.StatusBadRequest} @@ -416,11 +417,7 @@ func ServeHTTP(responseWriter http.ResponseWriter, request *http.Request) error func ServeHTTPSrv(serverIdx int, responseWriter http.ResponseWriter, request *http.Request, opts ...RequestOption) error { s, ok := servers[serverIdx] if !ok { - existinggIdxs := "" - for idx := range servers { - existinggIdxs += fmt.Sprintf("%d, ", idx) - } - return errors.Join(ServerNotFoundError, fmt.Errorf("server with idx %d not found or not started, existing servers: %s", serverIdx, existinggIdxs)) + return fmt.Errorf("%w: no server with idx %d was registered (%d servers registered overall)", ErrServerNotFound, serverIdx, len(servers)) } return s.serveHTTP(responseWriter, request, opts...) diff --git a/server.go b/server.go index 215f29eb4c..46de90077b 100644 --- a/server.go +++ b/server.go @@ -34,10 +34,8 @@ func resetServers() { } func newServer(idx int, root string, splitPath []string, env map[string]string) (*server, error) { - existingServer, ok := servers[idx] - if ok { - globalLogger.Debug("server already registered, ignoring duplicate registration", "idx", idx) - return existingServer, nil + if _, ok := servers[idx]; ok { + return nil, fmt.Errorf("%w: duplicate registration of server with idx %d", ErrAlreadyRegistered, idx) } server := &server{ @@ -53,8 +51,8 @@ func newServer(idx int, root string, splitPath []string, env map[string]string) server.splitPath = []string{".php"} } - if env == nil { - env = PrepareEnv(nil) + if server.env == nil { + server.env = PrepareEnv(nil) } servers[server.idx] = server diff --git a/server_test.go b/server_test.go index d676395391..8df2a42511 100644 --- a/server_test.go +++ b/server_test.go @@ -137,7 +137,7 @@ func TestServer(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.Run("error_on_duplicate_worker_filenames", func(t *testing.T) { t.Cleanup(frankenphp.Shutdown) err := frankenphp.Init( @@ -150,20 +150,13 @@ func TestServer(t *testing.T) { assert.Contains(t, err.Error(), "two workers in a server cannot have the same filename") }) - t.Run("duplicate_registration", func(t *testing.T) { - initServer(t, - frankenphp.WithServer(1, testDataDir, nil, map[string]string{ - "PHP_SERVER_IDX": "first", - }), - frankenphp.WithServer(1, testDataDir+"/other/", nil, map[string]string{ - "PHP_SERVER_IDX": "second", - }), + t.Run("error_on_duplicate_registration", func(t *testing.T) { + err := frankenphp.Init( + frankenphp.WithServer(1, testDataDir, nil, nil), + frankenphp.WithServer(1, testDataDir, nil, nil), ) - body, _ := serverGet(t, 1, "http://example.com/server-variable.php") - - assert.Contains(t, body, "[PHP_SERVER_IDX] => first", "should contain the first server env variable") - assert.NotContains(t, body, "[PHP_SERVER_IDX] => second", "should not contain the duplicate server env variable") + assert.ErrorIs(t, err, frankenphp.ErrAlreadyRegistered) }) t.Run("serve_http_validation", func(t *testing.T) { @@ -183,6 +176,6 @@ func TestServer(t *testing.T) { err := frankenphp.ServeHTTPSrv(1, w, req) - assert.ErrorIs(t, err, frankenphp.ServerNotFoundError) + assert.ErrorIs(t, err, frankenphp.ErrServerNotFound) }) } From 48ac7d2817e3f035a4c4364e3056d08b694bbacd Mon Sep 17 00:00:00 2001 From: Alliballibaba Date: Sun, 5 Jul 2026 00:05:05 +0200 Subject: [PATCH 30/39] context cleanup. --- cgi.go | 6 +++--- frankenphp.go | 33 +++++++++++++++++---------------- mercure.go | 2 +- phpthread.go | 6 +----- threadworker.go | 4 ++-- 5 files changed, 24 insertions(+), 27 deletions(-) diff --git a/cgi.go b/cgi.go index 185c64dc3e..d3e22708c5 100644 --- a/cgi.go +++ b/cgi.go @@ -177,11 +177,11 @@ func registerPreparedEnv(fc *frankenPHPContext, preparedEnvLen int) { //export go_register_server_variables func go_register_server_variables(threadIndex C.uintptr_t, trackVarsArray *C.zval) { thread := phpThreads[threadIndex] - fc := thread.frankenPHPContext() + fc := thread.handler.frankenPHPContext() if fc.request != nil { addKnownVariablesToServer(fc, trackVarsArray) - addHeadersToServer(thread.context(), fc.request, trackVarsArray) + addHeadersToServer(fc.ctx, fc.request, trackVarsArray) } // The Prepared Environment is registered last and can overwrite any previous values @@ -293,7 +293,7 @@ func splitPos(path string, splitPath []string) int { //export go_update_request_info func go_update_request_info(threadIndex C.uintptr_t, info *C.sapi_request_info) *C.char { thread := phpThreads[threadIndex] - fc := thread.frankenPHPContext() + fc := thread.handler.frankenPHPContext() request := fc.request if request == nil { diff --git a/frankenphp.go b/frankenphp.go index 4fadeb6772..52a33c9c54 100644 --- a/frankenphp.go +++ b/frankenphp.go @@ -426,7 +426,7 @@ func ServeHTTPSrv(serverIdx int, responseWriter http.ResponseWriter, request *ht //export go_ub_write func go_ub_write(threadIndex C.uintptr_t, cBuf *C.char, length C.size_t) (C.size_t, C.bool) { thread := phpThreads[threadIndex] - fc := thread.frankenPHPContext() + fc := thread.handler.frankenPHPContext() if fc.isDone { return 0, C.bool(true) @@ -471,7 +471,7 @@ func go_ub_write(threadIndex C.uintptr_t, cBuf *C.char, length C.size_t) (C.size func go_apache_request_headers(threadIndex C.uintptr_t) (*C.go_string, C.size_t) { thread := phpThreads[threadIndex] ctx := thread.context() - fc := thread.frankenPHPContext() + fc := thread.handler.frankenPHPContext() if fc.responseWriter == nil { // worker mode, not handling a request @@ -552,7 +552,7 @@ func splitRawHeader(rawHeader *C.char, length int) (string, string) { //export go_write_headers func go_write_headers(threadIndex C.uintptr_t, status C.int, headers *C.zend_llist) C.bool { thread := phpThreads[threadIndex] - fc := thread.frankenPHPContext() + fc := thread.handler.frankenPHPContext() if fc == nil { return C.bool(false) } @@ -604,7 +604,7 @@ func go_write_headers(threadIndex C.uintptr_t, status C.int, headers *C.zend_lli //export go_sapi_flush func go_sapi_flush(threadIndex C.uintptr_t) bool { thread := phpThreads[threadIndex] - fc := thread.frankenPHPContext() + fc := thread.handler.frankenPHPContext() if fc == nil { return false } @@ -633,7 +633,7 @@ func go_sapi_flush(threadIndex C.uintptr_t) bool { //export go_read_post func go_read_post(threadIndex C.uintptr_t, cBuf *C.char, countBytes C.size_t) (readBytes C.size_t) { - fc := phpThreads[threadIndex].frankenPHPContext() + fc := phpThreads[threadIndex].handler.frankenPHPContext() if fc.responseWriter == nil { return 0 @@ -652,7 +652,7 @@ func go_read_post(threadIndex C.uintptr_t, cBuf *C.char, countBytes C.size_t) (r //export go_read_cookies func go_read_cookies(threadIndex C.uintptr_t) *C.char { - request := phpThreads[threadIndex].frankenPHPContext().request + request := phpThreads[threadIndex].handler.frankenPHPContext().request if request == nil { return nil } @@ -669,23 +669,24 @@ func go_read_cookies(threadIndex C.uintptr_t) *C.char { return C.CString(cookie) } +// getLogger returns the logger and context savely even if phpThreads have not been created yet func getLogger(threadIndex C.uintptr_t) (*slog.Logger, context.Context) { - ctxHolder := phpThreads[threadIndex] - if ctxHolder == nil { + if threadIndex < 0 || threadIndex >= C.uintptr_t(len(phpThreads)) { return globalLogger, globalCtx } - ctx := ctxHolder.context() - if ctxHolder.handler == nil { - return globalLogger, ctx + thread := phpThreads[threadIndex] + if thread == nil || thread.handler == nil { + return globalLogger, globalCtx } - fCtx := ctxHolder.frankenPHPContext() - if fCtx == nil || fCtx.logger == nil { - return globalLogger, ctx + fc := thread.handler.frankenPHPContext() + if fc == nil { + return globalLogger, globalCtx } - return fCtx.logger, ctx + // logger and context must always be defined on fc + return fc.logger, fc.ctx } //export go_log @@ -753,7 +754,7 @@ func mapToAttr(input map[string]any) []slog.Attr { //export go_is_context_done func go_is_context_done(threadIndex C.uintptr_t) C.bool { - return C.bool(phpThreads[threadIndex].frankenPHPContext().isDone) + return C.bool(phpThreads[threadIndex].handler.frankenPHPContext().isDone) } //export go_schedule_opcache_reset diff --git a/mercure.go b/mercure.go index d7cf33609e..9f464c147c 100644 --- a/mercure.go +++ b/mercure.go @@ -21,7 +21,7 @@ type mercureContext struct { func go_mercure_publish(threadIndex C.uintptr_t, topics *C.struct__zval_struct, data *C.zend_string, private bool, id, typ *C.zend_string, retry uint64) (generatedID *C.zend_string, error C.short) { thread := phpThreads[threadIndex] ctx := thread.context() - fc := thread.frankenPHPContext() + fc := thread.handler.frankenPHPContext() if fc.mercureHub == nil { if fc.logger.Enabled(ctx, slog.LevelError) { diff --git a/phpthread.go b/phpthread.go index 2d053b6d89..2e01a7b4d1 100644 --- a/phpthread.go +++ b/phpthread.go @@ -176,12 +176,8 @@ func (thread *phpThread) transitionToNewHandler() string { return thread.handler.beforeScriptExecution() } -func (thread *phpThread) frankenPHPContext() *frankenPHPContext { - return thread.handler.frankenPHPContext() -} - func (thread *phpThread) context() context.Context { - if fc := thread.frankenPHPContext(); fc != nil && fc.ctx != nil { + if fc := thread.handler.frankenPHPContext(); fc != nil && fc.ctx != nil { return fc.ctx } diff --git a/threadworker.go b/threadworker.go index 01285fef95..a5b57c6624 100644 --- a/threadworker.go +++ b/threadworker.go @@ -277,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 := thread.frankenPHPContext() + fc := thread.handler.frankenPHPContext() if retval != nil { r, err := GoValue[any](unsafe.Pointer(retval)) @@ -309,7 +309,7 @@ func go_frankenphp_finish_worker_request(threadIndex C.uintptr_t, retval *C.zval //export go_frankenphp_finish_php_request func go_frankenphp_finish_php_request(threadIndex C.uintptr_t) { thread := phpThreads[threadIndex] - fc := thread.frankenPHPContext() + fc := thread.handler.frankenPHPContext() fc.closeContext() From bb8ce0bb329d6eecd4f2cb479cd40f7e70d66bdd Mon Sep 17 00:00:00 2001 From: Alliballibaba Date: Sun, 5 Jul 2026 00:17:22 +0200 Subject: [PATCH 31/39] naming --- caddy/workerconfig.go | 2 +- options.go | 4 ++-- server_test.go | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/caddy/workerconfig.go b/caddy/workerconfig.go index 2464306a17..6cb61e37bb 100644 --- a/caddy/workerconfig.go +++ b/caddy/workerconfig.go @@ -170,7 +170,7 @@ func (wc *workerConfig) toWorkerOptions() []frankenphp.WorkerOption { if len(wc.MatchPath) > 0 { matchFunc := caddyhttp.MatchPath(append([]string(nil), wc.MatchPath...)) _ = matchFunc.Provision(caddy.Context{}) - opts = append(opts, frankenphp.WithWorkerMatchOn(matchFunc.Match)) + opts = append(opts, frankenphp.WithWorkerMatcher(matchFunc.Match)) } return opts } diff --git a/options.go b/options.go index 132eddb667..c9560b9bc7 100644 --- a/options.go +++ b/options.go @@ -217,10 +217,10 @@ func WithWorkerWatchMode(watch []string) WorkerOption { } } -// WithWorkerMatchOn sets a request matcher for this worker +// WithWorkerMatcher 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 { +func WithWorkerMatcher(matcherFunc func(*http.Request) bool) WorkerOption { return func(w *workerOpt) error { w.matchRequest = matcherFunc return nil diff --git a/server_test.go b/server_test.go index 8df2a42511..b0f6ef4a09 100644 --- a/server_test.go +++ b/server_test.go @@ -104,7 +104,7 @@ func TestServer(t *testing.T) { frankenphp.WithWorkers("counter", testDataDir+"worker-with-counter.php", 1, frankenphp.WithWorkerServerScope(server1Idx)), frankenphp.WithWorkers("match", testDataDir+"worker-with-counter.php", 1, frankenphp.WithWorkerServerScope(server2Idx), - frankenphp.WithWorkerMatchOn(func(r *http.Request) bool { + frankenphp.WithWorkerMatcher(func(r *http.Request) bool { return strings.HasPrefix(r.URL.Path, "/match/") }), ), From 94fe75e9079c406ba9df0527fbcb65a26c1cf47b Mon Sep 17 00:00:00 2001 From: Alliballibaba Date: Sun, 5 Jul 2026 00:18:28 +0200 Subject: [PATCH 32/39] linting --- frankenphp.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frankenphp.go b/frankenphp.go index 52a33c9c54..0ed695d836 100644 --- a/frankenphp.go +++ b/frankenphp.go @@ -671,7 +671,7 @@ func go_read_cookies(threadIndex C.uintptr_t) *C.char { // getLogger returns the logger and context savely even if phpThreads have not been created yet func getLogger(threadIndex C.uintptr_t) (*slog.Logger, context.Context) { - if threadIndex < 0 || threadIndex >= C.uintptr_t(len(phpThreads)) { + if threadIndex >= C.uintptr_t(len(phpThreads)) { return globalLogger, globalCtx } From e56326c249614fb7f3916cc02e03062cf4ff862a Mon Sep 17 00:00:00 2001 From: Alliballibaba Date: Sun, 5 Jul 2026 10:35:28 +0200 Subject: [PATCH 33/39] ensures context and logger must be defined --- context.go | 4 ++++ phpmainthread_test.go | 19 +++++++++++++++++++ 2 files changed, 23 insertions(+) diff --git a/context.go b/context.go index 40e4bc60db..861cf0082a 100644 --- a/context.go +++ b/context.go @@ -160,6 +160,10 @@ func newContextFromMessage(message any, rw http.ResponseWriter, ctx context.Cont fc.server = fallbackServer } + if fc.ctx == nil { + fc.ctx = globalCtx + } + return fc } diff --git a/phpmainthread_test.go b/phpmainthread_test.go index 0158425e31..428b02b0be 100644 --- a/phpmainthread_test.go +++ b/phpmainthread_test.go @@ -383,3 +383,22 @@ func testThreadCalculationError(t *testing.T, o *opt) { _, err := calculateMaxThreads(o) assert.Error(t, err, "configuration must error") } + +func TestContextAndLoggerMustNotBeNil(t *testing.T) { + log, ctx := getLogger(0) + assert.NotNil(t, log, "logger is defined if all threads are inactive") + assert.NotNil(t, ctx, "context is defined if all threads are inactive") + + fc := newContextFromMessage(nil, nil, nil, &worker{}) + assert.NotNil(t, fc.logger, "logger is defined for message context") + assert.NotNil(t, fc.ctx, "context is defined for message context") + + r := httptest.NewRequest("GET", "http://localhost/index.php", nil) + fc, _ = newContextFromRequest(r, nil, &server{}) + assert.NotNil(t, fc.logger, "logger is defined for request context") + assert.NotNil(t, fc.ctx, "context is defined for request context") + + fc, _ = newWorkerDummyContext(&worker{}) + assert.NotNil(t, fc.logger, "logger is defined for worker dummy context") + assert.NotNil(t, fc.ctx, "context is defined for worker dummy context") +} From 081ef2a8a0f6b88d82a7da58a1a785bc9abcf2e3 Mon Sep 17 00:00:00 2001 From: Alliballibaba Date: Sun, 5 Jul 2026 11:16:35 +0200 Subject: [PATCH 34/39] more context cleanup. --- context.go | 4 +- frankenphp.go | 45 ++++++------------ mercure.go | 17 ++++--- phpthread.go | 9 ---- server_test.go | 119 +++++++++++++++++++++++------------------------- threadworker.go | 16 +++---- 6 files changed, 88 insertions(+), 122 deletions(-) diff --git a/context.go b/context.go index 861cf0082a..d99cfed39b 100644 --- a/context.go +++ b/context.go @@ -200,8 +200,8 @@ func (fc *frankenPHPContext) validate() error { } func (fc *frankenPHPContext) clientHasClosed() bool { - if fc.ctx == nil { - return false + if fc.request == nil { + return false // not in HTTP context } select { diff --git a/frankenphp.go b/frankenphp.go index 0ed695d836..22f13780a9 100644 --- a/frankenphp.go +++ b/frankenphp.go @@ -441,27 +441,14 @@ func go_ub_write(threadIndex C.uintptr_t, cBuf *C.char, length C.size_t) (C.size writer = fc.responseWriter } - var ctx context.Context - i, e := writer.Write(unsafe.Slice((*byte)(unsafe.Pointer(cBuf)), length)) - if e != nil { - ctx = thread.context() - - if fc.logger.Enabled(ctx, slog.LevelWarn) { - fc.logger.LogAttrs(ctx, slog.LevelWarn, "write error", slog.Any("error", e)) - } + if e != nil && fc.logger.Enabled(fc.ctx, slog.LevelWarn) { + fc.logger.LogAttrs(fc.ctx, slog.LevelWarn, "write error", slog.Any("error", e)) } - if fc.responseWriter == nil { + if fc.responseWriter == nil && fc.logger.Enabled(fc.ctx, slog.LevelInfo) { // probably starting a worker script, log the output - - if ctx == nil { - ctx = thread.context() - } - - if fc.logger.Enabled(ctx, slog.LevelInfo) { - fc.logger.LogAttrs(ctx, slog.LevelInfo, writer.(*bytes.Buffer).String()) - } + fc.logger.LogAttrs(fc.ctx, slog.LevelInfo, writer.(*bytes.Buffer).String()) } return C.size_t(i), C.bool(fc.clientHasClosed()) @@ -470,14 +457,13 @@ func go_ub_write(threadIndex C.uintptr_t, cBuf *C.char, length C.size_t) (C.size //export go_apache_request_headers func go_apache_request_headers(threadIndex C.uintptr_t) (*C.go_string, C.size_t) { thread := phpThreads[threadIndex] - ctx := thread.context() fc := thread.handler.frankenPHPContext() if fc.responseWriter == nil { // worker mode, not handling a request - if globalLogger.Enabled(ctx, slog.LevelDebug) { - globalLogger.LogAttrs(ctx, slog.LevelDebug, "apache_request_headers() called in non-HTTP context", slog.String("worker", fc.worker.name)) + if fc.logger.Enabled(fc.ctx, slog.LevelDebug) { + fc.logger.LogAttrs(fc.ctx, slog.LevelDebug, "apache_request_headers() called in non-HTTP context", slog.String("worker", fc.worker.name)) } return nil, 0 @@ -506,11 +492,11 @@ func go_apache_request_headers(threadIndex C.uintptr_t) (*C.go_string, C.size_t) return sd, C.size_t(len(fc.request.Header)) } -func addHeader(ctx context.Context, fc *frankenPHPContext, h *C.sapi_header_struct) { +func addHeader(fc *frankenPHPContext, h *C.sapi_header_struct) { key, val := splitRawHeader(h.header, int(h.header_len)) if key == "" { - if fc.logger.Enabled(ctx, slog.LevelDebug) { - fc.logger.LogAttrs(ctx, slog.LevelDebug, "invalid header", slog.String("header", C.GoStringN(h.header, C.int(h.header_len)))) + if fc.logger.Enabled(fc.ctx, slog.LevelDebug) { + fc.logger.LogAttrs(fc.ctx, slog.LevelDebug, "invalid header", slog.String("header", C.GoStringN(h.header, C.int(h.header_len)))) } return @@ -570,7 +556,7 @@ func go_write_headers(threadIndex C.uintptr_t, status C.int, headers *C.zend_lli for current != nil { h := (*C.sapi_header_struct)(unsafe.Pointer(&(current.data))) - addHeader(thread.context(), fc, h) + addHeader(fc, h) current = current.next } @@ -579,10 +565,9 @@ func go_write_headers(threadIndex C.uintptr_t, status C.int, headers *C.zend_lli // go panics on invalid status code // https://github.com/golang/go/blob/9b8742f2e79438b9442afa4c0a0139d3937ea33f/src/net/http/server.go#L1162 if goStatus < 100 || goStatus > 999 { - ctx := thread.context() - if globalLogger.Enabled(ctx, slog.LevelWarn) { - globalLogger.LogAttrs(ctx, slog.LevelWarn, "Invalid response status code", slog.Int("status_code", goStatus)) + if fc.logger.Enabled(fc.ctx, slog.LevelWarn) { + fc.logger.LogAttrs(fc.ctx, slog.LevelWarn, "Invalid response status code", slog.Int("status_code", goStatus)) } goStatus = 500 @@ -621,10 +606,8 @@ func go_sapi_flush(threadIndex C.uintptr_t) bool { fc.responseController = http.NewResponseController(fc.responseWriter) } if err := fc.responseController.Flush(); err != nil { - ctx := thread.context() - - if globalLogger.Enabled(ctx, slog.LevelWarn) { - globalLogger.LogAttrs(ctx, slog.LevelWarn, "the current responseWriter is not a flusher, if you are not using a custom build, please report this issue", slog.Any("error", err)) + if fc.logger.Enabled(fc.ctx, slog.LevelWarn) { + fc.logger.LogAttrs(fc.ctx, slog.LevelWarn, "the current responseWriter is not a flusher, if you are not using a custom build, please report this issue", slog.Any("error", err)) } } diff --git a/mercure.go b/mercure.go index 9f464c147c..821b057915 100644 --- a/mercure.go +++ b/mercure.go @@ -20,12 +20,11 @@ type mercureContext struct { //export go_mercure_publish func go_mercure_publish(threadIndex C.uintptr_t, topics *C.struct__zval_struct, data *C.zend_string, private bool, id, typ *C.zend_string, retry uint64) (generatedID *C.zend_string, error C.short) { thread := phpThreads[threadIndex] - ctx := thread.context() fc := thread.handler.frankenPHPContext() if fc.mercureHub == nil { - if fc.logger.Enabled(ctx, slog.LevelError) { - fc.logger.LogAttrs(ctx, slog.LevelError, "No Mercure hub configured") + if fc.logger.Enabled(fc.ctx, slog.LevelError) { + fc.logger.LogAttrs(fc.ctx, slog.LevelError, "No Mercure hub configured") } return nil, 1 @@ -39,7 +38,7 @@ func go_mercure_publish(threadIndex C.uintptr_t, topics *C.struct__zval_struct, Type: GoString(unsafe.Pointer(typ)), }, Private: private, - Debug: fc.logger.Enabled(ctx, slog.LevelDebug), + Debug: fc.logger.Enabled(fc.ctx, slog.LevelDebug), } zvalType := C.zval_get_type(topics) @@ -49,8 +48,8 @@ func go_mercure_publish(threadIndex C.uintptr_t, topics *C.struct__zval_struct, case C.IS_ARRAY: ts, err := GoPackedArray[string](unsafe.Pointer(*(**C.zend_array)(unsafe.Pointer(&topics.value[0])))) if err != nil { - if fc.logger.Enabled(ctx, slog.LevelError) { - fc.logger.LogAttrs(ctx, slog.LevelError, "invalid topics type", slog.Any("error", err)) + if fc.logger.Enabled(fc.ctx, slog.LevelError) { + fc.logger.LogAttrs(fc.ctx, slog.LevelError, "invalid topics type", slog.Any("error", err)) } return nil, 1 @@ -62,9 +61,9 @@ func go_mercure_publish(threadIndex C.uintptr_t, topics *C.struct__zval_struct, panic("invalid topics type") } - if err := fc.mercureHub.Publish(ctx, u); err != nil { - if fc.logger.Enabled(ctx, slog.LevelError) { - fc.logger.LogAttrs(ctx, slog.LevelError, "Unable to publish Mercure update", slog.Any("error", err)) + if err := fc.mercureHub.Publish(fc.ctx, u); err != nil { + if fc.logger.Enabled(fc.ctx, slog.LevelError) { + fc.logger.LogAttrs(fc.ctx, slog.LevelError, "Unable to publish Mercure update", slog.Any("error", err)) } return nil, 2 diff --git a/phpthread.go b/phpthread.go index 2e01a7b4d1..39ee24b5d4 100644 --- a/phpthread.go +++ b/phpthread.go @@ -4,7 +4,6 @@ package frankenphp // #include "frankenphp.h" import "C" import ( - "context" "log/slog" "runtime" "sync" @@ -176,14 +175,6 @@ func (thread *phpThread) transitionToNewHandler() string { return thread.handler.beforeScriptExecution() } -func (thread *phpThread) context() context.Context { - if fc := thread.handler.frankenPHPContext(); fc != nil && fc.ctx != nil { - return fc.ctx - } - - return globalCtx -} - func (thread *phpThread) name() string { thread.handlerMu.RLock() defer thread.handlerMu.RUnlock() diff --git a/server_test.go b/server_test.go index b0f6ef4a09..46ddc8e501 100644 --- a/server_test.go +++ b/server_test.go @@ -1,7 +1,6 @@ package frankenphp_test import ( - "errors" "io" "net/http" "net/http/httptest" @@ -14,7 +13,7 @@ import ( "github.com/stretchr/testify/require" ) -func initServer(t *testing.T, opts ...frankenphp.Option) { +func initServers(t *testing.T, opts ...frankenphp.Option) { t.Helper() t.Cleanup(frankenphp.Shutdown) require.NoError(t, frankenphp.Init(opts...)) @@ -24,13 +23,7 @@ func serverRequest(t *testing.T, serverIdx int, req *http.Request) (string, *htt t.Helper() w := httptest.NewRecorder() - err := frankenphp.ServeHTTPSrv(serverIdx, w, req) - if err != nil { - var rejected frankenphp.ErrRejected - if !errors.As(err, &rejected) { - require.NoError(t, err) - } - } + require.NoError(t, frankenphp.ServeHTTPSrv(serverIdx, w, req)) resp := w.Result() body, err := io.ReadAll(resp.Body) @@ -39,55 +32,52 @@ func serverRequest(t *testing.T, serverIdx int, req *http.Request) (string, *htt return string(body), resp } -func serverGet(t *testing.T, serverIdx int, url string) (string, *http.Response) { +func serverGet(t *testing.T, serverIdx int, url string) string { t.Helper() - return serverRequest(t, serverIdx, httptest.NewRequest(http.MethodGet, url, nil)) + body, _ := serverRequest(t, serverIdx, httptest.NewRequest(http.MethodGet, url, nil)) + + return body } func TestServer(t *testing.T) { t.Run("idx", func(t *testing.T) { - initServer(t, - frankenphp.WithServer(1, testDataDir, nil, map[string]string{ - "PHP_SERVER_IDX": "1", - }), - frankenphp.WithServer(2, testDataDir, nil, map[string]string{ - "PHP_SERVER_IDX": "2", - }), + initServers( + t, + frankenphp.WithServer(1, testDataDir, nil, map[string]string{"PHP_SERVER_IDX_1": "1"}), + frankenphp.WithServer(2, testDataDir, nil, map[string]string{"PHP_SERVER_IDX_2": "2"}), ) - body1, _ := serverGet(t, 1, "http://example.com/server-variable.php") - body2, _ := serverGet(t, 2, "http://example.com/server-variable.php") + body1 := serverGet(t, 1, "http://example.com/server-variable.php") + body2 := serverGet(t, 2, "http://example.com/server-variable.php") - 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") + assert.Contains(t, body1, "[PHP_SERVER_IDX_1] => 1") + assert.Contains(t, body2, "[PHP_SERVER_IDX_2] => 2") + assert.NotContains(t, body1, "[PHP_SERVER_IDX_2]") + assert.NotContains(t, body2, "[PHP_SERVER_IDX_1]") }) t.Run("root", func(t *testing.T) { - initServer(t, frankenphp.WithServer(1, testDataDir, nil, nil)) + initServers(t, frankenphp.WithServer(1, testDataDir, nil, nil)) - body, _ := serverGet(t, 1, "http://example.com/server-globals.php") + body := serverGet(t, 1, "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) { - initServer(t, frankenphp.WithServer(1, testDataDir, nil, map[string]string{ - "PHP_SERVER_TEST_KEY": "from_php_server", - })) + initServers(t, frankenphp.WithServer(1, testDataDir, nil, map[string]string{"TEST_123": "123"})) - body, _ := serverGet(t, 1, "http://example.com/server-variable.php") + body := serverGet(t, 1, "http://example.com/server-variable.php") - assert.Contains(t, body, "[PHP_SERVER_TEST_KEY] => from_php_server") + assert.Contains(t, body, "[TEST_123] => 123") }) t.Run("split_path", func(t *testing.T) { - initServer(t, frankenphp.WithServer(1, testDataDir, []string{".custom"}, nil)) + initServers(t, frankenphp.WithServer(1, testDataDir, []string{".custom"}, nil)) - body, _ := serverGet(t, 1, "http://example.com/split-path.custom/pathinfo") + body := serverGet(t, 1, "http://example.com/split-path.custom/pathinfo") assert.Contains(t, body, "PATH_INFO: /pathinfo\n") assert.Contains(t, body, "SCRIPT_NAME: /split-path.custom\n") @@ -97,7 +87,7 @@ func TestServer(t *testing.T) { t.Run("workers_by_path_and_request_matcher", func(t *testing.T) { server1Idx := 1 server2Idx := 2 - initServer( + initServers( t, frankenphp.WithServer(server1Idx, testDataDir, nil, nil), frankenphp.WithServer(server2Idx, testDataDir, nil, nil), @@ -110,31 +100,47 @@ func TestServer(t *testing.T) { ), ) - body1, _ := serverGet(t, 1, "http://example.com/worker-with-counter.php") - body2, _ := serverGet(t, 1, "http://example.com/worker-with-counter.php") - body3, _ := serverGet(t, 2, "http://example.com/match/anything") - body4, _ := serverGet(t, 2, "http://example.com/match/anything") - body5, _ := serverGet(t, 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") + body1 := serverGet(t, 1, "http://example.com/worker-with-counter.php") + body2 := serverGet(t, 1, "http://example.com/worker-with-counter.php") + body3 := serverGet(t, 2, "http://example.com/match/anything") + body4 := serverGet(t, 2, "http://example.com/match/anything") + body5 := serverGet(t, 2, "http://example.com/index.php") + body6 := serverGet(t, 1, "http://example.com/match/anything") + + assert.Equal(t, "requests:1", body1, "could not access the worker by path on server 1") + assert.Equal(t, "requests:2", body2, "could not access the worker by path on server 1") + assert.Equal(t, "requests:1", body3, "could not access the worker by matcher on server 2") + assert.Equal(t, "requests:2", body4, "could not access the worker by matcher on server 2") + assert.Contains(t, body5, "I am by birth a Genevese (i not set)", "could not access the worker by path on server 2") + assert.Contains(t, body6, "Failed opening required", "worker is scoped to server 1, so should not be found on server 2") }) t.Run("worker_env_inheritance", func(t *testing.T) { - initServer( + initServers( t, frankenphp.WithServer(1, testDataDir, nil, map[string]string{ - "APP_ENV": "staging", + "FROM_SERVER_ENV": "original", + "FROM_WORKER_ENV": "overridden", }), - frankenphp.WithWorkers("env", testDataDir+"worker-with-env.php", 1, frankenphp.WithWorkerServerScope(1)), + frankenphp.WithWorkers( + "env", + testDataDir+"env/env.php", + 1, + frankenphp.WithWorkerServerScope(1), + frankenphp.WithWorkerEnv(map[string]string{ + "FROM_WORKER_ENV": "original", + }), + ), ) - body, _ := serverGet(t, 1, "http://example.com/worker-with-env.php") + body := serverGet(t, 1, "http://example.com/env/env.php?keys[]=FROM_SERVER_ENV&keys[]=FROM_WORKER_ENV") - assert.Equal(t, "Worker has APP_ENV=staging", body) + assert.Equal( + t, + "FROM_SERVER_ENV=original,FROM_WORKER_ENV=original", + body, + "should contain the server env and not override the worker env", + ) }) t.Run("error_on_duplicate_worker_filenames", func(t *testing.T) { @@ -159,18 +165,7 @@ func TestServer(t *testing.T) { assert.ErrorIs(t, err, frankenphp.ErrAlreadyRegistered) }) - t.Run("serve_http_validation", func(t *testing.T) { - initServer(t, frankenphp.WithServer(1, testDataDir, nil, nil)) - - req := httptest.NewRequest(http.MethodGet, "http://example.com/server-variable.php", nil) - req.Header.Add("Content-Length", "-1") - body, resp := serverRequest(t, 1, req) - - assert.Equal(t, 400, resp.StatusCode) - assert.Contains(t, body, "invalid") - }) - - t.Run("idx_not_found", func(t *testing.T) { + t.Run("error_on_missing_server", func(t *testing.T) { req := httptest.NewRequest(http.MethodGet, "http://example.com/server-variable.php", nil) w := httptest.NewRecorder() diff --git a/threadworker.go b/threadworker.go index a5b57c6624..110e90e5ec 100644 --- a/threadworker.go +++ b/threadworker.go @@ -276,13 +276,12 @@ func go_frankenphp_worker_handle_request_start(threadIndex C.uintptr_t) (C.bool, //export go_frankenphp_finish_worker_request func go_frankenphp_finish_worker_request(threadIndex C.uintptr_t, retval *C.zval) { thread := phpThreads[threadIndex] - ctx := thread.context() fc := thread.handler.frankenPHPContext() if retval != nil { r, err := GoValue[any](unsafe.Pointer(retval)) - if err != nil && globalLogger.Enabled(ctx, slog.LevelError) { - globalLogger.LogAttrs(ctx, slog.LevelError, "cannot convert return value", slog.Any("error", err), slog.Int("thread", thread.threadIndex)) + if err != nil && fc.logger.Enabled(fc.ctx, slog.LevelError) { + fc.logger.LogAttrs(fc.ctx, slog.LevelError, "cannot convert return value", slog.Any("error", err), slog.Int("thread", thread.threadIndex)) } fc.handlerReturn = r @@ -295,11 +294,11 @@ func go_frankenphp_finish_worker_request(threadIndex C.uintptr_t, retval *C.zval thread.handler.(*workerThread).workerFrankenPHPContext = nil thread.contextMu.Unlock() - if globalLogger.Enabled(ctx, slog.LevelDebug) { + if fc.logger.Enabled(fc.ctx, slog.LevelDebug) { if fc.request == nil { - fc.logger.LogAttrs(ctx, slog.LevelDebug, "request handling finished", slog.String("worker", fc.worker.name), slog.Int("thread", thread.threadIndex)) + fc.logger.LogAttrs(fc.ctx, slog.LevelDebug, "request handling finished", slog.String("worker", fc.worker.name), slog.Int("thread", thread.threadIndex)) } else { - fc.logger.LogAttrs(ctx, slog.LevelDebug, "request handling finished", slog.String("worker", fc.worker.name), slog.Int("thread", thread.threadIndex), slog.String("url", fc.request.RequestURI)) + fc.logger.LogAttrs(fc.ctx, slog.LevelDebug, "request handling finished", slog.String("worker", fc.worker.name), slog.Int("thread", thread.threadIndex), slog.String("url", fc.request.RequestURI)) } } } @@ -313,8 +312,7 @@ func go_frankenphp_finish_php_request(threadIndex C.uintptr_t) { fc.closeContext() - ctx := thread.context() - if fc.logger.Enabled(ctx, slog.LevelDebug) { - fc.logger.LogAttrs(ctx, slog.LevelDebug, "request handling finished", slog.Int("thread", thread.threadIndex), slog.String("url", fc.request.RequestURI)) + if fc.logger.Enabled(fc.ctx, slog.LevelDebug) { + fc.logger.LogAttrs(fc.ctx, slog.LevelDebug, "request handling finished", slog.Int("thread", thread.threadIndex), slog.String("url", fc.request.RequestURI)) } } From e2a1e9d32c7127612228ab173ec47056c1a54a04 Mon Sep 17 00:00:00 2001 From: Alliballibaba Date: Sun, 5 Jul 2026 11:33:47 +0200 Subject: [PATCH 35/39] makes original request logic more consistent. --- cgi.go | 11 ++--------- context.go | 31 +++++++++++++++---------------- debugstate.go | 9 ++------- requestoptions.go | 6 +++++- server.go | 2 -- 5 files changed, 24 insertions(+), 35 deletions(-) diff --git a/cgi.go b/cgi.go index d3e22708c5..c38ce51373 100644 --- a/cgi.go +++ b/cgi.go @@ -93,13 +93,6 @@ func addKnownVariablesToServer(fc *frankenPHPContext, trackVarsArray *C.zval) { serverPort := reqPort contentLength := request.Header.Get("Content-Length") - var requestURI string - if fc.originalRequest != nil { - requestURI = fc.originalRequest.URL.RequestURI() - } else { - requestURI = fc.requestURI - } - phpSelf := fc.scriptName + fc.pathInfo C.frankenphp_register_server_vars(trackVarsArray, C.frankenphp_server_vars{ @@ -136,8 +129,8 @@ func addKnownVariablesToServer(fc *frankenPHPContext, trackVarsArray *C.zval) { server_protocol_len: C.size_t(len(request.Proto)), http_host: toUnsafeChar(request.Host), http_host_len: C.size_t(len(request.Host)), - request_uri: toUnsafeChar(requestURI), - request_uri_len: C.size_t(len(requestURI)), + request_uri: toUnsafeChar(fc.requestURI), + request_uri_len: C.size_t(len(fc.requestURI)), ssl_cipher: toUnsafeChar(sslCipher), ssl_cipher_len: C.size_t(len(sslCipher)), diff --git a/context.go b/context.go index d99cfed39b..e329ed04bd 100644 --- a/context.go +++ b/context.go @@ -17,21 +17,20 @@ import ( type frankenPHPContext struct { mercureContext - ctx context.Context - documentRoot string - splitPath []string - env PreparedEnv - logger *slog.Logger - request *http.Request - originalRequest *http.Request - worker *worker + ctx context.Context + documentRoot string + splitPath []string + env PreparedEnv + logger *slog.Logger + request *http.Request + worker *worker + server *server docURI string pathInfo string scriptName string scriptFilename string requestURI string - server *server // Whether the request is already closed by us isDone bool @@ -68,11 +67,10 @@ func newContextFromRequest(request *http.Request, responseWriter http.ResponseWr startedAt: time.Now(), server: s, splitPath: s.splitPath, - logger: s.logger, + logger: globalLogger, request: request, documentRoot: s.root, responseWriter: responseWriter, - requestURI: request.URL.RequestURI(), } for _, o := range opts { @@ -81,7 +79,7 @@ func newContextFromRequest(request *http.Request, responseWriter http.ResponseWr } } - // see if a worker matches the request + // assign a worker directly if it has a request matcher if fc.worker == nil { for _, w := range s.workersWithRequestMatcher { if w.matchRequest(request) { @@ -91,10 +89,6 @@ func newContextFromRequest(request *http.Request, responseWriter http.ResponseWr } } - if fc.logger == nil { - fc.logger = globalLogger - } - if fc.documentRoot == "" { if EmbeddedAppPath != "" { fc.documentRoot = EmbeddedAppPath @@ -106,6 +100,11 @@ func newContextFromRequest(request *http.Request, responseWriter http.ResponseWr } } + if fc.requestURI == "" { + // if no WithOriginalRequest() was passed, use the URI from the actual request + fc.requestURI = fc.request.URL.RequestURI() + } + splitCgiPath(fc) return fc, nil diff --git a/debugstate.go b/debugstate.go index 62f3f9f7a0..ea9f3e364f 100644 --- a/debugstate.go +++ b/debugstate.go @@ -80,13 +80,8 @@ func threadDebugState(thread *phpThread) ThreadDebugState { return s } - if fc.originalRequest == nil { - s.CurrentURI = fc.requestURI - s.CurrentMethod = fc.request.Method - } else { - s.CurrentURI = fc.originalRequest.URL.RequestURI() - s.CurrentMethod = fc.originalRequest.Method - } + s.CurrentURI = fc.requestURI + s.CurrentMethod = fc.request.Method if !fc.startedAt.IsZero() { s.RequestStartedAt = fc.startedAt.UnixMilli() diff --git a/requestoptions.go b/requestoptions.go index 700cdd60de..e3dad1af95 100644 --- a/requestoptions.go +++ b/requestoptions.go @@ -173,7 +173,7 @@ func ensurePreparedEnv(env PreparedEnv) PreparedEnv { func WithOriginalRequest(r *http.Request) RequestOption { return func(o *frankenPHPContext) error { - o.originalRequest = r + o.requestURI = r.URL.RequestURI() return nil } @@ -184,6 +184,10 @@ func WithRequestLogger(logger *slog.Logger) RequestOption { return func(o *frankenPHPContext) error { o.logger = logger + if o.logger == nil { + o.logger = globalLogger // fall back to global logger + } + return nil } } diff --git a/server.go b/server.go index 46de90077b..aaa7c0f938 100644 --- a/server.go +++ b/server.go @@ -2,7 +2,6 @@ package frankenphp import ( "fmt" - "log/slog" "net/http" ) @@ -17,7 +16,6 @@ type server struct { workersByPath map[string]*worker workersWithRequestMatcher []*worker workerOpts []workerOpt - logger *slog.Logger } var ( From a81552d0836c49b134235aa901da274063fa3b6a Mon Sep 17 00:00:00 2001 From: Alliballibaba Date: Sun, 5 Jul 2026 12:10:07 +0200 Subject: [PATCH 36/39] pipeline test --- server_test.go | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/server_test.go b/server_test.go index 46ddc8e501..f5c98d633e 100644 --- a/server_test.go +++ b/server_test.go @@ -116,24 +116,26 @@ func TestServer(t *testing.T) { }) t.Run("worker_env_inheritance", func(t *testing.T) { + envTestDataDir := testDataDir + "env" initServers( t, - frankenphp.WithServer(1, testDataDir, nil, map[string]string{ + frankenphp.WithServer(1, envTestDataDir, nil, map[string]string{ "FROM_SERVER_ENV": "original", "FROM_WORKER_ENV": "overridden", }), frankenphp.WithWorkers( "env", - testDataDir+"env/env.php", + envTestDataDir+"/env.php", 1, frankenphp.WithWorkerServerScope(1), + frankenphp.WithWorkerMatcher(func(r *http.Request) bool { return true }), frankenphp.WithWorkerEnv(map[string]string{ "FROM_WORKER_ENV": "original", }), ), ) - body := serverGet(t, 1, "http://example.com/env/env.php?keys[]=FROM_SERVER_ENV&keys[]=FROM_WORKER_ENV") + body := serverGet(t, 1, "http://example.com/env.php?keys[]=FROM_SERVER_ENV&keys[]=FROM_WORKER_ENV") assert.Equal( t, From 17c2823ff55f41a34c5e402da36c765b35eb204a Mon Sep 17 00:00:00 2001 From: Alliballibaba Date: Sun, 5 Jul 2026 12:17:18 +0200 Subject: [PATCH 37/39] ensures $_ENV is populated for the test --- server_test.go | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/server_test.go b/server_test.go index f5c98d633e..ee4e79f897 100644 --- a/server_test.go +++ b/server_test.go @@ -116,26 +116,25 @@ func TestServer(t *testing.T) { }) t.Run("worker_env_inheritance", func(t *testing.T) { - envTestDataDir := testDataDir + "env" initServers( t, - frankenphp.WithServer(1, envTestDataDir, nil, map[string]string{ + frankenphp.WithPhpIni(map[string]string{"variables_order": "EGPCS"}), + frankenphp.WithServer(1, testDataDir, nil, map[string]string{ "FROM_SERVER_ENV": "original", "FROM_WORKER_ENV": "overridden", }), frankenphp.WithWorkers( "env", - envTestDataDir+"/env.php", + testDataDir+"env/env.php", 1, frankenphp.WithWorkerServerScope(1), - frankenphp.WithWorkerMatcher(func(r *http.Request) bool { return true }), frankenphp.WithWorkerEnv(map[string]string{ "FROM_WORKER_ENV": "original", }), ), ) - body := serverGet(t, 1, "http://example.com/env.php?keys[]=FROM_SERVER_ENV&keys[]=FROM_WORKER_ENV") + body := serverGet(t, 1, "http://example.com/env/env.php?keys[]=FROM_SERVER_ENV&keys[]=FROM_WORKER_ENV") assert.Equal( t, From 9c987ccbec24fc763a97e1e325d9cf3f9d2731d4 Mon Sep 17 00:00:00 2001 From: Alliballibaba Date: Sun, 5 Jul 2026 17:47:38 +0200 Subject: [PATCH 38/39] refactors server public api and fixes restart configuration --- caddy/admin_test.go | 4 +- caddy/app.go | 47 +++++++++++++++++------ caddy/hotreload.go | 1 - caddy/module.go | 34 ++++++----------- frankenphp.go | 12 ------ options.go | 43 +++++++++++---------- server.go | 16 ++++++++ server_test.go | 92 +++++++++++++++++++++++---------------------- worker.go | 6 +-- 9 files changed, 137 insertions(+), 118 deletions(-) diff --git a/caddy/admin_test.go b/caddy/admin_test.go index d2946bb179..f86c3cfce9 100644 --- a/caddy/admin_test.go +++ b/caddy/admin_test.go @@ -40,9 +40,6 @@ func TestRestartWorkerViaAdminApi(t *testing.T) { } `, "caddyfile") - // make sure workers are not still running from any previous tests - assertAdminResponse(t, tester, "POST", "workers/restart", http.StatusOK, "workers restarted successfully\n") - tester.AssertGetResponse("http://localhost:"+testPort+"/", http.StatusOK, "requests:1") tester.AssertGetResponse("http://localhost:"+testPort+"/", http.StatusOK, "requests:2") @@ -79,6 +76,7 @@ func TestShowTheCorrectThreadDebugStatus(t *testing.T) { debugState := getDebugState(t, tester) // assert that the correct threads are present in the thread info + assert.Len(t, debugState.ThreadDebugStates, 3) assert.Equal(t, debugState.ThreadDebugStates[0].State, "ready") assert.Contains(t, debugState.ThreadDebugStates[1].Name, "worker-with-counter.php") assert.Contains(t, debugState.ThreadDebugStates[2].Name, "index.php") diff --git a/caddy/app.go b/caddy/app.go index f4d8861a2a..5c2ed973e7 100644 --- a/caddy/app.go +++ b/caddy/app.go @@ -59,11 +59,11 @@ 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 - moduleWorkers map[int][]workerConfig + opts []frankenphp.Option + metrics frankenphp.Metrics + ctx context.Context + logger *slog.Logger + modules map[int]*FrankenPHPModule } var errIni = errors.New(`"php_ini" must be in the format: php_ini "" ""`) @@ -127,10 +127,13 @@ func (f *FrankenPHPApp) Start() error { } // register module workers - for serverIdx, workers := range f.moduleWorkers { - for _, w := range workers { + for _, module := range f.modules { + for _, w := range module.Workers { w.FileName = repl.ReplaceKnown(w.FileName, "") - workerOptions := append(w.toWorkerOptions(), frankenphp.WithWorkerServerScope(serverIdx)) + if module.server == nil { + return fmt.Errorf("module %d has no server", module.ServerIdx) + } + workerOptions := append(w.toWorkerOptions(), frankenphp.WithWorkerServerScope(module.server)) f.opts = append(f.opts, frankenphp.WithWorkers(w.Name, w.FileName, w.Num, workerOptions...)) } } @@ -140,6 +143,10 @@ func (f *FrankenPHPApp) Start() error { return err } + // after startup, reset all configuration for future reloads or tests + // it is necessary to do this here since caddy will re-use the app instance + f.reset() + return nil } @@ -155,19 +162,35 @@ func (f *FrankenPHPApp) Stop() error { frankenphp.Shutdown() } - // reset the configuration so it doesn't bleed into later tests + return nil +} + +func (f *FrankenPHPApp) reset() { f.Workers = nil f.NumThreads = 0 f.MaxWaitTime = 0 f.MaxIdleTime = 0 f.MaxRequests = 0 - f.moduleWorkers = nil - + f.PhpIni = nil + f.modules = nil + f.opts = nil + f.ctx = nil + f.metrics = nil optionsMU.Lock() options = nil optionsMU.Unlock() +} - return nil +func (f *FrankenPHPApp) registerModule(m *FrankenPHPModule, serverOpt frankenphp.Option) { + if f.modules == nil { + f.modules = make(map[int]*FrankenPHPModule) + } + + if _, ok := f.modules[m.ServerIdx]; !ok { + // only register the module if it's not already registered + f.modules[m.ServerIdx] = m + f.opts = append(f.opts, serverOpt) + } } // UnmarshalCaddyfile implements caddyfile.Unmarshaler. diff --git a/caddy/hotreload.go b/caddy/hotreload.go index bcab369372..bec8b16fbd 100644 --- a/caddy/hotreload.go +++ b/caddy/hotreload.go @@ -26,7 +26,6 @@ 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 diff --git a/caddy/module.go b/caddy/module.go index a84eaa5a91..3866e53c85 100644 --- a/caddy/module.go +++ b/caddy/module.go @@ -50,6 +50,7 @@ type FrankenPHPModule struct { resolvedDocumentRoot string preparedEnv frankenphp.PreparedEnv requestOptions []frankenphp.RequestOption + server *frankenphp.Server } // CaddyModule returns the Caddy module information. @@ -74,6 +75,12 @@ func (f *FrankenPHPModule) Provision(ctx caddy.Context) error { return fmt.Errorf(`expected ctx.App("frankenphp") to return *FrankenPHPApp, got nil`) } + if f.ServerIdx <= 0 { + // when registering via JSON configuration, it's possible that no idx was yet assigned to this module + f.ServerIdx = len(fapp.modules) + 1 + caddy.Log().Warn("\"php\" is missing a \"server_idx\", assigning one on-the-fly") + } + f.assignMercureHub(ctx) for i, wc := range f.Workers { @@ -160,29 +167,10 @@ func (f *FrankenPHPModule) Provision(ctx caddy.Context) error { f.preparedEnv = frankenphp.PrepareEnv(requestEnv) - if fapp.moduleWorkers == nil { - fapp.moduleWorkers = make(map[int][]workerConfig) - } - - if f.ServerIdx <= 0 { - // when registering via JSON configuration, it's possible that no idx was yet assigned - f.ServerIdx = len(fapp.moduleWorkers) + 1 - caddy.Log().Warn("\"php\" is missing a \"server_idx\", assigning one on-the-fly") - } - - if _, ok := fapp.moduleWorkers[f.ServerIdx]; ok { - // server was already registered, avoid duplicate registrations - return nil - } - - fapp.moduleWorkers[f.ServerIdx] = f.Workers + server, serverOpt := frankenphp.WithServer(f.ServerIdx, f.resolvedDocumentRoot, f.SplitPath, resolvedEnv) + f.server = server - fapp.opts = append(fapp.opts, frankenphp.WithServer( - f.ServerIdx, - f.resolvedDocumentRoot, - f.SplitPath, - resolvedEnv, - )) + fapp.registerModule(f, serverOpt) return nil } @@ -242,7 +230,7 @@ func (f *FrankenPHPModule) ServeHTTP(w http.ResponseWriter, r *http.Request, _ c opts = append(opts, frankenphp.WithRequestPreparedEnv(env)) } - err := frankenphp.ServeHTTPSrv(f.ServerIdx, w, r, opts...) + err := f.server.ServeHTTP(w, r, opts...) if err != nil && !errors.As(err, &frankenphp.ErrRejected{}) { return caddyhttp.Error(http.StatusInternalServerError, err) diff --git a/frankenphp.go b/frankenphp.go index 22f13780a9..ad3193f119 100644 --- a/frankenphp.go +++ b/frankenphp.go @@ -411,18 +411,6 @@ func ServeHTTP(responseWriter http.ResponseWriter, request *http.Request) error return fallbackServer.serveHTTP(responseWriter, request, opts...) } -// ServeHTTPSrv executes a PHP script with a registered server. -// this allows using a pre-configured server instance. -// otherwise, it is equivalent to calling ServeHTTP. -func ServeHTTPSrv(serverIdx int, responseWriter http.ResponseWriter, request *http.Request, opts ...RequestOption) error { - s, ok := servers[serverIdx] - if !ok { - return fmt.Errorf("%w: no server with idx %d was registered (%d servers registered overall)", ErrServerNotFound, serverIdx, len(servers)) - } - - return s.serveHTTP(responseWriter, request, opts...) -} - //export go_ub_write func go_ub_write(threadIndex C.uintptr_t, cBuf *C.char, length C.size_t) (C.size_t, C.bool) { thread := phpThreads[threadIndex] diff --git a/options.go b/options.go index c9560b9bc7..1a869e0dae 100644 --- a/options.go +++ b/options.go @@ -54,7 +54,7 @@ type workerOpt struct { onThreadShutdown func(int) onServerStartup func() onServerShutdown func() - serverIdx int + server *Server } // WithContext sets the main context to use. @@ -229,9 +229,10 @@ func WithWorkerMatcher(matcherFunc func(*http.Request) bool) WorkerOption { // WithWorkerServerScope scopes the worker to a server instance. // Only requests that are handled by the server instance will reach the worker. -func WithWorkerServerScope(serverIdx int) WorkerOption { +func WithWorkerServerScope(server *Server) WorkerOption { return func(w *workerOpt) error { - w.serverIdx = serverIdx + w.server = server + return nil } } @@ -296,26 +297,28 @@ func WithServer( resolvedDocumentRoot string, splitPath []string, env map[string]string, -) Option { - return func(o *opt) error { - if idx <= 0 { - return fmt.Errorf("server idx must be > 0, got %d", idx) - } +) (*Server, Option) { + return &Server{ + idx: idx, + }, func(o *opt) error { + if idx <= 0 { + return fmt.Errorf("server idx must be > 0, got %d", idx) + } - root, err := fastabs.FastAbs(resolvedDocumentRoot) - if err != nil { - return err - } + root, err := fastabs.FastAbs(resolvedDocumentRoot) + if err != nil { + return err + } - if err := normalizeSplitPath(splitPath); err != nil { - return err - } + if err := normalizeSplitPath(splitPath); err != nil { + return err + } - _, err = newServer(idx, root, splitPath, PrepareEnv(env)) + _, err = newServer(idx, root, splitPath, PrepareEnv(env)) - if err != nil { - return err + if err != nil { + return err + } + return nil } - return nil - } } diff --git a/server.go b/server.go index aaa7c0f938..62db67bd71 100644 --- a/server.go +++ b/server.go @@ -18,6 +18,10 @@ type server struct { workerOpts []workerOpt } +type Server struct { + idx int +} + var ( servers = make(map[int]*server) fallbackServer = &server{ @@ -73,6 +77,18 @@ func (s *server) addWorker(w *worker) error { return nil } +// ServeHTTPSrv executes a PHP script with a registered server. +// this allows using a pre-configured server instance. +// otherwise, it is equivalent to calling ServeHTTP. +func (se *Server) ServeHTTP(responseWriter http.ResponseWriter, request *http.Request, opts ...RequestOption) error { + s, ok := servers[se.idx] + if !ok { + return fmt.Errorf("%w: no server with idx %d was registered (%d servers registered overall)", ErrServerNotFound, se.idx, len(servers)) + } + + return s.serveHTTP(responseWriter, request, opts...) +} + func (s *server) serveHTTP(responseWriter http.ResponseWriter, request *http.Request, opts ...RequestOption) error { h := responseWriter.Header() if h["Server"] == nil { diff --git a/server_test.go b/server_test.go index ee4e79f897..1466ca29ce 100644 --- a/server_test.go +++ b/server_test.go @@ -19,11 +19,11 @@ func initServers(t *testing.T, opts ...frankenphp.Option) { require.NoError(t, frankenphp.Init(opts...)) } -func serverRequest(t *testing.T, serverIdx int, req *http.Request) (string, *http.Response) { +func serverRequest(t *testing.T, server *frankenphp.Server, req *http.Request) (string, *http.Response) { t.Helper() w := httptest.NewRecorder() - require.NoError(t, frankenphp.ServeHTTPSrv(serverIdx, w, req)) + require.NoError(t, server.ServeHTTP(w, req)) resp := w.Result() body, err := io.ReadAll(resp.Body) @@ -32,24 +32,23 @@ func serverRequest(t *testing.T, serverIdx int, req *http.Request) (string, *htt return string(body), resp } -func serverGet(t *testing.T, serverIdx int, url string) string { +func serverGet(t *testing.T, server *frankenphp.Server, url string) string { t.Helper() - body, _ := serverRequest(t, serverIdx, httptest.NewRequest(http.MethodGet, url, nil)) + body, _ := serverRequest(t, server, httptest.NewRequest(http.MethodGet, url, nil)) return body } func TestServer(t *testing.T) { t.Run("idx", func(t *testing.T) { - initServers( - t, - frankenphp.WithServer(1, testDataDir, nil, map[string]string{"PHP_SERVER_IDX_1": "1"}), - frankenphp.WithServer(2, testDataDir, nil, map[string]string{"PHP_SERVER_IDX_2": "2"}), - ) + server1, opt1 := frankenphp.WithServer(1, testDataDir, nil, map[string]string{"PHP_SERVER_IDX_1": "1"}) + server2, opt2 := frankenphp.WithServer(2, testDataDir, nil, map[string]string{"PHP_SERVER_IDX_2": "2"}) - body1 := serverGet(t, 1, "http://example.com/server-variable.php") - body2 := serverGet(t, 2, "http://example.com/server-variable.php") + initServers(t, opt1, opt2) + + body1 := serverGet(t, server1, "http://example.com/server-variable.php") + body2 := serverGet(t, server2, "http://example.com/server-variable.php") assert.Contains(t, body1, "[PHP_SERVER_IDX_1] => 1") assert.Contains(t, body2, "[PHP_SERVER_IDX_2] => 2") @@ -58,26 +57,29 @@ func TestServer(t *testing.T) { }) t.Run("root", func(t *testing.T) { - initServers(t, frankenphp.WithServer(1, testDataDir, nil, nil)) + server, opt := frankenphp.WithServer(1, testDataDir, nil, nil) + initServers(t, opt) - body := serverGet(t, 1, "http://example.com/server-globals.php") + body := serverGet(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) { - initServers(t, frankenphp.WithServer(1, testDataDir, nil, map[string]string{"TEST_123": "123"})) + server, opt := frankenphp.WithServer(1, testDataDir, nil, map[string]string{"TEST_123": "123"}) + initServers(t, opt) - body := serverGet(t, 1, "http://example.com/server-variable.php") + body := serverGet(t, server, "http://example.com/server-variable.php") assert.Contains(t, body, "[TEST_123] => 123") }) t.Run("split_path", func(t *testing.T) { - initServers(t, frankenphp.WithServer(1, testDataDir, []string{".custom"}, nil)) + server, opt := frankenphp.WithServer(1, testDataDir, []string{".custom"}, nil) + initServers(t, opt) - body := serverGet(t, 1, "http://example.com/split-path.custom/pathinfo") + body := serverGet(t, server, "http://example.com/split-path.custom/pathinfo") assert.Contains(t, body, "PATH_INFO: /pathinfo\n") assert.Contains(t, body, "SCRIPT_NAME: /split-path.custom\n") @@ -85,27 +87,27 @@ func TestServer(t *testing.T) { }) t.Run("workers_by_path_and_request_matcher", func(t *testing.T) { - server1Idx := 1 - server2Idx := 2 + server1, opt1 := frankenphp.WithServer(1, testDataDir, nil, nil) + server2, opt2 := frankenphp.WithServer(2, testDataDir, nil, nil) initServers( t, - frankenphp.WithServer(server1Idx, testDataDir, nil, nil), - frankenphp.WithServer(server2Idx, testDataDir, nil, nil), - frankenphp.WithWorkers("counter", testDataDir+"worker-with-counter.php", 1, frankenphp.WithWorkerServerScope(server1Idx)), + opt1, + opt2, + frankenphp.WithWorkers("counter", testDataDir+"worker-with-counter.php", 1, frankenphp.WithWorkerServerScope(server1)), frankenphp.WithWorkers("match", testDataDir+"worker-with-counter.php", 1, - frankenphp.WithWorkerServerScope(server2Idx), + frankenphp.WithWorkerServerScope(server2), frankenphp.WithWorkerMatcher(func(r *http.Request) bool { return strings.HasPrefix(r.URL.Path, "/match/") }), ), ) - body1 := serverGet(t, 1, "http://example.com/worker-with-counter.php") - body2 := serverGet(t, 1, "http://example.com/worker-with-counter.php") - body3 := serverGet(t, 2, "http://example.com/match/anything") - body4 := serverGet(t, 2, "http://example.com/match/anything") - body5 := serverGet(t, 2, "http://example.com/index.php") - body6 := serverGet(t, 1, "http://example.com/match/anything") + body1 := serverGet(t, server1, "http://example.com/worker-with-counter.php") + body2 := serverGet(t, server1, "http://example.com/worker-with-counter.php") + body3 := serverGet(t, server2, "http://example.com/match/anything") + body4 := serverGet(t, server2, "http://example.com/match/anything") + body5 := serverGet(t, server2, "http://example.com/index.php") + body6 := serverGet(t, server1, "http://example.com/match/anything") assert.Equal(t, "requests:1", body1, "could not access the worker by path on server 1") assert.Equal(t, "requests:2", body2, "could not access the worker by path on server 1") @@ -116,25 +118,26 @@ func TestServer(t *testing.T) { }) t.Run("worker_env_inheritance", func(t *testing.T) { + server, opt := frankenphp.WithServer(1, testDataDir, nil, map[string]string{ + "FROM_SERVER_ENV": "original", + "FROM_WORKER_ENV": "overridden", + }) initServers( t, frankenphp.WithPhpIni(map[string]string{"variables_order": "EGPCS"}), - frankenphp.WithServer(1, testDataDir, nil, map[string]string{ - "FROM_SERVER_ENV": "original", - "FROM_WORKER_ENV": "overridden", - }), + opt, frankenphp.WithWorkers( "env", testDataDir+"env/env.php", 1, - frankenphp.WithWorkerServerScope(1), + frankenphp.WithWorkerServerScope(server), frankenphp.WithWorkerEnv(map[string]string{ "FROM_WORKER_ENV": "original", }), ), ) - body := serverGet(t, 1, "http://example.com/env/env.php?keys[]=FROM_SERVER_ENV&keys[]=FROM_WORKER_ENV") + body := serverGet(t, server, "http://example.com/env/env.php?keys[]=FROM_SERVER_ENV&keys[]=FROM_WORKER_ENV") assert.Equal( t, @@ -147,10 +150,11 @@ func TestServer(t *testing.T) { t.Run("error_on_duplicate_worker_filenames", func(t *testing.T) { t.Cleanup(frankenphp.Shutdown) + server, opt := frankenphp.WithServer(1, testDataDir, nil, nil) err := frankenphp.Init( - frankenphp.WithServer(1, testDataDir, nil, nil), - frankenphp.WithWorkers("worker1", testDataDir+"worker-with-counter.php", 1, frankenphp.WithWorkerServerScope(1)), - frankenphp.WithWorkers("worker2", testDataDir+"worker-with-counter.php", 1, frankenphp.WithWorkerServerScope(1)), + opt, + frankenphp.WithWorkers("worker1", testDataDir+"worker-with-counter.php", 1, frankenphp.WithWorkerServerScope(server)), + frankenphp.WithWorkers("worker2", testDataDir+"worker-with-counter.php", 1, frankenphp.WithWorkerServerScope(server)), ) assert.Error(t, err) @@ -158,19 +162,19 @@ func TestServer(t *testing.T) { }) t.Run("error_on_duplicate_registration", func(t *testing.T) { - err := frankenphp.Init( - frankenphp.WithServer(1, testDataDir, nil, nil), - frankenphp.WithServer(1, testDataDir, nil, nil), - ) + _, opt1 := frankenphp.WithServer(1, testDataDir, nil, nil) + _, opt2 := frankenphp.WithServer(1, testDataDir, nil, nil) + err := frankenphp.Init(opt1, opt2) assert.ErrorIs(t, err, frankenphp.ErrAlreadyRegistered) }) - t.Run("error_on_missing_server", func(t *testing.T) { + t.Run("error_on_missing_registration", func(t *testing.T) { + server, _ := frankenphp.WithServer(1, testDataDir, nil, nil) req := httptest.NewRequest(http.MethodGet, "http://example.com/server-variable.php", nil) w := httptest.NewRecorder() - err := frankenphp.ServeHTTPSrv(1, w, req) + err := server.ServeHTTP(w, req) assert.ErrorIs(t, err, frankenphp.ErrServerNotFound) }) diff --git a/worker.go b/worker.go index b136a72fed..2cd820b26a 100644 --- a/worker.go +++ b/worker.go @@ -124,11 +124,11 @@ func newWorker(o workerOpt) (*worker, error) { } var server *server - if o.serverIdx != 0 { - server = servers[o.serverIdx] + if o.server != nil { + server = servers[o.server.idx] if server == nil { - return nil, fmt.Errorf("worker was registered with a non-existent server idx %d: %q", o.serverIdx, absFileName) + return nil, fmt.Errorf("worker was registered with a non-existent server idx %d: %q", o.server.idx, absFileName) } } else if w := globalWorkersByPath[absFileName]; w != nil { return w, fmt.Errorf("two global workers cannot have the same filename: %q", absFileName) From 00405604954d63ee72747b74f0a9ffa56f296d0f Mon Sep 17 00:00:00 2001 From: Alliballibaba Date: Sun, 5 Jul 2026 20:26:35 +0200 Subject: [PATCH 39/39] final cleanup. --- caddy/admin_test.go | 3 +-- caddy/app.go | 15 ++++++++++----- caddy/caddy_test.go | 10 +++++----- caddy/module.go | 46 ++++++++++++++++++++------------------------- context.go | 4 +++- server.go | 12 ++++++------ 6 files changed, 45 insertions(+), 45 deletions(-) diff --git a/caddy/admin_test.go b/caddy/admin_test.go index f86c3cfce9..4353a556f7 100644 --- a/caddy/admin_test.go +++ b/caddy/admin_test.go @@ -76,7 +76,6 @@ func TestShowTheCorrectThreadDebugStatus(t *testing.T) { debugState := getDebugState(t, tester) // assert that the correct threads are present in the thread info - assert.Len(t, debugState.ThreadDebugStates, 3) assert.Equal(t, debugState.ThreadDebugStates[0].State, "ready") assert.Contains(t, debugState.ThreadDebugStates[1].Name, "worker-with-counter.php") assert.Contains(t, debugState.ThreadDebugStates[2].Name, "index.php") @@ -323,7 +322,7 @@ func TestAddModuleWorkerViaAdminApi(t *testing.T) { assert.NoError(t, err) r.Header.Set("Content-Type", "text/caddyfile") resp := tester.AssertResponseCode(r, http.StatusOK) - require.NoError(t, resp.Body.Close()) + defer func() { require.NoError(t, resp.Body.Close()) }() // Get the updated debug state to check if the worker was added updatedDebugState := getDebugState(t, tester) diff --git a/caddy/app.go b/caddy/app.go index 5c2ed973e7..371e329e27 100644 --- a/caddy/app.go +++ b/caddy/app.go @@ -126,7 +126,7 @@ func (f *FrankenPHPApp) Start() error { f.opts = append(f.opts, frankenphp.WithWorkers(w.Name, w.FileName, w.Num, w.toWorkerOptions()...)) } - // register module workers + // register module workers that are scoped to a php_server or php block for _, module := range f.modules { for _, w := range module.Workers { w.FileName = repl.ReplaceKnown(w.FileName, "") @@ -143,8 +143,8 @@ func (f *FrankenPHPApp) Start() error { return err } - // after startup, reset all configuration for future reloads or tests - // it is necessary to do this here since caddy will re-use the app instance + // after startup, reset all configuration on the app instance + // this must happen here since the instance is re-used across reloads and tests f.reset() return nil @@ -181,13 +181,18 @@ func (f *FrankenPHPApp) reset() { optionsMU.Unlock() } -func (f *FrankenPHPApp) registerModule(m *FrankenPHPModule, serverOpt frankenphp.Option) { +// register the php_server or php block to the app instance +func (f *FrankenPHPApp) registerServer(m *FrankenPHPModule) { if f.modules == nil { f.modules = make(map[int]*FrankenPHPModule) } + server, serverOpt := frankenphp.WithServer(m.ServerIdx, m.resolvedDocumentRoot, m.SplitPath, m.resolvedEnv) + m.server = server + + // only register the server module if it's not already registered to avoid duplicate modules + // this can happen if multiple "php" directives are used in the same caddy route block (like with worker matches) if _, ok := f.modules[m.ServerIdx]; !ok { - // only register the module if it's not already registered f.modules[m.ServerIdx] = m f.opts = append(f.opts, serverOpt) } diff --git a/caddy/caddy_test.go b/caddy/caddy_test.go index 5467f18186..86987239c7 100644 --- a/caddy/caddy_test.go +++ b/caddy/caddy_test.go @@ -442,7 +442,7 @@ func TestCustomCaddyVariablesInEnv(t *testing.T) { tester.AssertGetResponse("http://localhost:"+testPort+"/worker-env.php", http.StatusOK, "hello world") } -func TestServerDirective(t *testing.T) { +func TestPHPServerDirective(t *testing.T) { tester := caddytest.NewTester(t) initServer(t, tester, ` { @@ -463,7 +463,7 @@ func TestServerDirective(t *testing.T) { tester.AssertGetResponse("http://localhost:"+testPort+"/not-found.txt", http.StatusOK, "I am by birth a Genevese (i not set)") } -func TestServerDirectiveDisableFileServer(t *testing.T) { +func TestPHPServerDirectiveDisableFileServer(t *testing.T) { tester := caddytest.NewTester(t) initServer(t, tester, ` { @@ -487,7 +487,7 @@ func TestServerDirectiveDisableFileServer(t *testing.T) { tester.AssertGetResponse("http://localhost:"+testPort+"/not-found.txt", http.StatusOK, "I am by birth a Genevese (i not set)") } -func TestServerGlobals(t *testing.T) { +func TestPHPServerGlobals(t *testing.T) { documentRoot, _ := filepath.Abs("../testdata") scriptFilename := filepath.Join(documentRoot, "server-globals.php") @@ -539,7 +539,7 @@ REQUEST_URI: /server-globals.php/en ) } -func TestWorkerServerGlobals(t *testing.T) { +func TestWorkerPHPServerGlobals(t *testing.T) { documentRoot, _ := filepath.Abs("../testdata") documentRoot2, _ := filepath.Abs("../caddy") scriptFilename := documentRoot + string(filepath.Separator) + "server-globals.php" @@ -859,7 +859,7 @@ func TestWorkerMetrics(t *testing.T) { } // #2477: verify one pool per worker, no "_0" duplicates. -func TestServerWorkerMatchPoolCount(t *testing.T) { +func TestPhpServerWorkerMatchPoolCount(t *testing.T) { tester := caddytest.NewTester(t) initServer(t, tester, ` { diff --git a/caddy/module.go b/caddy/module.go index 3866e53c85..f4f3647a19 100644 --- a/caddy/module.go +++ b/caddy/module.go @@ -24,6 +24,7 @@ import ( // FrankenPHPModule represents the "php_server" and "php" directives in the Caddyfile // they are responsible for forwarding requests to FrankenPHP via "ServeHTTP" +// keep in mind that the module reference gets lost between Caddy parsing and Caddy provisioning // // example.com { // php_server { @@ -48,7 +49,8 @@ type FrankenPHPModule struct { ServerIdx int `json:"server_idx,omitempty"` resolvedDocumentRoot string - preparedEnv frankenphp.PreparedEnv + resolvedEnv map[string]string + requestEnv frankenphp.PreparedEnv requestOptions []frankenphp.RequestOption server *frankenphp.Server } @@ -75,10 +77,11 @@ func (f *FrankenPHPModule) Provision(ctx caddy.Context) error { return fmt.Errorf(`expected ctx.App("frankenphp") to return *FrankenPHPApp, got nil`) } - if f.ServerIdx <= 0 { - // when registering via JSON configuration, it's possible that no idx was yet assigned to this module + if f.ServerIdx == 0 { + // if no idx was yet assigned to this module, assign one on-the-fly + // this is possible in JSON configuration when using "php" directives in route blocks f.ServerIdx = len(fapp.modules) + 1 - caddy.Log().Warn("\"php\" is missing a \"server_idx\", assigning one on-the-fly") + caddy.Log().Warn("\"php\" in route block is missing a \"server_idx\", assigning one on-the-fly") } f.assignMercureHub(ctx) @@ -90,14 +93,11 @@ func (f *FrankenPHPModule) Provision(ctx caddy.Context) error { wc.FileName = filepath.Join(f.Root, wc.FileName) } - f.Workers[i] = wc - } - - for i, wc := range f.Workers { if frankenphp.EmbeddedAppPath != "" && filepath.IsLocal(wc.FileName) { wc.FileName = filepath.Join(frankenphp.EmbeddedAppPath, wc.FileName) } + // module worker names are prefixed with m#, to distinguish them from global workers serverPrefix := fmt.Sprintf("m%d#", f.ServerIdx) if wc.Name == "" { wc.Name = f.generateUniqueModuleWorkerName(wc.FileName, serverPrefix) @@ -147,30 +147,24 @@ func (f *FrankenPHPModule) Provision(ctx caddy.Context) error { } } } - } if err := f.configureHotReload(fapp); err != nil { return err } - resolvedEnv := 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} + f.resolvedEnv = make(map[string]string) // env variables that do not need replacement + f.requestEnv = make(map[string]string) // env variables that need replacement, e.g. {http.vars.root} for k, e := range f.Env { if needReplacement(e) { - requestEnv[k] = e + f.requestEnv[k+"\x00"] = e // prepare the env with a null byte } else { - resolvedEnv[k] = e + f.resolvedEnv[k] = e } } - f.preparedEnv = frankenphp.PrepareEnv(requestEnv) - - server, serverOpt := frankenphp.WithServer(f.ServerIdx, f.resolvedDocumentRoot, f.SplitPath, resolvedEnv) - f.server = server - - fapp.registerModule(f, serverOpt) + fapp.registerServer(f) return nil } @@ -203,14 +197,13 @@ func (f *FrankenPHPModule) ServeHTTP(w http.ResponseWriter, r *http.Request, _ c ctx := r.Context() repl := ctx.Value(caddy.ReplacerCtxKey).(*caddy.Replacer) - documentRoot := f.resolvedDocumentRoot - 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, "") + // if the root contains a caddy placeholder, it needs to be resolved here in the hot path + if f.resolvedDocumentRoot == "" { + documentRoot := repl.ReplaceKnown(f.Root, "") if documentRoot == "" && frankenphp.EmbeddedAppPath != "" { documentRoot = frankenphp.EmbeddedAppPath } @@ -221,9 +214,10 @@ func (f *FrankenPHPModule) ServeHTTP(w http.ResponseWriter, r *http.Request, _ c opts = append(opts, frankenphp.WithRequestDocumentRoot(documentRoot, false)) } - if len(f.preparedEnv) > 0 { - env := make(frankenphp.PreparedEnv, len(f.preparedEnv)) - for k, v := range f.preparedEnv { + // all env variables that contain caddy placeholders need to be resolved here in the hot path + if len(f.requestEnv) > 0 { + env := make(frankenphp.PreparedEnv, len(f.requestEnv)) + for k, v := range f.requestEnv { env[k] = repl.ReplaceKnown(v, "") } diff --git a/context.go b/context.go index e329ed04bd..c4ef37d9b0 100644 --- a/context.go +++ b/context.go @@ -100,8 +100,10 @@ func newContextFromRequest(request *http.Request, responseWriter http.ResponseWr } } + // if no originalRequest was passed, use the URI from the actual request + // when using Caddy's http module, the original unchanged uri will be used here + // request.URL is often already rewritten to match a PHP script path if fc.requestURI == "" { - // if no WithOriginalRequest() was passed, use the URI from the actual request fc.requestURI = fc.request.URL.RequestURI() } diff --git a/server.go b/server.go index 62db67bd71..05352f5897 100644 --- a/server.go +++ b/server.go @@ -77,13 +77,13 @@ func (s *server) addWorker(w *worker) error { return nil } -// ServeHTTPSrv executes a PHP script with a registered server. -// this allows using a pre-configured server instance. -// otherwise, it is equivalent to calling ServeHTTP. -func (se *Server) ServeHTTP(responseWriter http.ResponseWriter, request *http.Request, opts ...RequestOption) error { - s, ok := servers[se.idx] +// ServeHTTP executes a PHP script on the registered server. +// The request will be scoped to the server instance that was registered via WithServer(). +// Otherwise, it is equivalent to calling ServeHTTP. +func (publicServer *Server) ServeHTTP(responseWriter http.ResponseWriter, request *http.Request, opts ...RequestOption) error { + s, ok := servers[publicServer.idx] if !ok { - return fmt.Errorf("%w: no server with idx %d was registered (%d servers registered overall)", ErrServerNotFound, se.idx, len(servers)) + return fmt.Errorf("%w: server with idx %d was not initialized (%d servers initialized overall)", ErrServerNotFound, publicServer.idx, len(servers)) } return s.serveHTTP(responseWriter, request, opts...)