Skip to content

Commit c2741b2

Browse files
committed
bug #27267 [DependencyInjection] resolve array env vars (jamesthomasonjr)
This PR was squashed before being merged into the 3.4 branch (closes #27267). Discussion ---------- [DependencyInjection] resolve array env vars | Q | A | ------------- | --- | Branch? | 3.4 | Bug fix? | yes | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | #27239 | License | MIT | Doc PR | n/a ## Why This bugfix solves a problem where environment variables resolved as an array would cause an error while compiling the container if they aren't the last parameter in the ParameterBag: the next parameter to be resolved would fail at the `stripos()` check. More information about the bug is available at #27239 ## Tests - This PR modifies existing ContainerBuilder tests to make use of the EnvVarProcessor to resolve json strings into arrays, instead of relying upon a TestingEnvPlaceholderParameterBag class. - I would liked to have kept EnvVarProcessor logic out of the ContainerBuilder tests, but it was the interaction between the ContainerBuilder and EnvVarProcessor that caused the bug - This PR adds a new ContainerBuilder test to verify that an environment variable resolved into an array doesn't cause an error when the next variable attempts to be resolved ## Code - ~This PR adds an `\is_string()` sanity check before the `stripos()` method call so that only a string are passed into `stripos()`~ - This PR also adds a `$completed` flag so that completely resolved environment variables (currently only determined by `$placeholder === $value`) can break out of the loop early (handled via `break 2;` Commits ------- 4c3b950dc2 [DependencyInjection] resolve array env vars
2 parents 13bf53d + f96dd29 commit c2741b2

File tree

2 files changed

+45
-15
lines changed

2 files changed

+45
-15
lines changed

ContainerBuilder.php

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1408,6 +1408,7 @@ public function resolveEnvPlaceholders($value, $format = null, array &$usedEnvs
14081408
}
14091409
$envPlaceholders = $bag instanceof EnvPlaceholderParameterBag ? $bag->getEnvPlaceholders() : $this->envPlaceholders;
14101410

1411+
$completed = false;
14111412
foreach ($envPlaceholders as $env => $placeholders) {
14121413
foreach ($placeholders as $placeholder) {
14131414
if (false !== stripos($value, $placeholder)) {
@@ -1418,14 +1419,19 @@ public function resolveEnvPlaceholders($value, $format = null, array &$usedEnvs
14181419
}
14191420
if ($placeholder === $value) {
14201421
$value = $resolved;
1422+
$completed = true;
14211423
} else {
14221424
if (!is_string($resolved) && !is_numeric($resolved)) {
1423-
throw new RuntimeException(sprintf('A string value must be composed of strings and/or numbers, but found parameter "env(%s)" of type %s inside string value "%s".', $env, gettype($resolved), $value));
1425+
throw new RuntimeException(sprintf('A string value must be composed of strings and/or numbers, but found parameter "env(%s)" of type %s inside string value "%s".', $env, gettype($resolved), $this->resolveEnvPlaceholders($value)));
14241426
}
14251427
$value = str_ireplace($placeholder, $resolved, $value);
14261428
}
14271429
$usedEnvs[$env] = $env;
14281430
$this->envCounters[$env] = isset($this->envCounters[$env]) ? 1 + $this->envCounters[$env] : 1;
1431+
1432+
if ($completed) {
1433+
break 2;
1434+
}
14291435
}
14301436
}
14311437
}

Tests/ContainerBuilderTest.php

Lines changed: 38 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -665,17 +665,49 @@ public function testCompileWithResolveEnv()
665665
putenv('DUMMY_ENV_VAR');
666666
}
667667

668+
public function testCompileWithArrayResolveEnv()
669+
{
670+
putenv('ARRAY={"foo":"bar"}');
671+
672+
$container = new ContainerBuilder();
673+
$container->setParameter('foo', '%env(json:ARRAY)%');
674+
$container->compile(true);
675+
676+
$this->assertSame(array('foo' => 'bar'), $container->getParameter('foo'));
677+
678+
putenv('ARRAY');
679+
}
680+
681+
public function testCompileWithArrayAndAnotherResolveEnv()
682+
{
683+
putenv('DUMMY_ENV_VAR=abc');
684+
putenv('ARRAY={"foo":"bar"}');
685+
686+
$container = new ContainerBuilder();
687+
$container->setParameter('foo', '%env(json:ARRAY)%');
688+
$container->setParameter('bar', '%env(DUMMY_ENV_VAR)%');
689+
$container->compile(true);
690+
691+
$this->assertSame(array('foo' => 'bar'), $container->getParameter('foo'));
692+
$this->assertSame('abc', $container->getParameter('bar'));
693+
694+
putenv('DUMMY_ENV_VAR');
695+
putenv('ARRAY');
696+
}
697+
668698
/**
669699
* @expectedException \Symfony\Component\DependencyInjection\Exception\RuntimeException
670-
* @expectedExceptionMessage A string value must be composed of strings and/or numbers, but found parameter "env(ARRAY)" of type array inside string value "ABC %env(ARRAY)%".
700+
* @expectedExceptionMessage A string value must be composed of strings and/or numbers, but found parameter "env(json:ARRAY)" of type array inside string value "ABC %env(json:ARRAY)%".
671701
*/
672-
public function testCompileWithArrayResolveEnv()
702+
public function testCompileWithArrayInStringResolveEnv()
673703
{
674-
$bag = new TestingEnvPlaceholderParameterBag();
675-
$container = new ContainerBuilder($bag);
676-
$container->setParameter('foo', '%env(ARRAY)%');
677-
$container->setParameter('bar', 'ABC %env(ARRAY)%');
704+
putenv('ARRAY={"foo":"bar"}');
705+
706+
$container = new ContainerBuilder();
707+
$container->setParameter('foo', 'ABC %env(json:ARRAY)%');
678708
$container->compile(true);
709+
710+
putenv('ARRAY');
679711
}
680712

681713
/**
@@ -1418,11 +1450,3 @@ public function __construct(A $a)
14181450
{
14191451
}
14201452
}
1421-
1422-
class TestingEnvPlaceholderParameterBag extends EnvPlaceholderParameterBag
1423-
{
1424-
public function get($name)
1425-
{
1426-
return 'env(array)' === strtolower($name) ? array(123) : parent::get($name);
1427-
}
1428-
}

0 commit comments

Comments
 (0)