Skip to content

[Draft][WebProfiler] Removed intercept_redirects config option #33607

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from
Closed

[Draft][WebProfiler] Removed intercept_redirects config option #33607

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
[WebProfiler] Removed intercept_redirects config option
  • Loading branch information
@dorumd
dorumd committed Sep 17, 2019
commit 32ceea69b06573a238f4adb44bbf352d2c7d2c1c
1 change: 0 additions & 1 deletion src/Symfony/Bundle/WebProfilerBundle/DependencyInjection/Configuration.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,6 @@ public function getConfigTreeBuilder()
$treeBuilder->getRootNode()
->children()
->booleanNode('toolbar')->defaultFalse()->end()
->booleanNode('intercept_redirects')->defaultFalse()->setDeprecated('The "intercept_redirects" option is deprecated since version 4.4 and will be removed in 5.0.')->end()
->scalarNode('excluded_ajax_paths')->defaultValue('^/((index|app(_[\w]+)?)\.php/)?_wdt')->end()
->end()
;
Expand Down
5 changes: 2 additions & 3 deletions src/Symfony/Bundle/WebProfilerBundle/DependencyInjection/WebProfilerExtension.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,10 +46,9 @@ public function load(array $configs, ContainerBuilder $container)
$loader = new XmlFileLoader($container, new FileLocator(__DIR__.'/../Resources/config'));
$loader->load('profiler.xml');

if ($config['toolbar'] || $config['intercept_redirects']) {
if ($config['toolbar']) {
$loader->load('toolbar.xml');
$container->getDefinition('web_profiler.debug_toolbar')->replaceArgument(4, $config['excluded_ajax_paths']);
$container->setParameter('web_profiler.debug_toolbar.intercept_redirects', $config['intercept_redirects']);
$container->getDefinition('web_profiler.debug_toolbar')->replaceArgument(3, $config['excluded_ajax_paths']);
$container->setParameter('web_profiler.debug_toolbar.mode', $config['toolbar'] ? WebDebugToolbarListener::ENABLED : WebDebugToolbarListener::DISABLED);
}

Expand Down
16 changes: 1 addition & 15 deletions src/Symfony/Bundle/WebProfilerBundle/EventListener/WebDebugToolbarListener.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@
use Symfony\Component\EventDispatcher\EventSubscriberInterface;
use Symfony\Component\HttpFoundation\Request;
use Symfony\Component\HttpFoundation\Response;
use Symfony\Component\HttpFoundation\Session\Flash\AutoExpireFlashBag;
use Symfony\Component\HttpKernel\Event\ResponseEvent;
use Symfony\Component\HttpKernel\KernelEvents;
use Symfony\Component\Routing\Generator\UrlGeneratorInterface;
Expand All @@ -40,16 +39,14 @@ class WebDebugToolbarListener implements EventSubscriberInterface

protected $twig;
protected $urlGenerator;
protected $interceptRedirects;
protected $mode;
protected $excludedAjaxPaths;
private $cspHandler;

public function __construct(Environment $twig, bool $interceptRedirects = false, int $mode = self::ENABLED, UrlGeneratorInterface $urlGenerator = null, string $excludedAjaxPaths = '^/bundles|^/_wdt', ContentSecurityPolicyHandler $cspHandler = null)
public function __construct(Environment $twig, int $mode = self::ENABLED, UrlGeneratorInterface $urlGenerator = null, string $excludedAjaxPaths = '^/bundles|^/_wdt', ContentSecurityPolicyHandler $cspHandler = null)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should we have a BC layer for this signature change ? This class is not marked as @internal

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should mark it as @internal in 4.4 IMHO.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This class is marked as final, it should be enough ?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@maxhelias no. BC breaks in argument position is related to using the class, not to extending it.

{
$this->twig = $twig;
$this->urlGenerator = $urlGenerator;
$this->interceptRedirects = $interceptRedirects;
$this->mode = $mode;
$this->excludedAjaxPaths = $excludedAjaxPaths;
$this->cspHandler = $cspHandler;
Expand Down Expand Up @@ -87,17 +84,6 @@ public function onKernelResponse(ResponseEvent $event)
return;
}

if ($response->headers->has('X-Debug-Token') && $response->isRedirect() && $this->interceptRedirects && 'html' === $request->getRequestFormat()) {
if ($request->hasSession() && ($session = $request->getSession())->isStarted() && $session->getFlashBag() instanceof AutoExpireFlashBag) {
// keep current flashes for one more request if using AutoExpireFlashBag
$session->getFlashBag()->setAll($session->getFlashBag()->peekAll());
}

$response->setContent($this->twig->render('@WebProfiler/Profiler/toolbar_redirect.html.twig', ['location' => $response->headers->get('Location')]));
$response->setStatusCode(200);
$response->headers->remove('Location');
}

if (self::DISABLED === $this->mode
|| !$response->headers->has('X-Debug-Token')
|| $response->isRedirection()
Expand Down
1 change: 0 additions & 1 deletion src/Symfony/Bundle/WebProfilerBundle/Resources/config/toolbar.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@
<service id="web_profiler.debug_toolbar" class="Symfony\Bundle\WebProfilerBundle\EventListener\WebDebugToolbarListener">
<tag name="kernel.event_subscriber" />
<argument type="service" id="twig" />
<argument>%web_profiler.debug_toolbar.intercept_redirects%</argument>
<argument>%web_profiler.debug_toolbar.mode%</argument>
<argument type="service" id="router" on-invalid="ignore" />
<argument /> <!-- paths that should be excluded from the AJAX requests shown in the toolbar -->
Expand Down
39 changes: 0 additions & 39 deletions src/Symfony/Bundle/WebProfilerBundle/Tests/DependencyInjection/ConfigurationTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,63 +35,24 @@ public function getDebugModes()
[
'options' => [],
'expectedResult' => [
'intercept_redirects' => false,
'toolbar' => false,
'excluded_ajax_paths' => '^/((index|app(_[\w]+)?)\.php/)?_wdt',
],
],
[
'options' => ['toolbar' => true],
'expectedResult' => [
'intercept_redirects' => false,
'toolbar' => true,
'excluded_ajax_paths' => '^/((index|app(_[\w]+)?)\.php/)?_wdt',
],
],
[
'options' => ['excluded_ajax_paths' => 'test'],
'expectedResult' => [
'intercept_redirects' => false,
'toolbar' => false,
'excluded_ajax_paths' => 'test',
],
],
];
}

/**
* @group legacy
*
* @dataProvider getInterceptRedirectsConfiguration
*/
public function testConfigTreeUsingInterceptRedirects(bool $interceptRedirects, array $expectedResult)
{
$processor = new Processor();
$configuration = new Configuration();
$config = $processor->processConfiguration($configuration, [['intercept_redirects' => $interceptRedirects]]);

$this->assertEquals($expectedResult, $config);
}

public function getInterceptRedirectsConfiguration()
{
return [
[
'interceptRedirects' => true,
'expectedResult' => [
'intercept_redirects' => true,
'toolbar' => false,
'excluded_ajax_paths' => '^/((index|app(_[\w]+)?)\.php/)?_wdt',
],
],
[
'interceptRedirects' => false,
'expectedResult' => [
'intercept_redirects' => false,
'toolbar' => false,
'excluded_ajax_paths' => '^/((index|app(_[\w]+)?)\.php/)?_wdt',
],
],
];
}
}
51 changes: 0 additions & 51 deletions src/Symfony/Bundle/WebProfilerBundle/Tests/DependencyInjection/WebProfilerExtensionTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -141,57 +141,6 @@ public function getToolbarConfig()
];
}

/**
* @group legacy
*
* @dataProvider getInterceptRedirectsToolbarConfig
*/
public function testToolbarConfigUsingInterceptRedirects(
bool $toolbarEnabled,
bool $interceptRedirects,
bool $listenerInjected,
bool $listenerEnabled
) {
$extension = new WebProfilerExtension();
$extension->load(
[['toolbar' => $toolbarEnabled, 'intercept_redirects' => $interceptRedirects]],
$this->container
);
$this->container->removeDefinition('web_profiler.controller.exception');

$this->assertSame($listenerInjected, $this->container->has('web_profiler.debug_toolbar'));

self::assertSaneContainer($this->getCompiledContainer(), '', ['web_profiler.csp.handler']);

if ($listenerInjected) {
$this->assertSame($listenerEnabled, $this->container->get('web_profiler.debug_toolbar')->isEnabled());
}
}

public function getInterceptRedirectsToolbarConfig()
{
return [
[
'toolbarEnabled' => false,
'interceptRedirects' => true,
'listenerInjected' => true,
'listenerEnabled' => false,
],
[
'toolbarEnabled' => false,
'interceptRedirects' => false,
'listenerInjected' => false,
'listenerEnabled' => false,
],
[
'toolbarEnabled' => true,
'interceptRedirects' => true,
'listenerInjected' => true,
'listenerEnabled' => true,
],
];
}

private function getCompiledContainer()
{
if ($this->container->has('web_profiler.debug_toolbar')) {
Expand Down