8000 [DI] Fix merging of env vars in configs by nicolas-grekas · Pull Request #23903 · symfony/symfony · GitHub
[go: up one dir, main page]

Skip to content

[DI] Fix merging of env vars in configs #23903

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Aug 18, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,9 @@

use Symfony\Component\DependencyInjection\ContainerBuilder;
use Symfony\Component\DependencyInjection\Extension\ConfigurationExtensionInterface;
use Symfony\Component\DependencyInjection\Extension\Extension;
use Symfony\Component\DependencyInjection\Extension\PrependExtensionInterface;
use Symfony\Component\DependencyInjection\Parameterbag\EnvPlaceholderParameterBag;

/**
* Merges extension configs into the container builder.
Expand Down Expand Up @@ -43,7 +45,10 @@ public function process(ContainerBuilder $container)
// this extension was not called
continue;
}
$config = $container->getParameterBag()->resolveValue($config);
// EnvPlaceholderParameterBag tracks env vars when calling resolveValue().
// Clone so that tracking is done in a dedicated bag.
$resolvingBag = clone $container->getParameterBag();
$config = $resolvingBag->resolveValue($config);

$tmpContainer = new ContainerBuilder($container->getParameterBag());
$tmpContainer->setResourceTracking($container->isTrackingResources());
Expand All @@ -58,6 +63,15 @@ public function process(ContainerBuilder $container)

$extension->load($config, $tmpContainer);

if ($resolvingBag instanceof EnvPlaceholderParameterBag) {
// $resolvingBag keeps track of env vars encoutered *before* merging configs
if ($extension instanceof Extension) {
// but we don't want to keep track of env vars that are *overridden* when configs are merged
$resolvingBag = new MergeExtensionConfigurationParameterBag($extension, $resolvingBag);
}
$container->getParameterBag()->mergeEnvPlaceholders($resolvingBag);
}

$container->merge($tmpContainer);
$container->getParameterBag()->add($parameters);
}
Expand All @@ -66,3 +80,58 @@ public function process(ContainerBuilder $container)
$container->addAliases($aliases);
}
}

/**
* @internal
*/
class MergeExtensionConfigurationParameterBag extends EnvPlaceholderParameterBag
{
private $beforeProcessingEnvPlaceholders;

public function __construct(Extension $extension, parent $resolvingBag)
{
$this->beforeProcessingEnvPlaceholders = $resolvingBag->getEnvPlaceholders();
$config = $this->resolveEnvPlaceholders($extension->getProcessedConfigs());
parent::__construct($this->resolveEnvReferences($config));
}

/**
* {@inheritdoc}
*/
public function getEnvPlaceholders()
{
// contains the list of env vars that are still used after configs have been merged
$envPlaceholders = parent::getEnvPlaceholders();

foreach ($envPlaceholders as $env => $placeholders) {
if (isset($this->beforeProcessingEnvPlaceholders[$env])) {
// for still-used env vars, keep track of their before-processing placeholders
$envPlaceholders[$env] += $this->beforeProcessingEnvPlaceholders[$env];
}
}

return $envPlaceholders;
}

/**
* Replaces-back env placeholders to their original "%env(FOO)%" version.
*/
private function resolveEnvPlaceholders($value)
{
if (is_array($value)) {
foreach ($value as $k => $v) {
$value[$this->resolveEnvPlaceholders($k)] = $this->resolveEnvPlaceholders($v);
}
} elseif (is_string($value)) {
foreach ($this->beforeProcessingEnvPlaceholders as $env => $placeholders) {
foreach ($placeholders as $placeholder) {
if (false !== stripos($value, $placeholder)) {
$value = str_ireplace($placeholder, "%env($env)%", $value);
}
}
}
}

return $value;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -729,10 +729,9 @@ public function compile(/*$resolveEnvPlaceholders = false*/)
$bag = $this->getParameterBag();

if ($resolveEnvPlaceholders && $bag instanceof EnvPlaceholderParameterBag) {
$bag->resolveEnvReferences();
$this->parameterBag = new ParameterBag($bag->all());
$this->parameterBag = new ParameterBag($bag->resolveEnvReferences($bag->all()));
$this->envPlaceholders = $bag->getEnvPlaceholders();
$this->parameterBag = $bag = new ParameterBag($this->resolveEnvPlaceholders($bag->all(), true));
$this->parameterBag = $bag = new ParameterBag($this->resolveEnvPlaceholders($this->parameterBag->all(), true));
}

$compiler->compile($this);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,8 @@
*/
abstract class Extension implements ExtensionInterface, ConfigurationExtensionInterface
{
p 10000 rivate $processedConfigs = array();

/**
* {@inheritdoc}
*/
Expand Down Expand Up @@ -91,7 +93,19 @@ final protected function processConfiguration(ConfigurationInterface $configurat
{
$processor = new Processor();

return $processor->processConfiguration($configuration, $configs);
return $this->processedConfigs[] = $processor->processConfiguration($configuration, $configs);
}

/**
* @internal
*/
final public function getProcessedConfigs()
{
try {
return $this->processedConfigs;
} finally {
$this->processedConfigs = array();
}
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -106,11 +106,11 @@ public function resolve()
/**
* Replaces "%env(FOO)%" references by their placeholder, keeping regular "%parameters%" references as is.
*/
public function resolveEnvReferences()
public function resolveEnvReferences(array $value)
Copy link
Contributor

Choose a reason for hiding this comment

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

isn't this a BC break ?

Copy link
Member Author

Choose a reason for hiding this comment

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

this is not released yet, 3.3@dev only

{
$this->resolveEnvReferences = true;
try {
$this->resolve();
return $this->resolveValue($value);
} finally {
$this->resolveEnvReferences = false;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
use Symfony\Component\Config\Resource\FileResource;
use Symfony\Component\DependencyInjection\Compiler\MergeExtensionConfigurationPass;
use Symfony\Component\DependencyInjection\ContainerBuilder;
use Symfony\Component\DependencyInjection\Extension\Extension;
use Symfony\Component\DependencyInjection\ParameterBag\ParameterBag;

class MergeExtensionConfigurationPassTest extends TestCase
Expand Down Expand Up @@ -55,13 +56,10 @@ public function testExpressionLanguageProviderForwarding()

public function testExtensionConfigurationIsTrackedByDefault()
{
$extension = $this->getMockBuilder('Symfony\\Component\\DependencyInjection\\Extension\\Extension')->getMock();
$extension->expects($this->once())
$extension = $this->getMockBuilder(FooExtension::class)->setMethods(array('getConfiguration'))->getMock();
$extension->expects($this->exactly(2))
->method('getConfiguration')
->will($this->returnValue(new FooConfiguration()));
$extension->expects($this->any())
->method('getAlias')
->will($this->returnValue('foo'));

$container = new ContainerBuilder(new ParameterBag());
$container->registerExtension($extension);
Expand All @@ -72,12 +70,51 @@ public function testExtensionConfigurationIsTrackedByDefault()

$this->assertContains(new FileResource(__FILE__), $container->getResources(), '', false, false);
}

public function testOverriddenEnvsAreMerged()
{
$container = new ContainerBuilder();
$container->registerExtension(new FooExtension());
$container->prependExtensionConfig('foo', array('bar' => '%env(FOO)%'));
$container->prependExtensionConfig('foo', array('bar' => '%env(BAR)%'));

$pass = new MergeExtensionConfigurationPass();
$pass->process($container);

$this->assertSame(array('FOO'), array_keys($container->getParameterBag()->getEnvPlaceholders()));
}
}

class FooConfiguration implements ConfigurationInterface
{
public function getConfigTreeBuilder()
{
return new TreeBuilder();
$treeBuilder = new TreeBuilder();
$rootNode = $treeBuilder->root('foo');
$rootNode
->children()
->scalarNode('bar')->end()
->end();

return $treeBuilder;
}
}

class FooExtension extends Extension
{
public function getAlias()
{
return 'foo';
}

public function getConfiguration(array $config, ContainerBuilder $container)
{
return new FooConfiguration();
}

public function load(array $configs, ContainerBuilder $container)
{
$configuration = $this->getConfiguration($configs, $container);
$config = $this->processConfiguration($configuration, $configs);
}
}
0