8000 [DI] Case sensitive parameter names by ro0NL · Pull Request #23874 · symfony/symfony · GitHub
[go: up one dir, main page]
Skip to content

[DI] Case sensitive parameter names #23874

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.

Sign up for GitHub

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 22, 2017
Merged

[DI] Case sensitive parameter names #23874

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
1 change: 1 addition & 0 deletions src/Symfony/Component/DependencyInjection/CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ CHANGELOG
* deprecated service auto-registration while autowiring
* deprecated the ability to check for the initialization of a private service with the `Container::initialized()` method
* deprecated support for top-level anonymous services in XML
* deprecated case insensitivity of parameter names

3.3.0
-----
Expand Down
34 changes: 28 additions & 6 deletions src/Symfony/Component/DependencyInjection/Dumper/PhpDumper.php
View file
Open in desktop
67ED
Original file line number Diff line number Diff line change
Expand Up @@ -1056,11 +1056,15 @@ private function addDefaultParametersMethod()

$php = array();
$dynamicPhp = array();
$normalizedParams = array();

foreach ($this->container->getParameterBag()->all() as $key => $value) {
if ($key !== $resolvedKey = $this->container->resolveEnvPlaceholders($key)) {
throw new InvalidArgumentException(sprintf('Parameter name cannot use env parameters: %s.', $resolvedKey));
}
if ($key !== $lcKey = strtolower($key)) {
$normalizedParams[] = sprintf(' %s => %s,', $this->export($lcKey), $this->export($key));
}
$export = $this->exportParameters(array($value));
$export = explode('0 => ', substr(rtrim($export, " )\n"), 7, -1), 2);

Expand All @@ -1082,7 +1086,7 @@ private function addDefaultParametersMethod()
public function getParameter($name)
{
if (!(isset($this->parameters[$name]) || isset($this->loadedDynamicParameters[$name]) || array_key_exists($name, $this->parameters))) {
$name = strtolower($name);
$name = $this->normalizeParameterName($name);

if (!(isset($this->parameters[$name]) || isset($this->loadedDynamicParameters[$name]) || array_key_exists($name, $this->parameters))) {
throw new InvalidArgumentException(sprintf('The parameter "%s" must be defined.', $name));
Expand All @@ -1100,7 +1104,7 @@ public function getParameter($name)
*/
public function hasParameter($name)
{
$name = strtolower($name);
$name = $this->normalizeParameterName($name);

return isset($this->parameters[$name]) || isset($this->loadedDynamicParameters[$name]) || array_key_exists($name, $this->parameters);
}
Expand Down Expand Up @@ -1170,6 +1174,26 @@ private function getDynamicParameter(\$name)
{$getDynamicParameter}
}


EOF;

$code .= ' private $normalizedParameterNames = '.($normalizedParams ? sprintf("array(\n%s\n );", implode("\n", $normalizedParams)) : 'array();')."\n";
$code .= <<<'EOF'

private function normalizeParameterName($name)
{
if (isset($this->normalizedParameterNames[$normalizedName = strtolower($name)]) || isset($this->parameters[$normalizedName]) || array_key_exists($normalizedName, $this->parameters)) {
$normalizedName = isset($this->normalizedParameterNames[$normalizedName]) ? $this->normalizedParameterNames[$normalizedName] : $normalizedName;
Copy link
Contributor

Choose a reason for hiding this comment

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

that is not the normalized (lowercased) parameter name, but the original name. using the same variable for both things is confusing

Copy link
Contributor

Choose a reason for hiding this comment

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

Or it might be easier to rename the variable $normalizedName = strtolower($name) to $lowercasedName to remove the duplicate usage

Copy link
Contributor Author

Choose a reason for hiding this comment

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

not sure we care.. it's optimized code. I believe this will be a hot code path.

if ((string) $name !== $normalizedName) {
@trigger_error(sprintf('Parameter names will be made case sensitive in Symfony 4.0. Using "%s" instead of "%s" is deprecated since version 3.4.', $name, $normalizedName), E_USER_DEPRECATED);
}
} else {
$normalizedName = $this->normalizedParameterNames[$normalizedName] = (string) $name;
}

return $normalizedName;
}

EOF;
} elseif ($dynamicPhp) {
throw new RuntimeException('You cannot dump a not-frozen container with dynamic parameters.');
Expand Down Expand Up @@ -1555,10 +1579,10 @@ private function dumpValue($value, $interpolate = true)
if (preg_match('/^%([^%]+)%$/', $value, $match)) {
// we do this to deal with non string values (Boolean, integer, ...)
// the preg_replace_callback converts them to strings
return $this->dumpParameter(strtolower($match[1]));
return $this->dumpParameter($match[1]);
} else {
$replaceParameters = function ($match) {
return "'.".$this->dumpParameter(strtolower($match[2])).".'";
return "'.".$this->dumpParameter($match[2]).".'";
};

$code = str_replace('%%', '%', preg_replace_callback('/(?<!%)(%)([^%]+)\1/', $replaceParameters, $this->export($value)));
Expand Down Expand Up @@ -1604,8 +1628,6 @@ private function dumpLiteralClass($class)
*/
private function dumpParameter($name)
{
$name = strtolower($name);

if ($this->container->isCompiled() && $this->container->hasParameter($name)) {
$value = $this->container->getParameter($name);
$dumpedValue = $this->dumpValue($value, false);
Expand Down
5 changes: 0 additions & 5 deletions src/Symfony/Component/DependencyInjection/Loader/XmlFileLoader.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -486,11 +486,6 @@ private function getArgumentsAsPhp(\DOMElement $node, $name, $file, $lowercase =
$key = array_pop($keys);
} else {
$key = $arg->getAttribute('key');

// parameter keys are case insensitive
if ('parameter' == $name && $lowercase) {
$key = strtolower($key);
}
}

$onInvalid = $arg->getAttribute('on-invalid');
Expand Down
2 changes: 1 addition & 1 deletion src/Symfony/Component/DependencyInjection/ParameterBag/EnvPlaceholderParameterBag.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ public function resolve()
parent::resolve();

foreach ($this->envPlaceholders as $env => $placeholders) {
if (!isset($this->parameters[$name = strtolower("env($env)")])) {
if (!$this->has($name = "env($env)")) {
continue;
}
if (is_numeric($default = $this->parameters[$name])) {
Expand Down
30 changes: 23 additions & 7 deletions src/Symfony/Component/DependencyInjection/ParameterBag/ParameterBag.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,8 @@ class ParameterBag implements ParameterBagInterface
protected $parameters = array();
protected $resolved = false;

private $normalizedNames = array();

/**
* @param array $parameters An array of parameters
*/
Expand All @@ -49,7 +51,7 @@ public function clear()
public function add(array $parameters)
{
foreach ($parameters as $key => $value) {
$this->parameters[strtolower($key)] = $value;
$this->set($key, $value);
}
}

Expand All @@ -66,7 +68,7 @@ public function all()
*/
public function get($name)
{
$name = strtolower($name);
$name = $this->normalizeName($name);

if (!array_key_exists($name, $this->parameters)) {
if (!$name) {
Expand Down Expand Up @@ -111,15 +113,15 @@ public function get($name)
*/
public function set($name, $value)
{
$this->parameters[strtolower($name)] = $value;
$this->parameters[$this->normalizeName($name)] = $value;
}

/**
* {@inheritdoc}
*/
public function has($name)
{
return array_key_exists(strtolower($name), $this->parameters);
return array_key_exists($this->normalizeName($name), $this->parameters);
}

/**
Expand All @@ -129,7 +131,7 @@ public function has($name)
*/
public function remove($name)
{
unset($this->parameters[strtolower($name)]);
unset($this->parameters[$this->normalizeName($name)]);
}

/**
Expand Down Expand Up @@ -206,7 +208,7 @@ public function resolveString($value, array $resolving = array())
// a non-string in a parameter value
if (preg_match('/^%([^%\s]+)%$/', $value, $match)) {
$key = $match[1];
$lcKey = strtolower($key);
$lcKey = strtolower($key); // strtolower() to be removed in 4.0

if (isset($resolving[$lcKey])) {
throw new ParameterCircularReferenceException(array_keys($resolving));
Expand All @@ -224,7 +226,7 @@ public function resolveString($value, array $resolving = array())
}

$key = $match[1];
$lcKey = strtolower($key);
$lcKey = strtolower($key); // strtolower() to be removed in 4.0
if (isset($resolving[$lcKey])) {
throw new ParameterCircularReferenceException(array_keys($resolving));
}
Expand Down Expand Up @@ -288,4 +290,18 @@ public function unescapeValue($value)

return $value;
}

private function normalizeName($name)
{
if (isset($this->normalizedNames[$normalizedName = strtolower($name)])) {
$normalizedName = $this->normalizedNames[$normalizedName];
if ((string) $name !== $normalizedName) {
@trigger_error(sprintf('Parameter names will be made case sensitive in Symfony 4.0. Using "%s" instead of "%s" is deprecated since version 3.4.', $name, $normalizedName), E_USER_DEPRECATED);
}
} else {
$normalizedName = $this->normalizedNames[$normalizedName] = (string) $name;
}

return $normalizedName;
}
}
15 changes: 15 additions & 0 deletions src/Symfony/Component/DependencyInjection/Tests/ContainerBuilderTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1103,6 +1103,21 @@ public function testPrivateServiceTriggersDeprecation()

$container->get('bar');
}

/**
* @group legacy
* @expectedDeprecation Parameter names will be made case sensitive in Symfony 4.0. Using "FOO" instead of "foo" is deprecated since version 3.4.
*/
public function testParameterWithMixedCase()
{
$container = new ContainerBuilder(new ParameterBag(array('foo' => 'bar')));
$container->register('foo', 'stdClass')
->setProperty('foo', '%FOO%');

$container->compile();

$this->assertSame('bar', $container->get('foo')->foo);
}
}

class FooClass
Expand Down
18 changes: 14 additions & 4 deletions src/Symfony/Component/DependencyInjection/Tests/ContainerTest.php
View file
Open in desktop
< F42D td id="diff-b8fdab49473bab5ee23e6cba0ef47de4abbb2e91ba2f6546af1c8523d5f2fe54L139" data-line-number="139" class="blob-num blob-num-context js-linkable-line-number">
Original file line number Diff line number Diff line change
Expand Up @@ -125,10 +125,6 @@ public function testGetSetParameter()
$sc->setParameter('foo', 'baz');
$this->assertEquals('baz', $sc->getParameter('foo'), '->setParameter() overrides previously set parameter');

$sc->setParameter('Foo', 'baz1');
$this->assertEquals('baz1', $sc->getParameter('foo'), '->setParameter() converts the key to lowercase');
$this->assertEquals('baz1', $sc->getParameter('FOO'), '->getParameter() converts the key to lowercase');

try {
$sc->getParameter('baba');
$this->fail('->getParameter() thrown an \InvalidArgumentException if the key does not exist');
Expand All @@ -138,6 +134,20 @@ public function testGetSetParameter()
}
}

/**
* @group legacy
* @expectedDeprecation Parameter names will be made case sensitive in Symfony 4.0. Using "Foo" instead of "foo" is deprecated since version 3.4.
* @expectedDeprecation Parameter names will be made case sensitive in Symfony 4.0. Using "FOO" instead of "foo" is deprecated since version 3.4.
*/
public function testGetSetParameterWithMixedCase()
{
$sc = new Container(new ParameterBag(array('foo' => 'bar')));

$sc->setParameter('Foo', 'baz1');
$this->assertEquals('baz1', $sc->getParameter('foo'), '->setParameter() converts the key to lowercase');
$this->assertEquals('baz1', $sc->getParameter('FOO'), '->getParameter() converts the key to lowercase');
}

public function testGetServiceIds()
{
$sc = new Container();
Expand Down
39 changes: 39 additions & 0 deletions src/Symfony/Component/DependencyInjection/Tests/Dumper/PhpDumperTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -670,4 +670,43 @@ public function testPrivateServiceTriggersDeprecation()

$container->get('bar');
}

/**
* @group legacy
* @expectedDeprecation Parameter names will be made case sensitive in Symfony 4.0. Using "foo" instead of "Foo" is deprecated since version 3.4.
* @expectedDeprecation Parameter names will be made case sensitive in Symfony 4.0. Using "FOO" instead of "Foo" is deprecated since version 3.4.
* @expectedDeprecation Parameter names will be made case sensitive in Symfony 4.0. Using "bar" instead of "BAR" is deprecated since version 3.4.
*/
public function testParameterWithMixedCase()
{
$container = new ContainerBuilder(new ParameterBag(array('Foo' => 'bar', 'BAR' => 'foo')));
$container->compile();

$dumper = new PhpDumper($container);
eval('?>'.$dumper->dump(array('class' => 'Symfony_DI_PhpDumper_Test_Parameter_With_Mixed_Case')));

$container = new \Symfony_DI_PhpDumper_Test_Parameter_With_Mixed_Case();

$this->assertSame('bar', $container->getParameter('foo'));
$this->assertSame('bar', $container->getParameter('FOO'));
$this->assertSame('foo', $container->getParameter('bar'));
$this->assertSame('foo', $container->getParameter('BAR'));
}

/**
* @group legacy
* @expectedDeprecation Parameter names will be made case sensitive in Symfony 4.0. Using "FOO" instead of "foo" is deprecated since version 3.4.
*/
public function testParameterWithLowerCase()
{
$container = new ContainerBuilder(new ParameterBag(array('foo' => 'bar')));
$container->compile();

$dumper = new PhpDumper($container);
eval('?>'.$dumper->dump(array('class' => 'Symfony_DI_PhpDumper_Test_Parameter_With_Lower_Case')));

$container = new \Symfony_DI_PhpDumper_Test_Parameter_With_Lower_Case();

$this->assertSame('bar', $container->getParameter('FOO'));
}
}
2 changes: 1 addition & 1 deletion src/Symfony/Component/DependencyInjection/Tests/Fixtures/containers/container8.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
use Symfony\Component\DependencyInjection\ParameterBag\ParameterBag;

$container = new ContainerBuilder(new ParameterBag(array(
'FOO' => '%baz%',
'foo' => '%baz%',
'baz' => 'bar',
'bar' => 'foo is %%foo bar',
'escape' => '@escapeme',
Expand Down
20 changes: 18 additions & 2 deletions src/Symfony/Component/DependencyInjection/Tests/Fixtures/php/services10.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ protected function getTestService()
public function getParameter($name)
{
if (!(isset($this->parameters[$name]) || isset($this->loadedDynamicParameters[$name]) || array_key_exists($name, $this->parameters))) {
$name = strtolower($name);
$name = $this->normalizeParameterName($name);

if (!(isset($this->parameters[$name]) || isset($this->loadedDynamicParameters[$name]) || array_key_exists($name, $this->parameters))) {
throw new InvalidArgumentException(sprintf('The parameter "%s" must be defined.', $name));
Expand All @@ -96,7 +96,7 @@ public function getParameter($name)
*/
public function hasParameter($name)
{
$name = strtolower($name);
$name = $this->normalizeParameterName($name);

return isset($this->parameters[$name]) || isset($this->loadedDynamicParameters[$name]) || array_key_exists($name, $this->parameters);
}
Expand Down Expand Up @@ -142,6 +142,22 @@ private function getDynamicParameter($name)
throw new InvalidArgumentException(sprintf('The dynamic parameter "%s" must be defined.', $name));
}

private $normalizedParameterNames = array();

private function normalizeParameterName($name)
{
if (isset($this->normalizedParameterNames[$normalizedName = strtolower($name)]) || isset($this->parameters[$normalizedName]) || array_key_exists($normalizedName, $this->parameters)) {
$normalizedName = isset($this->normalizedParameterNames[$normalizedName]) ? $this->normalizedParameterNames[$normalizedName] : $normalizedName;
if ((string) $name !== $normalizedName) {
@trigger_error(sprintf('Parameter names will be made case sensitive in Symfony 4.0. Using "%s" instead of "%s" is deprecated since version 3.4.', $name, $normalizedName), E_USER_DEPRECATED);
}
} else {
$normalizedName = $this->normalizedParameterNames[$normalizedName] = (string) $name;
}

return $normalizedName;
}

/**
* Gets the default parameters.
*
Expand Down
20 changes: 18 additions & 2 deletions src/Symfony/Component/DependencyInjection/Tests/Fixtures/php/services12.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ protected function getTestService()
public function getParameter($name)
{
if (!(isset($this->parameters[$name]) || isset($this->loadedDynamicParameters[$name]) || array_key_exists($name, $this->parameters))) {
$name = strtolower($name);
$name = $this->normalizeParameterName($name);

if (!(isset($this->parameters[$name]) || isset($this->loadedDynamicParameters[$name]) || array_key_exists($name, $this->parameters))) {
throw new InvalidArgumentException(sprintf('The parameter "%s" must be defined.', $name));
Expand All @@ -100,7 +100,7 @@ public function getParameter($name)
*/
public function hasParameter($name)
{
$name = strtolower($name);
$name = $this->normalizeParameterName($name);

return isset($this->parameters[$name]) || isset($this->loadedDynamicParameters[$name]) || array_key_exists($name, $this->parameters);
}
Expand Down Expand Up @@ -156,6 +156,22 @@ private function getDynamicParameter($name)
return $this->dynamicParameters[$name] = $value;
}

private $normalizedParameterNames = array();

private function normalizeParameterName($name)
{
if (isset($this->normalizedParameterNames[$normalizedName = strtolower($name)]) || isset($this->parameters[$normalizedName]) || array_key_exists($normalizedName, $this->parameters)) {
$normalizedName = isset($this->normalizedParameterNames[$normalizedName]) ? $this->normalizedParameterNames[$normalizedName] : $normalizedName;
if ((string) $name !== $normalizedName) {
@trigger_error(sprintf('Parameter names will be made case sensitive in Symfony 4.0. Using "%s" instead of "%s" is deprecated since version 3.4.', $name, $normalizedName), E_USER_DEPRECATED);
}
} else {
$normalizedName = $this->normalizedParameterNames[$normalizedName] = (string) $name;
}

return $normalizedName;
}

/**
* Gets the default parameters.
*
Expand Down
Loading
0