bug #23940 [DI] Fix resolving env vars when compiling a ContainerBuil…

…der (nicolas-grekas)

This PR was merged into the 3.3 branch.

Discussion
----------

[DI] Fix resolving env vars when compiling a ContainerBuilder

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

As spotted by @ro0NL, `$container->compile(true)` doesn't resolve direct env var references found in service definitions. Fixed here.

Commits
-------

9219594 [DI] Fix resolving env vars when compiling a ContainerBuilder

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

@@ -92,7 +92,15 @@ public function __construct(Extension $extension, parent $resolvingBag)
     {
         $this->beforeProcessingEnvPlaceholders = $resolvingBag->getEnvPlaceholders();
         $config = $this->resolveEnvPlaceholders($extension->getProcessedConfigs());
-        parent::__construct($this->resolveEnvReferences($config));
+        parent::__construct($this->resolveValue($config));
+    }
+
+    /**
+     * {@inheritdoc}
+     */
+    public function get($name)
+    {
+        return $this->has($name) || (0 === strpos($name, 'env(') && ')' === substr($name, -1) && 'env()' !== $name) ? parent::get($name) : '';
     }
 
     /**

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

@@ -0,0 +1,44 @@
+<?php
+
+/*
+ * This file is part of the Symfony package.
+ *
+ * (c) Fabien Potencier <fabien@symfony.com>
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+namespace Symfony\Component\DependencyInjection\Compiler;
+
+use Symfony\Component\DependencyInjection\Definition;
+
+/**
+ * Replaces env var placeholders by their current values.
+ */
+class ResolveEnvPlaceholdersPass extends AbstractRecursivePass
+{
+    protected function processValue($value, $isRoot = false)
+    {
+        if (is_string($value)) {
+            return $this->container->resolveEnvPlaceholders($value, true);
+        }
+        if ($value instanceof Definition) {
+            $changes = $value->getChanges();
+            if (isset($changes['class'])) {
+                $value->setClass($this->container->resolveEnvPlaceholders($value->getClass(), true));
+            }
+            if (isset($changes['file'])) {
+                $value->setFile($this->container->resolveEnvPlaceholders($value->getFile(), true));
+            }
+        }
+
+        $value = parent::processValue($value, $isRoot);
+
+        if ($value && is_array($value)) {
+            $value = array_combine($this->container->resolveEnvPlaceholders(array_keys($value), true), $value);
+        }
+
+        return $value;
+    }
+}

src/Symfony/Component/DependencyInjection/ContainerBuilder.php

@@ -18,6 +18,7 @@
 use Symfony\Component\DependencyInjection\Compiler\Compiler;
 use Symfony\Component\DependencyInjection\Compiler\CompilerPassInterface;
 use Symfony\Component\DependencyInjection\Compiler\PassConfig;
+use Symfony\Component\DependencyInjection\Compiler\ResolveEnvPlaceholdersPass;
 use Symfony\Component\DependencyInjection\Exception\BadMethodCallException;
 use Symfony\Component\DependencyInjection\Exception\InvalidArgumentException;
 use Symfony\Component\DependencyInjection\Exception\LogicException;
@@ -729,9 +730,7 @@ public function compile(/*$resolveEnvPlaceholders = false*/)
         $bag = $this->getParameterBag();
 
         if ($resolveEnvPlaceholders && $bag instanceof EnvPlaceholderParameterBag) {
-            $this->parameterBag = new ParameterBag($bag->resolveEnvReferences($bag->all()));
-            $this->envPlaceholders = $bag->getEnvPlaceholders();
-            $this->parameterBag = $bag = new ParameterBag($this->resolveEnvPlaceholders($this->parameterBag->all(), true));
+            $compiler->addPass(new ResolveEnvPlaceholdersPass(), PassConfig::TYPE_AFTER_REMOVING, -1000);
         }
 
         $compiler->compile($this);
@@ -744,11 +743,15 @@ public function compile(/*$resolveEnvPlaceholders = false*/)
 
         $this->extensionConfigs = array();
 
-        parent::compile();
-
         if ($bag instanceof EnvPlaceholderParameterBag) {
+            if ($resolveEnvPlaceholders) {
+                $this->parameterBag = new ParameterBag($this->resolveEnvPlaceholders($bag->all(), true));
+            }
+
             $this->envPlaceholders = $bag->getEnvPlaceholders();
         }
+
+        parent::compile();
     }
 
     /**
@@ -1313,12 +1316,19 @@ public function resolveEnvPlaceholders($value, $format = null, array &$usedEnvs
         foreach ($envPlaceholders as $env => $placeholders) {
             foreach ($placeholders as $placeholder) {
                 if (false !== stripos($value, $placeholder)) {
-                    if (true !== $format) {
+                    if (true === $format) {
+                        $resolved = $bag->escapeValue($this->getEnv($env));
+                    } else {
                         $resolved = sprintf($format, $env);
-                    } elseif ($placeholder === $resolved = $bag->escapeValue($this->getEnv($env))) {
-                        $resolved = $bag->all()[strtolower("env($env)")];
                     }
-                    $value = str_ireplace($placeholder, $resolved, $value);
+                    if ($placeholder === $value) {
+                        $value = $resolved;
+                    } else {
+                        if (!is_string($resolved) && !is_numeric($resolved)) {
+                            throw new RuntimeException(sprintf('A string value must be composed of strings and/or numbers, but found parameter "env(%s)" of type %s inside string value "%s".', $env, gettype($resolved), $value));
+                        }
+                        $value = str_ireplace($placeholder, $resolved, $value);
+                    }
                     $usedEnvs[$env] = $env;
                     $this->envCounters[$env] = isset($this->envCounters[$env]) ? 1 + $this->envCounters[$env] : 1;
                 }
@@ -1393,6 +1403,28 @@ public static function getServiceConditionals($value)
         return $services;
     }
 
+    /**
+     * {@inheritdoc}
+     */
+    protected function getEnv($name)
+    {
+        $value = parent::getEnv($name);
+
+        if (!is_string($value) || !$this->getParameterBag() instanceof EnvPlaceholderParameterBag) {
+            return $value;
+        }
+
+        foreach ($this->getParameterBag()->getEnvPlaceholders() as $env => $placeholders) {
+            if (isset($placeholders[$value])) {
+                $bag = new ParameterBag($this->getParameterBag()->all());
+
+                return $bag->unescapeValue($bag->get("env($name)"));
+            }
+        }
+
+        return $value;
+    }
+
     /**
      * Retrieves the currently set proxy instantiator or instantiates one.
      *

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

@@ -20,7 +20,6 @@
 class EnvPlaceholderParameterBag extends ParameterBag
 {
     private $envPlaceholders = array();
-    private $resolveEnvReferences = false;
 
     /**
      * {@inheritdoc}
@@ -102,29 +101,4 @@ public function resolve()
             }
         }
     }
-
-    /**
-     * Replaces "%env(FOO)%" references by their placeholder, keeping regular "%parameters%" references as is.
-     */
-    public function resolveEnvReferences(array $value)
-    {
-        $this->resolveEnvReferences = true;
-        try {
-            return $this->resolveValue($value);
-        } finally {
-            $this->resolveEnvReferences = false;
-        }
-    }
-
-    /**
-     * {@inheritdoc}
-     */
-    public function resolveString($value, array $resolving = array())
-    {
-        if ($this->resolveEnvReferences) {
-            return preg_replace_callback('/%%|%(env\([^%\s]+\))%/', function ($match) { return isset($match[1]) ? $this->get($match[1]) : '%%'; }, $value);
-        }
-
-        return parent::resolveString($value, $resolving);
-    }
 }

src/Symfony/Component/DependencyInjection/Tests/ContainerBuilderTest.php

@@ -623,19 +623,37 @@ public function testCompileWithResolveEnv()
 
         $container = new ContainerBuilder();
         $container->setParameter('env(FOO)', 'Foo');
+        $container->setParameter('env(DUMMY_ENV_VAR)', 'GHI');
         $container->setParameter('bar', '%% %env(DUMMY_ENV_VAR)% %env(DUMMY_SERVER_VAR)% %env(HTTP_DUMMY_VAR)%');
         $container->setParameter('foo', '%env(FOO)%');
         $container->setParameter('baz', '%foo%');
         $container->setParameter('env(HTTP_DUMMY_VAR)', '123');
+        $container->register('teatime', 'stdClass')
+            ->setProperty('foo', '%env(DUMMY_ENV_VAR)%')
+        ;
         $container->compile(true);
 
         $this->assertSame('% du%%y ABC 123', $container->getParameter('bar'));
         $this->assertSame('Foo', $container->getParameter('baz'));
+        $this->assertSame('du%%y', $container->get('teatime')->foo);
 
         unset($_SERVER['DUMMY_SERVER_VAR'], $_SERVER['HTTP_DUMMY_VAR']);
         putenv('DUMMY_ENV_VAR');
     }
 
+    /**
+     * @expectedException \Symfony\Component\DependencyInjection\Exception\RuntimeException
+     * @expectedExceptionMessage A string value must be composed of strings and/or numbers, but found parameter "env(ARRAY)" of type array inside string value "ABC %env(ARRAY)%".
+     */
+    public function testCompileWithArrayResolveEnv()
+    {
+        $bag = new TestingEnvPlaceholderParameterBag();
+        $container = new ContainerBuilder($bag);
+        $container->setParameter('foo', '%env(ARRAY)%');
+        $container->setParameter('bar', 'ABC %env(ARRAY)%');
+        $container->compile(true);
+    }
+
     /**
      * @expectedException \Symfony\Component\DependencyInjection\Exception\EnvNotFoundException
      * @expectedExceptionMessage Environment variable not found: "FOO".
@@ -1127,3 +1145,11 @@ public function __construct(A $a)
     {
     }
 }
+
+class TestingEnvPlaceholderParameterBag extends EnvPlaceholderParameterBag
+{
+    public function get($name)
+    {
+        return 'env(array)' === strtolower($name) ? array(123) : parent::get($name);
+    }
+}