Handle invalid JSON in API request more gracefully, update tests. Clo…

…ses issue #22.
abivia · Apr 18, 2024 · caac970 · caac970
1 parent 96d1624
commit caac970
Show file tree

Hide file tree

Showing 3 changed files with 45 additions and 5 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1,3 +1,9 @@
+## 1.11.1
+
+### Bugs:
+- Requests with a JSON payload that was not valid JSON caused a PHP error.
+  Now an error response is returned. Thanks to @marvoh for reporting the problem.
+
 ## 1.11.0
 
 Note: there is a new entry in the Ledger config, `session_key_prefix`, which defaults
@@ -37,8 +43,8 @@ entity must have at least one name in any language.
 ### Bugs
 - [Issue 10](https://github.com/abivia/ledger/issues/10) exposed several cases where performing operations
 before the ledger was created resulted in confusing responses instead of an error.
-- [Issue 12](https://github.com/abivia/ledger/issues/12) The sub-journal was bing validated byt not stored
-in the journal entry.
+- [Issue 12](https://github.com/abivia/ledger/issues/12) The sub-journal was being validated but
+not stored in the journal entry.
 
 ### Changes
 - ControllerResultHandler::unexpectedException() returns a generic error string suitable for

diff --git a/src/Messages/Message.php b/src/Messages/Message.php
@@ -115,9 +115,22 @@ public abstract static function fromArray(array $data, int $opFlags = 0): self;
      */
     public static function fromRequest(Request $request, int $opFlags = 0): self
     {
-        $content = json_decode(
-            $request->getContent(), true, 512, JSON_BIGINT_AS_STRING
-        );
+        try {
+            $content = json_decode(
+                $request->getContent(),
+                true,
+                512,
+                JSON_BIGINT_AS_STRING | JSON_THROW_ON_ERROR
+            );
+        } catch (\JsonException $exception) {
+            throw Breaker::withCode(
+                Breaker::BAD_REQUEST,
+                [__(
+                    'Request is not valid JSON: :message',
+                    ['message' => $exception->getMessage()]
+                )]
+            );
+        }
         return static::fromArray($content, $opFlags);
     }
 

diff --git a/tests/Feature/RootTest.php b/tests/Feature/RootTest.php
@@ -30,6 +30,27 @@ public function testBadOperation()
         $this->assertCount(1, $actual->errors);
     }
 
+    public function testBadRequest()
+    {
+        $badJson = 'this is not valid JSON.';
+        $headers = [
+            'CONTENT_LENGTH' => mb_strlen($badJson, '8bit'),
+            'CONTENT_TYPE' => 'application/json',
+            'Accept' => 'application/json',
+        ];
+        $response = $this->call(
+            'POST',
+            'api/ledger/currency/query',
+            [],
+            $this->prepareCookiesForJsonRequest(),
+            [],
+            $this->transformHeadersToServerVars($headers),
+            $badJson
+        );
+        $actual = $this->isFailure($response);
+        $this->assertCount(2, $actual->errors);
+    }
+
     public function testDebug()
     {
         // Ensure debugging is off