From 8dfef785340a16f5151daea25598dc1c99c562f1 Mon Sep 17 00:00:00 2001
From: Sam Stenvall <sam.stenvall@nordsoftware.com>
Date: Tue, 6 Mar 2018 17:21:33 +0200
Subject: [PATCH 1/2] Handle more JSON errors gracefully when
 JSON_PARTIAL_OUTPUT_ON_ERROR  is set

It's fairly common for stack traces to be circular, i.e. they can't be encoded as JSON without limiting the recursion depth somehow. PHP automatically does this if JSON_PARTIAL_OUTPUT_ON_ERROR is true, but we still considered it an error.

By treating JSON_ERROR_RECURSION as okay if JSON_PARTIAL_OUTPUT_ON_ERROR is set we prevent things from blowing up if the user uses a JSON-based exception handler.

Since exceptions thrown in exception handlers are generally a recipe for disaster I added JSON_ERROR_INF_OR_NAN to the list of "acceptable" errors too.
---
 src/Illuminate/Http/JsonResponse.php | 21 ++++++++++++++++++---
 tests/Http/HttpJsonResponseTest.php  | 23 +++++++++++++++++++++++
 2 files changed, 41 insertions(+), 3 deletions(-)

diff --git a/src/Illuminate/Http/JsonResponse.php b/src/Illuminate/Http/JsonResponse.php
index 3e065d8603ab..7bf257b93b37 100755
--- a/src/Illuminate/Http/JsonResponse.php
+++ b/src/Illuminate/Http/JsonResponse.php
@@ -86,9 +86,24 @@ public function setData($data = [])
      */
     protected function hasValidJson($jsonError)
     {
-        return $jsonError === JSON_ERROR_NONE ||
-                ($jsonError === JSON_ERROR_UNSUPPORTED_TYPE &&
-                $this->hasEncodingOption(JSON_PARTIAL_OUTPUT_ON_ERROR));
+        // No error is obviously fine
+        if ($jsonError === JSON_ERROR_NONE) {
+            return true;
+        }
+
+        // If the JSON_PARTIAL_OUTPUT_ON_ERROR option is set, some additional errors are fine
+        // (see https://secure.php.net/manual/en/json.constants.php)
+        if ($this->hasEncodingOption(JSON_PARTIAL_OUTPUT_ON_ERROR)) {
+            $acceptableErrors = [
+                JSON_ERROR_RECURSION,
+                JSON_ERROR_INF_OR_NAN,
+                JSON_ERROR_UNSUPPORTED_TYPE,
+            ];
+
+            return \in_array($jsonError, $acceptableErrors);
+        }
+
+        return false;
     }
 
     /**
diff --git a/tests/Http/HttpJsonResponseTest.php b/tests/Http/HttpJsonResponseTest.php
index 46761942d043..def5310322d4 100644
--- a/tests/Http/HttpJsonResponseTest.php
+++ b/tests/Http/HttpJsonResponseTest.php
@@ -92,6 +92,29 @@ public function testJsonErrorResourceWithPartialOutputOnError()
         $this->assertInstanceOf('stdClass', $data);
         $this->assertNull($data->resource);
     }
+
+    public function testJsonErrorRecursionDetectedWithPartialOutputOnError()
+    {
+        $objectA = new \stdClass();
+        $objectB = new \stdClass();
+        $objectA->b = $objectB;
+        $objectB->a = $objectA;
+
+        $response = new \Illuminate\Http\JsonResponse($objectA, 200, [], JSON_PARTIAL_OUTPUT_ON_ERROR);
+        $data = $response->getData();
+
+        $this->assertNotNull($data);
+    }
+
+    public function testJsonErrorInfOrNanWithPartialOutputOnError()
+    {
+        $data = ['product' => NAN];
+
+        $response = new \Illuminate\Http\JsonResponse($data, 200, [], JSON_PARTIAL_OUTPUT_ON_ERROR);
+        $data = $response->getData();
+
+        $this->assertNotNull($data);
+    }
 }
 
 class JsonResponseTestJsonableObject implements Jsonable

From 01090f79df8df4df285bdb0d040f112c06747cec Mon Sep 17 00:00:00 2001
From: Sam Stenvall <sam.stenvall@nordsoftware.com>
Date: Tue, 6 Mar 2018 17:43:37 +0200
Subject: [PATCH 2/2] Refactor relevant tests to be more comprehensive

---
 tests/Http/HttpJsonResponseTest.php | 58 +++++++++++++++--------------
 1 file changed, 31 insertions(+), 27 deletions(-)

diff --git a/tests/Http/HttpJsonResponseTest.php b/tests/Http/HttpJsonResponseTest.php
index def5310322d4..e310a976c99f 100644
--- a/tests/Http/HttpJsonResponseTest.php
+++ b/tests/Http/HttpJsonResponseTest.php
@@ -75,45 +75,49 @@ public function testSetAndRetrieveStatusCode()
     }
 
     /**
+     * @param mixed $data
+     *
      * @expectedException \InvalidArgumentException
-     * @expectedExceptionMessage Type is not supported
+     *
+     * @dataProvider jsonErrorDataProvider
      */
-    public function testJsonErrorResource()
+    public function testInvalidArgumentExceptionOnJsonError($data)
     {
-        $resource = tmpfile();
-        $response = new \Illuminate\Http\JsonResponse(['resource' => $resource]);
+        new \Illuminate\Http\JsonResponse(['data' => $data]);
     }
 
-    public function testJsonErrorResourceWithPartialOutputOnError()
+    /**
+     * @param mixed $data
+     *
+     * @dataProvider jsonErrorDataProvider
+     */
+    public function testGracefullyHandledSomeJsonErrorsWithPartialOutputOnError($data)
     {
-        $resource = tmpfile();
-        $response = new \Illuminate\Http\JsonResponse(['resource' => $resource], 200, [], JSON_PARTIAL_OUTPUT_ON_ERROR);
-        $data = $response->getData();
-        $this->assertInstanceOf('stdClass', $data);
-        $this->assertNull($data->resource);
+        new \Illuminate\Http\JsonResponse(['data' => $data], 200, [], JSON_PARTIAL_OUTPUT_ON_ERROR);
     }
 
-    public function testJsonErrorRecursionDetectedWithPartialOutputOnError()
+    /**
+     * @return array
+     */
+    public function jsonErrorDataProvider()
     {
-        $objectA = new \stdClass();
-        $objectB = new \stdClass();
-        $objectA->b = $objectB;
-        $objectB->a = $objectA;
-
-        $response = new \Illuminate\Http\JsonResponse($objectA, 200, [], JSON_PARTIAL_OUTPUT_ON_ERROR);
-        $data = $response->getData();
-
-        $this->assertNotNull($data);
-    }
+        // Resources can't be encoded
+        $resource = tmpfile();
 
-    public function testJsonErrorInfOrNanWithPartialOutputOnError()
-    {
-        $data = ['product' => NAN];
+        // Recursion can't be encoded
+        $recursiveObject = new \stdClass();
+        $objectB = new \stdClass();
+        $recursiveObject->b = $objectB;
+        $objectB->a = $recursiveObject;
 
-        $response = new \Illuminate\Http\JsonResponse($data, 200, [], JSON_PARTIAL_OUTPUT_ON_ERROR);
-        $data = $response->getData();
+        // NAN or INF can't be encoded
+        $nan = NAN;
 
-        $this->assertNotNull($data);
+        return [
+            [$resource],
+            [$recursiveObject],
+            [$nan],
+        ];
     }
 }