From bf6778f4c152f39086028fb27df60ce1348c916f 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/4] 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 e835205a29..3caf197089 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 9eccf2ed48..af36a1f71e 100644 --- a/frankenphp.go +++ b/frankenphp.go @@ -53,6 +53,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"} @@ -678,6 +679,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 @@ -764,6 +766,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/4] 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 142923319e..a5e50c0344 100644 --- a/frankenphp.c +++ b/frankenphp.c @@ -1419,7 +1419,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 af36a1f71e..20c8dec319 100644 --- a/frankenphp.go +++ b/frankenphp.go @@ -732,12 +732,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 @@ -767,8 +767,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 @@ + Date: Mon, 2 Nov 2026 15:30:45 +0100 Subject: [PATCH 3/4] fix: throw FrankenPHP\RequestBodyTooLargeException instead of exiting The uncatchable exit skipped finally blocks and restarted the worker, so a client could force restarts with an oversized chunked body. Reads by user code now throw a catchable exception, scripts are still skipped when PHP hits the limit while parsing the request, and the body drained after a worker callback no longer stops the worker loop. --- docs/config.md | 14 +++ frankenphp.c | 38 ++++-- frankenphp.stub.php | 88 ++++++++------ frankenphp_arginfo.h | 42 +++++-- requestbodylimit_test.go | 186 ++++++++++++++++++++++------- testdata/read-input-stream.php | 6 + testdata/read-input-then-touch.php | 6 +- testdata/worker-read-input.php | 12 +- 8 files changed, 289 insertions(+), 103 deletions(-) create mode 100644 testdata/read-input-stream.php diff --git a/docs/config.md b/docs/config.md index 3dc5a9cbbe..922a57899a 100644 --- a/docs/config.md +++ b/docs/config.md @@ -365,6 +365,20 @@ CADDY_GLOBAL_OPTIONS="servers { You can find more information about this setting in the [Caddy documentation](https://caddyserver.com/docs/caddyfile/options#enable-full-duplex). +### Request body size limit + +To limit the size of request bodies, use the [`request_body`](https://caddyserver.com/docs/caddyfile/directives/request_body) directive: + +```caddyfile +request_body { + max_size 10MB +} +``` + +Requests with a larger body get a `413 Request Entity Too Large` response. +If PHP reads the body before the script starts (`POST` requests with a `Content-Type`, such as forms and file uploads), the script doesn't run. +If the script reads the body itself, through `php://input` or `request_parse_body()`, the read throws a `FrankenPHP\RequestBodyTooLargeException` once the limit is reached, so a truncated body is never mistaken for a complete one. + ## Enable the debug mode When using the Docker image, set the `CADDY_GLOBAL_OPTIONS` environment variable to `debug` to enable the debug mode: diff --git a/frankenphp.c b/frankenphp.c index a5e50c0344..a408fea6db 100644 --- a/frankenphp.c +++ b/frankenphp.c @@ -130,6 +130,12 @@ HashTable *main_thread_env = NULL; static THREAD_LOCAL uintptr_t thread_index; static THREAD_LOCAL bool is_worker_thread = false; +/* a read hit the request body size limit, the script must not start on the + * truncated body */ +static THREAD_LOCAL bool request_body_too_large = false; +/* worker request startup and shutdown run in frankenphp_handle_request()'s + * frame too */ +static THREAD_LOCAL bool in_worker_callback = false; /* putenv() overrides layered over the other env tables, an IS_NULL entry * unsets the variable */ static THREAD_LOCAL HashTable *sandboxed_env = NULL; @@ -375,6 +381,9 @@ static void frankenphp_update_request_context() { SG(server_context) = &frankenphp_local_server_ctx; /* status It is not reset by zend engine, set it to 200. */ SG(sapi_headers).http_response_code = 200; + request_body_too_large = false; + /* not cleared when the callback bailed out */ + in_worker_callback = false; char *authorization_header = go_update_request_info(thread_index, &SG(request_info)); @@ -980,7 +989,10 @@ PHP_FUNCTION(frankenphp_handle_request) { fci.params = result.r1; fci.param_count = result.r1 == NULL ? 0 : 1; - if (zend_call_function(&fci, &fcc) == SUCCESS && Z_TYPE(retval) != IS_UNDEF) { + in_worker_callback = true; + /* a body over the limit was rejected while parsing the request */ + if (EXPECTED(!request_body_too_large) && + zend_call_function(&fci, &fcc) == SUCCESS && Z_TYPE(retval) != IS_UNDEF) { callback_ret = &retval; /* pass NULL instead of the NULL zval as return value */ @@ -988,6 +1000,7 @@ PHP_FUNCTION(frankenphp_handle_request) { callback_ret = NULL; } } + in_worker_callback = false; /* * If an exception occurred, print the message to the client before @@ -1193,8 +1206,12 @@ static const zend_function_entry frankenphp_test_hook_functions[] = { arginfo_frankenphp_test_persist_roundtrip) PHP_FE_END}; #endif +static zend_class_entry *frankenphp_ce_request_body_too_large_exception; + PHP_MINIT_FUNCTION(frankenphp) { register_frankenphp_symbols(module_number); + frankenphp_ce_request_body_too_large_exception = + register_class_FrankenPHP_RequestBodyTooLargeException(zend_ce_exception); #ifndef PHP_WIN32 /* MINIT runs once per ZTS thread — guard the atfork registration */ static pthread_once_t atfork_once = PTHREAD_ONCE_INIT; @@ -1422,10 +1439,15 @@ static size_t frankenphp_read_post(char *buffer, size_t 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(); + if (result.r1) { + request_body_too_large = true; + + /* php://input can't report read errors, throw so user code doesn't take + * the truncated body for a complete one */ + if (EG(current_execute_data) && (!is_worker_thread || in_worker_callback)) { + zend_throw_exception(frankenphp_ce_request_body_too_large_exception, + "Request body too large", 0); + } } return result.r0; @@ -1799,8 +1821,10 @@ static void *php_thread(void *arg) { file_handle.primary_script = 1; EG(exit_status) = 0; - /* Execute the PHP script, potential bailout to zend_catch */ - if (EXPECTED(open_primary_script(&file_handle))) { + /* Execute the PHP script, potential bailout to zend_catch. A body over + * the limit was rejected while parsing the request. */ + if (EXPECTED(!request_body_too_large) && + EXPECTED(open_primary_script(&file_handle))) { php_execute_script(&file_handle); } #ifndef PHP_WIN32 diff --git a/frankenphp.stub.php b/frankenphp.stub.php index d61397caaa..0fc316d40a 100644 --- a/frankenphp.stub.php +++ b/frankenphp.stub.php @@ -1,56 +1,66 @@ $context Values of the array will be converted to the corresponding Go type (if supported by FrankenPHP) and added to the context of the structured logs using https://pkg.go.dev/log/slog#Attr - */ -function frankenphp_log(string $message, int $level = 0, array $context = []): void {} + /** + * @param int $level The importance or severity of a log event. The higher the level, the more important or severe the event. For more details, see: https://pkg.go.dev/log/slog#Level + * array $context Values of the array will be converted to the corresponding Go type (if supported by FrankenPHP) and added to the context of the structured logs using https://pkg.go.dev/log/slog#Attr + */ + function frankenphp_log(string $message, int $level = 0, array $context = []): void {} +} + +namespace FrankenPHP { + /** Thrown when reading a request body larger than the configured limit */ + class RequestBodyTooLargeException extends \Exception {} +} diff --git a/frankenphp_arginfo.h b/frankenphp_arginfo.h index 052cfc67c6..f6ddc4696e 100644 --- a/frankenphp_arginfo.h +++ b/frankenphp_arginfo.h @@ -1,5 +1,5 @@ /* This is a generated file, edit the .stub.php file instead. - * Stub hash: 85fabd314ab885307e0032b9f7635fa13cc0c4b5 */ + * Stub hash: 57299a756b23c7443d6b09f759106ddf0ec8df20 */ ZEND_BEGIN_ARG_WITH_RETURN_TYPE_INFO_EX(arginfo_frankenphp_handle_request, 0, 1, _IS_BOOL, 0) ZEND_ARG_TYPE_INFO(0, callback, IS_CALLABLE, 0) @@ -41,7 +41,6 @@ ZEND_BEGIN_ARG_WITH_RETURN_TYPE_INFO_EX(arginfo_frankenphp_log, 0, 1, IS_VOID, 0 ZEND_ARG_TYPE_INFO_WITH_DEFAULT_VALUE(0, context, IS_ARRAY, 0, "[]") ZEND_END_ARG_INFO() - ZEND_FUNCTION(frankenphp_handle_request); ZEND_FUNCTION(headers_send); ZEND_FUNCTION(frankenphp_finish_request); @@ -50,17 +49,32 @@ ZEND_FUNCTION(frankenphp_response_headers); ZEND_FUNCTION(mercure_publish); ZEND_FUNCTION(frankenphp_log); - static const zend_function_entry ext_functions[] = { ZEND_FE(frankenphp_handle_request, arginfo_frankenphp_handle_request) ZEND_FE(headers_send, arginfo_headers_send) ZEND_FE(frankenphp_finish_request, arginfo_frankenphp_finish_request) - ZEND_FALIAS(fastcgi_finish_request, frankenphp_finish_request, arginfo_fastcgi_finish_request) +#if (PHP_VERSION_ID >= 80400) + ZEND_RAW_FENTRY("fastcgi_finish_request", zif_frankenphp_finish_request, arginfo_fastcgi_finish_request, 0, NULL, NULL) +#else + ZEND_RAW_FENTRY("fastcgi_finish_request", zif_frankenphp_finish_request, arginfo_fastcgi_finish_request, 0) +#endif ZEND_FE(frankenphp_request_headers, arginfo_frankenphp_request_headers) - ZEND_FALIAS(apache_request_headers, frankenphp_request_headers, arginfo_apache_request_headers) - ZEND_FALIAS(getallheaders, frankenphp_request_headers, arginfo_getallheaders) +#if (PHP_VERSION_ID >= 80400) + ZEND_RAW_FENTRY("apache_request_headers", zif_frankenphp_request_headers, arginfo_apache_request_headers, 0, NULL, NULL) +#else + ZEND_RAW_FENTRY("apache_request_headers", zif_frankenphp_request_headers, arginfo_apache_request_headers, 0) +#endif +#if (PHP_VERSION_ID >= 80400) + ZEND_RAW_FENTRY("getallheaders", zif_frankenphp_request_headers, arginfo_getallheaders, 0, NULL, NULL) +#else + ZEND_RAW_FENTRY("getallheaders", zif_frankenphp_request_headers, arginfo_getallheaders, 0) +#endif ZEND_FE(frankenphp_response_headers, arginfo_frankenphp_response_headers) - ZEND_FALIAS(apache_response_headers, frankenphp_response_headers, arginfo_apache_response_headers) +#if (PHP_VERSION_ID >= 80400) + ZEND_RAW_FENTRY("apache_response_headers", zif_frankenphp_response_headers, arginfo_apache_response_headers, 0, NULL, NULL) +#else + ZEND_RAW_FENTRY("apache_response_headers", zif_frankenphp_response_headers, arginfo_apache_response_headers, 0) +#endif ZEND_FE(mercure_publish, arginfo_mercure_publish) ZEND_FE(frankenphp_log, arginfo_frankenphp_log) ZEND_FE_END @@ -73,3 +87,17 @@ static void register_frankenphp_symbols(int module_number) REGISTER_LONG_CONSTANT("FRANKENPHP_LOG_LEVEL_WARN", 4, CONST_PERSISTENT); REGISTER_LONG_CONSTANT("FRANKENPHP_LOG_LEVEL_ERROR", 8, CONST_PERSISTENT); } + +static zend_class_entry *register_class_FrankenPHP_RequestBodyTooLargeException(zend_class_entry *class_entry_Exception) +{ + zend_class_entry ce, *class_entry; + + INIT_NS_CLASS_ENTRY(ce, "FrankenPHP", "RequestBodyTooLargeException", NULL); +#if (PHP_VERSION_ID >= 80400) + class_entry = zend_register_internal_class_with_flags(&ce, class_entry_Exception, 0); +#else + class_entry = zend_register_internal_class_ex(&ce, class_entry_Exception); +#endif + + return class_entry; +} diff --git a/requestbodylimit_test.go b/requestbodylimit_test.go index 59dd093ce0..816067ec45 100644 --- a/requestbodylimit_test.go +++ b/requestbodylimit_test.go @@ -1,82 +1,178 @@ package frankenphp_test import ( + "bytes" + "fmt" + "io" + "mime/multipart" "net/http" "net/http/httptest" "os" "path/filepath" "strings" "testing" + "time" "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() +// limitBody caps request bodies like Caddy's request_body max_size does. +func limitBody(limit int64, handler func(http.ResponseWriter, *http.Request)) func(http.ResponseWriter, *http.Request) { + return func(w http.ResponseWriter, r *http.Request) { + r.Body = http.MaxBytesReader(w, r.Body, limit) + handler(w, r) + } +} + +func newBodyRequest(t *testing.T, method, target, contentType string, size int, chunked bool) *http.Request { + t.Helper() - 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)) + var body bytes.Buffer + switch contentType { + case "multipart/form-data": + mw := multipart.NewWriter(&body) + fw, err := mw.CreateFormFile("file", "file.txt") require.NoError(t, err) - require.NoError(t, frankenphp.ServeHTTP(w, req)) + _, err = fw.Write(bytes.Repeat([]byte("x"), size)) + require.NoError(t, err) + require.NoError(t, mw.Close()) + contentType = mw.FormDataContentType() + case "application/x-www-form-urlencoded": + body.WriteString("foo=" + strings.Repeat("x", size-len("foo="))) + default: + body.WriteString(strings.Repeat("x", size)) } - 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) + var r io.Reader = &body + if chunked { + // hides the length + r = io.MultiReader(r) + } - 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") - } - }) + req := httptest.NewRequest(method, target, r) + if contentType != "" { + req.Header.Set("Content-Type", contentType) } + + return req +} + +// waitForThread returns once the only PHP thread is done with the previous script: a 413 is sent before. +func waitForThread(t *testing.T, handler func(http.ResponseWriter, *http.Request)) { + t.Helper() + + testGet("http://example.com/read-input.php", handler, t) +} + +// The script must not run on a truncated body, whoever reads it. +func TestRequestBodyTooLarge(t *testing.T) { + runTest(t, func(handler func(http.ResponseWriter, *http.Request), _ *httptest.Server, _ int) { + handler = limitBody(1024, handler) + + for _, tc := range []struct { + method, contentType string + // PHP reads the body before the script starts + readAtStartup bool + }{ + {"POST", "application/x-www-form-urlencoded", true}, + {"POST", "multipart/form-data", true}, + {"POST", "text/csv", true}, + {"POST", "", false}, + {"PUT", "application/json", false}, + {"PUT", "application/x-www-form-urlencoded", false}, + } { + for _, size := range []int{512, 4096} { + for _, chunked := range []bool{false, true} { + t.Run(fmt.Sprintf("%s %q %d chunked=%t", tc.method, tc.contentType, size, chunked), func(t *testing.T) { + marker := filepath.Join(t.TempDir(), "marker") + w := httptest.NewRecorder() + handler(w, newBodyRequest(t, tc.method, "http://example.com/read-input-then-touch.php?marker="+marker, tc.contentType, size, chunked)) + + if size <= 1024 { + assert.Equal(t, http.StatusOK, w.Code) + assert.FileExists(t, marker) + + return + } + + assert.Equal(t, http.StatusRequestEntityTooLarge, w.Code) + waitForThread(t, handler) + assert.NoFileExists(t, marker) + if tc.readAtStartup { + assert.NoFileExists(t, marker+"-caught") + } else { + assert.FileExists(t, marker+"-caught") + } + }) + } + } + } + }, &testOptions{nbParallelRequests: 1, initOpts: []frankenphp.Option{frankenphp.WithNumThreads(1)}}) } 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) + handler = limitBody(1024, handler) + + for _, tc := range []struct { + method, query, contentType string + size, status int + expected string + }{ + {"POST", "", "", 512, http.StatusOK, "count=1 finally=0 read=512"}, + // rejected before the callback runs + {"POST", "", "application/x-www-form-urlencoded", 4096, http.StatusRequestEntityTooLarge, ""}, + // the callback throws while reading it + {"PUT", "", "", 4096, http.StatusRequestEntityTooLarge, ""}, + // drained after the callback returns + {"PUT", "?ignore-body", "", 4096, http.StatusOK, "count=3 finally=2 read=0"}, + // the worker wasn't restarted + {"POST", "", "", 512, http.StatusOK, "count=4 finally=3 read=512"}, + } { w := httptest.NewRecorder() - handler(w, req) + handler(w, newBodyRequest(t, tc.method, "http://example.com/worker-read-input.php"+tc.query, tc.contentType, tc.size, false)) - if size > 1024 { - assert.Equal(t, http.StatusRequestEntityTooLarge, w.Code) - assert.NotContains(t, w.Body.String(), "read=") + assert.Equal(t, tc.status, w.Code) + if tc.expected == "" { + assert.NotContains(t, w.Body.String(), "count=") } else { - assert.Equal(t, http.StatusOK, w.Code) - assert.Equal(t, "read=512", w.Body.String()) + assert.Equal(t, tc.expected, w.Body.String()) } } }, &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() +// The script starts before the end of the body, which it reads as it arrives. +func TestRequestBodyStreamed(t *testing.T) { + runTest(t, func(handler func(http.ResponseWriter, *http.Request), _ *httptest.Server, _ int) { + marker := filepath.Join(t.TempDir(), "marker") + body, bodyWriter := io.Pipe() + req := httptest.NewRequest("PUT", "http://example.com/read-input-stream.php?marker="+marker, body) + req.ContentLength = 16384 - cwd, _ := os.Getwd() - marker := filepath.Join(t.TempDir(), "marker") + w := httptest.NewRecorder() + done := make(chan struct{}) + go func() { + defer close(done) + limitBody(1<<20, handler)(w, req) + }() - 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)) + chunk := strings.Repeat("x", 8192) + _, err := io.WriteString(bodyWriter, chunk) + require.NoError(t, err) + assert.Eventually(t, func() bool { + _, err := os.Stat(marker) + + return err == nil + }, 5*time.Second, 10*time.Millisecond) + + _, err = io.WriteString(bodyWriter, chunk) + require.NoError(t, err) + require.NoError(t, bodyWriter.Close()) + <-done - require.Equal(t, http.StatusRequestEntityTooLarge, w.Code) - require.NoFileExists(t, marker) + assert.Equal(t, "read=16384", w.Body.String()) + }, &testOptions{nbParallelRequests: 1}) } diff --git a/testdata/read-input-stream.php b/testdata/read-input-stream.php new file mode 100644 index 0000000000..995a0e7930 --- /dev/null +++ b/testdata/read-input-stream.php @@ -0,0 +1,6 @@ + Date: Tue, 3 Nov 2026 08:49:37 +0100 Subject: [PATCH 4/4] refactor: detect an oversized body without thread-locals Skip the script when the context is already done, and throw only from user code: worker request startup and shutdown run in frankenphp_handle_request()'s own frame. --- frankenphp.c | 34 ++++++++++++---------------------- 1 file changed, 12 insertions(+), 22 deletions(-) diff --git a/frankenphp.c b/frankenphp.c index a408fea6db..50a5ad5fc3 100644 --- a/frankenphp.c +++ b/frankenphp.c @@ -130,12 +130,6 @@ HashTable *main_thread_env = NULL; static THREAD_LOCAL uintptr_t thread_index; static THREAD_LOCAL bool is_worker_thread = false; -/* a read hit the request body size limit, the script must not start on the - * truncated body */ -static THREAD_LOCAL bool request_body_too_large = false; -/* worker request startup and shutdown run in frankenphp_handle_request()'s - * frame too */ -static THREAD_LOCAL bool in_worker_callback = false; /* putenv() overrides layered over the other env tables, an IS_NULL entry * unsets the variable */ static THREAD_LOCAL HashTable *sandboxed_env = NULL; @@ -381,9 +375,6 @@ static void frankenphp_update_request_context() { SG(server_context) = &frankenphp_local_server_ctx; /* status It is not reset by zend engine, set it to 200. */ SG(sapi_headers).http_response_code = 200; - request_body_too_large = false; - /* not cleared when the callback bailed out */ - in_worker_callback = false; char *authorization_header = go_update_request_info(thread_index, &SG(request_info)); @@ -989,9 +980,8 @@ PHP_FUNCTION(frankenphp_handle_request) { fci.params = result.r1; fci.param_count = result.r1 == NULL ? 0 : 1; - in_worker_callback = true; /* a body over the limit was rejected while parsing the request */ - if (EXPECTED(!request_body_too_large) && + if (EXPECTED(!go_is_context_done(thread_index)) && zend_call_function(&fci, &fcc) == SUCCESS && Z_TYPE(retval) != IS_UNDEF) { callback_ret = &retval; @@ -1000,7 +990,6 @@ PHP_FUNCTION(frankenphp_handle_request) { callback_ret = NULL; } } - in_worker_callback = false; /* * If an exception occurred, print the message to the client before @@ -1439,15 +1428,16 @@ static size_t frankenphp_read_post(char *buffer, size_t count_bytes) { struct go_read_post_return result = go_read_post(frankenphp_thread_index(), buffer, count_bytes); - if (result.r1) { - request_body_too_large = true; - - /* php://input can't report read errors, throw so user code doesn't take - * the truncated body for a complete one */ - if (EG(current_execute_data) && (!is_worker_thread || in_worker_callback)) { - zend_throw_exception(frankenphp_ce_request_body_too_large_exception, - "Request body too large", 0); - } + /* php://input can't report read errors, throw so user code doesn't take + * the truncated body for a complete one. Worker request startup and + * shutdown run in frankenphp_handle_request()'s frame, not in user code. */ + zend_execute_data *ex = EG(current_execute_data); + if (result.r1 && ex != NULL && + !(ex->func != NULL && ex->func->type == ZEND_INTERNAL_FUNCTION && + ex->func->internal_function.handler == + ZEND_FN(frankenphp_handle_request))) { + zend_throw_exception(frankenphp_ce_request_body_too_large_exception, + "Request body too large", 0); } return result.r0; @@ -1823,7 +1813,7 @@ static void *php_thread(void *arg) { /* Execute the PHP script, potential bailout to zend_catch. A body over * the limit was rejected while parsing the request. */ - if (EXPECTED(!request_body_too_large) && + if (EXPECTED(!go_is_context_done(thread_index)) && EXPECTED(open_primary_script(&file_handle))) { php_execute_script(&file_handle); }