From 43b522c37706fa4867dff9c404cac5af4d825c7d Mon Sep 17 00:00:00 2001
From: Benedikt Franke <benedikt.franke@mll.com>
Date: Tue, 26 Apr 2022 17:09:27 +0200
Subject: [PATCH] Consistently throw `BadRequestGraphQLException` on invalid
 requests (#10)

---
 .php-cs-fixer.php                  |  4 ++--
 CHANGELOG.md                       |  9 ++++++++-
 composer.json                      |  5 +++++
 src/BadRequestGraphQLException.php |  9 +++++++++
 src/RequestParser.php              | 23 ++++++++++++-----------
 tests/Unit/RequestParserTest.php   | 19 ++++++++-----------
 6 files changed, 44 insertions(+), 25 deletions(-)
 create mode 100644 src/BadRequestGraphQLException.php

diff --git a/.php-cs-fixer.php b/.php-cs-fixer.php
index 213c898..0efaa95 100644
--- a/.php-cs-fixer.php
+++ b/.php-cs-fixer.php
@@ -1,6 +1,6 @@
 <?php declare(strict_types=1);
 
-use function MLL\PhpCsFixerConfig\config;
+use function MLL\PhpCsFixerConfig\risky;
 
 $finder = PhpCsFixer\Finder::create()
     ->notPath('vendor')
@@ -9,4 +9,4 @@
     ->ignoreDotFiles(true)
     ->ignoreVCS(true);
 
-return config($finder);
+return risky($finder);
diff --git a/CHANGELOG.md b/CHANGELOG.md
index c80a4f9..e10a410 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -2,10 +2,17 @@
 
 All notable changes to this project will be documented in this file.
 
-The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
+The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0),
+and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 
 ## Unreleased
 
+## v1.4.0
+
+### Changed
+
+- Consistently throw `BadRequestGraphQLException` on invalid requests
+
 ## v1.3.0
 
 ### Added
diff --git a/composer.json b/composer.json
index 2b49775..dcbfa35 100644
--- a/composer.json
+++ b/composer.json
@@ -50,6 +50,11 @@
     ]
   },
   "config": {
+    "allow-plugins": {
+      "infection/extension-installer": true,
+      "ergebnis/composer-normalize": true,
+      "phpstan/extension-installer": true
+    },
     "preferred-install": "dist",
     "sort-packages": true
   }
diff --git a/src/BadRequestGraphQLException.php b/src/BadRequestGraphQLException.php
new file mode 100644
index 0000000..d94b8e1
--- /dev/null
+++ b/src/BadRequestGraphQLException.php
@@ -0,0 +1,9 @@
+<?php declare(strict_types=1);
+
+namespace Laragraph\Utils;
+
+use Symfony\Component\HttpKernel\Exception\BadRequestHttpException;
+
+class BadRequestGraphQLException extends BadRequestHttpException
+{
+}
diff --git a/src/RequestParser.php b/src/RequestParser.php
index f108c2f..4f0a900 100644
--- a/src/RequestParser.php
+++ b/src/RequestParser.php
@@ -1,16 +1,13 @@
-<?php
-
-declare(strict_types=1);
+<?php declare(strict_types=1);
 
 namespace Laragraph\Utils;
 
 use GraphQL\Server\Helper;
-use GraphQL\Server\OperationParams;
-use GraphQL\Server\RequestError;
 use GraphQL\Utils\Utils;
 use Illuminate\Http\Request;
 use Illuminate\Support\Arr;
 use Illuminate\Support\Str;
+use Safe\Exceptions\JsonException;
 
 /**
  * Follows https://github.com/graphql/graphql-over-http/blob/main/spec/GraphQLOverHTTP.md.
@@ -30,7 +27,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<int, \GraphQL\Server\OperationParams>
      */
@@ -52,10 +49,14 @@ public function parseRequest(Request $request)
             if (Str::startsWith($contentType, ['application/json', 'application/graphql+json'])) {
                 /** @var string $content */
                 $content = $request->getContent();
-                $bodyParams = \Safe\json_decode($content, true);
+                try {
+                    $bodyParams = \Safe\json_decode($content, true);
+                } catch (JsonException $e) {
+                    throw new BadRequestGraphQLException("Invalid JSON: {$e->getMessage()}");
+                }
 
                 if (! is_array($bodyParams)) {
-                    throw new RequestError(
+                    throw new BadRequestGraphQLException(
                         'GraphQL Server expects JSON object or array, but got '
                         . Utils::printSafeJson($bodyParams)
                     );
@@ -70,7 +71,7 @@ public function parseRequest(Request $request)
             } elseif (Str::startsWith($contentType, 'multipart/form-data')) {
                 $bodyParams = $this->inlineFiles($request);
             } else {
-                throw new RequestError('Unexpected content type: ' . Utils::printSafeJson($contentType));
+                throw new BadRequestGraphQLException('Unexpected content type: ' . Utils::printSafeJson($contentType));
             }
         }
 
@@ -87,7 +88,7 @@ protected function inlineFiles(Request $request): array
         /** @var string|null $mapParam */
         $mapParam = $request->post('map');
         if (null === $mapParam) {
-            throw new RequestError(
+            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'
             );
         }
@@ -95,7 +96,7 @@ protected function inlineFiles(Request $request): array
         /** @var string|null $operationsParam */
         $operationsParam = $request->post('operations');
         if (null === $operationsParam) {
-            throw new RequestError(
+            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'
             );
         }
diff --git a/tests/Unit/RequestParserTest.php b/tests/Unit/RequestParserTest.php
index a52023d..d4a0528 100644
--- a/tests/Unit/RequestParserTest.php
+++ b/tests/Unit/RequestParserTest.php
@@ -1,19 +1,16 @@
-<?php
-
-declare(strict_types=1);
+<?php declare(strict_types=1);
 
 namespace Laragraph\Utils\Tests\Unit;
 
 use GraphQL\Server\OperationParams;
-use GraphQL\Server\RequestError;
 use Illuminate\Http\Request;
 use Illuminate\Http\UploadedFile;
+use Laragraph\Utils\BadRequestGraphQLException;
 use Laragraph\Utils\RequestParser;
 use Orchestra\Testbench\TestCase;
-use Safe\Exceptions\JsonException;
 use Symfony\Component\HttpFoundation\Request as SymfonyRequest;
 
-class RequestParserTest extends TestCase
+final class RequestParserTest extends TestCase
 {
     public function testGetWithQuery(): void
     {
@@ -138,7 +135,7 @@ public function testNonsensicalContentTypes(string $contentType): void
         );
         $parser = new RequestParser();
 
-        $this->expectException(RequestError::class);
+        $this->expectException(BadRequestGraphQLException::class);
         $parser->parseRequest($request);
     }
 
@@ -175,7 +172,7 @@ public function testInvalidJson(): void
         );
         $parser = new RequestParser();
 
-        $this->expectException(JsonException::class);
+        $this->expectException(BadRequestGraphQLException::class);
         $parser->parseRequest($request);
     }
 
@@ -190,7 +187,7 @@ public function testNonArrayJson(): void
         );
         $parser = new RequestParser();
 
-        $this->expectException(RequestError::class);
+        $this->expectException(BadRequestGraphQLException::class);
         $parser->parseRequest($request);
     }
 
@@ -258,7 +255,7 @@ public function testMultipartFormWithoutMap(): void
         );
         $parser = new RequestParser();
 
-        $this->expectException(RequestError::class);
+        $this->expectException(BadRequestGraphQLException::class);
         $parser->parseRequest($request);
     }
 
@@ -281,7 +278,7 @@ public function testMultipartFormWithoutOperations(): void
 
         $parser = new RequestParser();
 
-        $this->expectException(RequestError::class);
+        $this->expectException(BadRequestGraphQLException::class);
         $parser->parseRequest($request);
     }