[DI] Validate env vars in config

symfony · ro0NL · Mar 3, 2018 · Mar 3, 2018 · Mar 21, 2018 · Mar 21, 2018
commit 68ee20eaf187529b029d606a4e5d8673b62df973
diff --git a/src/Symfony/Component/Config/Definition/ArrayNode.php b/src/Symfony/Component/Config/Definition/ArrayNode.php
@@ -390,4 +390,12 @@ protected function mergeValues($leftSide, $rightSide)

        return $leftSide;
    }
+
+    /**
+     * {@inheritdoc}
+     */
+    protected function allowPlaceholders(): bool
+    {
+        return false;
+    }
 }
diff --git a/src/Symfony/Component/Config/Definition/BaseNode.php b/src/Symfony/Component/Config/Definition/BaseNode.php
@@ -15,6 +15,7 @@
 use Symfony\Component\Config\Definition\Exception\ForbiddenOverwriteException;
 use Symfony\Component\Config\Definition\Exception\InvalidConfigurationException;
 use Symfony\Component\Config\Definition\Exception\InvalidTypeException;
+use Symfony\Component\Config\Definition\Exception\UnsetKeyException;

 /**
 * The base node class.
@@ -25,6 +26,9 @@ abstract class BaseNode implements NodeInterface
 {
    const DEFAULT_PATH_SEPARATOR = '.';

+    private static $placeholderUniquePrefix;
+    private static $placeholders = array();
+
    protected $name;
    protected $parent;
    protected $normalizationClosures = array();
@@ -36,6 +40,8 @@ abstract class BaseNode implements NodeInterface
    protected $attributes = array();
    protected $pathSeparator;

+    private $handlingPlaceholder = false;
+
    /**
     * @throws \InvalidArgumentException if the name contains a period
     */
@@ -50,6 +56,39 @@ public function __construct(?string $name, NodeInterface $parent = null, string
        $this->pathSeparator = $pathSeparator;
    }

+    /**
+     * Register possible (dummy) values for a dynamic placeholder value. Matching configuration values will be processed
+     * with a provided value, one by one. After a provided value is successfully processed the configuration value is
+     * returned as is, thus preserving the placeholder.
+     */
+    public static function setPlaceholder(string $placeholder, array $values): void
+    {
+        if (!$values) {
+            throw new \InvalidArgumentException('At least one value must be provided.');
+        }
+
+        self::$placeholders[$placeholder] = $values;
+    }
+
+    /**
+     * Set a common prefix for dynamic placeholder values. Matching configuration values will be skipped from being
+     * processed and are returned as is, thus preserving the placeholder. An exact match provided by {@see setPlaceholder()}
+     * might take precedence.
+     */
+    public static function setPlaceholderUniquePrefix(string $prefix): void
+    {
+        self::$placeholderUniquePrefix = $prefix;
+    }
+
+    /**
+     * Reset all current placeholders available.
+     */
+    public static function resetPlaceholders(): void
+    {
+        self::$placeholderUniquePrefix = null;
+        self::$placeholders = array();
+    }
+
    public function setAttribute($key, $value)
    {
        $this->attributes[$key] = $value;
@@ -249,6 +288,46 @@ final public function merge($leftSide, $rightSide)
            ));
        }

+        if ($leftSide !== $leftPlaceholders = self::resolvePlaceholderValue($leftSide)) {
+            if (!$leftPlaceholders) {
+                return $rightSide;
+            }
+
+            foreach ($leftPlaceholders as $leftPlaceholder) {
+                $this->handlingPlaceholder = true;
+                try {
+                    $this->merge($leftPlaceholder, $rightSide);
+
+                    return $rightSide;
+                } catch (InvalidConfigurationException $e) {
+                } finally {
+                    $this->handlingPlaceholder = false;
+                }
+            }
+
+            throw $e;
+        }
+
+        if ($rightSide !== $rightPlaceholders = self::resolvePlaceholderValue($rightSide)) {
+            if (!$rightPlaceholders) {
+                return $rightSide;
+            }
+
+            foreach ($rightPlaceholders as $rightPlaceholder) {
+                $this->handlingPlaceholder = true;
+                try {
+                    $this->merge($leftSide, $rightPlaceholder);
+
+                    return $rightSide;
+                } catch (InvalidConfigurationException $e) {
+                } finally {
+                    $this->handlingPlaceholder = false;
+                }
+            }
+
+            throw $e;
+        }
+
        $this->validateType($leftSide);
        $this->validateType($rightSide);

@@ -267,6 +346,27 @@ final public function normalize($value)
            $value = $closure($value);
        }

+        // resolve placeholder value
+        if ($value !== $placeholders = self::resolvePlaceholderValue($value)) {
+            if (!$placeholders) {
+                return $value;
+            }
+
+            foreach ($placeholders as $placeholder) {
+                $this->handlingPlaceholder = true;
+                try {
+                    $this->normalize($placeholder);
+
+                    return $value;
+                } catch (InvalidConfigurationException $e) {
+                } finally {
+                    $this->handlingPlaceholder = false;
+                }
+            }
+
+            throw $e;
+        }
+
        // replace value with their equivalent
        foreach ($this->equivalentValues as $data) {
            if ($data[0] === $value) {
@@ -308,8 +408,35 @@ public function getParent()
     */
    final public function finalize($value)
    {
+        if ($value !== $placeholders = self::resolvePlaceholderValue($value)) {
+            if (!$placeholders) {
+                return $value;
+            }
+
+            foreach ($placeholders as $placeholder) {
+                $this->handlingPlaceholder = true;
+                try {
+                    $this->finalize($placeholder);
+
+                    return $value;
+                } catch (InvalidConfigurationException $e) {
+                } finally {
+                    $this->handlingPlaceholder = false;
+                }
+            }
+
+            throw $e;
+        }
+
        $this->validateType($value);

+        if ($this->handlingPlaceholder && !$this->allowPlaceholders()) {
+            $e = new InvalidConfigurationException(sprintf('A dynamic value is not compatible with a "%s" node type at path "%s".', get_class($this), $this->getPath()));
+            $e->setPath($this->getPath());
+
+            throw $e;
+        }
+
        $value = $this->finalizeValue($value);

        // Perform validation on the final value if a closure has been set.
@@ -318,6 +445,10 @@ final public function finalize($value)
            try {
                $value = $closure($value);
            } catch (Exception $e) {
+                if ($e instanceof UnsetKeyException && $this->handlingPlaceholder) {
+                    continue;
+                }
+
                throw $e;
            } catch (\Exception $e) {
                throw new InvalidConfigurationException(sprintf('Invalid configuration for path "%s": %s', $this->getPath(), $e->getMessage()), $e->getCode(), $e);
@@ -363,4 +494,27 @@ abstract protected function mergeValues($leftSide, $rightSide);
     * @return mixed The finalized value
     */
    abstract protected function finalizeValue($value);
+
+    /**
+     * Test if placeholder values are allowed for this node.
+     */
+    protected function allowPlaceholders(): bool
+    {
+        return true;
+    }
+
+    private static function resolvePlaceholderValue($value)
+    {
+        if (\is_string($value)) {
+            if (isset(self::$placeholders[$value])) {
+                return self::$placeholders[$value];
+            }
+
+            if (0 === strpos($value, self::$placeholderUniquePrefix, 0)) {
+                return array();
+            }
+        }
+
+        return $value;
+    }
 }
diff --git a/src/Symfony/Component/Config/Definition/EnumNode.php b/src/Symfony/Component/Config/Definition/EnumNode.php
@@ -55,4 +55,12 @@ protected function finalizeValue($value)

        return $value;
    }
+
+    /**
+     * {@inheritdoc}
+     */
+    protected function allowPlaceholders(): bool
+    {
+        return false;
+    }
 }
diff --git a/src/Symfony/Component/Config/Definition/Processor.php b/src/Symfony/Component/Config/Definition/Processor.php
@@ -15,6 +15,8 @@
 * This class is the entry point for config normalization/merging/finalization.
 *
 * @author Johannes M. Schmitt <schmittjoh@gmail.com>
+ *
+ * @final since version 4.1
 */
 class Processor
 {

diff --git a/src/Symfony/Component/DependencyInjection/CHANGELOG.md b/src/Symfony/Component/DependencyInjection/CHANGELOG.md
@@ -8,6 +8,7 @@ CHANGELOG
 * added PSR-11 `ContainerBagInterface` and its `ContainerBag` implementation to access parameters as-a-service
 * added support for service's decorators autowiring
 * deprecated the `TypedReference::canBeAutoregistered()` and  `TypedReference::getRequiringClass()` methods
+ * environment variables are validated when used in extension configuration

 4.0.0
 -----

diff --git a/src/Symfony/Component/DependencyInjection/Compiler/MergeExtensionConfigurationPass.php b/src/Symfony/Component/DependencyInjection/Compiler/MergeExtensionConfigurationPass.php
@@ -11,6 +11,7 @@

 namespace Symfony\Component\DependencyInjection\Compiler;

+use Symfony\Component\Config\Definition\BaseNode;
 use Symfony\Component\DependencyInjection\ContainerBuilder;
 use Symfony\Component\DependencyInjection\Exception\LogicException;
 use Symfony\Component\DependencyInjection\Exception\RuntimeException;
@@ -37,6 +38,7 @@ public function process(ContainerBuilder $container)
        $definitions = $container->getDefinitions();
        $aliases = $container->getAliases();
        $exprLangProviders = $container->getExpressionLanguageProviders();
+        $configAvailable = class_exists(BaseNode::class);

        foreach ($container->getExtensions() as $extension) {
            if ($extension instanceof PrependExtensionInterface) {
@@ -53,6 +55,9 @@ public function process(ContainerBuilder $container)
            if ($resolvingBag instanceof EnvPlaceholderParameterBag && $extension instanceof Extension) {
                // create a dedicated bag so that we can track env vars per-extension
                $resolvingBag = new MergeExtensionConfigurationParameterBag($resolvingBag);
+                if ($configAvailable) {
+                    BaseNode::setPlaceholderUniquePrefix($resolvingBag->getEnvPlaceholderUniquePrefix());
+                }
            }
            $config = $resolvingBag->resolveValue($config);

@@ -75,6 +80,10 @@ public function process(ContainerBuilder $container)
                }

                throw $e;
+            } finally {
+                if ($configAvailable) {
+                    BaseNode::resetPlaceholders();
+                }
            }

            if ($resolvingBag instanceof MergeExtensionConfigurationParameterBag) {
@@ -132,6 +141,11 @@ public function getEnvPlaceholders()
    {
        return null !== $this->processedEnvPlaceholders ? $this->processedEnvPlaceholders : parent::getEnvPlaceholders();
    }
+
+    public function getUnusedEnvPlaceholders(): array
+    {
+        return null === $this->processedEnvPlaceholders ? array() : array_diff_key(parent::getEnvPlaceholders(), $this->processedEnvPlaceholders);
+    }
 }

 /**

diff --git a/src/Symfony/Component/DependencyInjection/Compiler/PassConfig.php b/src/Symfony/Component/DependencyInjection/Compiler/PassConfig.php
@@ -49,6 +49,7 @@ public function __construct()
        );

        $this->optimizationPasses = array(array(
+            new ValidateEnvPlaceholdersPass(),
            new ResolveChildDefinitionsPass(),
            new ServiceLocatorTagPass(),
            new DecoratorServicePass(),
-Original file line number
+Diff line change
@@ -390,4 +390,12 @@ protected function mergeValues($leftSide, $rightSide)
             return $leftSide;
         }
+        /**
+         * {@inheritdoc}
+         */
+        protected function allowPlaceholders(): bool
+        {
+            return false;
             Copy link

  
      
    
  

  
      

  
    Contributor


      

  

  
    
      

      
            mcfedr
  

      

      

      


        Mar 27, 2018


      
    

  


        
      
  
  
  
    

    There was a problem hiding this comment.


  

 
  
    

    Choose a reason for hiding this comment

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

    
      
      


  


  
    
      Why have you made it false by default for ArrayNode? It seems this will limit how much of a config you can change in other peoples bundles, i.e. you can only put env(json:FOO) if the owner things of you in advance.
    
  
  


    

        
      
  
  
    
  
  
  
    
    Sorry, something went wrong.
  

  
    
  
    
      

              Uh oh!

              
There was an error while loading. Please reload this page.


  
  


          
      
  
    
    
      
        
            
    All reactions
  


          
          
        
      
    

    



  
        
  
    
        
    
  


      
          
  
      
            Copy link

  
      
    
  

  
      

  
  Member


      

  

  
    
      

      
            chalasr
  

      

      

      


        Mar 27, 2018


      
    

  


        
      
  
  
  
    

    There was a problem hiding this comment.


  

 
  
    

    Choose a reason for hiding this comment

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

    
      
      


  


  
    
      comments on closed PRs are likely to be lost, please consider opening a new issue/PR if you think something is buggy or can be improved. Thanks
    
  
  


    

        
      
  
  
    
  
  
  
    
    Sorry, something went wrong.
  

  
    
  
    
      

              Uh oh!

              
There was an error while loading. Please reload this page.


  
  


          
      
  
    
    
            ro0NL reacted with thumbs up emoji
      
        
            
    All reactions
  


          
              
                👍
                  1 reaction
              
          
        
      
    

    



  
        
  
    
        
    
  


      
          
  
      
            Copy link

  
      
    
  

  
      

  
    Contributor


      

  Author


  

  
    
      

      
            ro0NL
  

      

      

      


        Mar 27, 2018


      
    

  


        
      
  
  
  
    

    There was a problem hiding this comment.


  

 
  
    

    Choose a reason for hiding this comment

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

    
      
      


  


  
    
      In this case i asked for it on another closed PR :P  But yeah.. not sure if there's any real issue now (hence i asked :D)
@mcfedr see symfony/symfony-docs#8382 (comment) ,  tried to explain the general proces there.
    
  
  


    

        
      
  
  
    
  
  
  
    
    Sorry, something went wrong.
  

  
    
  
    
      

              Uh oh!

              
There was an error while loading. Please reload this page.


  
  


          
      
  
    
    
      
        
            
    All reactions
  


          
          
        
      
    

    



  
        
  
    
        
    
  


      
          
  
      
            Copy link

  
      
    
  

  
      

  
    Contributor


      

  

  
    
      

      
            mcfedr
  

      

      

      


        Mar 27, 2018


      
    

  


        
      
  
  
  
    

    There was a problem hiding this comment.


  

 
  
    

    Choose a reason for hiding this comment

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

    
      
      


  


  
    
      I see, makes sense, so for simple_array nodes it will accept env(json:FOO) but not when it should validate the structure.
    
  
  


    

        
      
  
  
    
  
  
  
    
    Sorry, something went wrong.
  

  
    
  
    
      

              Uh oh!

              
There was an error while loading. Please reload this page.


  
  


          
      
  
    
    
      
        
            
    All reactions
+        }
     }