From 8fcec23ca89c6e5c70d17cc63fa72dbb7cb22635 Mon Sep 17 00:00:00 2001 From: Pawel Date: Sun, 11 Oct 2026 20:28:21 +0200 Subject: [PATCH 1/5] Reject JSON objects with the key 0 instead of parsing them as a batch RequestParser::bodyParams() accepted any associative array as a single operation. A JSON object such as {"query":"...","0":"..."} decodes to a PHP array with the integer key 0, which Arr::isAssoc() still reports as associative, but which GraphQL\Server\Helper::parseRequestParams() treats as a batch. webonyx then calls OperationParams::create() with the string entries and the request ends in an uncaught TypeError (HTTP 500) instead of a BadRequestGraphQLException (HTTP 400). Use the same rule as webonyx to tell a batch from a single operation: a body with the key 0 is a batch, and every entry of a batch must be an object. Co-Authored-By: Claude Fable 5.1 --- CHANGELOG.md | 4 ++++ src/RequestParser.php | 5 ++++- tests/Unit/RequestParserTest.php | 36 ++++++++++++++++++++++++++++++++ 3 files changed, 44 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 19ae1ea..5dbfd4e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## Unreleased +### Fixed + +- Reject JSON objects that carry the key `0` instead of passing them to webonyx as a batch, where they caused a `TypeError` + ## v2.3.1 ### Fixed diff --git a/src/RequestParser.php b/src/RequestParser.php index 450e551..04305d5 100644 --- a/src/RequestParser.php +++ b/src/RequestParser.php @@ -66,7 +66,10 @@ protected function bodyParams(Request $request): array $bodyParams = $request->post(); if (is_array($bodyParams) && count($bodyParams) > 0) { - if (Arr::isAssoc($bodyParams)) { + // Mirror \GraphQL\Server\Helper::parseRequestParams(), which treats + // any body with the key 0 as a batch - including a JSON object such as + // {"query": "...", "0": "..."}, where Arr::isAssoc() would say otherwise. + if (! isset($bodyParams[0])) { return $bodyParams; } diff --git a/tests/Unit/RequestParserTest.php b/tests/Unit/RequestParserTest.php index 1a91f9c..8b03796 100644 --- a/tests/Unit/RequestParserTest.php +++ b/tests/Unit/RequestParserTest.php @@ -207,6 +207,42 @@ public function testInvalidJson(): void $parser->parseRequest($request); } + public function testObjectWithNumericKeyIsNotABatch(): void + { + // A JSON object that also carries the key "0" decodes to a PHP array with the integer key 0, + // which webonyx takes for a batch and then fails with a TypeError on the string entries. + $request = $this->makeRequest( + 'POST', + [], + [], + ['Content-Type' => 'application/json'], + '{"query":"{ foo }","0":"not an operation"}' + ); + $parser = new RequestParser(); + + $this->expectException(BadRequestGraphQLException::class); + $this->expectExceptionMessage('GraphQL Server expects JSON object or array, but got: {"query":"{ foo }","0":"not an operation"}'); + + $parser->parseRequest($request); + } + + public function testBatchWithNonObjectEntry(): void + { + $request = $this->makeRequest( + 'POST', + [], + [], + ['Content-Type' => 'application/json'], + '[{"query":"{ foo }"},"not an operation"]' + ); + $parser = new RequestParser(); + + $this->expectException(BadRequestGraphQLException::class); + $this->expectExceptionMessage('GraphQL Server expects JSON object or array, but got: [{"query":"{ foo }"},"not an operation"]'); + + $parser->parseRequest($request); + } + public function testNonArrayJson(): void { $request = $this->makeRequest( From e881cf42fa1aa2dfecfc8e603c83ab94de0ea843 Mon Sep 17 00:00:00 2001 From: Benedikt Franke Date: Mon, 12 Oct 2026 14:48:19 +0200 Subject: [PATCH 2/5] Keep rejecting batches that start with null isset() is false for a null value, so [null] skipped the batch entry check. --- src/RequestParser.php | 2 +- tests/Unit/RequestParserTest.php | 17 +++++++++++++++++ 2 files changed, 18 insertions(+), 1 deletion(-) diff --git a/src/RequestParser.php b/src/RequestParser.php index 04305d5..390c2ba 100644 --- a/src/RequestParser.php +++ b/src/RequestParser.php @@ -69,7 +69,7 @@ protected function bodyParams(Request $request): array // Mirror \GraphQL\Server\Helper::parseRequestParams(), which treats // any body with the key 0 as a batch - including a JSON object such as // {"query": "...", "0": "..."}, where Arr::isAssoc() would say otherwise. - if (! isset($bodyParams[0])) { + if (! isset($bodyParams[0]) && Arr::isAssoc($bodyParams)) { return $bodyParams; } diff --git a/tests/Unit/RequestParserTest.php b/tests/Unit/RequestParserTest.php index 8b03796..69f30db 100644 --- a/tests/Unit/RequestParserTest.php +++ b/tests/Unit/RequestParserTest.php @@ -243,6 +243,23 @@ public function testBatchWithNonObjectEntry(): void $parser->parseRequest($request); } + public function testBatchStartingWithNull(): void + { + $request = $this->makeRequest( + 'POST', + [], + [], + ['Content-Type' => 'application/json'], + '[null]' + ); + $parser = new RequestParser(); + + $this->expectException(BadRequestGraphQLException::class); + $this->expectExceptionMessage('GraphQL Server expects JSON object or array, but got: [null]'); + + $parser->parseRequest($request); + } + public function testNonArrayJson(): void { $request = $this->makeRequest( From b698b50e63e2b82fdf1342e8e29bb6fc3c020d9a Mon Sep 17 00:00:00 2001 From: Benedikt Franke Date: Mon, 12 Oct 2026 14:49:21 +0200 Subject: [PATCH 3/5] Shorten comments --- src/RequestParser.php | 4 +--- tests/Unit/RequestParserTest.php | 2 -- 2 files changed, 1 insertion(+), 5 deletions(-) diff --git a/src/RequestParser.php b/src/RequestParser.php index 390c2ba..ec17ac7 100644 --- a/src/RequestParser.php +++ b/src/RequestParser.php @@ -66,9 +66,7 @@ protected function bodyParams(Request $request): array $bodyParams = $request->post(); if (is_array($bodyParams) && count($bodyParams) > 0) { - // Mirror \GraphQL\Server\Helper::parseRequestParams(), which treats - // any body with the key 0 as a batch - including a JSON object such as - // {"query": "...", "0": "..."}, where Arr::isAssoc() would say otherwise. + // \GraphQL\Server\Helper::parseRequestParams() treats any body with the key 0 as a batch if (! isset($bodyParams[0]) && Arr::isAssoc($bodyParams)) { return $bodyParams; } diff --git a/tests/Unit/RequestParserTest.php b/tests/Unit/RequestParserTest.php index 69f30db..89e9147 100644 --- a/tests/Unit/RequestParserTest.php +++ b/tests/Unit/RequestParserTest.php @@ -209,8 +209,6 @@ public function testInvalidJson(): void public function testObjectWithNumericKeyIsNotABatch(): void { - // A JSON object that also carries the key "0" decodes to a PHP array with the integer key 0, - // which webonyx takes for a batch and then fails with a TypeError on the string entries. $request = $this->makeRequest( 'POST', [], From 85146db723aefb6c89deeaca130fea29cadbaecc Mon Sep 17 00:00:00 2001 From: Benedikt Franke Date: Mon, 12 Oct 2026 15:02:27 +0200 Subject: [PATCH 4/5] Reject malformed multipart operations MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit inlineFiles() skipped the batch shape check, so multipart operations such as {"query":"…","0":"x"} still caused a TypeError in webonyx. --- CHANGELOG.md | 2 +- src/RequestParser.php | 38 +++++++++++-------- tests/Unit/RequestParserTest.php | 64 ++++++++++++++++++++++++++++++++ 3 files changed, 88 insertions(+), 16 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5dbfd4e..9f497f8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,7 +11,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed -- Reject JSON objects that carry the key `0` instead of passing them to webonyx as a batch, where they caused a `TypeError` +- Reject JSON objects that carry the key `0` and malformed multipart `operations` instead of passing them to webonyx as a batch, where they caused a `TypeError` ## v2.3.1 diff --git a/src/RequestParser.php b/src/RequestParser.php index ec17ac7..3271d48 100644 --- a/src/RequestParser.php +++ b/src/RequestParser.php @@ -65,21 +65,8 @@ protected function bodyParams(Request $request): array $bodyParams = $request->post(); - if (is_array($bodyParams) && count($bodyParams) > 0) { - // \GraphQL\Server\Helper::parseRequestParams() treats any body with the key 0 as a batch - if (! isset($bodyParams[0]) && Arr::isAssoc($bodyParams)) { - return $bodyParams; - } - - $allAssoc = true; - foreach ($bodyParams as $bodyParam) { - if (! is_array($bodyParam) || ! Arr::isAssoc($bodyParam)) { - $allAssoc = false; - } - } - if ($allAssoc) { - return $bodyParams; - } + if (is_array($bodyParams) && count($bodyParams) > 0 && $this->isOperationOrBatch($bodyParams)) { + return $bodyParams; } if ($request->isJson()) { @@ -159,6 +146,27 @@ protected function inlineFiles(Request $request): array } } + if (! $this->isOperationOrBatch($operations)) { + throw new BadMultipartRequestGraphQLException('Expected parameter operations to be an object or a list of objects.'); + } + return $operations; } + + /** @param array $params */ + protected function isOperationOrBatch(array $params): bool + { + // \GraphQL\Server\Helper::parseRequestParams() treats any body with the key 0 as a batch + if (! isset($params[0]) && Arr::isAssoc($params)) { + return true; + } + + foreach ($params as $operation) { + if (! is_array($operation) || ! Arr::isAssoc($operation)) { + return false; + } + } + + return true; + } } diff --git a/tests/Unit/RequestParserTest.php b/tests/Unit/RequestParserTest.php index 89e9147..a705d69 100644 --- a/tests/Unit/RequestParserTest.php +++ b/tests/Unit/RequestParserTest.php @@ -436,6 +436,70 @@ public function testMultipartFormWithNonArrayOperations(): void $parser->parseRequest($request); } + public function testMultipartFormWithOperationsObjectWithNumericKey(): void + { + $request = $this->makeRequest( + 'POST', + [ + 'operations' => /** @lang JSON */ '{"query":"{ foo }","0":"not an operation"}', + 'map' => /** @lang JSON */ '{}', + ], + [], + [ + 'Content-Type' => 'multipart/form-data', + ] + ); + + $parser = new RequestParser(); + + $this->expectException(BadMultipartRequestGraphQLException::class); + $parser->parseRequest($request); + } + + public function testMultipartFormWithBatchWithNonObjectEntry(): void + { + $request = $this->makeRequest( + 'POST', + [ + 'operations' => /** @lang JSON */ '[{"query":"{ foo }"},"not an operation"]', + 'map' => /** @lang JSON */ '{}', + ], + [], + [ + 'Content-Type' => 'multipart/form-data', + ] + ); + + $parser = new RequestParser(); + + $this->expectException(BadMultipartRequestGraphQLException::class); + $parser->parseRequest($request); + } + + public function testMultipartFormWithMapToNumericKey(): void + { + $file = UploadedFile::fake()->create('image.jpg', 500); + + $request = $this->makeRequest( + 'POST', + [ + 'operations' => /** @lang JSON */ '{"query":"{ foo }"}', + 'map' => /** @lang JSON */ '{"file":["0"]}', + ], + [ + 'file' => $file, + ], + [ + 'Content-Type' => 'multipart/form-data', + ] + ); + + $parser = new RequestParser(); + + $this->expectException(BadMultipartRequestGraphQLException::class); + $parser->parseRequest($request); + } + public function testMultipartFormWithoutMap(): void { $request = $this->makeRequest( From fbfffa8fc692cd415e5d9e4adf1794cce9f2c759 Mon Sep 17 00:00:00 2001 From: Benedikt Franke Date: Mon, 12 Oct 2026 15:15:17 +0200 Subject: [PATCH 5/5] Describe batch detection precisely --- CHANGELOG.md | 2 +- src/RequestParser.php | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9f497f8..a8068c7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,7 +11,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed -- Reject JSON objects that carry the key `0` and malformed multipart `operations` instead of passing them to webonyx as a batch, where they caused a `TypeError` +- Reject JSON and multipart bodies that webonyx parses as a batch with non-object entries, such as `{"query":"…","0":"x"}`, instead of letting them cause a `TypeError` ## v2.3.1 diff --git a/src/RequestParser.php b/src/RequestParser.php index 3271d48..da25171 100644 --- a/src/RequestParser.php +++ b/src/RequestParser.php @@ -156,7 +156,7 @@ protected function inlineFiles(Request $request): array /** @param array $params */ protected function isOperationOrBatch(array $params): bool { - // \GraphQL\Server\Helper::parseRequestParams() treats any body with the key 0 as a batch + // \GraphQL\Server\Helper::parseRequestParams() treats any body with a non-null key 0 as a batch if (! isset($params[0]) && Arr::isAssoc($params)) { return true; }