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/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 142923319e..50a5ad5fc3 100644 --- a/frankenphp.c +++ b/frankenphp.c @@ -980,7 +980,9 @@ 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) { + /* a body over the limit was rejected while parsing the request */ + if (EXPECTED(!go_is_context_done(thread_index)) && + zend_call_function(&fci, &fcc) == SUCCESS && Z_TYPE(retval) != IS_UNDEF) { callback_ret = &retval; /* pass NULL instead of the NULL zval as return value */ @@ -1193,8 +1195,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; @@ -1419,7 +1425,22 @@ 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); + + /* 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; } static char *frankenphp_read_cookies(void) { @@ -1790,8 +1811,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(!go_is_context_done(thread_index)) && + EXPECTED(open_primary_script(&file_handle))) { php_execute_script(&file_handle); } #ifndef PHP_WIN32 diff --git a/frankenphp.go b/frankenphp.go index 9eccf2ed48..20c8dec319 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 @@ -730,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 @@ -764,6 +766,15 @@ 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 { + if !fc.headersWritten { + fc.reject(ErrRequestBodyTooLarge) + } + + tooLarge = true + } + return } 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 new file mode 100644 index 0000000000..816067ec45 --- /dev/null +++ b/requestbodylimit_test.go @@ -0,0 +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" +) + +// 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() + + 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) + _, 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)) + } + + var r io.Reader = &body + if chunked { + // hides the length + r = io.MultiReader(r) + } + + 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) { + 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, newBodyRequest(t, tc.method, "http://example.com/worker-read-input.php"+tc.query, tc.contentType, tc.size, false)) + + assert.Equal(t, tc.status, w.Code) + if tc.expected == "" { + assert.NotContains(t, w.Body.String(), "count=") + } else { + assert.Equal(t, tc.expected, w.Body.String()) + } + } + }, &testOptions{workerScript: "worker-read-input.php", nbWorkers: 1, nbParallelRequests: 1}) +} + +// 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 + + w := httptest.NewRecorder() + done := make(chan struct{}) + go func() { + defer close(done) + limitBody(1<<20, handler)(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 + + 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 @@ +