From 76c42175b97f13a155282ee1f4591a59d35228b4 Mon Sep 17 00:00:00 2001 From: Nicolas Grekas Date: Tue, 5 Sep 2017 17:39:41 +0200 Subject: [PATCH] [FrameworkBundle] Fix Di config to allow for more private services --- .../Compiler/CachePoolClearerPass.php | 27 ++++------------ .../Compiler/CachePoolPass.php | 32 +++++++++++++------ .../Compiler/TemplatingPass.php | 7 ++++ .../Compiler/UnusedTagsPass.php | 1 + .../FrameworkExtension.php | 6 ++-- .../FrameworkBundle/FrameworkBundle.php | 2 +- .../Resources/config/templating_debug.xml | 2 +- .../Resources/config/templating_php.xml | 7 +++- .../Templating/Helper/FormHelper.php | 2 +- .../Controller/SubRequestController.php | 4 +-- .../Tests/Functional/app/Session/config.yml | 5 +++ .../WebProfilerExtensionTest.php | 23 ++++++------- .../Component/Form/FormRendererInterface.php | 2 +- 13 files changed, 65 insertions(+), 55 deletions(-) diff --git a/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Compiler/CachePoolClearerPass.php b/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Compiler/CachePoolClearerPass.php index 882f9b2dccf67..094712ded69d3 100644 --- a/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Compiler/CachePoolClearerPass.php +++ b/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Compiler/CachePoolClearerPass.php @@ -27,29 +27,16 @@ final class CachePoolClearerPass implements CompilerPassInterface public function process(ContainerBuilder $container) { $container->getParameterBag()->remove('cache.prefix.seed'); - $poolsByClearer = array(); - $pools = array(); - foreach ($container->findTaggedServiceIds('cache.pool') as $id => $attributes) { - $pools[$id] = new Reference($id); - foreach (array_reverse($attributes) as $attr) { - if (isset($attr['clearer'])) { - $poolsByClearer[$attr['clearer']][$id] = $pools[$id]; - } - if (!empty($attr['unlazy'])) { - $container->getDefinition($id)->setLazy(false); - } - if (array_key_exists('clearer', $attr) || array_key_exists('unlazy', $attr)) { - break; + foreach ($container->findTaggedServiceIds('cache.pool.clearer') as $id => $attr) { + $clearer = $container->getDefinition($id); + $pools = array(); + foreach ($clearer->getArgument(0) as $id => $ref) { + if ($container->hasDefinition($id)) { + $pools[$id] = new Reference($id); } } - } - - $container->getDefinition('cache.global_clearer')->addArgument($pools); - - foreach ($poolsByClearer as $clearer => $pools) { - $clearer = $container->getDefinition($clearer); - $clearer->addArgument($pools); + $clearer->replaceArgument(0, $pools); } if (!$container->has('cache.annotations')) { diff --git a/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Compiler/CachePoolPass.php b/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Compiler/CachePoolPass.php index c63175e2d3ac0..6a633c7b25361 100644 --- a/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Compiler/CachePoolPass.php +++ b/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Compiler/CachePoolPass.php @@ -37,6 +37,8 @@ public function process(ContainerBuilder $container) } $seed .= '.'.$container->getParameter('kernel.name').'.'.$container->getParameter('kernel.environment'); + $pools = array(); + $clearers = array(); $attributes = array( 'provider', 'namespace', @@ -47,10 +49,8 @@ public function process(ContainerBuilder $container) if ($pool->isAbstract()) { continue; } - $isLazy = $pool->isLazy(); while ($adapter instanceof ChildDefinition) { $adapter = $container->findDefinition($adapter->getParent()); - $isLazy = $isLazy || $adapter->isLazy(); if ($t = $adapter->getTag('cache.pool')) { $tags[0] += $t[0]; } @@ -82,17 +82,29 @@ public function process(ContainerBuilder $container) throw new InvalidArgumentException(sprintf('Invalid "cache.pool" tag for service "%s": accepted attributes are "clearer", "provider", "namespace" and "default_lifetime", found "%s".', $id, implode('", "', array_keys($tags[0])))); } - $attr = array(); if (null !== $clearer) { - $attr['clearer'] = $clearer; + $clearers[$clearer][$id] = new Reference($id, $container::IGNORE_ON_UNINITIALIZED_REFERENCE); } - if (!$isLazy) { - $pool->setLazy(true); - $attr['unlazy'] = true; - } - if ($attr) { - $pool->addTag('cache.pool', $attr); + + $pools[$id] = new Reference($id, $container::IGNORE_ON_UNINITIALIZED_REFERENCE); + } + + $clearer = 'cache.global_clearer'; + while ($container->hasAlias($clearer)) { + $clearer = (string) $container->getAlias($clearer); + } + if ($container->hasDefinition($clearer)) { + $clearers['cache.global_clearer'] = $pools; + } + + foreach ($clearers as $id => $pools) { + $clearer = $container->getDefinition($id); + if ($clearer instanceof ChilDefinition) { + $clearer->replaceArgument(0, $pools); + } else { + $clearer->setArgument(0, $pools); } + $clearer->addTag('cache.pool.clearer'); } } diff --git a/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Compiler/TemplatingPass.php b/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Compiler/TemplatingPass.php index 8e2e4d0dbfb34..b916a27a6d619 100644 --- a/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Compiler/TemplatingPass.php +++ b/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Compiler/TemplatingPass.php @@ -15,6 +15,7 @@ use Symfony\Component\DependencyInjection\Alias; use Symfony\Component\DependencyInjection\ContainerBuilder; use Symfony\Component\DependencyInjection\Compiler\CompilerPassInterface; +use Symfony\Component\DependencyInjection\Reference; use Symfony\Component\Templating\EngineInterface as ComponentEngineInterface; class TemplatingPass implements CompilerPassInterface @@ -31,16 +32,22 @@ public function process(ContainerBuilder $container) } if ($container->hasDefinition('templating.engine.php')) { + $refs = array(); $helpers = array(); foreach ($container->findTaggedServiceIds('templating.helper', true) as $id => $attributes) { if (isset($attributes[0]['alias'])) { $helpers[$attributes[0]['alias']] = $id; + $refs[$id] = new Reference($id); } } if (count($helpers) > 0) { $definition = $container->getDefinition('templating.engine.php'); $definition->addMethodCall('setHelpers', array($helpers)); + + if ($container->hasDefinition('templating.engine.php.helpers_locator')) { + $container->getDefinition('templating.engine.php.helpers_locator')->replaceArgument(0, $refs); + } } } } diff --git a/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Compiler/UnusedTagsPass.php b/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Compiler/UnusedTagsPass.php index ffc19c49dfeff..1ebb5562986ab 100644 --- a/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Compiler/UnusedTagsPass.php +++ b/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Compiler/UnusedTagsPass.php @@ -22,6 +22,7 @@ class UnusedTagsPass implements CompilerPassInterface { private $whitelist = array( + 'cache.pool.clearer', 'console.command', 'container.service_locator', 'container.service_subscriber', diff --git a/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php b/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php index 16f13ac79ad51..3b59ec9ef4583 100644 --- a/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php +++ b/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php @@ -597,15 +597,15 @@ private function registerWorkflowConfiguration(array $workflows, ContainerBuilde } // Create Workflow + $workflowId = sprintf('%s.%s', $type, $name); $workflowDefinition = new ChildDefinition(sprintf('%s.abstract', $type)); - $workflowDefinition->replaceArgument(0, $definitionDefinition); + $workflowDefinition->replaceArgument(0, new Reference(sprintf('%s.definition', $workflowId))); if (isset($markingStoreDefinition)) { $workflowDefinition->replaceArgument(1, $markingStoreDefinition); } $workflowDefinition->replaceArgument(3, $name); // Store to container - $workflowId = sprintf('%s.%s', $type, $name); $container->setDefinition($workflowId, $workflowDefinition); $container->setDefinition(sprintf('%s.definition', $workflowId), $definitionDefinition); @@ -680,7 +680,7 @@ private function registerDebugConfiguration(array $config, ContainerBuilder $con if (class_exists(Stopwatch::class)) { $container->register('debug.stopwatch', Stopwatch::class)->addArgument(true); - $container->setAlias(Stopwatch::class, 'debug.stopwatch'); + $container->setAlias(Stopwatch::class, new Alias('debug.stopwatch', false)); } $debug = $container->getParameter('kernel.debug'); diff --git a/src/Symfony/Bundle/FrameworkBundle/FrameworkBundle.php b/src/Symfony/Bundle/FrameworkBundle/FrameworkBundle.php index 47129136d7920..ba117ceeb82a2 100644 --- a/src/Symfony/Bundle/FrameworkBundle/FrameworkBundle.php +++ b/src/Symfony/Bundle/FrameworkBundle/FrameworkBundle.php @@ -106,7 +106,7 @@ public function build(ContainerBuilder $container) $container->addCompilerPass(new AddExpressionLanguageProvidersPass()); $this->addCompilerPassIfExists($container, TranslationExtractorPass::class); $this->addCompilerPassIfExists($container, TranslationDumperPass::class); - $container->addCompilerPass(new FragmentRendererPass(), PassConfig::TYPE_AFTER_REMOVING); + $container->addCompilerPass(new FragmentRendererPass()); $this->addCompilerPassIfExists($container, SerializerPass::class); $this->addCompilerPassIfExists($container, PropertyInfoPass::class); $container->addCompilerPass(new DataCollectorTranslatorPass()); diff --git a/src/Symfony/Bundle/FrameworkBundle/Resources/config/templating_debug.xml b/src/Symfony/Bundle/FrameworkBundle/Resources/config/templating_debug.xml index cc478a03eb349..0bdfbe24ea3bd 100644 --- a/src/Symfony/Bundle/FrameworkBundle/Resources/config/templating_debug.xml +++ b/src/Symfony/Bundle/FrameworkBundle/Resources/config/templating_debug.xml @@ -9,7 +9,7 @@ - + diff --git a/src/Symfony/Bundle/FrameworkBundle/Resources/config/templating_php.xml b/src/Symfony/Bundle/FrameworkBundle/Resources/config/templating_php.xml index 09a00c78e44a5..5eb7233592f65 100644 --- a/src/Symfony/Bundle/FrameworkBundle/Resources/config/templating_php.xml +++ b/src/Symfony/Bundle/FrameworkBundle/Resources/config/templating_php.xml @@ -9,12 +9,17 @@ - + %kernel.charset% + + + + + diff --git a/src/Symfony/Bundle/FrameworkBundle/Templating/Helper/FormHelper.php b/src/Symfony/Bundle/FrameworkBundle/Templating/Helper/FormHelper.php index 68d46c2c3facb..bc6c255f09d07 100644 --- a/src/Symfony/Bundle/FrameworkBundle/Templating/Helper/FormHelper.php +++ b/src/Symfony/Bundle/FrameworkBundle/Templating/Helper/FormHelper.php @@ -226,7 +226,7 @@ public function block(FormView $view, $blockName, array $variables = array()) * Check the token in your action using the same CSRF token id. * * - * $csrfProvider = $this->get('security.csrf.token_generator'); + * // $csrfProvider being an instance of Symfony\Component\Security\Csrf\TokenGenerator\TokenGeneratorInterface * if (!$csrfProvider->isCsrfTokenValid('rm_user_'.$user->getId(), $token)) { * throw new \RuntimeException('CSRF attack detected.'); * } diff --git a/src/Symfony/Bundle/FrameworkBundle/Tests/Functional/Bundle/TestBundle/Controller/SubRequestController.php b/src/Symfony/Bundle/FrameworkBundle/Tests/Functional/Bundle/TestBundle/Controller/SubRequestController.php index 5df6e29590a5b..1df462992be30 100644 --- a/src/Symfony/Bundle/FrameworkBundle/Tests/Functional/Bundle/TestBundle/Controller/SubRequestController.php +++ b/src/Symfony/Bundle/FrameworkBundle/Tests/Functional/Bundle/TestBundle/Controller/SubRequestController.php @@ -21,10 +21,8 @@ class SubRequestController implements ContainerAwareInterface { use ContainerAwareTrait; - public function indexAction() + public function indexAction($handler) { - $handler = $this->container->get('fragment.handler'); - $errorUrl = $this->generateUrl('subrequest_fragment_error', array('_locale' => 'fr', '_format' => 'json')); $altUrl = $this->generateUrl('subrequest_fragment', array('_locale' => 'fr', '_format' => 'json')); diff --git a/src/Symfony/Bundle/FrameworkBundle/Tests/Functional/app/Session/config.yml b/src/Symfony/Bundle/FrameworkBundle/Tests/Functional/app/Session/config.yml index 65dd6c7fa91f1..ad6bdb691ca52 100644 --- a/src/Symfony/Bundle/FrameworkBundle/Tests/Functional/app/Session/config.yml +++ b/src/Symfony/Bundle/FrameworkBundle/Tests/Functional/app/Session/config.yml @@ -1,2 +1,7 @@ imports: - { resource: ./../config/default.yml } + +services: + Symfony\Bundle\FrameworkBundle\Tests\Functional\Bundle\TestBundle\Controller\SubRequestController: + tags: + - { name: controller.service_arguments, action: indexAction, argument: handler, id: fragment.handler } diff --git a/src/Symfony/Bundle/WebProfilerBundle/Tests/DependencyInjection/WebProfilerExtensionTest.php b/src/Symfony/Bundle/WebProfilerBundle/Tests/DependencyInjection/WebProfilerExtensionTest.php index af0573e5a362d..25e352e7124ce 100644 --- a/src/Symfony/Bundle/WebProfilerBundle/Tests/DependencyInjection/WebProfilerExtensionTest.php +++ b/src/Symfony/Bundle/WebProfilerBundle/Tests/DependencyInjection/WebProfilerExtensionTest.php @@ -17,7 +17,8 @@ use Symfony\Component\DependencyInjection\ContainerBuilder; use Symfony\Component\DependencyInjection\Definition; use Symfony\Component\DependencyInjection\Reference; -use Symfony\Component\DependencyInjection\Dumper\PhpDumper; +use Symfony\Component\EventDispatcher\DependencyInjection\RegisterListenersPass; +use Symfony\Component\EventDispatcher\EventDispatcher; class WebProfilerExtensionTest extends TestCase { @@ -51,6 +52,7 @@ protected function setUp() $this->kernel = $this->getMockBuilder('Symfony\\Component\\HttpKernel\\KernelInterface')->getMock(); $this->container = new ContainerBuilder(); + $this->container->register('event_dispatcher', EventDispatcher::class); $this->container->register('router', $this->getMockClass('Symfony\\Component\\Routing\\RouterInterface')); $this->container->register('twig', 'Twig\Environment'); $this->container->register('twig_loader', 'Twig\Loader\ArrayLoader')->addArgument(array()); @@ -66,6 +68,7 @@ protected function setUp() ->addArgument(new Definition($this->getMockClass('Symfony\\Component\\HttpKernel\\Profiler\\ProfilerStorageInterface'))); $this->container->setParameter('data_collector.templates', array()); $this->container->set('kernel', $this->kernel); + $this->container->addCompilerPass(new RegisterListenersPass()); } protected function tearDown() @@ -88,7 +91,7 @@ public function testDefaultConfig($debug) $this->assertFalse($this->container->has('web_profiler.debug_toolbar')); - $this->assertSaneContainer($this->getDumpedContainer()); + $this->assertSaneContainer($this->getCompiledContainer()); } /** @@ -101,7 +104,7 @@ public function testToolbarConfig($toolbarEnabled, $interceptRedirects, $listene $this->assertSame($listenerInjected, $this->container->has('web_profiler.debug_toolbar')); - $this->assertSaneContainer($this->getDumpedContainer(), '', array('web_profiler.csp.handler')); + $this->assertSaneContainer($this->getCompiledContainer(), '', array('web_profiler.csp.handler')); if ($listenerInjected) { $this->assertSame($listenerEnabled, $this->container->get('web_profiler.debug_toolbar')->isEnabled()); @@ -118,19 +121,11 @@ public function getDebugModes() ); } - private function getDumpedContainer() + private function getCompiledContainer() { - static $i = 0; - $class = 'WebProfilerExtensionTestContainer'.$i++; - $this->container->compile(); + $this->container->set('kernel', $this->kernel); - $dumper = new PhpDumper($this->container); - eval('?>'.$dumper->dump(array('class' => $class))); - - $container = new $class(); - $container->set('kernel', $this->kernel); - - return $container; + return $this->container; } } diff --git a/src/Symfony/Component/Form/FormRendererInterface.php b/src/Symfony/Component/Form/FormRendererInterface.php index f0f51e4f59251..34c7822455c19 100644 --- a/src/Symfony/Component/Form/FormRendererInterface.php +++ b/src/Symfony/Component/Form/FormRendererInterface.php @@ -76,7 +76,7 @@ public function searchAndRenderBlock(FormView $view, $blockNameSuffix, array $va * Check the token in your action using the same token ID. * * - * $csrfProvider = $this->get('security.csrf.token_generator'); + * // $csrfProvider being an instance of Symfony\Component\Security\Csrf\TokenGenerator\TokenGeneratorInterface * if (!$csrfProvider->isCsrfTokenValid('rm_user_'.$user->getId(), $token)) { * throw new \RuntimeException('CSRF attack detected.'); * }