From c01e90c637b9e7e6414c3f5857a112374db77bb3 Mon Sep 17 00:00:00 2001 From: Michael Date: Mon, 20 Mar 2023 10:01:56 +0100 Subject: [PATCH] Rely on `$request->input()` instead of manually json decoding request body (#12) --- .gitignore | 1 + CHANGELOG.md | 2 + src/RequestParser.php | 90 +++++++++++++++----------------- tests/Unit/RequestParserTest.php | 6 ++- 4 files changed, 50 insertions(+), 49 deletions(-) diff --git a/.gitignore b/.gitignore index ec59755..2de1ed0 100644 --- a/.gitignore +++ b/.gitignore @@ -3,3 +3,4 @@ /.phpunit.result.cache /.php-cs-fixer.cache /composer.lock +/.idea diff --git a/CHANGELOG.md b/CHANGELOG.md index 2dc745c..c448308 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## Unreleased +- Rely on `$request->input()` instead of manually json decoding request body https://github.com/laragraph/utils/pull/12 + ## v1.6.0 ### Added diff --git a/src/RequestParser.php b/src/RequestParser.php index 4f0a900..05c1118 100644 --- a/src/RequestParser.php +++ b/src/RequestParser.php @@ -7,7 +7,8 @@ use Illuminate\Http\Request; use Illuminate\Support\Arr; use Illuminate\Support\Str; -use Safe\Exceptions\JsonException; + +use function Safe\json_decode; /** * Follows https://github.com/graphql/graphql-over-http/blob/main/spec/GraphQLOverHTTP.md. @@ -27,6 +28,7 @@ public function __construct() /** * Converts an incoming HTTP request to one or more OperationParams. * + * @throws \GraphQL\Server\RequestError * @throws \Laragraph\Utils\BadRequestGraphQLException * * @return \GraphQL\Server\OperationParams|array @@ -34,47 +36,12 @@ public function __construct() public function parseRequest(Request $request) { $method = $request->getMethod(); - $bodyParams = []; - /** @var array $queryParams */ + $bodyParams = 'POST' === $method + ? $this->bodyParams($request) + : []; + /** @var array $queryParams Laravel type is not precise enough */ $queryParams = $request->query(); - if ('POST' === $method) { - /** - * Never null, since Symfony defaults to application/x-www-form-urlencoded. - * - * @var string $contentType - */ - $contentType = $request->header('Content-Type'); - - if (Str::startsWith($contentType, ['application/json', 'application/graphql+json'])) { - /** @var string $content */ - $content = $request->getContent(); - try { - $bodyParams = \Safe\json_decode($content, true); - } catch (JsonException $e) { - throw new BadRequestGraphQLException("Invalid JSON: {$e->getMessage()}"); - } - - if (! is_array($bodyParams)) { - throw new BadRequestGraphQLException( - 'GraphQL Server expects JSON object or array, but got ' - . Utils::printSafeJson($bodyParams) - ); - } - } elseif (Str::startsWith($contentType, 'application/graphql')) { - /** @var string $content */ - $content = $request->getContent(); - $bodyParams = ['query' => $content]; - } elseif (Str::startsWith($contentType, 'application/x-www-form-urlencoded')) { - /** @var array $bodyParams */ - $bodyParams = $request->post(); - } elseif (Str::startsWith($contentType, 'multipart/form-data')) { - $bodyParams = $this->inlineFiles($request); - } else { - throw new BadRequestGraphQLException('Unexpected content type: ' . Utils::printSafeJson($contentType)); - } - } - return $this->helper->parseRequestParams($method, $bodyParams, $queryParams); } @@ -88,24 +55,20 @@ protected function inlineFiles(Request $request): array /** @var string|null $mapParam */ $mapParam = $request->post('map'); if (null === $mapParam) { - throw new BadRequestGraphQLException( - 'Could not find a valid map, be sure to conform to GraphQL multipart request specification: https://github.com/jaydenseric/graphql-multipart-request-spec' - ); + throw new BadRequestGraphQLException('Could not find a valid map, be sure to conform to GraphQL multipart request specification: https://github.com/jaydenseric/graphql-multipart-request-spec'); } /** @var string|null $operationsParam */ $operationsParam = $request->post('operations'); if (null === $operationsParam) { - throw new BadRequestGraphQLException( - 'Could not find valid operations, be sure to conform to GraphQL multipart request specification: https://github.com/jaydenseric/graphql-multipart-request-spec' - ); + throw new BadRequestGraphQLException('Could not find valid operations, be sure to conform to GraphQL multipart request specification: https://github.com/jaydenseric/graphql-multipart-request-spec'); } /** @var array|array> $operations */ - $operations = \Safe\json_decode($operationsParam, true); + $operations = json_decode($operationsParam, true); /** @var array> $map */ - $map = \Safe\json_decode($mapParam, true); + $map = json_decode($mapParam, true); foreach ($map as $fileKey => $operationsPaths) { /** @var array $operationsPaths */ @@ -118,4 +81,35 @@ protected function inlineFiles(Request $request): array return $operations; } + + /** + * Extracts the body parameters from the request. + * + * @return array + */ + protected function bodyParams(Request $request): array + { + $contentType = $request->header('Content-Type'); + assert(is_string($contentType), 'Never null, since Symfony defaults to application/x-www-form-urlencoded.'); + + if (Str::startsWith($contentType, 'multipart/form-data')) { + return $this->inlineFiles($request); + } + + $bodyParams = $request->input(); + + if (is_array($bodyParams) && Arr::isAssoc($bodyParams)) { + return $bodyParams; + } + + if (Str::startsWith($contentType, 'application/graphql')) { + return ['query' => $request->getContent()]; + } + + if ($request->isJson()) { + throw new BadRequestGraphQLException("GraphQL Server expects JSON object or array, but got: {$request->getContent()}."); + } + + throw new BadRequestGraphQLException('Unexpected content type: ' . Utils::printSafeJson($contentType)); + } } diff --git a/tests/Unit/RequestParserTest.php b/tests/Unit/RequestParserTest.php index 19b9f6a..fa8e961 100644 --- a/tests/Unit/RequestParserTest.php +++ b/tests/Unit/RequestParserTest.php @@ -136,6 +136,7 @@ public function testNonsensicalContentTypes(string $contentType): void $parser = new RequestParser(); $this->expectException(BadRequestGraphQLException::class); + $this->expectExceptionMessage('Unexpected content type: "' . $contentType . '"'); $parser->parseRequest($request); } @@ -148,7 +149,6 @@ public function nonsensicalContentTypes(): iterable yield ['application/foobar']; yield ['application/josn']; yield ['application/grapql']; - yield ['application/foo;charset=application/json']; } public function testNoQuery(): void @@ -172,6 +172,8 @@ public function testInvalidJson(): void $parser = new RequestParser(); $this->expectException(BadRequestGraphQLException::class); + $this->expectExceptionMessage('GraphQL Server expects JSON object or array, but got: this is not valid json'); + $parser->parseRequest($request); } @@ -187,6 +189,8 @@ public function testNonArrayJson(): void $parser = new RequestParser(); $this->expectException(BadRequestGraphQLException::class); + $this->expectExceptionMessage('GraphQL Server expects JSON object or array, but got: "this should be a map with query, variables, etc."'); + $parser->parseRequest($request); }