bug #14478 [DebugBundle] Fix dump() output in API / No-Toolbar context (nicolas-grekas)

This PR was merged into the 2.6 branch. Discussion ---------- [DebugBundle] Fix dump() output in API / No-Toolbar context | Q | A | ------------- | --- | Bug fix? | yes | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | - | License | MIT | Doc PR | - With this PR, dumps are written as plain text when the main content-type is not HTML. With HTML, dumps are inlined when the web debug toolbar is not enabled. Commits ------- 62f8469 [DebugBundle] Fix dump() output in API/No-Toolbar context
2015-05-05 03:50:03 +02:00 · 2015-05-05 03:50:03 +02:00 · c4867485d7
parent b11ea0a553 62f8469f25
commit c4867485d7
6 changed files with 160 additions and 28 deletions
--- a/src/Symfony/Bundle/DebugBundle/DependencyInjection/Compiler/DumpDataCollectorPass.php
+++ b/src/Symfony/Bundle/DebugBundle/DependencyInjection/Compiler/DumpDataCollectorPass.php
@ -11,6 +11,7 @@

 namespace Symfony\Bundle\DebugBundle\DependencyInjection\Compiler;

+use Symfony\Bundle\WebProfilerBundle\EventListener\WebDebugToolbarListener;
 use Symfony\Component\DependencyInjection\Compiler\CompilerPassInterface;
 use Symfony\Component\DependencyInjection\ContainerBuilder;

@ -35,5 +36,9 @@ class DumpDataCollectorPass implements CompilerPassInterface
        if ($container->hasParameter('templating.helper.code.file_link_format')) {
            $definition->replaceArgument(1, $container->getParameter('templating.helper.code.file_link_format'));
        }
+
+        if (!$container->hasParameter('web_profiler.debug_toolbar.mode') || WebDebugToolbarListener::DISABLED === $container->getParameter('web_profiler.debug_toolbar.mode')) {
+            $definition->replaceArgument(3, null);
+        }
    }
 }
--- a/src/Symfony/Bundle/DebugBundle/Resources/config/services.xml
+++ b/src/Symfony/Bundle/DebugBundle/Resources/config/services.xml
@ -15,6 +15,7 @@
            <argument type="service" id="debug.stopwatch" on-invalid="ignore" />
            <argument>null</argument><!-- %templating.helper.code.file_link_format% -->
            <argument>%kernel.charset%</argument>
+            <argument type="service" id="request_stack" />
        </service>

        <service id="debug.dump_listener" class="Symfony\Component\HttpKernel\EventListener\DumpListener">
--- a/src/Symfony/Bundle/DebugBundle/Tests/DependencyInjection/Compiler/DumpDataCollectorPassTest.php
+++ b/src/Symfony/Bundle/DebugBundle/Tests/DependencyInjection/Compiler/DumpDataCollectorPassTest.php
@ -12,8 +12,10 @@
 namespace Symfony\Bundle\DebugBundle\Tests\DependencyInjection\Compiler;

 use Symfony\Bundle\DebugBundle\DependencyInjection\Compiler\DumpDataCollectorPass;
+use Symfony\Bundle\WebProfilerBundle\EventListener\WebDebugToolbarListener;
 use Symfony\Component\DependencyInjection\ContainerBuilder;
 use Symfony\Component\DependencyInjection\Definition;
+use Symfony\Component\HttpFoundation\RequestStack;

 class DumpDataCollectorPassTest extends \PHPUnit_Framework_TestCase
 {
@ -23,7 +25,7 @@ class DumpDataCollectorPassTest extends \PHPUnit_Framework_TestCase
        $container->addCompilerPass(new DumpDataCollectorPass());
        $container->setParameter('templating.helper.code.file_link_format', 'file-link-format');

-        $definition = new Definition('Symfony\Component\HttpKernel\DataCollector\DumpDataCollector', array(null, null));
+        $definition = new Definition('Symfony\Component\HttpKernel\DataCollector\DumpDataCollector', array(null, null, null, null));
        $container->setDefinition('data_collector.dump', $definition);

        $container->compile();
@ -36,11 +38,53 @@ class DumpDataCollectorPassTest extends \PHPUnit_Framework_TestCase
        $container = new ContainerBuilder();
        $container->addCompilerPass(new DumpDataCollectorPass());

-        $definition = new Definition('Symfony\Component\HttpKernel\DataCollector\DumpDataCollector', array(null, null));
+        $definition = new Definition('Symfony\Component\HttpKernel\DataCollector\DumpDataCollector', array(null, null, null, null));
        $container->setDefinition('data_collector.dump', $definition);

        $container->compile();

        $this->assertNull($definition->getArgument(1));
    }
+
+    public function testProcessWithToolbarEnabled()
+    {
+        $container = new ContainerBuilder();
+        $container->addCompilerPass(new DumpDataCollectorPass());
+        $requestStack = new RequestStack();
+
+        $definition = new Definition('Symfony\Component\HttpKernel\DataCollector\DumpDataCollector', array(null, null, null, $requestStack));
+        $container->setDefinition('data_collector.dump', $definition);
+        $container->setParameter('web_profiler.debug_toolbar.mode', WebDebugToolbarListener::ENABLED);
+
+        $container->compile();
+
+        $this->assertSame($requestStack, $definition->getArgument(3));
+    }
+
+    public function testProcessWithToolbarDisabled()
+    {
+        $container = new ContainerBuilder();
+        $container->addCompilerPass(new DumpDataCollectorPass());
+
+        $definition = new Definition('Symfony\Component\HttpKernel\DataCollector\DumpDataCollector', array(null, null, null, new RequestStack()));
+        $container->setDefinition('data_collector.dump', $definition);
+        $container->setParameter('web_profiler.debug_toolbar.mode', WebDebugToolbarListener::DISABLED);
+
+        $container->compile();
+
+        $this->assertNull($definition->getArgument(3));
+    }
+
+    public function testProcessWithoutToolbar()
+    {
+        $container = new ContainerBuilder();
+        $container->addCompilerPass(new DumpDataCollectorPass());
+
+        $definition = new Definition('Symfony\Component\HttpKernel\DataCollector\DumpDataCollector', array(null, null, null, new RequestStack()));
+        $container->setDefinition('data_collector.dump', $definition);
+
+        $container->compile();
+
+        $this->assertNull($definition->getArgument(3));
+    }
 }
--- a/src/Symfony/Bundle/DebugBundle/composer.json
+++ b/src/Symfony/Bundle/DebugBundle/composer.json
@ -24,7 +24,8 @@
    "require-dev": {
        "symfony/phpunit-bridge": "~2.7",
        "symfony/config": "~2.3",
-        "symfony/dependency-injection": "~2.3"
+        "symfony/dependency-injection": "~2.3",
+        "symfony/web-profiler-bundle": "~2.3"
    },
    "suggest": {
        "symfony/config": "For service container configuration",
--- a/src/Symfony/Component/HttpKernel/DataCollector/DumpDataCollector.php
+++ b/src/Symfony/Component/HttpKernel/DataCollector/DumpDataCollector.php
@ -12,6 +12,7 @@
 namespace Symfony\Component\HttpKernel\DataCollector;

 use Symfony\Component\HttpFoundation\Request;
+use Symfony\Component\HttpFoundation\RequestStack;
 use Symfony\Component\HttpFoundation\Response;
 use Symfony\Component\Stopwatch\Stopwatch;
 use Symfony\Component\VarDumper\Cloner\Data;
@ -33,12 +34,14 @@ class DumpDataCollector extends DataCollector implements DataDumperInterface
    private $clonesIndex = 0;
    private $rootRefs;
    private $charset;
+    private $dumper;

-    public function __construct(Stopwatch $stopwatch = null, $fileLinkFormat = null, $charset = null)
+    public function __construct(Stopwatch $stopwatch = null, $fileLinkFormat = null, $charset = null, RequestStack $requestStack = null)
    {
        $this->stopwatch = $stopwatch;
        $this->fileLinkFormat = $fileLinkFormat ?: ini_get('xdebug.file_link_format') ?: get_cfg_var('xdebug.file_link_format');
        $this->charset = $charset ?: ini_get('php.output_encoding') ?: ini_get('default_charset') ?: 'UTF-8';
+        $this->requestStack = $requestStack;

        // All clones share these properties by reference:
        $this->rootRefs = array(
@ -118,8 +121,12 @@ class DumpDataCollector extends DataCollector implements DataDumperInterface
            $name = substr($name, strrpos($name, '/') + 1);
        }

-        $this->data[] = compact('data', 'name', 'file', 'line', 'fileExcerpt');
-        ++$this->dataCount;
+        if ($this->dumper) {
+            $this->doDump($data, $name, $file, $line);
+        } else {
+            $this->data[] = compact('data', 'name', 'file', 'line', 'fileExcerpt');
+            ++$this->dataCount;
+        }

        if ($this->stopwatch) {
            $this->stopwatch->stop('dump');
@ -128,6 +135,33 @@ class DumpDataCollector extends DataCollector implements DataDumperInterface

    public function collect(Request $request, Response $response, \Exception $exception = null)
    {
+        if ($this->requestStack && $this->requestStack->getMasterRequest() !== $request) {
+            return;
+        }
+
+        // In all conditions that remove the web debug toolbar, dumps are written on the output.
+        if (!$this->requestStack
+            || $request->isXmlHttpRequest()
+            || !$response->headers->has('X-Debug-Token')
+            || $response->isRedirection()
+            || ($response->headers->has('Content-Type') && false === strpos($response->headers->get('Content-Type'), 'html'))
+            || 'html' !== $request->getRequestFormat()
+            || false === strripos($response->getContent(), '</body>')
+        ) {
+            if ($response->headers->has('Content-Type') && false !== strpos($response->headers->get('Content-Type'), 'html')) {
+                $this->dumper = new HtmlDumper('php://output', $this->charset);
+            } else {
+                $this->dumper = new CliDumper('php://output', $this->charset);
+            }
+
+            foreach ($this->data as $i => $dump) {
+                $this->data[$i] = null;
+                $this->doDump($dump['data'], $dump['name'], $dump['file'], $dump['line']);
+            }
+
+            $this->data = array();
+            $this->dataCount = 0;
+        }
    }

    public function serialize()
@ -140,6 +174,7 @@ class DumpDataCollector extends DataCollector implements DataDumperInterface
        $this->data = array();
        $this->dataCount = 0;
        $this->isCollected = true;
+        $this->dumper = null;

        return $ser;
    }
@ -198,31 +233,14 @@ class DumpDataCollector extends DataCollector implements DataDumperInterface
            }

            if ('cli' !== PHP_SAPI && stripos($h[$i], 'html')) {
-                $dumper = new HtmlDumper('php://output', $this->charset);
+                $this->dumper = new HtmlDumper('php://output', $this->charset);
            } else {
-                $dumper = new CliDumper('php://output', $this->charset);
-                $dumper->setColors(false);
+                $this->dumper = new CliDumper('php://output', $this->charset);
            }

            foreach ($this->data as $i => $dump) {
                $this->data[$i] = null;
-
-                if ($dumper instanceof HtmlDumper) {
-                    $dump['name'] = $this->htmlEncode($dump['name']);
-                    $dump['file'] = $this->htmlEncode($dump['file']);
-                    if ('' !== $dump['file']) {
-                        if ($this->fileLinkFormat) {
-                            $link = strtr($this->fileLinkFormat, array('%f' => $dump['file'], '%l' => $dump['line']));
-                            $dump['name'] = sprintf('<a href="%s" title="%s">%s</a>', $link, $dump['file'], $dump['name']);
-                        } else {
-                            $dump['name'] = sprintf('<abbr title="%s">%s</abbr>', $dump['file'], $dump['name']);
-                        }
-                    }
-                    echo "\n<span class=\"sf-dump-meta\">{$dump['name']} on line {$dump['line']}:</span>";
-                } else {
-                    echo "{$dump['name']} on line {$dump['line']}:\n";
-                }
-                $dumper->dump($dump['data']);
+                $this->doDump($dump['data'], $dump['name'], $dump['file'], $dump['line']);
            }

            $this->data = array();
@ -230,6 +248,26 @@ class DumpDataCollector extends DataCollector implements DataDumperInterface
        }
    }

+    private function doDump($data, $name, $file, $line)
+    {
+        if ($this->dumper instanceof HtmlDumper) {
+            $name = $this->htmlEncode($name);
+            $file = $this->htmlEncode($file);
+            if ('' !== $file) {
+                if ($this->fileLinkFormat) {
+                    $link = strtr($this->fileLinkFormat, array('%f' => $file, '%l' => $line));
+                    $name = sprintf('<a href="%s" title="%s">%s</a>', $link, $file, $name);
+                } else {
+                    $name = sprintf('<abbr title="%s">%s</abbr>', $file, $name);
+                }
+            }
+            echo "\n<span class=\"sf-dump-meta\">{$name} on line {$line}:</span>";
+        } else {
+            echo "{$name} on line {$line}:\n";
+        }
+        $this->dumper->dump($data);
+    }
+
    private function htmlEncode($s)
    {
        $html = '';
--- a/src/Symfony/Component/HttpKernel/Tests/DataCollector/DumpDataCollectorTest.php
+++ b/src/Symfony/Component/HttpKernel/Tests/DataCollector/DumpDataCollectorTest.php
@ -12,11 +12,11 @@
 namespace Symfony\Component\HttpKernel\Tests\DataCollector;

 use Symfony\Component\HttpKernel\DataCollector\DumpDataCollector;
+use Symfony\Component\HttpFoundation\Request;
+use Symfony\Component\HttpFoundation\Response;
 use Symfony\Component\VarDumper\Cloner\Data;

 /**
- * DumpDataCollectorTest
- *
 * @author Nicolas Grekas <p@tchwork.com>
 */
 class DumpDataCollectorTest extends \PHPUnit_Framework_TestCase
@ -58,6 +58,49 @@ class DumpDataCollectorTest extends \PHPUnit_Framework_TestCase
        $this->assertSame('a:0:{}', $collector->serialize());
    }

+    public function testCollectDefault()
+    {
+        $data = new Data(array(array(123)));
+
+        $collector = new DumpDataCollector();
+
+        $collector->dump($data);
+        $line = __LINE__ - 1;
+
+        ob_start();
+        $collector->collect(new Request(), new Response());
+        $output = ob_get_clean();
+
+        $this->assertSame("DumpDataCollectorTest.php on line {$line}:\n123\n", $output);
+    }
+
+    public function testCollectHtml()
+    {
+        $data = new Data(array(array(123)));
+
+        $collector = new DumpDataCollector(null, 'test://%f:%l');
+
+        $collector->dump($data);
+        $line = __LINE__ - 1;
+        $file = __FILE__;
+        $xOutput = <<<EOTXT
+
+<span class="sf-dump-meta"><a href="test://{$file}:{$line}" title="{$file}">DumpDataCollectorTest.php</a> on line {$line}:</span> <pre class=sf-dump id=sf-dump data-indent-pad="  "><span class=sf-dump-num>123</span>
+</pre>
+
+EOTXT;
+
+        ob_start();
+        $response = new Response();
+        $response->headers->set('Content-Type', 'text/html');
+        $collector->collect(new Request(), $response);
+        $output = ob_get_clean();
+        $output = preg_replace('#<(script|style).*?</\1>#s', '', $output);
+        $output = preg_replace('/sf-dump-\d+/', 'sf-dump', $output);
+
+        $this->assertSame($xOutput, $output);
+    }
+
    public function testFlush()
    {
        $data = new Data(array(array(456)));