diff --git a/CHANGELOG.md b/CHANGELOG.md index 19ae1ea..a8068c7 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 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 ### Fixed diff --git a/src/RequestParser.php b/src/RequestParser.php index 450e551..da25171 100644 --- a/src/RequestParser.php +++ b/src/RequestParser.php @@ -65,20 +65,8 @@ protected function bodyParams(Request $request): array $bodyParams = $request->post(); - if (is_array($bodyParams) && count($bodyParams) > 0) { - if (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()) { @@ -158,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 a non-null 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 1a91f9c..a705d69 100644 --- a/tests/Unit/RequestParserTest.php +++ b/tests/Unit/RequestParserTest.php @@ -207,6 +207,57 @@ public function testInvalidJson(): void $parser->parseRequest($request); } + public function testObjectWithNumericKeyIsNotABatch(): void + { + $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 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( @@ -385,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(