bug #23903 [DI] Fix merging of env vars in configs (nicolas-grekas)

This PR was merged into the 3.3 branch.

Discussion
----------

[DI] Fix merging of env vars in configs

| Q             | A
| ------------- | ---
| Branch?       | 3.3
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | #22561, #23520
| License       | MIT
| Doc PR        | -

Commits
-------

00b9273 [DI] Fix merging of env vars in configs

src/Symfony/Component/DependencyInjection/Compiler/MergeExtensionConfigurationPass.php

@@ -13,7 +13,9 @@
 
 use Symfony\Component\DependencyInjection\ContainerBuilder;
 use Symfony\Component\DependencyInjection\Extension\ConfigurationExtensionInterface;
+use Symfony\Component\DependencyInjection\Extension\Extension;
 use Symfony\Component\DependencyInjection\Extension\PrependExtensionInterface;
+use Symfony\Component\DependencyInjection\Parameterbag\EnvPlaceholderParameterBag;
 
 /**
  * Merges extension configs into the container builder.
@@ -43,7 +45,10 @@ public function process(ContainerBuilder $container)
                 // this extension was not called
                 continue;
             }
-            $config = $container->getParameterBag()->resolveValue($config);
+            // EnvPlaceholderParameterBag tracks env vars when calling resolveValue().
+            // Clone so that tracking is done in a dedicated bag.
+            $resolvingBag = clone $container->getParameterBag();
+            $config = $resolvingBag->resolveValue($config);
 
             $tmpContainer = new ContainerBuilder($container->getParameterBag());
             $tmpContainer->setResourceTracking($container->isTrackingResources());
@@ -58,6 +63,15 @@ public function process(ContainerBuilder $container)
 
             $extension->load($config, $tmpContainer);
 
+            if ($resolvingBag instanceof EnvPlaceholderParameterBag) {
+                // $resolvingBag keeps track of env vars encoutered *before* merging configs
+                if ($extension instanceof Extension) {
+                    // but we don't want to keep track of env vars that are *overridden* when configs are merged
+                    $resolvingBag = new MergeExtensionConfigurationParameterBag($extension, $resolvingBag);
+                }
+                $container->getParameterBag()->mergeEnvPlaceholders($resolvingBag);
+            }
+
             $container->merge($tmpContainer);
             $container->getParameterBag()->add($parameters);
         }
@@ -66,3 +80,58 @@ public function process(ContainerBuilder $container)
         $container->addAliases($aliases);
     }
 }
+
+/**
+ * @internal
+ */
+class MergeExtensionConfigurationParameterBag extends EnvPlaceholderParameterBag
+{
+    private $beforeProcessingEnvPlaceholders;
+
+    public function __construct(Extension $extension, parent $resolvingBag)
+    {
+        $this->beforeProcessingEnvPlaceholders = $resolvingBag->getEnvPlaceholders();
+        $config = $this->resolveEnvPlaceholders($extension->getProcessedConfigs());
+        parent::__construct($this->resolveEnvReferences($config));
+    }
+
+    /**
+     * {@inheritdoc}
+     */
+    public function getEnvPlaceholders()
+    {
+        // contains the list of env vars that are still used after configs have been merged
+        $envPlaceholders = parent::getEnvPlaceholders();
+
+        foreach ($envPlaceholders as $env => $placeholders) {
+            if (isset($this->beforeProcessingEnvPlaceholders[$env])) {
+                // for still-used env vars, keep track of their before-processing placeholders
+                $envPlaceholders[$env] += $this->beforeProcessingEnvPlaceholders[$env];
+            }
+        }
+
+        return $envPlaceholders;
+    }
+
+    /**
+     * Replaces-back env placeholders to their original "%env(FOO)%" version.
+     */
+    private function resolveEnvPlaceholders($value)
+    {
+        if (is_array($value)) {
+            foreach ($value as $k => $v) {
+                $value[$this->resolveEnvPlaceholders($k)] = $this->resolveEnvPlaceholders($v);
+            }
+        } elseif (is_string($value)) {
+            foreach ($this->beforeProcessingEnvPlaceholders as $env => $placeholders) {
+                foreach ($placeholders as $placeholder) {
+                    if (false !== stripos($value, $placeholder)) {
+                        $value = str_ireplace($placeholder, "%env($env)%", $value);
+                    }
+                }
+            }
+        }
+
+        return $value;
+    }
+}

src/Symfony/Component/DependencyInjection/ContainerBuilder.php

@@ -729,10 +729,9 @@ public function compile(/*$resolveEnvPlaceholders = false*/)
         $bag = $this->getParameterBag();
 
         if ($resolveEnvPlaceholders && $bag instanceof EnvPlaceholderParameterBag) {
-            $bag->resolveEnvReferences();
-            $this->parameterBag = new ParameterBag($bag->all());
+            $this->parameterBag = new ParameterBag($bag->resolveEnvReferences($bag->all()));
             $this->envPlaceholders = $bag->getEnvPlaceholders();
-            $this->parameterBag = $bag = new ParameterBag($this->resolveEnvPlaceholders($bag->all(), true));
+            $this->parameterBag = $bag = new ParameterBag($this->resolveEnvPlaceholders($this->parameterBag->all(), true));
         }
 
         $compiler->compile($this);

src/Symfony/Component/DependencyInjection/Extension/Extension.php

@@ -25,6 +25,8 @@
  */
 abstract class Extension implements ExtensionInterface, ConfigurationExtensionInterface
 {
+    private $processedConfigs = array();
+
     /**
      * {@inheritdoc}
      */
@@ -91,7 +93,19 @@ final protected function processConfiguration(ConfigurationInterface $configurat
     {
         $processor = new Processor();
 
-        return $processor->processConfiguration($configuration, $configs);
+        return $this->processedConfigs[] = $processor->processConfiguration($configuration, $configs);
+    }
+
+    /**
+     * @internal
+     */
+    final public function getProcessedConfigs()
+    {
+        try {
+            return $this->processedConfigs;
+        } finally {
+            $this->processedConfigs = array();
+        }
     }
 
     /**

src/Symfony/Component/DependencyInjection/ParameterBag/EnvPlaceholderParameterBag.php

@@ -106,11 +106,11 @@ public function resolve()
     /**
      * Replaces "%env(FOO)%" references by their placeholder, keeping regular "%parameters%" references as is.
      */
-    public function resolveEnvReferences()
+    public function resolveEnvReferences(array $value)
     {
         $this->resolveEnvReferences = true;
         try {
-            $this->resolve();
+            return $this->resolveValue($value);
         } finally {
             $this->resolveEnvReferences = false;
         }

src/Symfony/Component/DependencyInjection/Tests/Compiler/MergeExtensionConfigurationPassTest.php

@@ -17,6 +17,7 @@
 use Symfony\Component\Config\Resource\FileResource;
 use Symfony\Component\DependencyInjection\Compiler\MergeExtensionConfigurationPass;
 use Symfony\Component\DependencyInjection\ContainerBuilder;
+use Symfony\Component\DependencyInjection\Extension\Extension;
 use Symfony\Component\DependencyInjection\ParameterBag\ParameterBag;
 
 class MergeExtensionConfigurationPassTest extends TestCase
@@ -55,13 +56,10 @@ public function testExpressionLanguageProviderForwarding()
 
     public function testExtensionConfigurationIsTrackedByDefault()
     {
-        $extension = $this->getMockBuilder('Symfony\\Component\\DependencyInjection\\Extension\\Extension')->getMock();
-        $extension->expects($this->once())
+        $extension = $this->getMockBuilder(FooExtension::class)->setMethods(array('getConfiguration'))->getMock();
+        $extension->expects($this->exactly(2))
             ->method('getConfiguration')
             ->will($this->returnValue(new FooConfiguration()));
-        $extension->expects($this->any())
-            ->method('getAlias')
-            ->will($this->returnValue('foo'));
 
         $container = new ContainerBuilder(new ParameterBag());
         $container->registerExtension($extension);
@@ -72,12 +70,51 @@ public function testExtensionConfigurationIsTrackedByDefault()
 
         $this->assertContains(new FileResource(__FILE__), $container->getResources(), '', false, false);
     }
+
+    public function testOverriddenEnvsAreMerged()
+    {
+        $container = new ContainerBuilder();
+        $container->registerExtension(new FooExtension());
+        $container->prependExtensionConfig('foo', array('bar' => '%env(FOO)%'));
+        $container->prependExtensionConfig('foo', array('bar' => '%env(BAR)%'));
+
+        $pass = new MergeExtensionConfigurationPass();
+        $pass->process($container);
+
+        $this->assertSame(array('FOO'), array_keys($container->getParameterBag()->getEnvPlaceholders()));
+    }
 }
 
 class FooConfiguration implements ConfigurationInterface
 {
     public function getConfigTreeBuilder()
     {
-        return new TreeBuilder();
+        $treeBuilder = new TreeBuilder();
+        $rootNode = $treeBuilder->root('foo');
+        $rootNode
+            ->children()
+                ->scalarNode('bar')->end()
+            ->end();
+
+        return $treeBuilder;
+    }
+}
+
+class FooExtension extends Extension
+{
+    public function getAlias()
+    {
+        return 'foo';
+    }
+
+    public function getConfiguration(array $config, ContainerBuilder $container)
+    {
+        return new FooConfiguration();
+    }
+
+    public function load(array $configs, ContainerBuilder $container)
+    {
+        $configuration = $this->getConfiguration($configs, $container);
+        $config = $this->processConfiguration($configuration, $configs);
     }
 }