diff --git a/CHANGELOG.md b/CHANGELOG.md index e43a1b87..557a511a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -20,6 +20,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Changed +- **A query string past `max_input_vars` or `max_input_nesting_level` is refused with 400, as a form body is, instead of cut (#413).** `getQuery()` kept the first `max_input_vars` pairs and dropped the rest without a word (a name nested too deep was dropped the same way), so a handler served a filter or a list that was silently incomplete; `$_GET` cuts too, but warns. `getQuery()` and `getQueryParam()` now throw `HttpException` 400 ("the query string was refused: more fields than max_input_vars") on every call, and uncaught it answers the request with 400; `getPath()` is unaffected. A handler that read past the limit has to catch the exception or raise the ini value. Edmond, 2026-10-20. Evidence: `h1/074` answers 200 with three of four pairs before and 400 after; `http1parser/017` and `018` now expect the refusal. - **An uncaught `HttpException` on HTTP/3 answers with its code as the status, as on HTTP/1 and HTTP/2 (#412).** HTTP/3 answered 500 for every exception a handler let escape, so a form over `max_input_vars` answered 400 on HTTP/1 and HTTP/2 and 500 on HTTP/3, and `throw new HttpException('gone', 404)` answered 500. A code from 400 to 599 is now the status and the message the reason and body; any other code still gives 500 (`h3/107`, `h3/108`). - **An uploaded file whose client filename has path components is saved under the part after the last `/` or `\`, as PHP saves it (#339).** The server refused a name containing `..`, starting with `/` or `\`, or with a drive letter, with error 101, a code PHP does not define, and did not save the file; `my..report.pdf` was refused too, while `a/b\c.txt` was saved and `getClientFilename()` returned the whole path. `getClientFilename()` now returns what PHP puts in `$_FILES[...]['name']`: `../etc/passwd` gives `passwd`, `C:\Users\me\report.pdf` gives `report.pdf`, `dir/` gives an empty name, and the file is saved with error 0 (`multipart/027`, checked against php-cgi). A name over 4096 bytes still gets 101. - **`start()` refuses a `CODEL_TARGET_MS` that is not a whole number of milliseconds from 0 to 10000 (#395).** The variable was read with `strtol`: `abc` became 0 and turned CoDel off, `50ms` became 50, and an out-of-range value was dropped, each without a word. `start()` now throws `HttpServerInvalidArgumentException` naming the value, once, before any worker starts; a deployment that sets `50ms` has to set `50` (`core/086`). On Windows ZTS a value set with PHP's `putenv()` was not seen at all, since that build sets only the process environment and the CRT's `getenv()` reads its own copy; the variable is now read through `php_getenv()`, as PHP's `getenv()` reads it. diff --git a/dev/PLAN.md b/dev/PLAN.md index e89f8765..cbe82412 100644 --- a/dev/PLAN.md +++ b/dev/PLAN.md @@ -644,8 +644,20 @@ Added by the health check of 2026-09-30 (`dev/HEALTH.md`), in this order: in-thread without `--ENV--`; readMessage's wait loop threw RuntimeException 0; no test of a handler's own status), all taken. `grpc/020`-`025` red on the base, 10 of 10 after; full phpt 635 passed, 2 skipped. +48a. #413 — a query string past `max_input_vars` was cut silently. Edmond, + 2026-10-20: refuse with 400 as a form body is. `getQuery()` and + `getQueryParam()` throw HttpException 400 on every call, `getPath()` is + unaffected; nesting past `max_input_nesting_level` is refused the same way. + `h1/074` red before (200 with three of four pairs), green after; + `http1parser/017`, `018` expected the cut and now expect the refusal. T1. 49. #329 — the accept split on the shared listen fd. Edmond, 2026-10-20: possibly a Linux trait rather than a bug. Check before any code. + Traced 2026-10-20 (comment on #329): epoll registers a shared fd with + `add_wait_queue`, which puts the entry at the head of the socket's wait + queue, so the worker registered last is woken first and wins `accept4`. + 3 workers, 32 sequential connections, 5 runs: shared fd [32], [30,2], + [29,3], [31,1], [32]; SO_REUSEPORT [13,11,8] to [16,9,7]. Linux uses + SO_REUSEPORT by default. Waiting on Edmond: close as a kernel trait. 43. #339 — a client filename with path components: done. Edmond, 2026-10-19: behave as PHP. The name is cut after the last `/` or `\` (`php_ap_basename`, static in `main/rfc1867.c`, so copied) and the file is diff --git a/ide-stubs/true-async-server.php b/ide-stubs/true-async-server.php index c2336951..b9707ae6 100644 --- a/ide-stubs/true-async-server.php +++ b/ide-stubs/true-async-server.php @@ -2349,7 +2349,14 @@ public function getPath(): string {} * Get all query parameters as an associative array. * Supports PHP array notation: foo[bar], foo[] * + * A query string over a limit is refused rather than cut, as a form body + * is: more pairs than max_input_vars, or a name nested deeper than + * max_input_nesting_level, makes this and getQueryParam() throw + * {@see HttpException} 400 on every call, and uncaught it answers the + * request with 400. $_GET keeps what fits instead. getPath() still works. + * * @return array + * @throws HttpException 400 when the query string was refused. */ public function getQuery(): array {} @@ -2359,6 +2366,7 @@ public function getQuery(): array {} * @param string $name Parameter name * @param mixed $default Value to return when the parameter is absent (default: null) * @return mixed + * @throws HttpException 400 when the query string was refused (see getQuery()). */ public function getQueryParam(string $name, mixed $default = null): mixed {} diff --git a/include/http1/http_parser.h b/include/http1/http_parser.h index d52261a6..d6d22fae 100644 --- a/include/http1/http_parser.h +++ b/include/http1/http_parser.h @@ -315,6 +315,10 @@ struct http_request_t { /* Why the form was refused, for the message of every getter that throws; * a string literal, so it is valid on any thread. NULL while it has not. */ const char *form_refused_reason; + /* Why the query string was refused (400: over max_input_vars or + * max_input_nesting_level), for the message of every query getter that + * throws; a string literal. NULL while it has not. The path is unaffected. */ + const char *query_refused_reason; }; /* Single chunk node in the streaming body queue (linked list). diff --git a/src/http_request.c b/src/http_request.c index fd94d1e9..92244c99 100644 --- a/src/http_request.c +++ b/src/http_request.c @@ -805,16 +805,42 @@ static void http_request_ensure_uri_parsed(http_request_t *req) if (qslen > 0) { /* Percent-decoding, '+' as space, PHP array notation, the input - * filter, max_input_vars and arg_separator.input, as PHP populates - * $_GET. */ - http_form_vars_decode(Z_ARRVAL(arr), q + 1, qslen, - ZSTR_VAL(PG(arg_separator).input), - PG(max_input_vars), PG(max_input_nesting_level)); + * filter and arg_separator.input, as PHP populates $_GET. A query + * past max_input_vars or max_input_nesting_level is refused, as a + * form body is (#413): $_GET keeps what fits and warns, which a + * handler cannot see. */ + switch (http_form_vars_decode(Z_ARRVAL(arr), q + 1, qslen, + ZSTR_VAL(PG(arg_separator).input), + PG(max_input_vars), PG(max_input_nesting_level))) { + case HTTP_FORM_VARS_TOO_MANY: + req->query_refused_reason = "more fields than max_input_vars"; + break; + case HTTP_FORM_VARS_TOO_DEEP: + req->query_refused_reason = "a name nested deeper than max_input_nesting_level"; + break; + default: + break; + } } req->query_params = Z_ARR(arr); } +/* Parses the URI on first use; false, with HttpException 400 thrown, when the + * query string was refused. Every query getter throws it again. */ +static bool http_request_query_ready(http_request_t *req) +{ + http_request_ensure_uri_parsed(req); + + if (UNEXPECTED(req->query_refused_reason != NULL)) { + zend_throw_exception_ex(http_exception_ce, 400, "the query string was refused: %s", + req->query_refused_reason); + return false; + } + + return true; +} + ZEND_METHOD(TrueAsync_HttpRequest, getPath) { http_request_object *intern = Z_HTTP_REQUEST_P(ZEND_THIS); @@ -829,7 +855,10 @@ ZEND_METHOD(TrueAsync_HttpRequest, getQuery) http_request_object *intern = Z_HTTP_REQUEST_P(ZEND_THIS); ZEND_PARSE_PARAMETERS_NONE(); - http_request_ensure_uri_parsed(intern->request); + if (!http_request_query_ready(intern->request)) { + RETURN_THROWS(); + } + http_request_retval_ht(return_value, intern->request->query_params); } @@ -845,7 +874,9 @@ ZEND_METHOD(TrueAsync_HttpRequest, getQueryParam) Z_PARAM_ZVAL(default_value) ZEND_PARSE_PARAMETERS_END(); - http_request_ensure_uri_parsed(intern->request); + if (!http_request_query_ready(intern->request)) { + RETURN_THROWS(); + } zval *val = zend_hash_find(intern->request->query_params, name); diff --git a/stubs/HttpRequest.php b/stubs/HttpRequest.php index 7145b3cc..17a35548 100644 --- a/stubs/HttpRequest.php +++ b/stubs/HttpRequest.php @@ -36,7 +36,14 @@ public function getPath(): string {} * Get all query parameters as an associative array. * Supports PHP array notation: foo[bar], foo[] * + * A query string over a limit is refused rather than cut, as a form body + * is: more pairs than max_input_vars, or a name nested deeper than + * max_input_nesting_level, makes this and getQueryParam() throw + * {@see HttpException} 400 on every call, and uncaught it answers the + * request with 400. $_GET keeps what fits instead. getPath() still works. + * * @return array + * @throws HttpException 400 when the query string was refused. */ public function getQuery(): array {} @@ -46,6 +53,7 @@ public function getQuery(): array {} * @param string $name Parameter name * @param mixed $default Value to return when the parameter is absent (default: null) * @return mixed + * @throws HttpException 400 when the query string was refused (see getQuery()). */ public function getQueryParam(string $name, mixed $default = null): mixed {} diff --git a/stubs/HttpRequest.php_arginfo.h b/stubs/HttpRequest.php_arginfo.h index 1ce0c738..416b6a26 100644 --- a/stubs/HttpRequest.php_arginfo.h +++ b/stubs/HttpRequest.php_arginfo.h @@ -1,5 +1,5 @@ /* This is a generated file, edit HttpRequest.php.stub.php instead. - * Stub hash: ab2c103d2000fd8fca24118341f0c0bfd41040fe */ + * Stub hash: 31247d2a50b2b5c6d3fce7e41f0eec0f0ad0dcb4 */ ZEND_BEGIN_ARG_INFO_EX(arginfo_class_TrueAsync_HttpRequest___construct, 0, 0, 0) ZEND_END_ARG_INFO() diff --git a/tests/phpt/http1parser/017-form-keys-depth-and-filter.phpt b/tests/phpt/http1parser/017-form-keys-depth-and-filter.phpt index 2a79e869..a2a8f1af 100644 --- a/tests/phpt/http1parser/017-form-keys-depth-and-filter.phpt +++ b/tests/phpt/http1parser/017-form-keys-depth-and-filter.phpt @@ -1,5 +1,5 @@ --TEST-- -HTTP/1 parser: past max_input_nesting_level a query drops the name, a form is refused; values pass the input filter +HTTP/1 parser: past max_input_nesting_level a query and a form are refused; values pass the input filter --EXTENSIONS-- true_async_server filter @@ -9,10 +9,11 @@ filter.default=special_chars error_reporting=E_ALL & ~E_DEPRECATED --FILE-- getQuery()), "\n"; +echo json_encode(TrueAsync\http_parse_request("GET /?$flat HTTP/1.1\r\nHost: t\r\n\r\n")->getQuery()), "\n"; + +try { + TrueAsync\http_parse_request("GET /?$deep HTTP/1.1\r\nHost: t\r\n\r\n")->getQuery(); + echo "deep query: read\n"; +} catch (TrueAsync\HttpException $e) { + echo 'deep query: ', get_class($e), ' ', $e->getCode(), "\n"; +} $urlencoded = static fn(string $body) => TrueAsync\http_parse_request("POST / HTTP/1.1\r\nHost: t\r\n" . "Content-Type: application/x-www-form-urlencoded\r\n" @@ -56,6 +64,7 @@ echo $read($multipart(['x' => '1', 'a[b][c][d]' => 'deep', 'after' => '2'])), "\ --EXPECT-- {"x":"1","q":"<b>","after":"2"} {"x":"1","q":"<b>","after":"2"} +deep query: TrueAsync\HttpException 400 {"x":"1","q":"<b>","after":"2"} {"x":"1","q":"<b>","after":"2"} TrueAsync\HttpException 400 diff --git a/tests/phpt/http1parser/018-urlencoded-over-max-input-vars.phpt b/tests/phpt/http1parser/018-urlencoded-over-max-input-vars.phpt index 783b8e84..08ee6e62 100644 --- a/tests/phpt/http1parser/018-urlencoded-over-max-input-vars.phpt +++ b/tests/phpt/http1parser/018-urlencoded-over-max-input-vars.phpt @@ -1,14 +1,35 @@ --TEST-- -HTTP/1 parser: an url-encoded form past max_input_vars is refused, a query past it is cut as $_GET is +HTTP/1 parser: an url-encoded form and a query string past max_input_vars are both refused (#413) --EXTENSIONS-- true_async_server --INI-- max_input_vars=3 +max_input_nesting_level=1 --FILE-- getQuery()), "\n"; +echo json_encode(TrueAsync\http_parse_request("GET /?a=1&b=2&c=3 HTTP/1.1\r\nHost: t\r\n\r\n")->getQuery()), "\n"; + +$query = TrueAsync\http_parse_request("GET /p?$pairs HTTP/1.1\r\nHost: t\r\n\r\n"); + +foreach (['getQuery' => [], 'getQueryParam' => ['a']] as $getter => $args) { + try { + $query->$getter(...$args); + echo "$getter: read\n"; + } catch (TrueAsync\HttpException $e) { + echo "$getter: ", $e->getCode(), ' ', $e->getMessage(), "\n"; + } +} + +echo 'path after the refusal: ', $query->getPath(), "\n"; + +try { + TrueAsync\http_parse_request("GET /?a[b][c]=1 HTTP/1.1\r\nHost: t\r\n\r\n")->getQuery(); + echo "deep: read\n"; +} catch (TrueAsync\HttpException $e) { + echo 'deep: ', $e->getCode(), ' ', $e->getMessage(), "\n"; +} $form = static fn(string $body) => TrueAsync\http_parse_request("POST / HTTP/1.1\r\nHost: t\r\n" . "Content-Type: application/x-www-form-urlencoded\r\n" @@ -31,6 +52,10 @@ echo 'body after the refusal: ', $request->getBody(), "\n"; ?> --EXPECT-- {"a":"1","b":"2","c":"3"} +getQuery: 400 the query string was refused: more fields than max_input_vars +getQueryParam: 400 the query string was refused: more fields than max_input_vars +path after the refusal: /p +deep: 400 the query string was refused: a name nested deeper than max_input_nesting_level {"a":"1","b":"2","c":"3"} attempt 1: 400 the request form was refused: more fields than max_input_vars attempt 2: 400 the request form was refused: more fields than max_input_vars diff --git a/tests/phpt/server/h1/074-query-over-max-input-vars-answers-400.phpt b/tests/phpt/server/h1/074-query-over-max-input-vars-answers-400.phpt new file mode 100644 index 00000000..54b782af --- /dev/null +++ b/tests/phpt/server/h1/074-query-over-max-input-vars-answers-400.phpt @@ -0,0 +1,56 @@ +--TEST-- +HttpServer: a query string past max_input_vars is answered 400 when the handler lets the refusal go (#413) +--EXTENSIONS-- +true_async_server +true_async +--INI-- +max_input_vars=3 +--FILE-- +addListener('127.0.0.1', $port) + ->setReadTimeout(5) + ->setWriteTimeout(5)); + +$server->addHttpHandler(function ($req, $res) { + $res->setStatusCode(200)->setBody(json_encode($req->getQuery())); +}); + +$get = static function (int $port, string $target): string { + $fp = stream_socket_client("tcp://127.0.0.1:$port", $errno, $errstr, 3); + fwrite($fp, "GET $target HTTP/1.1\r\nHost: t\r\nConnection: close\r\n\r\n"); + $reply = stream_get_contents($fp); + fclose($fp); + + $status = substr($reply, 9, 3); + $body = substr($reply, strpos($reply, "\r\n\r\n") + 4); + + return "$status $body"; +}; + +$client = spawn(function () use ($port, $server, $get) { + usleep(30000); + echo 'within: ', $get($port, '/?a=1&b=2&c=3'), "\n"; + echo 'past: ', $get($port, '/?a=1&b=2&c=3&d=4'), "\n"; + $server->stop(); +}); + +$server->start(); +await($client); +?> +--EXPECT-- +within: 200 {"a":"1","b":"2","c":"3"} +past: 400 the query string was refused: more fields than max_input_vars