Merge pull request #442 from danizord/hotfix/whoops-json-flags

Respect JsonResponseHandler flags when false
zendframework · Feb 13, 2017 · baed41d · baed41d
2 parents 036e0d0 + f2b3440
commit baed41d
Show file tree

Hide file tree

Showing 2 changed files with 45 additions and 12 deletions.
diff --git a/src/Container/WhoopsFactory.php b/src/Container/WhoopsFactory.php
@@ -75,11 +75,11 @@ private function registerJsonHandler(Whoops $whoops, $config)
 
         $handler = new JsonResponseHandler();
 
-        if (isset($config['json_exceptions']['show_trace'])) {
+        if (isset($config['json_exceptions']['show_trace']) && true === $config['json_exceptions']['show_trace']) {
             $handler->addTraceToOutput(true);
         }
 
-        if (isset($config['json_exceptions']['ajax_only'])) {
+        if (isset($config['json_exceptions']['ajax_only']) && true === $config['json_exceptions']['ajax_only']) {
             if (method_exists(\Whoops\Util\Misc::class, 'isAjaxRequest')) {
                 // Whoops 2.x
                 if (! \Whoops\Util\Misc::isAjaxRequest()) {

diff --git a/test/Container/WhoopsFactoryTest.php b/test/Container/WhoopsFactoryTest.php
@@ -12,6 +12,7 @@
 use PHPUnit_Framework_TestCase as TestCase;
 use Prophecy\Prophecy\ObjectProphecy;
 use ReflectionProperty;
+use Traversable;
 use Whoops\Handler\JsonResponseHandler;
 use Whoops\Handler\PrettyPageHandler;
 use Whoops\Run as Whoops;
@@ -76,33 +77,65 @@ public function testWillInjectJsonResponseHandlerIfConfigurationExpectsIt()
     }
 
     /**
-     * @depends testWillInjectJsonResponseHandlerIfConfigurationExpectsIt
+     * @depends      testWillInjectJsonResponseHandlerIfConfigurationExpectsIt
+     * @dataProvider provideConfig
+     *
+     * @param bool  $showsTrace
+     * @param bool  $isAjaxOnly
+     * @param bool  $requestIsAjax
      */
-    public function testJsonResponseHandlerCanBeConfigured()
+    public function testJsonResponseHandlerCanBeConfigured($showsTrace, $isAjaxOnly, $requestIsAjax)
     {
         // Set for Whoops 2.x json handler detection
-        $_SERVER['HTTP_X_REQUESTED_WITH'] = 'xmlhttprequest';
+        if ($requestIsAjax) {
+            $_SERVER['HTTP_X_REQUESTED_WITH'] = 'xmlhttprequest';
+        }
 
         $config = [
             'whoops' => [
                 'json_exceptions' => [
                     'display'    => true,
-                    'show_trace' => true,
-                    'ajax_only'  => true,
+                    'show_trace' => $showsTrace,
+                    'ajax_only'  => $isAjaxOnly,
                 ],
             ],
         ];
+
         $this->injectServiceInContainer($this->container, 'config', $config);
 
         $factory = $this->factory;
         $whoops  = $factory($this->container->reveal());
+        $handler = $whoops->popHandler();
+
+        // If ajax only, not ajax request and Whoops 2, it does not inject JsonResponseHandler
+        if ($isAjaxOnly
+            && ! $requestIsAjax
+            && method_exists(\Whoops\Util\Misc::class, 'isAjaxRequest')
+        ) {
+            $this->assertInstanceOf(PrettyPageHandler::class, $handler);
 
-        $jsonHandler = $whoops->popHandler();
-        $this->assertInstanceOf(JsonResponseHandler::class, $jsonHandler);
-        $this->assertAttributeSame(true, 'returnFrames', $jsonHandler);
+            // Skip remaining assertions
+            return;
+        }
+
+        $this->assertAttributeSame($showsTrace, 'returnFrames', $handler);
 
-        if (method_exists($jsonHandler, 'onlyForAjaxRequests')) {
-            $this->assertAttributeSame(true, 'onlyForAjaxRequests', $jsonHandler);
+        if (method_exists($handler, 'onlyForAjaxRequests')) {
+            $this->assertAttributeSame($isAjaxOnly, 'onlyForAjaxRequests', $handler);
         }
     }
+
+    /**
+     * @return Traversable
+     */
+    public function provideConfig()
+    {
+        yield 'Shows trace' => [true, true, true];
+        yield 'Does not show trace' => [false, true, true];
+
+        yield 'Ajax only, request is ajax' => [true, true, true];
+        yield 'Ajax only, request is not ajax' => [true, true, false];
+
+        yield 'Not ajax only' => [true, false, false];
+    }
 }