From 5fe240df7103c51dc7d7fff8a90d26627727ad7d Mon Sep 17 00:00:00 2001 From: Christophe VERGNE Date: Wed, 17 Apr 2024 19:02:08 +0200 Subject: [PATCH 1/6] Add Exception Event Listener --- EventListener/ExceptionListener.php | 41 +++++++++++++++++++ src/Resources/config/container/graphqlite.xml | 4 ++ 2 files changed, 45 insertions(+) create mode 100644 EventListener/ExceptionListener.php diff --git a/EventListener/ExceptionListener.php b/EventListener/ExceptionListener.php new file mode 100644 index 0000000..8041621 --- /dev/null +++ b/EventListener/ExceptionListener.php @@ -0,0 +1,41 @@ +httpCodeDecider = $httpCodeDecider ?? new HttpCodeDecider(); + } + + public function onKernelException(ExceptionEvent $event): void + { + $exception = $event->getThrowable(); + + if ($exception instanceof GraphQLExceptionInterface) { + $result = new ExecutionResult( + errors: [new Error( + message: $exception->getMessage(), + previous: $exception, + extensions: $exception->getExtensions() + )] + ); + + $response = new JsonResponse($result->toArray(), $this->httpCodeDecider->decideHttpStatusCode($result)); + + $event->setResponse($response); + } + } +} diff --git a/src/Resources/config/container/graphqlite.xml b/src/Resources/config/container/graphqlite.xml index 545ec0e..caa09d8 100644 --- a/src/Resources/config/container/graphqlite.xml +++ b/src/Resources/config/container/graphqlite.xml @@ -87,6 +87,10 @@ + + + + From 8611016dad2db145c4cc59c00901afc974d42a66 Mon Sep 17 00:00:00 2001 From: Christophe VERGNE Date: Wed, 17 Apr 2024 19:03:15 +0200 Subject: [PATCH 2/6] Throw GraphQLException in case of JSON syntax error --- Exceptions/JsonException.php | 19 +++++++++++++++++++ src/Controller/GraphQLiteController.php | 24 +++++++++++++++++++----- 2 files changed, 38 insertions(+), 5 deletions(-) create mode 100644 Exceptions/JsonException.php diff --git a/Exceptions/JsonException.php b/Exceptions/JsonException.php new file mode 100644 index 0000000..4b50de4 --- /dev/null +++ b/Exceptions/JsonException.php @@ -0,0 +1,19 @@ + $reason] + ); + } +} diff --git a/src/Controller/GraphQLiteController.php b/src/Controller/GraphQLiteController.php index aaa91fa..28bb261 100644 --- a/src/Controller/GraphQLiteController.php +++ b/src/Controller/GraphQLiteController.php @@ -27,6 +27,7 @@ use function array_map; use function class_exists; use function json_decode; +use TheCodingMachine\GraphQLite\Bundle\Exceptions\JsonException; /** * Listens to every single request and forward Graphql requests to Graphql Webonix standardServer. @@ -80,12 +81,25 @@ public function handleRequest(Request $request): Response if (strtoupper($request->getMethod()) === 'POST' && empty($psr7Request->getParsedBody())) { $content = $psr7Request->getBody()->getContents(); - $parsedBody = json_decode($content, true); - if (json_last_error() !== JSON_ERROR_NONE) { - throw new \RuntimeException('Invalid JSON received in POST body: '.json_last_error_msg()); + try { + $parsedBody = json_decode( + json: $content, + associative: true, + flags: \JSON_THROW_ON_ERROR + ); + } catch (\JsonException $e) { + throw JsonException::create( + reason: $e->getMessage(), + code: Response::HTTP_UNSUPPORTED_MEDIA_TYPE, + previous:$e + ); } - if (!is_array($parsedBody)){ - throw new \RuntimeException('Expecting associative array from request, got ' . gettype($parsedBody)); + + if (!is_array($parsedBody)) { + throw JsonException::create( + reason: 'Expecting associative array from request, got ' . gettype($parsedBody), + code: Response::HTTP_UNPROCESSABLE_ENTITY + ); } $psr7Request = $psr7Request->withParsedBody($parsedBody); } From 03af0fe99e13d1a08ffeaf1c131f1590a96d8d7c Mon Sep 17 00:00:00 2001 From: Christophe VERGNE Date: Wed, 17 Apr 2024 19:25:42 +0200 Subject: [PATCH 3/6] Add tests for JSON Exceptions --- tests/FunctionalTest.php | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/tests/FunctionalTest.php b/tests/FunctionalTest.php index cdc90ed..07848bc 100644 --- a/tests/FunctionalTest.php +++ b/tests/FunctionalTest.php @@ -104,6 +104,28 @@ public function testErrors(): void $kernel = new GraphQLiteTestingKernel(); $kernel->boot(); + $request = Request::create('/graphql', 'POST', [], [], [], ['CONTENT_TYPE' => 'application/json'], '{"query":"{ invalidJsonSyntax }"'); + + $response = $kernel->handle($request); + + $this->assertSame(415, $response->getStatusCode()); + + $result = json_decode($response->getContent(), true); + + $this->assertSame('Invalid JSON.', $result['errors'][0]['message']); + $this->assertSame('Syntax error', $result['errors'][0]['extensions']['reason']); + + $request = Request::create('/graphql', 'POST', [], [], [], ['CONTENT_TYPE' => 'application/json'], '"Unexpected Json Content"'); + + $response = $kernel->handle($request); + + $this->assertSame(422, $response->getStatusCode()); + + $result = json_decode($response->getContent(), true); + + $this->assertSame('Invalid JSON.', $result['errors'][0]['message']); + $this->assertSame('Expecting associative array from request, got string', $result['errors'][0]['extensions']['reason']); + $request = Request::create('/graphql', 'GET', ['query' => ' { notExists From 80cdfbb6114db0b04bb591c005a5752d66168674 Mon Sep 17 00:00:00 2001 From: Andrii Date: Wed, 1 Jan 2025 21:07:40 +0100 Subject: [PATCH 4/6] :package: Refactor to avoid using additional kernel exception listener --- EventListener/ExceptionListener.php | 41 ---------- src/Controller/GraphQLiteController.php | 79 ++++++++++++++----- src/Resources/config/container/graphqlite.xml | 4 - 3 files changed, 60 insertions(+), 64 deletions(-) delete mode 100644 EventListener/ExceptionListener.php diff --git a/EventListener/ExceptionListener.php b/EventListener/ExceptionListener.php deleted file mode 100644 index 8041621..0000000 --- a/EventListener/ExceptionListener.php +++ /dev/null @@ -1,41 +0,0 @@ -httpCodeDecider = $httpCodeDecider ?? new HttpCodeDecider(); - } - - public function onKernelException(ExceptionEvent $event): void - { - $exception = $event->getThrowable(); - - if ($exception instanceof GraphQLExceptionInterface) { - $result = new ExecutionResult( - errors: [new Error( - message: $exception->getMessage(), - previous: $exception, - extensions: $exception->getExtensions() - )] - ); - - $response = new JsonResponse($result->toArray(), $this->httpCodeDecider->decideHttpStatusCode($result)); - - $event->setResponse($response); - } - } -} diff --git a/src/Controller/GraphQLiteController.php b/src/Controller/GraphQLiteController.php index 28bb261..f5fe84c 100644 --- a/src/Controller/GraphQLiteController.php +++ b/src/Controller/GraphQLiteController.php @@ -1,20 +1,21 @@ getMessage(), - code: Response::HTTP_UNSUPPORTED_MEDIA_TYPE, - previous:$e - ); + return $this->invalidJsonBodyResponse($e); } if (!is_array($parsedBody)) { - throw JsonException::create( - reason: 'Expecting associative array from request, got ' . gettype($parsedBody), - code: Response::HTTP_UNPROCESSABLE_ENTITY - ); + return $this->invalidRequestBodyExpectedAssociativeResponse($parsedBody); } + $psr7Request = $psr7Request->withParsedBody($parsedBody); } @@ -139,4 +133,51 @@ private function handlePsr7Request(ServerRequestInterface $request, Request $sym throw new RuntimeException('Only SyncPromiseAdapter is supported'); } + + private function invalidJsonBodyResponse(\JsonException $e): JsonResponse + { + $jsonException = JsonException::create( + reason: $e->getMessage(), + code: Response::HTTP_UNSUPPORTED_MEDIA_TYPE, + previous: $e, + ); + $result = new ExecutionResult( + null, + [ + new Error( + 'Invalid JSON.', + previous: $jsonException, + extensions: $jsonException->getExtensions(), + ), + ] + ); + + return new JsonResponse( + $result->toArray($this->debug), + $this->httpCodeDecider->decideHttpStatusCode($result) + ); + } + + private function invalidRequestBodyExpectedAssociativeResponse(mixed $parsedBody): JsonResponse + { + $jsonException = JsonException::create( + reason: 'Expecting associative array from request, got ' . gettype($parsedBody), + code: Response::HTTP_UNPROCESSABLE_ENTITY, + ); + $result = new ExecutionResult( + null, + [ + new Error( + 'Invalid JSON.', + previous: $jsonException, + extensions: $jsonException->getExtensions(), + ), + ] + ); + + return new JsonResponse( + $result->toArray($this->debug), + $this->httpCodeDecider->decideHttpStatusCode($result) + ); + } } diff --git a/src/Resources/config/container/graphqlite.xml b/src/Resources/config/container/graphqlite.xml index caa09d8..545ec0e 100644 --- a/src/Resources/config/container/graphqlite.xml +++ b/src/Resources/config/container/graphqlite.xml @@ -87,10 +87,6 @@ - - - - From 1c23d87c1140476a1bb71cee52b555ae26e2a326 Mon Sep 17 00:00:00 2001 From: Christophe VERGNE Date: Mon, 6 Jan 2025 15:56:54 +0100 Subject: [PATCH 5/6] Fix Exceptions folder in wrong location --- {Exceptions => src/Exceptions}/JsonException.php | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename {Exceptions => src/Exceptions}/JsonException.php (100%) diff --git a/Exceptions/JsonException.php b/src/Exceptions/JsonException.php similarity index 100% rename from Exceptions/JsonException.php rename to src/Exceptions/JsonException.php From 84e77e9678a3752c04abb0ccfd4375a024a433d4 Mon Sep 17 00:00:00 2001 From: Christophe VERGNE Date: Mon, 6 Jan 2025 16:44:03 +0100 Subject: [PATCH 6/6] Fix implicitly nullable argument in JsonException constructor --- src/Exceptions/JsonException.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Exceptions/JsonException.php b/src/Exceptions/JsonException.php index 4b50de4..a474c8d 100644 --- a/src/Exceptions/JsonException.php +++ b/src/Exceptions/JsonException.php @@ -7,7 +7,7 @@ class JsonException extends GraphQLException { - public static function create(?string $reason = null, int $code = 400, Exception $previous = null): self + public static function create(?string $reason = null, int $code = 400, ?Exception $previous = null): self { return new self( message: 'Invalid JSON.',