From c0402d7896d3228cf9dbbcbab2d055ef858d2a10 Mon Sep 17 00:00:00 2001 From: Alon Noy Date: Fri, 21 Aug 2026 21:36:35 +0200 Subject: [PATCH 1/3] Fix filterHeaders() dropping purely-numeric header names A header name made up entirely of digits (e.g. "111") is a valid RFC 9110 token, but PHP itself coerces a canonical-integer string used as an array key into an int before filterHeaders() ever sees it. !\is_string($key) then treats that coerced int key as invalid input and deletes the header outright, silently dropping real data instead of the malformed input the check exists to guard against. Casts the key back to a string instead, which recovers the original header name losslessly (PHP guarantees (string) (int) $s === $s for exactly the strings it coerces this way). An empty string is still rejected, unchanged - that is the actual malformed case this method guards against. The existing test data for this method encoded the bug as the expected, correct behavior (a numeric-keyed header labelled "invalid-non-string-key" and asserted dropped); updated it to assert the header is recovered instead. --- src/HttpWorker.php | 35 +++++++++++++++++++++++++++-------- tests/Unit/HttpWorkerTest.php | 12 ++++++++++-- 2 files changed, 37 insertions(+), 10 deletions(-) diff --git a/src/HttpWorker.php b/src/HttpWorker.php index dd9528f..d9b40e6 100644 --- a/src/HttpWorker.php +++ b/src/HttpWorker.php @@ -205,23 +205,42 @@ private function requestFromProto(string $body, RequestProto $message): Request } /** - * Remove all non-string and empty-string keys + * Normalize header names back to strings, dropping only genuinely + * empty ones. + * + * A header name made up entirely of digits (e.g. "123", a valid + * RFC 9110 token) arrives here as an `int` array key: PHP itself + * coerces a canonical-integer string used as an array key into an + * int, before this method ever sees it. Casting the key back to a + * string recovers the original header name losslessly — PHP + * guarantees `(string) (int) $s === $s` for exactly the strings it + * coerces this way — instead of silently dropping a real header. + * + * An empty string is still rejected: that is the actual malformed + * input this method exists to guard against (otherwise, the worker + * might be crashed) — @see: . Every PHP array + * key is either an int or a string, so `(string) $key` is always + * safe. * * @param array> $headers * @return HeadersList */ private function filterHeaders(array $headers): array { - foreach ($headers as $key => $_) { - if (!\is_string($key) || $key === '') { - // ignore invalid header names or values (otherwise, the worker might be crashed) - // @see: - unset($headers[$key]); + $result = []; + + foreach ($headers as $key => $value) { + $key = (string) $key; + + if ($key === '') { + continue; } + + $result[$key] = $value; } - /** @var HeadersList $headers */ - return $headers; + /** @var HeadersList $result */ + return $result; } /** diff --git a/tests/Unit/HttpWorkerTest.php b/tests/Unit/HttpWorkerTest.php index e951d2e..1991b4e 100644 --- a/tests/Unit/HttpWorkerTest.php +++ b/tests/Unit/HttpWorkerTest.php @@ -53,12 +53,20 @@ public static function requestDataProvider(): \Traversable \array_merge(self::REQUIRED_PAYLOAD_DATA, [ 'headers' => [ 'Content-Type' => ['application/x-www-form-urlencoded'], - 111 => ['invalid-non-string-key'], + // A purely-numeric header name (e.g. "111") is a + // valid RFC 9110 token; PHP itself coerces it into + // an int array key on the way in, which is why this + // arrives as int(111) rather than the string "111". + // filterHeaders() must recover it, not drop it. + 111 => ['numeric-header-name'], '' => ['invalid-empty-string-key'], ], ]), \array_merge(self::REQUIRED_REQUEST_DATA, [ - 'headers' => ['Content-Type' => ['application/x-www-form-urlencoded']], + 'headers' => [ + 'Content-Type' => ['application/x-www-form-urlencoded'], + '111' => ['numeric-header-name'], + ], ]), ]; yield [ From c5582b7e1b572d3531be8e2a766bf6fbc43e3cf2 Mon Sep 17 00:00:00 2001 From: Alon Noy Date: Fri, 21 Aug 2026 22:39:16 +0200 Subject: [PATCH 2/3] Fix TypeError regression for numeric header names in PSR7Worker/GlobalState filterHeaders() now keeps purely-numeric header names instead of dropping them, but PHP always coerces such names into int array keys, so HeadersList can never guarantee string keys. PSR7Worker::mapRequest() and GlobalState::enrichServerVars() consumed those keys as strings under strict_types, so a numeric header name would throw a TypeError instead of being silently dropped as before. Cast the key to string at both consumption points, and correct the HeadersList type/docblocks that incorrectly implied string keys were guaranteed. Co-Authored-By: Claude Sonnet 5 --- src/GlobalState.php | 5 ++++- src/HttpWorker.php | 14 ++++++++------ src/PSR7Worker.php | 5 ++++- src/Request.php | 6 +++++- tests/Unit/HttpWorkerTest.php | 7 ++++++- tests/Unit/PSR7WorkerTest.php | 20 ++++++++++++++++++++ 6 files changed, 47 insertions(+), 10 deletions(-) diff --git a/src/GlobalState.php b/src/GlobalState.php index 86d26b3..bd69628 100644 --- a/src/GlobalState.php +++ b/src/GlobalState.php @@ -43,7 +43,10 @@ public static function enrichServerVars(Request $request): array : $parts['host']; } foreach ($request->headers as $key => $value) { - $key = \strtoupper(\str_replace('-', '_', $key)); + // $key may be an int here: a purely-numeric header name is + // coerced into an int array key by PHP (see HeadersList in + // Request.php), but str_replace()/strtoupper() require a string. + $key = \strtoupper(\str_replace('-', '_', (string) $key)); if ($key == 'CONTENT_TYPE' || $key == 'CONTENT_LENGTH') { $server[$key] = \implode(', ', $value); diff --git a/src/HttpWorker.php b/src/HttpWorker.php index d9b40e6..481ea66 100644 --- a/src/HttpWorker.php +++ b/src/HttpWorker.php @@ -205,16 +205,18 @@ private function requestFromProto(string $body, RequestProto $message): Request } /** - * Normalize header names back to strings, dropping only genuinely - * empty ones. + * Drop only genuinely empty header names; keep everything else. * * A header name made up entirely of digits (e.g. "123", a valid * RFC 9110 token) arrives here as an `int` array key: PHP itself * coerces a canonical-integer string used as an array key into an - * int, before this method ever sees it. Casting the key back to a - * string recovers the original header name losslessly — PHP - * guarantees `(string) (int) $s === $s` for exactly the strings it - * coerces this way — instead of silently dropping a real header. + * int, before this method ever sees it. The previous implementation + * treated that coercion as an invalid header name and dropped it; + * this one keeps it. Casting the key to a string only normalizes it + * for the emptiness check below — reinserting it into $result still + * leaves it as an `int` key, because PHP coerces it back the same + * way. There is no plain-array representation that can hold such a + * header name as a string key; see {@see HeadersList} in Request.php. * * An empty string is still rejected: that is the actual malformed * input this method exists to guard against (otherwise, the worker diff --git a/src/PSR7Worker.php b/src/PSR7Worker.php index af16370..02d1ca6 100644 --- a/src/PSR7Worker.php +++ b/src/PSR7Worker.php @@ -128,7 +128,10 @@ protected function mapRequest(Request $httpRequest, array $server): ServerReques } foreach ($httpRequest->headers as $name => $value) { - $request = $request->withHeader($name, $value); + // $name may be an int here: a purely-numeric header name is + // coerced into an int array key by PHP (see HeadersList in + // Request.php), but PSR-7 requires a string header name. + $request = $request->withHeader((string) $name, $value); } if ($httpRequest->parsed) { diff --git a/src/Request.php b/src/Request.php index b40c4c8..57ffac1 100644 --- a/src/Request.php +++ b/src/Request.php @@ -17,7 +17,11 @@ * mime: string * } * - * @psalm-type HeadersList = array> + * Header names are keyed by `array-key`, not `non-empty-string`: a header + * name made up entirely of digits (e.g. "123") is a valid RFC 9110 token, + * but PHP always coerces such a key into an `int` when it's used as an + * array key, so it cannot be represented as a string here. + * @psalm-type HeadersList = array> * @psalm-type AttributesList = array * @psalm-type QueryArgumentsList = array * @psalm-type CookiesList = array diff --git a/tests/Unit/HttpWorkerTest.php b/tests/Unit/HttpWorkerTest.php index 1991b4e..ee3eff7 100644 --- a/tests/Unit/HttpWorkerTest.php +++ b/tests/Unit/HttpWorkerTest.php @@ -65,7 +65,12 @@ public static function requestDataProvider(): \Traversable \array_merge(self::REQUIRED_REQUEST_DATA, [ 'headers' => [ 'Content-Type' => ['application/x-www-form-urlencoded'], - '111' => ['numeric-header-name'], + // Written as an int key on purpose: PHP coerces a + // canonical-integer string key back to int the moment + // it's used as an array key, so filterHeaders() cannot + // hand back a string "111" here — only preserve the + // header instead of dropping it. See HttpWorker.php. + 111 => ['numeric-header-name'], ], ]), ]; diff --git a/tests/Unit/PSR7WorkerTest.php b/tests/Unit/PSR7WorkerTest.php index c0a60f4..20c4dc8 100644 --- a/tests/Unit/PSR7WorkerTest.php +++ b/tests/Unit/PSR7WorkerTest.php @@ -61,6 +61,26 @@ public function testStateServerLeak(): void 'HTTP_HOST' => 'localhost', ], ], + [ + // A purely-numeric header name arrives as an int array + // key (see HeadersList in Request.php). Both withHeader() + // in PSR7Worker and the $_SERVER key building in + // GlobalState must cast it back to string themselves, or + // this throws a TypeError under strict_types. + [ + 'Content-Type' => ['application/json'], + 111 => ['numeric-header-name'], + ], + [ + 'REQUEST_URI' => 'http://localhost', + 'REMOTE_ADDR' => '127.0.0.1', + 'REQUEST_METHOD' => 'GET', + 'HTTP_USER_AGENT' => '', + 'CONTENT_TYPE' => 'application/json', + 'HTTP_111' => 'numeric-header-name', + 'HTTP_HOST' => 'localhost', + ], + ], ]; $_SERVER = []; From 7acc8b8dfb873c8c6cbff5983ee1a40e0bff9203 Mon Sep 17 00:00:00 2001 From: roxblnfk Date: Fri, 9 Oct 2026 23:48:27 +0400 Subject: [PATCH 3/3] refactor: simplify filterHeaders() and trim header-name comments test: cover numeric header names in GlobalState The rebuild loop re-coerced the cast key back to int, so unsetting the empty key is equivalent. HeadersList narrows to int|non-empty-string instead of array-key, keeping the non-empty guarantee for string names. Assisted-By: Claude Opus 5.5 --- src/GlobalState.php | 4 +--- src/HttpWorker.php | 36 +++++----------------------------- src/PSR7Worker.php | 4 +--- src/Request.php | 7 ++----- tests/Unit/GlobalStateTest.php | 12 ++++++++++++ tests/Unit/HttpWorkerTest.php | 11 +---------- tests/Unit/PSR7WorkerTest.php | 6 +----- 7 files changed, 23 insertions(+), 57 deletions(-) diff --git a/src/GlobalState.php b/src/GlobalState.php index bd69628..43fdf18 100644 --- a/src/GlobalState.php +++ b/src/GlobalState.php @@ -43,9 +43,7 @@ public static function enrichServerVars(Request $request): array : $parts['host']; } foreach ($request->headers as $key => $value) { - // $key may be an int here: a purely-numeric header name is - // coerced into an int array key by PHP (see HeadersList in - // Request.php), but str_replace()/strtoupper() require a string. + // A numeric header name is an int key, which strict types reject in str_replace(). $key = \strtoupper(\str_replace('-', '_', (string) $key)); if ($key == 'CONTENT_TYPE' || $key == 'CONTENT_LENGTH') { diff --git a/src/HttpWorker.php b/src/HttpWorker.php index 481ea66..19f5a1c 100644 --- a/src/HttpWorker.php +++ b/src/HttpWorker.php @@ -205,44 +205,18 @@ private function requestFromProto(string $body, RequestProto $message): Request } /** - * Drop only genuinely empty header names; keep everything else. - * - * A header name made up entirely of digits (e.g. "123", a valid - * RFC 9110 token) arrives here as an `int` array key: PHP itself - * coerces a canonical-integer string used as an array key into an - * int, before this method ever sees it. The previous implementation - * treated that coercion as an invalid header name and dropped it; - * this one keeps it. Casting the key to a string only normalizes it - * for the emptiness check below — reinserting it into $result still - * leaves it as an `int` key, because PHP coerces it back the same - * way. There is no plain-array representation that can hold such a - * header name as a string key; see {@see HeadersList} in Request.php. - * - * An empty string is still rejected: that is the actual malformed - * input this method exists to guard against (otherwise, the worker - * might be crashed) — @see: . Every PHP array - * key is either an int or a string, so `(string) $key` is always - * safe. + * Drops the empty header name: PSR-7 `withHeader()` rejects it, which would crash the worker. + * Int keys are kept: PHP stores a numeric header name such as "123" as one. * * @param array> $headers * @return HeadersList */ private function filterHeaders(array $headers): array { - $result = []; - - foreach ($headers as $key => $value) { - $key = (string) $key; + unset($headers['']); - if ($key === '') { - continue; - } - - $result[$key] = $value; - } - - /** @var HeadersList $result */ - return $result; + /** @var HeadersList $headers */ + return $headers; } /** diff --git a/src/PSR7Worker.php b/src/PSR7Worker.php index 02d1ca6..3b768c5 100644 --- a/src/PSR7Worker.php +++ b/src/PSR7Worker.php @@ -128,9 +128,7 @@ protected function mapRequest(Request $httpRequest, array $server): ServerReques } foreach ($httpRequest->headers as $name => $value) { - // $name may be an int here: a purely-numeric header name is - // coerced into an int array key by PHP (see HeadersList in - // Request.php), but PSR-7 requires a string header name. + // A numeric header name is an int key, which strict types reject in withHeader(). $request = $request->withHeader((string) $name, $value); } diff --git a/src/Request.php b/src/Request.php index 57ffac1..2f538d0 100644 --- a/src/Request.php +++ b/src/Request.php @@ -17,11 +17,8 @@ * mime: string * } * - * Header names are keyed by `array-key`, not `non-empty-string`: a header - * name made up entirely of digits (e.g. "123") is a valid RFC 9110 token, - * but PHP always coerces such a key into an `int` when it's used as an - * array key, so it cannot be represented as a string here. - * @psalm-type HeadersList = array> + * An int key is a numeric header name such as "123": PHP coerces it on insertion. + * @psalm-type HeadersList = array> * @psalm-type AttributesList = array * @psalm-type QueryArgumentsList = array * @psalm-type CookiesList = array diff --git a/tests/Unit/GlobalStateTest.php b/tests/Unit/GlobalStateTest.php index e3d4941..4d3d525 100644 --- a/tests/Unit/GlobalStateTest.php +++ b/tests/Unit/GlobalStateTest.php @@ -50,6 +50,18 @@ public static function enrichServerVarsDataProvider(): iterable 'HTTP_HOST' => 'roadrunner.dev:8080', ], ]; + + yield [ + new Request(headers: [111 => ['numeric-header-name']]), + [ + 'REQUEST_URI' => 'http://localhost', + 'REMOTE_ADDR' => '127.0.0.1', + 'REQUEST_METHOD' => 'GET', + 'HTTP_USER_AGENT' => '', + 'HTTP_111' => 'numeric-header-name', + 'HTTP_HOST' => 'localhost', + ], + ]; } /** diff --git a/tests/Unit/HttpWorkerTest.php b/tests/Unit/HttpWorkerTest.php index ee3eff7..3b34666 100644 --- a/tests/Unit/HttpWorkerTest.php +++ b/tests/Unit/HttpWorkerTest.php @@ -53,11 +53,7 @@ public static function requestDataProvider(): \Traversable \array_merge(self::REQUIRED_PAYLOAD_DATA, [ 'headers' => [ 'Content-Type' => ['application/x-www-form-urlencoded'], - // A purely-numeric header name (e.g. "111") is a - // valid RFC 9110 token; PHP itself coerces it into - // an int array key on the way in, which is why this - // arrives as int(111) rather than the string "111". - // filterHeaders() must recover it, not drop it. + // A numeric header name decodes to an int key and must survive. 111 => ['numeric-header-name'], '' => ['invalid-empty-string-key'], ], @@ -65,11 +61,6 @@ public static function requestDataProvider(): \Traversable \array_merge(self::REQUIRED_REQUEST_DATA, [ 'headers' => [ 'Content-Type' => ['application/x-www-form-urlencoded'], - // Written as an int key on purpose: PHP coerces a - // canonical-integer string key back to int the moment - // it's used as an array key, so filterHeaders() cannot - // hand back a string "111" here — only preserve the - // header instead of dropping it. See HttpWorker.php. 111 => ['numeric-header-name'], ], ]), diff --git a/tests/Unit/PSR7WorkerTest.php b/tests/Unit/PSR7WorkerTest.php index 20c4dc8..c33306b 100644 --- a/tests/Unit/PSR7WorkerTest.php +++ b/tests/Unit/PSR7WorkerTest.php @@ -62,11 +62,7 @@ public function testStateServerLeak(): void ], ], [ - // A purely-numeric header name arrives as an int array - // key (see HeadersList in Request.php). Both withHeader() - // in PSR7Worker and the $_SERVER key building in - // GlobalState must cast it back to string themselves, or - // this throws a TypeError under strict_types. + // A numeric header name decodes to an int key. [ 'Content-Type' => ['application/json'], 111 => ['numeric-header-name'],