8000 bug #37511 [DependencyInjection][Config] Use several placeholder uniq… · symfony/symfony@a3bd36c · GitHub
[go: up one dir, main page]
Skip to content

Commit a3bd36c

Browse files
nicolas-grekasnicolas-grekas
committed
bug #37511 [DependencyInjection][Config] Use several placeholder unique prefixes for dynamic placeholder values (fancyweb)
This PR was merged into the 4.4 branch. Discussion ---------- [DependencyInjection][Config] Use several placeholder unique prefixes for dynamic placeholder values | Q | A | ------------- | --- | Branch? | 4.4 | Bug fix? | yes | New feature? | no | Deprecations? | no | Tickets | #37426 | License | MIT | Doc PR |- Currently, in Config BaseNode, we are only able to check on one dynamic placeholder unique prefix (ie one env placeholder unique prefix in our usage) to ignore the validation of config values that contain placeholder values. It isn't enough in some scenarios, we would need to be able to check on multiple prefixes. In MergeExtensionConfigurationPass we need to not validate config values that are placeholders (they are checked later by a dedicated pass), so we set the BaseNode placeholder unique prefix for each processed extension. Because the env placeholder unique prefix depends of the bag data, if a first extension creates the env placeholder and adds a parameter to the bag, the second extension reusing the same env placeholder will generate a different env placeholder unique prefix. Therefore the validation of the value will not be skipped because the env placeholder contains the first env placeholder unique prefix while the BaseNode placeholder unique prefix is set with the second. Another solution might be to mutate the existing env placeholders to use the new unique prefix when merging the env placeholders ? I guess it depends on which side we want to fix this (DI or Config). cc @ro0NL Commits ------- 3d754ad [DependencyInjection][Config] Use several placeholder unique prefixes for dynamic placeholder values
2 parents 05fe56b + 3d754ad commit a3bd36c

File tree

3 files changed

+68
-8
lines changed

3 files changed

+68
-8
lines changed

‎src/Symfony/Component/Config/Definition/BaseNode.php

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ abstract class BaseNode implements NodeInterface
2626
{
2727
const DEFAULT_PATH_SEPARATOR = '.';
2828

29-
private static $placeholderUniquePrefix;
29+
private static $placeholderUniquePrefixes = [];
3030
private static $placeholders = [];
3131

3232
protected $name;
@@ -74,7 +74,7 @@ public static function setPlaceholder(string $placeholder, array $values): void
7474
}
7575

7676
/**
77-
* Sets a common prefix for dynamic placeholder values.
77+
* Adds a common prefix for dynamic placeholder values.
7878
*
7979
* Matching configuration values will be skipped from being processed and are returned as is, thus preserving the
8080
* placeholder. An exact match provided by {@see setPlaceholder()} might take precedence.
@@ -83,7 +83,7 @@ public static function setPlaceholder(string $placeholder, array $values): void
8383
*/
8484
public static function setPlaceholderUniquePrefix(string $prefix): void
8585
{
86-
self::$placeholderUniquePrefix = $prefix;
86+
self::$placeholderUniquePrefixes[] = $prefix;
8787
}
8888

8989
/**
@@ -93,7 +93,7 @@ public static function setPlaceholderUniquePrefix(string $prefix): void
9393
*/
9494
public static function resetPlaceholders(): void
9595
{
96-
self::$placeholderUniquePrefix = null;
96+
self::$placeholderUniquePrefixes = [];
9797
self::$placeholders = [];
9898
}
9999

@@ -513,8 +513,10 @@ private static function resolvePlaceholderValue($value)
513513
return self::$placeholders[$value];
514514
}
515515

516-
if (self::$placeholderUniquePrefix && 0 === strpos($value, self::$placeholderUniquePrefix)) {
517-
return [];
516+
foreach (self::$placeholderUniquePrefixes as $placeholderUniquePrefix) {
517+
if (0 === strpos($value, $placeholderUniquePrefix)) {
518+
return [];
519+
}
518520
}
519521
}
520522

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

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -79,11 +79,11 @@ public function process(ContainerBuilder $container)
7979
$container->getParameterBag()->mergeEnvPlaceholders($resolvingBag);
8080
}
8181

82-
throw $e;
83-
} finally {
8482
if ($configAvailable) {
8583
BaseNode::resetPlaceholders();
8684
}
85+
86+
throw $e;
8787
}
8888

8989
if ($resolvingBag instanceof MergeExtensionConfigurationParameterBag) {
@@ -95,6 +95,10 @@ p 8000 ublic function process(ContainerBuilder $container)
9595
$container->getParameterBag()->add($parameters);
9696
}
9797

98+
if ($configAvailable) {
99+
BaseNode::resetPlaceholders();
100+
}
101+
98102
$container->addDefinitions($definitions);
99103
$container->addAliases($aliases);
100104
}

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

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
namespace Symfony\Component\DependencyInjection\Tests\Compiler;
1313

1414
use PHPUnit\Framework\TestCase;
15+
use Symfony\Component\Config\Definition\BaseNode;
1516
use Symfony\Component\Config\Definition\Builder\TreeBuilder;
1617
use Symfony\Component\Config\Definition\ConfigurationInterface;
1718
use Symfony\Component\Config\Resource\FileResource;
@@ -128,6 +129,23 @@ public function testThrowingExtensionsGetMergedBag()
128129

129130
$this->assertSame(['FOO'], array_keys($container->getParameterBag()->getEnvPlaceholders()));
130131
}
132+
133+
public function testReuseEnvPlaceholderGeneratedByPreviousExtension()
134+
{
135+
if (!property_exists(BaseNode::class, 'placeholderUniquePrefixes')) {
136+
$this->markTestSkipped('This test requires symfony/config ^4.4.11|^5.0.11|^5.1.3');
137+
}
138+
139+
$container = new ContainerBuilder();
140+
$container->registerExtension(new FooExtension());
141+
$container->registerExtension(new TestCccExtension());
142+
$container->prependExtensionConfig('foo', ['bool_node' => '%env(bool:MY_ENV_VAR)%']);
143+
$container->prependExtensionConfig('test_ccc', ['bool_node' =& A8C6 gt; '%env(bool:MY_ENV_VAR)%']);
144+
145+
(new MergeExtensionConfigurationPass())->process($container);
146+
147+
$this->addToAssertionCount(1);
148+
}
131149
}
132150

133151
class FooConfiguration implements ConfigurationInterface
@@ -139,6 +157,7 @@ public function getConfigTreeBuilder(): TreeBuilder
139157
->children()
140158
->scalarNode('bar')->end()
141159
->scalarNode('baz')->end()
160+
->booleanNode('bool_node')->end()
142161
->end();
143162

144163
return $treeBuilder;
@@ -166,6 +185,8 @@ public function load(array $configs, ContainerBuilder $container)
166185
$container->getParameterBag()->get('env(BOZ)');
167186
$container->resolveEnvPlaceholders($config['baz']);
168187
}
188+
189+
$container->setParameter('foo.param', 'ccc');
169190
}
170191
}
171192

@@ -194,3 +215,36 @@ public function load(array $configs, ContainerBuilder $container)
194215
throw new \Exception();
195216
}
196217
}
218+
219+
final class TestCccConfiguration implements ConfigurationInterface
220+
{
221+
public function getConfigTreeBuilder(): TreeBuilder
222+
{
223+
$treeBuilder = new TreeBuilder('test_ccc');
224+
$treeBuilder->getRootNode()
225+
->children()
226+
->booleanNode('bool_node')->end()
227+
->end();
228+
229+
return $treeBuilder;
230+
}
231+
}
232+
233+
final class TestCccExtension extends Extension
234+
{
235+
public function getAlias(): string
236+
{
237+
return 'test_ccc';
238+
}
239+
240+
public function getConfiguration(array $config, ContainerBuilder $container): ?ConfigurationInterface
241+
{
242+
return new TestCccConfiguration();
243+
}
244+
245+
public function load(array $configs, ContainerBuilder $container)
246+
{
247+
$configuration = $this->getConfiguration($configs, $container);
248+
$this->processConfiguration($configuration, $configs);
249+
}
250+
}

0 commit comments

Comments
 (0)
0