fix: race condition introduced in 04fdc0c (#2180)

Fix issue introduced in 04fdc0c

Author: dunglas

caddy/module.go

@@ -118,7 +118,12 @@ func (f *FrankenPHPModule) Provision(ctx caddy.Context) error {
 	if len(f.SplitPath) == 0 {
 		f.SplitPath = []string{".php"}
 	}
-	f.requestOptions = append(f.requestOptions, frankenphp.WithRequestSplitPath(f.SplitPath))
+
+	if opt, err := frankenphp.WithRequestSplitPath(f.SplitPath); err == nil {
+		f.requestOptions = append(f.requestOptions, opt)
+	} else {
+		f.requestOptions = append(f.requestOptions, opt)
+	}
 
 	if f.ResolveRootSymlink == nil {
 		rrs := true
@@ -188,6 +193,10 @@ func (f *FrankenPHPModule) ServeHTTP(w http.ResponseWriter, r *http.Request, _ c
 	repl := ctx.Value(caddy.ReplacerCtxKey).(*caddy.Replacer)
 
 	documentRoot := f.resolvedDocumentRoot
+
+	opts := make([]frankenphp.RequestOption, 0, len(f.requestOptions)+4)
+	opts = append(opts, f.requestOptions...)
+
 	if documentRoot == "" {
 		documentRoot = repl.ReplaceKnown(f.Root, "")
 		if documentRoot == "" && frankenphp.EmbeddedAppPath != "" {
@@ -197,7 +206,7 @@ func (f *FrankenPHPModule) ServeHTTP(w http.ResponseWriter, r *http.Request, _ c
 		// If we do not have a resolved document root, then we cannot resolve the symlink of our cwd because it may
 		// resolve to a different directory than the one we are currently in.
 		// This is especially important if there are workers running.
-		f.requestOptions = append(f.requestOptions, frankenphp.WithRequestDocumentRoot(documentRoot, false))
+		opts = append(opts, frankenphp.WithRequestDocumentRoot(documentRoot, false))
 	}
 
 	if f.preparedEnvNeedsReplacement {
@@ -206,7 +215,7 @@ func (f *FrankenPHPModule) ServeHTTP(w http.ResponseWriter, r *http.Request, _ c
 			env[k] = repl.ReplaceKnown(v, "")
 		}
 
-		f.requestOptions = append(f.requestOptions, frankenphp.WithRequestPreparedEnv(env))
+		opts = append(opts, frankenphp.WithRequestPreparedEnv(env))
 	}
 
 	workerName := ""
@@ -220,7 +229,7 @@ func (f *FrankenPHPModule) ServeHTTP(w http.ResponseWriter, r *http.Request, _ c
 	fr, err := frankenphp.NewRequestWithContext(
 		r,
 		append(
-			f.requestOptions,
+			opts,
 			frankenphp.WithOriginalRequest(&origReq),
 			frankenphp.WithWorkerName(workerName),
 		)...,

go.mod

@@ -12,6 +12,7 @@ require (
 	github.com/prometheus/client_golang v1.23.2
 	github.com/stretchr/testify v1.11.1
 	golang.org/x/net v0.49.0
+	golang.org/x/text v0.33.0
 )
 
 require (
@@ -60,7 +61,6 @@ require (
 	go.yaml.in/yaml/v3 v3.0.4 // indirect
 	golang.org/x/crypto v0.47.0 // indirect
 	golang.org/x/sys v0.40.0 // indirect
-	golang.org/x/text v0.33.0 // indirect
 	google.golang.org/protobuf v1.36.11 // indirect
 	gopkg.in/yaml.v3 v3.0.1 // indirect
 )

request_options.go requestoptions.gorequest_options.go renamed to requestoptions.go

@@ -82,34 +82,34 @@ func WithRequestResolvedDocumentRoot(documentRoot string) RequestOption {
 // Future enhancements should be careful to avoid CVE-2019-11043,
 // 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 {
-	return func(o *frankenPHPContext) 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)
-				}
+func WithRequestSplitPath(splitPath []string) (RequestOption, 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 nil, ErrInvalidSplitPath
 			}
 
-			splitPath[i] = b.String()
-			b.Reset()
+			if 'A' <= c && c <= 'Z' {
+				b.WriteByte(c + 'a' - 'A')
+			} else {
+				b.WriteByte(c)
+			}
 		}
 
+		splitPath[i] = b.String()
+		b.Reset()
+	}
+
+	return func(o *frankenPHPContext) error {
 		o.splitPath = splitPath
 
 		return nil
-	}
+	}, nil
 }
 
 type PreparedEnv = map[string]string

request_options_test.go requestoptions_test.gorequest_options_test.go renamed to requestoptions_test.go

@@ -8,6 +8,8 @@ import (
 )
 
 func TestWithRequestSplitPath(t *testing.T) {
+	t.Parallel()
+
 	tests := []struct {
 		name          string
 		splitPath     []string
@@ -52,9 +54,10 @@ func TestWithRequestSplitPath(t *testing.T) {
 
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
+			t.Parallel()
+
 			ctx := &frankenPHPContext{}
-			opt := WithRequestSplitPath(tt.splitPath)
-			err := opt(ctx)
+			opt, err := WithRequestSplitPath(tt.splitPath)
 
 			if tt.wantErr != nil {
 				require.ErrorIs(t, err, tt.wantErr)
@@ -63,6 +66,7 @@ func TestWithRequestSplitPath(t *testing.T) {
 			}
 
 			require.NoError(t, err)
+			require.NoError(t, opt(ctx))
 			assert.Equal(t, tt.wantSplitPath, ctx.splitPath)
 		})
 	}