Skip to content

Commit

Permalink
bug #27267 [DependencyInjection] resolve array env vars (jamesthomaso…
Browse files Browse the repository at this point in the history
…njr)

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
-------

4c3b950 [DependencyInjection] resolve array env vars
  • Loading branch information
fabpot committed May 18, 2018
2 parents c29f34e + 4c3b950 commit 4f9d907
Show file tree
Hide file tree
Showing 2 changed files with 45 additions and 15 deletions.
Expand Up @@ -1408,6 +1408,7 @@ public function resolveEnvPlaceholders($value, $format = null, array &$usedEnvs
}
$envPlaceholders = $bag instanceof EnvPlaceholderParameterBag ? $bag->getEnvPlaceholders() : $this->envPlaceholders;

$completed = false;
foreach ($envPlaceholders as $env => $placeholders) {
foreach ($placeholders as $placeholder) {
if (false !== stripos($value, $placeholder)) {
Expand All @@ -1418,14 +1419,19 @@ public function resolveEnvPlaceholders($value, $format = null, array &$usedEnvs
}
if ($placeholder === $value) {
$value = $resolved;
$completed = true;
} else {
if (!is_string($resolved) && !is_numeric($resolved)) {
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));
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)));
}
$value = str_ireplace($placeholder, $resolved, $value);
}
$usedEnvs[$env] = $env;
$this->envCounters[$env] = isset($this->envCounters[$env]) ? 1 + $this->envCounters[$env] : 1;

if ($completed) {
break 2;
}
}
}
}
Expand Down
Expand Up @@ -665,17 +665,49 @@ public function testCompileWithResolveEnv()
putenv('DUMMY_ENV_VAR');
}

public function testCompileWithArrayResolveEnv()
{
putenv('ARRAY={"foo":"bar"}');

$container = new ContainerBuilder();
$container->setParameter('foo', '%env(json:ARRAY)%');
$container->compile(true);

$this->assertSame(array('foo' => 'bar'), $container->getParameter('foo'));

putenv('ARRAY');
}

public function testCompileWithArrayAndAnotherResolveEnv()
{
putenv('DUMMY_ENV_VAR=abc');
putenv('ARRAY={"foo":"bar"}');

$container = new ContainerBuilder();
$container->setParameter('foo', '%env(json:ARRAY)%');
$container->setParameter('bar', '%env(DUMMY_ENV_VAR)%');
$container->compile(true);

$this->assertSame(array('foo' => 'bar'), $container->getParameter('foo'));
$this->assertSame('abc', $container->getParameter('bar'));

putenv('DUMMY_ENV_VAR');
putenv('ARRAY');
}

/**
* @expectedException \Symfony\Component\DependencyInjection\Exception\RuntimeException
* @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)%".
* @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)%".
*/
public function testCompileWithArrayResolveEnv()
public function testCompileWithArrayInStringResolveEnv()
{
$bag = new TestingEnvPlaceholderParameterBag();
$container = new ContainerBuilder($bag);
$container->setParameter('foo', '%env(ARRAY)%');
$container->setParameter('bar', 'ABC %env(ARRAY)%');
putenv('ARRAY={"foo":"bar"}');

$container = new ContainerBuilder();
$container->setParameter('foo', 'ABC %env(json:ARRAY)%');
$container->compile(true);

putenv('ARRAY');
}

/**
Expand Down Expand Up @@ -1418,11 +1450,3 @@ public function __construct(A $a)
{
}
}

class TestingEnvPlaceholderParameterBag extends EnvPlaceholderParameterBag
{
public function get($name)
{
return 'env(array)' === strtolower($name) ? array(123) : parent::get($name);
}
}

0 comments on commit 4f9d907

Please sign in to comment.