From 1390195f24d780832f5c30a2285ff62606b62b55 Mon Sep 17 00:00:00 2001 From: Jakub Zelenka Date: Thu, 29 Oct 2026 20:01:38 +0100 Subject: [PATCH] Fix bug #61471: Incomplete POST does not timeout but is passed to PHP The apache2handler read_post callback ignored errors returned by ap_get_brigade() so a request body that could not be read completely (the client disconnected or the body read timed out, e.g. when mod_reqtimeout is used) was passed to the script as a complete body and the request was answered with 200 and a partial or empty $_POST. The read error is now remembered in the request context and if it happens during the request startup (POST data reading), the script is not executed and the error is reported to httpd using the status mapped by ap_map_http_request_error() (400 Bad Request, 408 Request Timeout or AP_FILTER_ERROR when the input filter already responded). The request shutdown is still performed so that all request resources like the temporary files of a partial upload are released, which was the problem of the previously reverted fix (bug #74318). The body read during the script execution (php://input) is not affected apart from no longer dropping the data read before the error. --- NEWS | 4 + sapi/apache2handler/php_apache.h | 2 + sapi/apache2handler/sapi_apache2.c | 133 ++++++++++++++++++++++------- 3 files changed, 110 insertions(+), 29 deletions(-) diff --git a/NEWS b/NEWS index 186d729a28af..8395b0cc0e61 100644 --- a/NEWS +++ b/NEWS @@ -30,6 +30,10 @@ PHP NEWS . Fixed unmapping of an unrelated range when the aligned chunk mmap fails (Marti2203). +- Apache: + . Fixed bug #61471 (Incomplete POST does not timeout but is passed to PHP). + (Jakub Zelenka) + - CLI: . Fixed bug GH-24697 (PHP_HTTP_PARSER_STRICT build option doesn't work). (ndossche, devnexen) diff --git a/sapi/apache2handler/php_apache.h b/sapi/apache2handler/php_apache.h index c49dd2970880..520120e679f4 100644 --- a/sapi/apache2handler/php_apache.h +++ b/sapi/apache2handler/php_apache.h @@ -45,6 +45,8 @@ typedef struct php_struct { int request_processed; /* final content type */ char *content_type; + /* error status of the request body read (APR_SUCCESS if no error occurred) */ + apr_status_t read_post_error; } php_struct; void *merge_php_config(apr_pool_t *p, void *base_conf, void *new_conf); diff --git a/sapi/apache2handler/sapi_apache2.c b/sapi/apache2handler/sapi_apache2.c index 88fc584a8bcb..05d0b020f9c5 100644 --- a/sapi/apache2handler/sapi_apache2.c +++ b/sapi/apache2handler/sapi_apache2.c @@ -67,6 +67,28 @@ char *apache2_php_ini_path_override = NULL; ZEND_TSRMLS_CACHE_DEFINE() #endif +/* ap_map_http_request_error() is available since httpd 2.4.16 (MMN 20120211.46) */ +#if AP_MODULE_MAGIC_AT_LEAST(20120211, 46) +# define php_apache_map_http_request_error ap_map_http_request_error +#else +static int php_apache_map_http_request_error(apr_status_t rv, int status) +{ + switch (rv) { + case AP_FILTER_ERROR: + return AP_FILTER_ERROR; + case APR_ENOSPC: + return HTTP_REQUEST_ENTITY_TOO_LARGE; + case APR_ENOTIMPL: + return HTTP_NOT_IMPLEMENTED; + case APR_TIMEUP: + case APR_ETIMEDOUT: + return HTTP_REQUEST_TIME_OUT; + default: + return status; + } +} +#endif + static size_t php_apache_sapi_ub_write(const char *str, size_t str_length) { @@ -188,6 +210,11 @@ php_apache_sapi_read_post(char *buf, size_t count_bytes) brigade = ctx->brigade; len = count_bytes; + if (ctx->read_post_error != APR_SUCCESS) { + /* The request body read already failed, do not try to read it again. */ + return 0; + } + /* * This loop is needed because ap_get_brigade() can return us partial data * which would cause premature termination of request read. Therefore we @@ -206,7 +233,19 @@ php_apache_sapi_read_post(char *buf, size_t count_bytes) } if (status != APR_SUCCESS) { - return 0; + /* The input filter might have left the data read before the error in the brigade so keep them. */ + if (!APR_BRIGADE_EMPTY(brigade)) { + if (apr_brigade_flatten(brigade, buf, &len) == APR_SUCCESS) { + tlen += len; + } + apr_brigade_cleanup(brigade); + } + /* APR_EOF is a clean end of the input. Anything else means that the request body could not be read + * completely, e.g. the client disconnected (APR_INCOMPLETE) or the read timed out (APR_TIMEUP), and + * the error is remembered so that the handler can report it to httpd. */ + if (status != APR_EOF) { + ctx->read_post_error = status; + } } return tlen; @@ -521,6 +560,7 @@ static int php_apache_request_ctor(request_rec *r, php_struct *ctx) char *content_length; const char *auth; + ctx->read_post_error = APR_SUCCESS; SG(sapi_headers).http_response_code = !r->status ? HTTP_OK : r->status; SG(request_info).content_type = apr_table_get(r->headers_in, "Content-Type"); SG(request_info).query_string = apr_pstrdup(r->pool, r->args); @@ -554,6 +594,27 @@ static int php_apache_request_ctor(request_rec *r, php_struct *ctx) return php_request_startup(); } +/* Returns the HTTP status to be returned to httpd when the request body read failed during the request + * startup (reading of the POST data) or OK if the request can be processed. */ +static int php_apache_request_body_status(request_rec *r, php_struct *ctx) +{ + int status; + + if (ctx->read_post_error == APR_SUCCESS) { + return OK; + } + + /* The request body could not be read completely (e.g. the client disconnected or the read timed out) so the + * request cannot be processed by the script. The error is reported to httpd which generates the appropriate + * error response (e.g. 400 Bad Request or 408 Request Timeout) or has already done so (AP_FILTER_ERROR). */ + status = php_apache_map_http_request_error(ctx->read_post_error, HTTP_BAD_REQUEST); + /* AP_FILTER_ERROR is not an APR status so it is not logged */ + ap_log_rerror(APLOG_MARK, APLOG_INFO, status == AP_FILTER_ERROR ? APR_SUCCESS : ctx->read_post_error, r, + "Error reading the request body for script '%s', request is not processed", r->filename); + + return status; +} + static void php_apache_request_dtor(request_rec *r) { php_request_shutdown(NULL); @@ -588,6 +649,7 @@ static int php_handler(request_rec *r) apr_bucket_brigade * volatile brigade; apr_bucket *bucket; apr_status_t rv; + int volatile status = OK; request_rec * volatile parent_req = NULL; #ifdef ZTS /* initial resource fetch */ @@ -672,6 +734,7 @@ zend_first_try { if (php_apache_request_ctor(r, ctx)!=SUCCESS) { zend_bailout(); } + status = php_apache_request_body_status(r, ctx); } else { if (!parent_req) { parent_req = ctx->r; @@ -683,6 +746,7 @@ zend_first_try { if (php_apache_request_ctor(r, ctx)!=SUCCESS) { zend_bailout(); } + status = php_apache_request_body_status(r, ctx); } /* @@ -699,54 +763,65 @@ zend_first_try { brigade = ctx->brigade; } - if (AP2(last_modified)) { - ap_update_mtime(r, r->finfo.mtime); - ap_set_last_modified(r); - } - - /* Determine if we need to parse the file or show the source */ - if (strncmp(r->handler, PHP_SOURCE_MAGIC_TYPE, sizeof(PHP_SOURCE_MAGIC_TYPE) - 1) == 0) { - zend_syntax_highlighter_ini syntax_highlighter_ini; - php_get_highlight_struct(&syntax_highlighter_ini); - highlight_file((char *)r->filename, &syntax_highlighter_ini); - } else { - zend_file_handle zfd; - zend_stream_init_filename(&zfd, (char *) r->filename); - zfd.primary_script = 1; + /* The script is executed only if the request body was read successfully */ + if (status == OK) { + if (AP2(last_modified)) { + ap_update_mtime(r, r->finfo.mtime); + ap_set_last_modified(r); + } - if (!parent_req) { - php_execute_script(&zfd); + /* Determine if we need to parse the file or show the source */ + if (strncmp(r->handler, PHP_SOURCE_MAGIC_TYPE, sizeof(PHP_SOURCE_MAGIC_TYPE) - 1) == 0) { + zend_syntax_highlighter_ini syntax_highlighter_ini; + php_get_highlight_struct(&syntax_highlighter_ini); + highlight_file((char *)r->filename, &syntax_highlighter_ini); } else { - zend_execute_scripts(ZEND_INCLUDE, NULL, 1, &zfd); - } - zend_destroy_file_handle(&zfd); + zend_file_handle zfd; + zend_stream_init_filename(&zfd, (char *) r->filename); + zfd.primary_script = 1; - apr_table_set(r->notes, "mod_php_memory_usage", - apr_psprintf(ctx->r->pool, "%" APR_SIZE_T_FMT, zend_memory_peak_usage(1))); + if (!parent_req) { + php_execute_script(&zfd); + } else { + zend_execute_scripts(ZEND_INCLUDE, NULL, 1, &zfd); + } + zend_destroy_file_handle(&zfd); + + apr_table_set(r->notes, "mod_php_memory_usage", + apr_psprintf(ctx->r->pool, "%" APR_SIZE_T_FMT, zend_memory_peak_usage(1))); + } + } else { + /* Discard the output produced during the request startup (e.g. displayed warnings) as httpd generates + * the error response. */ + php_output_discard_all(); } } zend_end_try(); if (!parent_req) { + /* The request shutdown is always done (even if the request body read failed) so that all the request + * resources like the uploaded files are released. */ php_apache_request_dtor(r); ctx->request_processed = 1; apr_brigade_cleanup(brigade); - bucket = apr_bucket_eos_create(r->connection->bucket_alloc); - APR_BRIGADE_INSERT_TAIL(brigade, bucket); + if (status == OK) { + bucket = apr_bucket_eos_create(r->connection->bucket_alloc); + APR_BRIGADE_INSERT_TAIL(brigade, bucket); - rv = ap_pass_brigade(r->output_filters, brigade); - if (rv != APR_SUCCESS || r->connection->aborted) { + rv = ap_pass_brigade(r->output_filters, brigade); + if (rv != APR_SUCCESS || r->connection->aborted) { zend_first_try { - php_handle_aborted_connection(); + php_handle_aborted_connection(); } zend_end_try(); + } + apr_brigade_cleanup(brigade); } - apr_brigade_cleanup(brigade); apr_pool_cleanup_run(r->pool, (void *)&SG(server_context), php_server_context_cleanup); } else { ctx->r = parent_req; } - return OK; + return status; } static void php_apache_child_init(apr_pool_t *pchild, server_rec *s)