From 397c19ee5fe02bd9073f87c19fa70f38cbd7cbe0 Mon Sep 17 00:00:00 2001 From: Roland Franssen Date: Sun, 14 Oct 2018 14:18:54 +0200 Subject: [PATCH] [DI] Deprecated using env vars with cannotBeEmpty() --- UPGRADE-4.2.md | 1 + UPGRADE-5.0.md | 1 + src/Symfony/Component/Config/CHANGELOG.md | 1 + .../Config/Definition/ScalarNode.php | 2 + .../Config/Definition/VariableNode.php | 15 +++++++ .../ValidateEnvPlaceholdersPassTest.php | 40 +++++++++++++++++++ 6 files changed, 60 insertions(+) diff --git a/UPGRADE-4.2.md b/UPGRADE-4.2.md index f04124d9bc4c..d838b7c2b24e 100644 --- a/UPGRADE-4.2.md +++ b/UPGRADE-4.2.md @@ -16,6 +16,7 @@ Config * Deprecated constructing a `TreeBuilder` without passing root node information. * Deprecated `FileLoaderLoadException`, use `LoaderLoadException` instead. + * Deprecated using environment variables with `cannotBeEmpty()` if the value is validated with `validate()` Console ------- diff --git a/UPGRADE-5.0.md b/UPGRADE-5.0.md index 80403f038d62..81709247cae8 100644 --- a/UPGRADE-5.0.md +++ b/UPGRADE-5.0.md @@ -18,6 +18,7 @@ Config * Added the `getChildNodeDefinitions()` method to `ParentNodeDefinitionInterface`. * The `Processor` class has been made final * Removed `FileLoaderLoadException`, use `LoaderLoadException` instead. + * Using environment variables with `cannotBeEmpty()` if the value is validated with `validate()` will throw an exception. Console ------- diff --git a/src/Symfony/Component/Config/CHANGELOG.md b/src/Symfony/Component/Config/CHANGELOG.md index fea35f9e47be..a2c1a2076a47 100644 --- a/src/Symfony/Component/Config/CHANGELOG.md +++ b/src/Symfony/Component/Config/CHANGELOG.md @@ -6,6 +6,7 @@ CHANGELOG * deprecated constructing a `TreeBuilder` without passing root node information * renamed `FileLoaderLoadException` to `LoaderLoadException` + * deprecated using environment variables with `cannotBeEmpty()` if the value is validated with `validate()` 4.1.0 ----- diff --git a/src/Symfony/Component/Config/Definition/ScalarNode.php b/src/Symfony/Component/Config/Definition/ScalarNode.php index ee870b6a85ea..77503bfd6e1f 100644 --- a/src/Symfony/Component/Config/Definition/ScalarNode.php +++ b/src/Symfony/Component/Config/Definition/ScalarNode.php @@ -48,6 +48,8 @@ protected function validateType($value) */ protected function isValueEmpty($value) { + // assume environment variables are never empty (which in practice is likely to be true during runtime) + // not doing so breaks many configs that are valid today if ($this->isHandlingPlaceholder()) { return false; } diff --git a/src/Symfony/Component/Config/Definition/VariableNode.php b/src/Symfony/Component/Config/Definition/VariableNode.php index 1a3442d9613d..93576870cf3e 100644 --- a/src/Symfony/Component/Config/Definition/VariableNode.php +++ b/src/Symfony/Component/Config/Definition/VariableNode.php @@ -81,6 +81,19 @@ protected function validateType($value) */ protected function finalizeValue($value) { + // deny environment variables only when using custom validators + // this avoids ever passing an empty value to final validation closures + if (!$this->allowEmptyValue && $this->isHandlingPlaceholder() && $this->finalValidationClosures) { + @trigger_error(sprintf('Setting path "%s" to an environment variable is deprecated since Symfony 4.2. Remove "cannotBeEmpty()", "validate()" or include a prefix/suffix value instead.', $this->getPath()), E_USER_DEPRECATED); +// $e = new InvalidConfigurationException(sprintf('The path "%s" cannot contain an environment variable when empty values are not allowed by definition and are validated.', $this->getPath(), json_encode($value))); +// if ($hint = $this->getInfo()) { +// $e->addHint($hint); +// } +// $e->setPath($this->getPath()); +// +// throw $e; + } + if (!$this->allowEmptyValue && $this->isValueEmpty($value)) { $ex = new InvalidConfigurationException(sprintf('The path "%s" cannot contain an empty value, but got %s.', $this->getPath(), json_encode($value))); if ($hint = $this->getInfo()) { @@ -120,6 +133,8 @@ protected function mergeValues($leftSide, $rightSide) * @param mixed $value * * @return bool + * + * @see finalizeValue() */ protected function isValueEmpty($value) { diff --git a/src/Symfony/Component/DependencyInjection/Tests/Compiler/ValidateEnvPlaceholdersPassTest.php b/src/Symfony/Component/DependencyInjection/Tests/Compiler/ValidateEnvPlaceholdersPassTest.php index 32a5bb1ab6de..4121a84131ee 100644 --- a/src/Symfony/Component/DependencyInjection/Tests/Compiler/ValidateEnvPlaceholdersPassTest.php +++ b/src/Symfony/Component/DependencyInjection/Tests/Compiler/ValidateEnvPlaceholdersPassTest.php @@ -210,6 +210,38 @@ public function testEmptyEnvWhichCannotBeEmptyForScalarNode(): void $this->assertSame($expected, $container->resolveEnvPlaceholders($ext->getConfig())); } + /** + * NOT LEGACY (test exception in 5.0). + * + * @group legacy + * @expectedDeprecation Setting path "env_extension.scalar_node_not_empty_validated" to an environment variable is deprecated since Symfony 4.2. Remove "cannotBeEmpty()", "validate()" or include a prefix/suffix value instead. + */ + public function testEmptyEnvWhichCannotBeEmptyForScalarNodeWithValidation(): void + { + $container = new ContainerBuilder(); + $container->registerExtension($ext = new EnvExtension()); + $container->prependExtensionConfig('env_extension', $expected = array( + 'scalar_node_not_empty_validated' => '%env(SOME)%', + )); + + $this->doProcess($container); + + $this->assertSame($expected, $container->resolveEnvPlaceholders($ext->getConfig())); + } + + public function testPartialEnvWhichCannotBeEmptyForScalarNode(): void + { + $container = new ContainerBuilder(); + $container->registerExtension($ext = new EnvExtension()); + $container->prependExtensionConfig('env_extension', $expected = array( + 'scalar_node_not_empty_validated' => 'foo %env(SOME)% bar', + )); + + $this->doProcess($container); + + $this->assertSame($expected, $container->resolveEnvPlaceholders($ext->getConfig())); + } + public function testEnvWithVariableNode(): void { $container = new ContainerBuilder(); @@ -281,6 +313,14 @@ public function getConfigTreeBuilder() ->children() ->scalarNode('scalar_node')->end() ->scalarNode('scalar_node_not_empty')->cannotBeEmpty()->end() + ->scalarNode('scalar_node_not_empty_validated') + ->cannotBeEmpty() + ->validate() + ->always(function ($value) { + return $value; + }) + ->end() + ->end() ->integerNode('int_node')->end() ->floatNode('float_node')->end() ->booleanNode('bool_node')->end()