From d5f0b8fc3057582079cbb187c8638e3548f5b728 Mon Sep 17 00:00:00 2001 From: Ilia Alshanetsky Date: Mon, 2 Nov 2026 12:29:15 -0500 Subject: [PATCH] sapi/cli: Accept request paths split across reads The HTTP parser invokes on_path once per read that carries part of the path, but the built-in server normalized the first piece and rejected any further call, so a path longer than one 16 KiB read, or one that arrived in several TCP segments, failed as a malformed request. Collect the raw path and normalize it once the message is complete, which also resolves percent escapes and dot segments split across reads. A second request in the same read is now rejected in on_message_begin, since its path, URI, and query string would otherwise be appended to the first. --- NEWS | 4 ++ sapi/cli/php_cli_server.c | 27 ++++--- sapi/cli/tests/bug73630a.phpt | 4 +- .../cli/tests/cli_server_fragmented_path.phpt | 72 +++++++++++++++++++ .../tests/cli_server_pipelined_requests.phpt | 25 +++++++ 5 files changed, 120 insertions(+), 12 deletions(-) create mode 100644 sapi/cli/tests/cli_server_fragmented_path.phpt create mode 100644 sapi/cli/tests/cli_server_pipelined_requests.phpt diff --git a/NEWS b/NEWS index 26abaf2f03ab..d84c5cb44f17 100644 --- a/NEWS +++ b/NEWS @@ -2,6 +2,10 @@ PHP NEWS ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||| ?? ??? ????, PHP 8.4.29 +- CLI: + . Fixed built-in server rejecting long request paths, or paths received in + several pieces, as malformed. (Ilia Alshanetsky) + - Core: . Fixed AST printing of interface extends list as implements. (Jorg Sowa) . Fixed AST printing dropping fully qualified and namespace-relative prefixes diff --git a/sapi/cli/php_cli_server.c b/sapi/cli/php_cli_server.c index 671d5d5cd15f..94275e1abc00 100644 --- a/sapi/cli/php_cli_server.c +++ b/sapi/cli/php_cli_server.c @@ -1624,21 +1624,22 @@ static void normalize_vpath(char **retval, size_t *retval_len, const char *vpath /* {{{ php_cli_server_client_read_request */ static int php_cli_server_client_read_request_on_message_begin(php_http_parser *parser) { - return 0; + php_cli_server_client *client = parser->data; + return client->request_read; } static int php_cli_server_client_read_request_on_path(php_http_parser *parser, const char *at, size_t length) { php_cli_server_client *client = parser->data; - { - char *vpath; - size_t vpath_len; - if (UNEXPECTED(client->request.vpath != NULL)) { - return 1; - } - normalize_vpath(&vpath, &vpath_len, at, length, 1); - client->request.vpath = vpath; - client->request.vpath_len = vpath_len; + if (EXPECTED(client->request.vpath == NULL)) { + client->request.vpath = pestrndup(at, length, true); + client->request.vpath_len = length; + } else { + ZEND_ASSERT(length <= PHP_HTTP_MAX_HEADER_SIZE && PHP_HTTP_MAX_HEADER_SIZE - length >= client->request.vpath_len); + client->request.vpath = perealloc(client->request.vpath, client->request.vpath_len + length + 1, true); + memcpy(client->request.vpath + client->request.vpath_len, at, length); + client->request.vpath_len += length; + client->request.vpath[client->request.vpath_len] = '\0'; } return 0; } @@ -1822,6 +1823,12 @@ static int php_cli_server_client_read_request_on_message_complete(php_http_parse { php_cli_server_client *client = parser->data; client->request.protocol_version = parser->http_major * 100 + parser->http_minor; + if (client->request.vpath) { + char *vpath = client->request.vpath; + normalize_vpath(&client->request.vpath, &client->request.vpath_len, + vpath, client->request.vpath_len, true); + pefree(vpath, true); + } php_cli_server_request_translate_vpath(client->server, &client->request, client->server->document_root, client->server->document_root_len); if (client->request.vpath) { const char *vpath = client->request.vpath; diff --git a/sapi/cli/tests/bug73630a.phpt b/sapi/cli/tests/bug73630a.phpt index 45880c6cda09..4279b85b5b33 100644 --- a/sapi/cli/tests/bug73630a.phpt +++ b/sapi/cli/tests/bug73630a.phpt @@ -1,7 +1,7 @@ --TEST-- Bug #73630 (Built-in Weberver - overwrite $_SERVER['request_uri']) --DESCRIPTION-- -Check that too long paths result in invalid request +Check that paths exceeding the header size limit result in invalid request --SKIPIF-- --EXPECTF-- diff --git a/sapi/cli/tests/cli_server_fragmented_path.phpt b/sapi/cli/tests/cli_server_fragmented_path.phpt new file mode 100644 index 000000000000..cf1c28e6d145 --- /dev/null +++ b/sapi/cli/tests/cli_server_fragmented_path.phpt @@ -0,0 +1,72 @@ +--TEST-- +CLI server normalizes complete paths spanning multiple reads +--SKIPIF-- + +--FILE-- + ['/a/../%69ndex.php?check=ok'], + 'percent escape' => ['/a/../%', '69ndex.php?check=ok'], + 'percent digit' => ['/a/../%6', '9ndex.php?check=ok'], + 'parent segment' => ['/a/.', './index.php?check=ok'], + 'encoded parent segment' => ['/a/%2e%', '2e/index.php?check=ok'], + 'encoded slash' => ['/a/..%2', 'findex.php?check=ok'], + 'query boundary' => ['/index.php', '?check=', 'ok'], + 'large path' => ['/' . str_repeat('./', 9000) . 'index.php?check=ok'], +] as $name => $parts) { + $socket = php_cli_server_connect(); + fwrite($socket, 'GET '); + foreach ($parts as $i => $part) { + fwrite($socket, $part); + if ($i < count($parts) - 1) { + // Let the server process this fragment before sending the next one. + $control = php_cli_server_connect(); + fwrite($control, "GET /index.php?control HTTP/1.0\r\n\r\n"); + $response = stream_get_contents($control); + fclose($control); + if (!str_starts_with($response, 'HTTP/1.0 200 OK')) { + die("Control request failed\n"); + } + } + } + fwrite($socket, " HTTP/1.0\r\n\r\n"); + $response = stream_get_contents($socket); + fclose($socket); + $body = explode("\r\n\r\n", $response, 2)[1] ?? ''; + echo $name, "\n"; + var_dump(str_starts_with($response, 'HTTP/1.0 200 OK')); + var_dump($body === "/index.php\ncheck=ok\n" . implode('', $parts)); +} +?> +--EXPECT-- +unsplit +bool(true) +bool(true) +percent escape +bool(true) +bool(true) +percent digit +bool(true) +bool(true) +parent segment +bool(true) +bool(true) +encoded parent segment +bool(true) +bool(true) +encoded slash +bool(true) +bool(true) +query boundary +bool(true) +bool(true) +large path +bool(true) +bool(true) diff --git a/sapi/cli/tests/cli_server_pipelined_requests.phpt b/sapi/cli/tests/cli_server_pipelined_requests.phpt new file mode 100644 index 000000000000..d2207668b417 --- /dev/null +++ b/sapi/cli/tests/cli_server_pipelined_requests.phpt @@ -0,0 +1,25 @@ +--TEST-- +CLI server does not merge pipelined requests into one request +--SKIPIF-- + +--FILE-- + +--EXPECT-- +array(0) { +} +string(24) "[/third.php?three|three]"