refactor: removes context on the C side

caddy/caddy.go

@@ -11,6 +11,7 @@ import (
 	"path/filepath"
 	"strconv"
 	"strings"
+	"time"
 
 	"github.com/dunglas/frankenphp/internal/fastabs"
 
@@ -63,6 +64,8 @@ type FrankenPHPApp struct {
 	Workers []workerConfig `json:"workers,omitempty"`
 	// Overwrites the default php ini configuration
 	PhpIni map[string]string `json:"php_ini,omitempty"`
+	// The maximum amount of time a request may be stalled wainting for threads
+	BusyTimeout time.Duration `json:"busy_timeout,omitempty"`
 
 	metrics frankenphp.Metrics
 	logger  *zap.Logger
@@ -93,6 +96,7 @@ func (f *FrankenPHPApp) Start() error {
 		frankenphp.WithLogger(f.logger),
 		frankenphp.WithMetrics(f.metrics),
 		frankenphp.WithPhpIni(f.PhpIni),
+		frankenphp.WithBusyTimeout(f.BusyTimeout),
 	}
 	for _, w := range f.Workers {
 		opts = append(opts, frankenphp.WithWorkers(repl.ReplaceKnown(w.FileName, ""), w.Num, w.Env, w.Watch))
@@ -119,6 +123,8 @@ func (f *FrankenPHPApp) Stop() error {
 	// reset configuration so it doesn't bleed into later tests
 	f.Workers = nil
 	f.NumThreads = 0
+	f.BusyTimeout = 0
+	f.PhpIni = nil
 
 	return nil
 }
@@ -155,6 +161,17 @@ func (f *FrankenPHPApp) UnmarshalCaddyfile(d *caddyfile.Dispenser) error {
 				}
 
 				f.MaxThreads = int(v)
+			case "busy_timeout":
+				if !d.NextArg() {
+					return d.ArgErr()
+				}
+
+				v, err := time.ParseDuration(d.Val())
+				if err != nil {
+					return errors.New("busy_timeout must be a valid duration (example: 10s)")
+				}
+
+				f.BusyTimeout = v
 			case "php_ini":
 				parseIniLine := func(d *caddyfile.Dispenser) error {
 					key := d.Val()

caddy/caddy_test.go

@@ -8,6 +8,7 @@ import (
 	"path/filepath"
 	"strings"
 	"sync"
+	"sync/atomic"
 	"testing"
 
 	"github.com/dunglas/frankenphp"
@@ -716,3 +717,53 @@ func testSingleIniConfiguration(tester *caddytest.Tester, key string, value stri
 		)
 	}
 }
+
+func TestGatewayTimeout(t *testing.T) {
+	tester := caddytest.NewTester(t)
+	tester.InitServer(`
+		{
+			skip_install_trust
+			admin localhost:2999
+			http_port `+testPort+`
+
+			frankenphp {
+				num_threads 1
+				busy_timeout 1ns
+			}
+		}
+
+		localhost:`+testPort+` {
+			route {
+				root ../testdata
+				php
+			}
+		}
+		`, "caddyfile")
+
+	// send 10 requests simultaneously, at least one request should be stalled longer than 1ns
+	// since we only have 1 thread, this will cause a 504 Gateway Timeout
+	wg := sync.WaitGroup{}
+	success := atomic.Bool{}
+	wg.Add(10)
+	for i := 0; i < 10; i++ {
+		go func() {
+			statusCode := getStatusCode("http://localhost:"+testPort+"/sleep.php?sleep=10", t)
+			if statusCode == http.StatusGatewayTimeout {
+				success.Store(true)
+			}
+			wg.Done()
+		}()
+	}
+	wg.Wait()
+
+	require.True(t, success.Load(), "At least one request should have failed with a 504 Gateway Timeout status")
+}
+
+func getStatusCode(url string, t *testing.T) int {
+	req, err := http.NewRequest("GET", url, nil)
+	require.NoError(t, err)
+	resp, err := http.DefaultClient.Do(req)
+	require.NoError(t, err)
+	defer resp.Body.Close()
+	return resp.StatusCode
+}

cgi.go

@@ -6,7 +6,6 @@ import "C"
 import (
 	"crypto/tls"
 	"net"
-	"net/http"
 	"path/filepath"
 	"strings"
 	"unsafe"
@@ -48,7 +47,8 @@ var knownServerKeys = []string{
 //
 // TODO: handle this case https://github.com/caddyserver/caddy/issues/3718
 // Inspired by https://github.com/caddyserver/caddy/blob/master/modules/caddyhttp/reverseproxy/fastcgi/fastcgi.go
-func addKnownVariablesToServer(thread *phpThread, request *http.Request, fc *FrankenPHPContext, trackVarsArray *C.zval) {
+func addKnownVariablesToServer(thread *phpThread, fc *FrankenPHPContext, trackVarsArray *C.zval) {
+	request := fc.request
 	keys := mainThread.knownServerKeys
 	// Separate remote IP and port; more lenient than net.SplitHostPort
 	var ip, port string
@@ -160,8 +160,8 @@ func packCgiVariable(key *C.zend_string, value string) C.ht_key_value_pair {
 	return C.ht_key_value_pair{key, toUnsafeChar(value), C.size_t(len(value))}
 }
 
-func addHeadersToServer(request *http.Request, thread *phpThread, fc *FrankenPHPContext, trackVarsArray *C.zval) {
-	for field, val := range request.Header {
+func addHeadersToServer(thread *phpThread, fc *FrankenPHPContext, trackVarsArray *C.zval) {
+	for field, val := range fc.request.Header {
 		if k := mainThread.commonHeaders[field]; k != nil {
 			v := strings.Join(val, ", ")
 			C.frankenphp_register_single(k, toUnsafeChar(v), C.size_t(len(v)), trackVarsArray)
@@ -186,11 +186,10 @@ func addPreparedEnvToServer(fc *FrankenPHPContext, trackVarsArray *C.zval) {
 //export go_register_variables
 func go_register_variables(threadIndex C.uintptr_t, trackVarsArray *C.zval) {
 	thread := phpThreads[threadIndex]
-	r := thread.getActiveRequest()
-	fc := r.Context().Value(contextKey).(*FrankenPHPContext)
+	fc := thread.getRequestContext()
 
-	addKnownVariablesToServer(thread, r, fc, trackVarsArray)
-	addHeadersToServer(r, thread, fc, trackVarsArray)
+	addKnownVariablesToServer(thread, fc, trackVarsArray)
+	addHeadersToServer(thread, fc, trackVarsArray)
 
 	// The Prepared Environment is registered last and can overwrite any previous values
 	addPreparedEnvToServer(fc, trackVarsArray)

context.go

@@ -0,0 +1,176 @@
+package frankenphp
+
+import (
+	"context"
+	"net/http"
+	"os"
+	"strings"
+	"time"
+
+	"go.uber.org/zap"
+)
+
+var busyTimeout time.Duration
+
+// FrankenPHPContext provides contextual information about the Request to handle.
+type FrankenPHPContext struct {
+	documentRoot    string
+	splitPath       []string
+	env             PreparedEnv
+	logger          *zap.Logger
+	request         *http.Request
+	originalRequest *http.Request
+
+	docURI         string
+	pathInfo       string
+	scriptName     string
+	scriptFilename string
+
+	// Whether the request is already closed by us
+	isDone bool
+
+	responseWriter http.ResponseWriter
+	exitStatus     int
+
+	done      chan interface{}
+	startedAt time.Time
+}
+
+// FromContext extracts the FrankenPHPContext from a context.
+func FromContext(ctx context.Context) (fctx *FrankenPHPContext, ok bool) {
+	fctx, ok = ctx.Value(contextKey).(*FrankenPHPContext)
+	return
+}
+
+// NewRequestWithContext creates a new FrankenPHP request context.
+func NewRequestWithContext(r *http.Request, opts ...RequestOption) (*http.Request, error) {
+	fc := &FrankenPHPContext{
+		done:      make(chan interface{}),
+		startedAt: time.Now(),
+		request:   r,
+	}
+	for _, o := range opts {
+		if err := o(fc); err != nil {
+			return nil, err
+		}
+	}
+
+	if fc.logger == nil {
+		fc.logger = logger
+	}
+
+	if fc.documentRoot == "" {
+		if EmbeddedAppPath != "" {
+			fc.documentRoot = EmbeddedAppPath
+		} else {
+			var err error
+			if fc.documentRoot, err = os.Getwd(); err != nil {
+				return nil, err
+			}
+		}
+	}
+
+	if fc.splitPath == nil {
+		fc.splitPath = []string{".php"}
+	}
+
+	if fc.env == nil {
+		fc.env = make(map[string]string)
+	}
+
+	if splitPos := splitPos(fc, r.URL.Path); splitPos > -1 {
+		fc.docURI = r.URL.Path[:splitPos]
+		fc.pathInfo = r.URL.Path[splitPos:]
+
+		// Strip PATH_INFO from SCRIPT_NAME
+		fc.scriptName = strings.TrimSuffix(r.URL.Path, fc.pathInfo)
+
+		// Ensure the SCRIPT_NAME has a leading slash for compliance with RFC3875
+		// Info: https://tools.ietf.org/html/rfc3875#section-4.1.13
+		if fc.scriptName != "" && !strings.HasPrefix(fc.scriptName, "/") {
+			fc.scriptName = "/" + fc.scriptName
+		}
+	}
+
+	// SCRIPT_FILENAME is the absolute path of SCRIPT_NAME
+	fc.scriptFilename = sanitizedPathJoin(fc.documentRoot, fc.scriptName)
+
+	c := context.WithValue(r.Context(), contextKey, fc)
+
+	return r.WithContext(c), nil
+}
+
+func newFakeContext(requestPath string, opts ...RequestOption) (*FrankenPHPContext, error) {
+	r, err := http.NewRequest(http.MethodGet, requestPath, nil)
+	if err != nil {
+		return nil, err
+	}
+
+	fr, err := NewRequestWithContext(r, opts...)
+	if err != nil {
+		return nil, err
+	}
+
+	fc, _ := FromContext(fr.Context())
+
+	return fc, nil
+}
+
+// closeContext sends the response to the client
+func (fc *FrankenPHPContext) closeContext() {
+	if !fc.isDone {
+		close(fc.done)
+		fc.isDone = true
+	}
+}
+
+// validate checks if the request should be outright rejected
+func (fc *FrankenPHPContext) validate() bool {
+	if !strings.Contains(fc.request.URL.Path, "\x00") {
+		return true
+	}
+	fc.rejectBadRequest("Invalid request path")
+	return false
+}
+
+// busyTimeout returns a channel that will be closed after busy_timeout
+func (fc *FrankenPHPContext) busyTimeout() <-chan time.Time {
+	if busyTimeout <= 0 {
+		return nil
+	}
+	return time.After(busyTimeout)
+}
+
+func (fc *FrankenPHPContext) clientHasClosed() bool {
+	select {
+	case <-fc.request.Context().Done():
+		return true
+	default:
+		return false
+	}
+}
+
+// reject sends a response with the given status code and message
+func (fc *FrankenPHPContext) reject(statusCode int, message string) {
+	if fc.isDone {
+		return
+	}
+
+	rw := fc.responseWriter
+	if rw != nil {
+		rw.WriteHeader(statusCode)
+		_, _ = rw.Write([]byte(message))
+		rw.(http.Flusher).Flush()
+	}
+
+	fc.closeContext()
+}
+
+func (fc *FrankenPHPContext) rejectBadRequest(message string) {
+	fc.reject(http.StatusBadRequest, message)
+}
+
+func (fc *FrankenPHPContext) rejectGatewayTimeout() {
+	logger.Debug("Request timed out")
+	fc.reject(http.StatusGatewayTimeout, "")
+}

docs/config.md

@@ -246,6 +246,20 @@ You can also change the PHP configuration using the `php_ini` directive in the `
 }
 ```
 
+## Busy Timeout
+
+Caddy will usually accept an unlimited amount of connections. These connections will be stalled until a
+worker is available to handle them. To set a limit on how long a request may be stalled before rejecting it
+with a 504 error, you can use the `busy_timeout` directive:
+
+```caddyfile
+{
+    frankenphp {
+        busy_timeout 10s
+    }
+}
+```
+
 ## Enable the Debug Mode
 
 When using the Docker image, set the `CADDY_GLOBAL_OPTIONS` environment variable to `debug` to enable the debug mode: