From b6c5dd9ff579889da4c885beceff4aa87b2792c4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?K=C3=A9vin=20Dunglas?= Date: Mon, 26 Oct 2026 17:06:18 +0100 Subject: [PATCH 1/2] fix: return 413 when the request body exceeds the configured limit go_read_post dropped read errors, so PHP saw a truncated php://input when Caddy's request_body max_size was exceeded. --- context.go | 2 ++ frankenphp.go | 7 ++++ requestbodylimit_test.go | 62 ++++++++++++++++++++++++++++++++++ testdata/worker-read-input.php | 8 +++++ 4 files changed, 79 insertions(+) create mode 100644 requestbodylimit_test.go create mode 100644 testdata/worker-read-input.php diff --git a/context.go b/context.go index ace1e3dcdf..2dff05492c 100644 --- a/context.go +++ b/context.go @@ -35,6 +35,8 @@ type frankenPHPContext struct { scriptFilename string requestURI string + // Whether the final response headers were already sent + headersWritten bool // Whether the request is already closed by us isDone bool // The client's connection state as of the moment isDone was set. Captured diff --git a/frankenphp.go b/frankenphp.go index e560031e62..2118d01329 100644 --- a/frankenphp.go +++ b/frankenphp.go @@ -52,6 +52,7 @@ var ( ErrInvalidRequestPath = ErrRejected{"invalid request path", http.StatusBadRequest} ErrInvalidContentLengthHeader = ErrRejected{"invalid Content-Length header", http.StatusBadRequest} ErrMaxWaitTimeExceeded = ErrRejected{"maximum request handling time exceeded", http.StatusServiceUnavailable} + ErrRequestBodyTooLarge = ErrRejected{"request body too large", http.StatusRequestEntityTooLarge} contextKey = contextKeyStruct{} serverHeader = []string{"FrankenPHP"} @@ -688,6 +689,7 @@ func go_write_headers(threadIndex C.uintptr_t, status C.int, headers *C.zend_lli } fc.responseWriter.WriteHeader(goStatus) + fc.headersWritten = goStatus >= 200 if goStatus < 200 { // Clear headers, it's not automatically done by ResponseWriter.WriteHeader() for 1xx responses @@ -773,6 +775,11 @@ func go_read_post(threadIndex C.uintptr_t, cBuf *C.char, countBytes C.size_t) (r _ = rc.SetReadDeadline(time.Time{}) } + // PHP can't see read errors: fail the request rather than expose a truncated body + if _, ok := errors.AsType[*http.MaxBytesError](err); ok && !fc.headersWritten { + fc.reject(ErrRequestBodyTooLarge) + } + return } diff --git a/requestbodylimit_test.go b/requestbodylimit_test.go new file mode 100644 index 0000000000..98e1f7ad6f --- /dev/null +++ b/requestbodylimit_test.go @@ -0,0 +1,62 @@ +package frankenphp_test + +import ( + "net/http" + "net/http/httptest" + "os" + "strings" + "testing" + + "github.com/dunglas/frankenphp" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestRequestBodyTooLarge(t *testing.T) { + require.NoError(t, frankenphp.Init()) + defer frankenphp.Shutdown() + + cwd, _ := os.Getwd() + handler := func(w http.ResponseWriter, r *http.Request) { + r.Body = http.MaxBytesReader(w, r.Body, 1024) + req, err := frankenphp.NewRequestWithContext(r, frankenphp.WithRequestDocumentRoot(cwd+"/testdata/", false)) + require.NoError(t, err) + require.NoError(t, frankenphp.ServeHTTP(w, req)) + } + + for name, size := range map[string]int{"under": 512, "over": 4096} { + t.Run(name, func(t *testing.T) { + req := httptest.NewRequest("POST", "http://example.com/read-input.php", strings.NewReader(strings.Repeat("x", size))) + w := httptest.NewRecorder() + handler(w, req) + + if size > 1024 { + require.Equal(t, http.StatusRequestEntityTooLarge, w.Code) + require.NotContains(t, w.Body.String(), "read=") + } else { + require.Equal(t, http.StatusOK, w.Code) + require.Contains(t, w.Body.String(), "read=512") + } + }) + } +} + +func TestWorkerRequestBodyTooLarge(t *testing.T) { + runTest(t, func(handler func(http.ResponseWriter, *http.Request), _ *httptest.Server, _ int) { + // the worker must stay usable after each rejected request + for _, size := range []int{4096, 512, 4096, 512} { + req := httptest.NewRequest("POST", "http://example.com/worker-read-input.php", strings.NewReader(strings.Repeat("x", size))) + req.Body = http.MaxBytesReader(httptest.NewRecorder(), req.Body, 1024) + w := httptest.NewRecorder() + handler(w, req) + + if size > 1024 { + assert.Equal(t, http.StatusRequestEntityTooLarge, w.Code) + assert.NotContains(t, w.Body.String(), "read=") + } else { + assert.Equal(t, http.StatusOK, w.Code) + assert.Equal(t, "read=512", w.Body.String()) + } + } + }, &testOptions{workerScript: "worker-read-input.php", nbWorkers: 1, nbParallelRequests: 1}) +} diff --git a/testdata/worker-read-input.php b/testdata/worker-read-input.php new file mode 100644 index 0000000000..04618a24af --- /dev/null +++ b/testdata/worker-read-input.php @@ -0,0 +1,8 @@ + Date: Tue, 27 Oct 2026 15:05:55 +0100 Subject: [PATCH 2/2] fix: stop the script when the request body is too large --- frankenphp.c | 11 ++++++++++- frankenphp.go | 12 ++++++++---- requestbodylimit_test.go | 20 ++++++++++++++++++++ testdata/read-input-then-touch.php | 8 ++++++++ 4 files changed, 46 insertions(+), 5 deletions(-) create mode 100644 testdata/read-input-then-touch.php diff --git a/frankenphp.c b/frankenphp.c index 4066845349..24d06e17aa 100644 --- a/frankenphp.c +++ b/frankenphp.c @@ -1448,7 +1448,16 @@ static void frankenphp_sapi_flush(void *server_context) { } static size_t frankenphp_read_post(char *buffer, size_t count_bytes) { - return go_read_post(frankenphp_thread_index(), buffer, count_bytes); + struct go_read_post_return result = + go_read_post(frankenphp_thread_index(), buffer, count_bytes); + + // stop the script like exit() does: it can't be caught, so it can't keep + // processing the truncated body + if (result.r1 && EG(current_execute_data)) { + zend_throw_unwind_exit(); + } + + return result.r0; } static char *frankenphp_read_cookies(void) { diff --git a/frankenphp.go b/frankenphp.go index 2118d01329..1ace441ae8 100644 --- a/frankenphp.go +++ b/frankenphp.go @@ -742,12 +742,12 @@ 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) { +func go_read_post(threadIndex C.uintptr_t, cBuf *C.char, countBytes C.size_t) (readBytes C.size_t, tooLarge C.bool) { fc := phpThreads[threadIndex].handler.frankenPHPContext() // net/http forbids reading the body after the handler returns; body wrappers may touch the dead stream if fc.responseWriter == nil || fc.isDone { - return 0 + return 0, false } var rc *http.ResponseController @@ -776,8 +776,12 @@ func go_read_post(threadIndex C.uintptr_t, cBuf *C.char, countBytes C.size_t) (r } // PHP can't see read errors: fail the request rather than expose a truncated body - if _, ok := errors.AsType[*http.MaxBytesError](err); ok && !fc.headersWritten { - fc.reject(ErrRequestBodyTooLarge) + if _, ok := errors.AsType[*http.MaxBytesError](err); ok { + if !fc.headersWritten { + fc.reject(ErrRequestBodyTooLarge) + } + + tooLarge = true } return diff --git a/requestbodylimit_test.go b/requestbodylimit_test.go index 98e1f7ad6f..59dd093ce0 100644 --- a/requestbodylimit_test.go +++ b/requestbodylimit_test.go @@ -4,6 +4,7 @@ import ( "net/http" "net/http/httptest" "os" + "path/filepath" "strings" "testing" @@ -60,3 +61,22 @@ func TestWorkerRequestBodyTooLarge(t *testing.T) { } }, &testOptions{workerScript: "worker-read-input.php", nbWorkers: 1, nbParallelRequests: 1}) } + +// The script must be stopped when the body is too large, not run on with a truncated one. +func TestRequestBodyTooLargeStopsScript(t *testing.T) { + require.NoError(t, frankenphp.Init()) + defer frankenphp.Shutdown() + + cwd, _ := os.Getwd() + marker := filepath.Join(t.TempDir(), "marker") + + r := httptest.NewRequest("POST", "http://example.com/read-input-then-touch.php?marker="+marker, strings.NewReader(strings.Repeat("x", 4096))) + w := httptest.NewRecorder() + r.Body = http.MaxBytesReader(w, r.Body, 1024) + req, err := frankenphp.NewRequestWithContext(r, frankenphp.WithRequestDocumentRoot(cwd+"/testdata/", false)) + require.NoError(t, err) + require.NoError(t, frankenphp.ServeHTTP(w, req)) + + require.Equal(t, http.StatusRequestEntityTooLarge, w.Code) + require.NoFileExists(t, marker) +} diff --git a/testdata/read-input-then-touch.php b/testdata/read-input-then-touch.php new file mode 100644 index 0000000000..c6abb4aaf6 --- /dev/null +++ b/testdata/read-input-then-touch.php @@ -0,0 +1,8 @@ +