Skip to content

[DependencyInjection] fixed ini file values conversion #20232

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
Nov 3, 2016
Merged

[DependencyInjection] fixed ini file values conversion #20232

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
36 changes: 35 additions & 1 deletion src/Symfony/Component/DependencyInjection/Loader/IniFileLoader.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
namespace Symfony\Component\DependencyInjection\Loader;

use Symfony\Component\Config\Resource\FileResource;
use Symfony\Component\Config\Util\XmlUtils;
use Symfony\Component\DependencyInjection\Exception\InvalidArgumentException;

/**
Expand All @@ -30,14 +31,18 @@ public function load($resource, $type = null)

$this->container->addResource(new FileResource($path));

// first pass to catch parsing errors
$result = parse_ini_file($path, true);
if (false === $result || array() === $result) {
throw new InvalidArgumentException(sprintf('The "%s" file is not valid.', $resource));
}

// real raw parsing
$result = parse_ini_file($path, true, INI_SCANNER_RAW);

if (isset($result['parameters']) && is_array($result['parameters'])) {
foreach ($result['parameters'] as $key => $value) {
$this->container->setParameter($key, $value);
$this->container->setParameter($key, $this->phpize($value));
}
}
}
Expand All @@ -49,4 +54,33 @@ public function supports($resource, $type = null)
{
return is_string($resource) && 'ini' === pathinfo($resource, PATHINFO_EXTENSION);
}

/**
* Note that the following features are not supported:
* * strings with escaped quotes are not supported "foo\"bar";
* * string concatenation ("foo" "bar").
*/
private function phpize($value)
{
// trim on the right as comments removal keep whitespaces
$value = rtrim($value);
$lowercaseValue = strtolower($value);

switch (true) {
case defined($value):
return constant($value);
case 'yes' === $lowercaseValue || 'on' === $lowercaseValue:
return true;
case 'no' === $lowercaseValue || 'off' === $lowercaseValue || 'none' === $lowercaseValue:
return false;
case isset($value[1]) && (
("'" === $value[0] && "'" === $value[strlen($value) - 1]) ||
('"' === $value[0] && '"' === $value[strlen($value) - 1])
):
// quoted string
Copy link
Contributor

Choose a reason for hiding this comment

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

Perhaps throw if 1 === strlen($value) (invalid syntax)?

Copy link
Member Author

Choose a reason for hiding this comment

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

the php.ini file would not be valid in that case.

Copy link
Contributor

Choose a reason for hiding this comment

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

It was fatal before :/ https://3v4l.org/VgDvJ

With SCANNER_RAW it parses though.. but that doesnt mean it's valid, it's raw :)

Copy link
Member Author

Choose a reason for hiding this comment

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

fixed

return substr($value, 1, -1);
default:
return XmlUtils::phpize($value);
}
}
}
2 changes: 2 additions & 0 deletions src/Symfony/Component/DependencyInjection/Tests/Fixtures/ini/almostvalid.ini
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
foo = '
bar = bar
27 changes: 27 additions & 0 deletions src/Symfony/Component/DependencyInjection/Tests/Fixtures/ini/types.ini
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
[parameters]
true = true
true_comment = true ; comment
false = false
null = null
on = on
off = off
yes = yes
no = no
none = none
constant = PHP_VERSION
12 = 12
12_string = '12'
12_comment = 12 ; comment
12_string_comment = '12' ; comment
12_string_comment_again = "12" ; comment
-12 = -12
0 = 0
1 = 1
0b0110 = 0b0110
11112222333344445555 = 1111,2222,3333,4444,5555
0777 = 0777
255 = 0xFF
100.0 = 1e2
-120.0 = -1.2E2
-10100.1 = -10100.1
-10,100.1 = -10,100.1
82 changes: 73 additions & 9 deletions src/Symfony/Component/DependencyInjection/Tests/Loader/IniFileLoaderTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,20 +17,13 @@

class IniFileLoaderTest extends \PHPUnit_Framework_TestCase
{
protected static $fixturesPath;

protected $container;
protected $loader;

public static function setUpBeforeClass()
{
self::$fixturesPath = realpath(__DIR__.'/../Fixtures/');
}

protected function setUp()
{
$this->container = new ContainerBuilder();
$this->loader = new IniFileLoader($this->container, new FileLocator(self::$fixturesPath.'/ini'));
$this->loader = new IniFileLoader($this->container, new FileLocator(realpath(__DIR__.'/../Fixtures/').'/ini'));
}

public function testIniFileCanBeLoaded()
Expand All @@ -39,6 +32,68 @@ public function testIniFileCanBeLoaded()
$this->assertEquals(array('foo' => 'bar', 'bar' => '%foo%'), $this->container->getParameterBag()->all(), '->load() takes a single file name as its first argument');
}

/**
* @dataProvider getTypeConversions
*/
public function testTypeConversions($key, $value, $supported)
{
$this->loader->load('types.ini');
$parameters = $this->container->getParameterBag()->all();
$this->assertSame($value, $parameters[$key], '->load() converts values to PHP types');
}

/**
* @dataProvider getTypeConversions
* @requires PHP 5.6.1
* This test illustrates where our conversions differs from INI_SCANNER_TYPED introduced in PHP 5.6.1
*/
public function testTypeConversionsWithNativePhp($key, $value, $supported)
{
if (defined('HHVM_VERSION_ID')) {
return $this->markTestSkipped();
}

if (!$supported) {
return;
}

$this->loader->load('types.ini');
$expected = parse_ini_file(__DIR__.'/../Fixtures/ini/types.ini', true, INI_SCANNER_TYPED);
$this->assertSame($value, $expected['parameters'][$key], '->load() converts values to PHP types');
}

public function getTypeConversions()
{
return array(
array('true_comment', true, true),
array('true', true, true),
array('false', false, true),
array('on', true, true),
array('off', false, true),
array('yes', true, true),
array('no', false, true),
array('none', false, true),
array('null', null, true),
array('constant', PHP_VERSION, true),
array('12', 12, true),
array('12_string', '12', true),
array('12_comment', 12, true),
array('12_string_comment', '12', true),
array('12_string_comment_again', '12', true),
array('-12', -12, true),
array('1', 1, true),
array('0', 0, true),
array('0b0110', bindec('0b0110'), false), // not supported by INI_SCANNER_TYPED
array('11112222333344445555', '1111,2222,3333,4444,5555', true),
array('0777', 0777, false), // not supported by INI_SCANNER_TYPED
array('255', 0xFF, false), // not supported by INI_SCANNER_TYPED
array('100.0', 1e2, false), // not supported by INI_SCANNER_TYPED
array('-120.0', -1.2E2, false), // not supported by INI_SCANNER_TYPED
array('-10100.1', -10100.1, false), // not supported by INI_SCANNER_TYPED
array('-10,100.1', '-10,100.1', true),
);
}

/**
* @expectedException \InvalidArgumentException
* @expectedExceptionMessage The file "foo.ini" does not exist (in:
Expand All @@ -49,14 +104,23 @@ public function testExceptionIsRaisedWhenIniFileDoesNotExist()
}

/**
* @expectedException \InvalidArgumentException
* @expectedException \Symfony\Component\DependencyInjection\Exception\InvalidArgumentException
* @expectedExceptionMessage The "nonvalid.ini" file is not valid.
*/
public function testExceptionIsRaisedWhenIniFileCannotBeParsed()
{
@$this->loader->load('nonvalid.ini');
}

/**
* @expectedException \Symfony\Component\DependencyInjection\Exception\InvalidArgumentException
* @expectedExceptionMessage The "almostvalid.ini" file is not valid.
*/
public function testExceptionIsRaisedWhenIniFileIsAlmostValid()
{
@$this->loader->load('almostvalid.ini');
}

public function testSupports()
{
$loader = new IniFileLoader(new ContainerBuilder(), new FileLocator());
Expand Down
1 change: 1 addition & 0 deletions src/Symfony/Component/DependencyInjection/Tests/Loader/XmlFileLoaderTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -164,6 +164,7 @@ public function testLoadImports()
);

$this->assertEquals(array_keys($expected), array_keys($actual), '->load() imports and merges imported files');
$this->assertTrue($actual['imported_from_ini']);

// Bad import throws no exception due to ignore_errors value.
$loader->load('services4_bad_import.xml');
Expand Down
1 change: 1 addition & 0 deletions src/Symfony/Component/DependencyInjection/Tests/Loader/YamlFileLoaderTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,7 @@ public function testLoadImports()
$actual = $container->getParameterBag()->all();
$expected = array('foo' => 'bar', 'values' => array(true, false, PHP_INT_MAX), 'bar' => '%foo%', 'escape' => '@escapeme', 'foo_bar' => new Reference('foo_bar'), 'mixedcase' => array('MixedCaseKey' => 'value'), 'imported_from_ini' => true, 'imported_from_xml' => true);
$this->assertEquals(array_keys($expected), array_keys($actual), '->load() imports and merges imported files');
$this->assertTrue($actual['imported_from_ini']);

// Bad import throws no exception due to ignore_errors value.
$loader->load('services4_bad_import.yml');
Expand Down