bug #23899 [DI] Fix reading env vars from fastcgi params (nicolas-grekas)

This PR was merged into the 3.3 branch.

Discussion
----------

[DI] Fix reading env vars from fastcgi params

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

Values in fastcgi_param populate `$_SERVER`, never `$_ENV`.
This PR makes `$container->getEnv()` read from `$_SERVER`, excluding any vars whose name start by `HTTP_` as that would be a security issue (values injection via HTTP headers.)

Embeds a few other fixes found meanwhile.

Commits
-------

adff65a [DI] Fix reading env vars from fastcgi params

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

@@ -54,20 +54,17 @@ final class ServiceLocatorTagPass extends AbstractRecursivePass
 
         $value->setArguments($arguments);
 
-        if ($public = $value->isPublic()) {
-            $value->setPublic(false);
-        }
         $id = 'service_locator.'.md5(serialize($value));
 
         if ($isRoot) {
             if ($id !== $this->currentId) {
-                $this->container->setAlias($id, new Alias($this->currentId, $public));
+                $this->container->setAlias($id, new Alias($this->currentId, false));
             }
 
             return $value;
         }
 
-        $this->container->setDefinition($id, $value);
+        $this->container->setDefinition($id, $value->setPublic(false));
 
         return new Reference($id);
     }

src/Symfony/Component/DependencyInjection/Container.php

@@ -429,7 +429,7 @@ class Container implements ResettableContainerInterface
      *
      * @param string $name The name of the environment variable
      *
-     * @return scalar The value to use for the provided environment variable name
+     * @return mixed The value to use for the provided environment variable name
      *
      * @throws EnvNotFoundException When the environment variable is not found and has no default value
      */
@@ -438,6 +438,9 @@ class Container implements ResettableContainerInterface
         if (isset($this->envCache[$name]) || array_key_exists($name, $this->envCache)) {
             return $this->envCache[$name];
         }
+        if (0 !== strpos($name, 'HTTP_') && isset($_SERVER[$name])) {
+            return $this->envCache[$name] = $_SERVER[$name];
+        }
         if (isset($_ENV[$name])) {
             return $this->envCache[$name] = $_ENV[$name];
         }

src/Symfony/Component/DependencyInjection/ContainerBuilder.php

@@ -729,9 +729,10 @@ class ContainerBuilder extends Container implements TaggedContainerInterface
         $bag = $this->getParameterBag();
 
         if ($resolveEnvPlaceholders && $bag instanceof EnvPlaceholderParameterBag) {
-            $this->parameterBag = new ParameterBag($this->resolveEnvPlaceholders($bag->all(), true));
+            $bag->resolveEnvReferences();
+            $this->parameterBag = new ParameterBag($bag->all());
             $this->envPlaceholders = $bag->getEnvPlaceholders();
-            $this->parameterBag = $bag = new ParameterBag($this->resolveEnvPlaceholders($this->parameterBag->all()));
+            $this->parameterBag = $bag = new ParameterBag($this->resolveEnvPlaceholders($bag->all(), true));
         }
 
         $compiler->compile($this);
@@ -746,7 +747,9 @@ class ContainerBuilder extends Container implements TaggedContainerInterface
 
         parent::compile();
 
-        $this->envPlaceholders = $bag instanceof EnvPlaceholderParameterBag ? $bag->getEnvPlaceholders() : array();
+        if ($bag instanceof EnvPlaceholderParameterBag) {
+            $this->envPlaceholders = $bag->getEnvPlaceholders();
+        }
     }
 
     /**
@@ -1311,10 +1314,10 @@ class ContainerBuilder extends Container implements TaggedContainerInterface
         foreach ($envPlaceholders as $env => $placeholders) {
             foreach ($placeholders as $placeholder) {
                 if (false !== stripos($value, $placeholder)) {
-                    if (true === $format) {
-                        $resolved = $bag->escapeValue($this->getEnv($env));
-                    } else {
+                    if (true !== $format) {
                         $resolved = sprintf($format, $env);
+                    } elseif ($placeholder === $resolved = $bag->escapeValue($this->getEnv($env))) {
+                        $resolved = $bag->all()[strtolower("env($env)")];
                     }
                     $value = str_ireplace($placeholder, $resolved, $value);
                     $usedEnvs[$env] = $env;

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

@@ -20,6 +20,7 @@ use Symfony\Component\DependencyInjection\Exception\InvalidArgumentException;
 class EnvPlaceholderParameterBag extends ParameterBag
 {
     private $envPlaceholders = array();
+    private $resolveEnvReferences = false;
 
     /**
      * {@inheritdoc}
@@ -101,4 +102,29 @@ class EnvPlaceholderParameterBag extends ParameterBag
             }
         }
     }
+
+    /**
+     * Replaces "%env(FOO)%" references by their placeholder, keeping regular "%parameters%" references as is.
+     */
+    public function resolveEnvReferences()
+    {
+        $this->resolveEnvReferences = true;
+        try {
+            $this->resolve();
+        } 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

@@ -603,29 +603,37 @@ class ContainerBuilderTest extends TestCase
     public function testResolveEnvValues()
     {
         $_ENV['DUMMY_ENV_VAR'] = 'du%%y';
+        $_SERVER['DUMMY_SERVER_VAR'] = 'ABC';
+        $_SERVER['HTTP_DUMMY_VAR'] = 'DEF';
 
         $container = new ContainerBuilder();
-        $container->setParameter('bar', '%% %env(DUMMY_ENV_VAR)%');
+        $container->setParameter('bar', '%% %env(DUMMY_ENV_VAR)% %env(DUMMY_SERVER_VAR)% %env(HTTP_DUMMY_VAR)%');
+        $container->setParameter('env(HTTP_DUMMY_VAR)', '123');
 
-        $this->assertSame('%% du%%%%y', $container->resolveEnvPlaceholders('%bar%', true));
+        $this->assertSame('%% du%%%%y ABC 123', $container->resolveEnvPlaceholders('%bar%', true));
 
-        unset($_ENV['DUMMY_ENV_VAR']);
+        unset($_ENV['DUMMY_ENV_VAR'], $_SERVER['DUMMY_SERVER_VAR'], $_SERVER['HTTP_DUMMY_VAR']);
     }
 
     public function testCompileWithResolveEnv()
     {
-        $_ENV['DUMMY_ENV_VAR'] = 'du%%y';
+        putenv('DUMMY_ENV_VAR=du%%y');
+        $_SERVER['DUMMY_SERVER_VAR'] = 'ABC';
+        $_SERVER['HTTP_DUMMY_VAR'] = 'DEF';
 
         $container = new ContainerBuilder();
         $container->setParameter('env(FOO)', 'Foo');
-        $container->setParameter('bar', '%% %env(DUMMY_ENV_VAR)%');
+        $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->compile(true);
 
-        $this->assertSame('% du%%y', $container->getParameter('bar'));
-        $this->assertSame('Foo', $container->getParameter('foo'));
+        $this->assertSame('% du%%y ABC 123', $container->getParameter('bar'));
+        $this->assertSame('Foo', $container->getParameter('baz'));
 
-        unset($_ENV['DUMMY_ENV_VAR']);
+        unset($_SERVER['DUMMY_SERVER_VAR'], $_SERVER['HTTP_DUMMY_VAR']);
+        putenv('DUMMY_ENV_VAR');
     }
 
     /**