8000 feature #20232 [DependencyInjection] fixed ini file values conversion… · symfony/symfony@9e2ad93 · GitHub
[go: up one dir, main page]
Skip to content

Commit 9e2ad93

Browse files
fabpotfabpot
committed
feature #20232 [DependencyInjection] fixed ini file values conversion (fabpot)
This PR was merged into the 3.2-dev branch. Discussion ---------- [DependencyInjection] fixed ini file values conversion | Q | A | | --- | --- | | Branch? | master | | Bug fix? | yes | | New feature? | no | | BC breaks? | no-ish | | Deprecations? | no | | Tests pass? | yes | | Fixed tickets | n/a | | License | MIT | | Doc PR | n/a | When using the ini format to load parameters in the Container, the parameter values were converted by PHP directly (`'true'` => `1` for instance). But when using the YAML or XML format, the conversions are much broader and more precise (`'true'` => `true` for instance). This PR fixed fixes this discrepancy by using the same rules as XML (we could use `INI_SCANNER_TYPED` for recent versions of PHP but the rules are not exactly the same, so I prefer consistency here). One might argue that this is a new feature and that this should be merged into master, which I can accept as well. In master, the `XmlUtils::phpize()` method should be deprecated and replaced by a more generic phpize class. ping @symfony/deciders Commits ------- 4ccfce6 [DependencyInjection] fixed ini file values conversion
2 parents 4459598 + 4ccfce6 commit 9e2ad93

File tree

6 files changed

+139
-10
lines changed

6 files changed

+139
-10
lines changed

‎src/Symfony/Component/DependencyInjection/Loader/IniFileLoader.php

Lines changed: 35 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
namespace Symfony\Component\DependencyInjection\Loader;
1313

1414
use Symfony\Component\Config\Resource\FileResource;
15+
use Symfony\Component\Config\Util\XmlUtils;
1516
use Symfony\Component\DependencyInjection\Exception\InvalidArgumentException;
1617

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

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

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

40+
// real raw parsing
41+
$result = parse_ini_file($path, true, INI_SCANNER_RAW);
42+
3843
if (isset($result['parameters']) && is_array($result['parameters'])) {
3944
foreach ($result['parameters'] as $key => $value) {
40-
$this->container->setParameter($key, $value);
45+
$this->container->setParameter($key, $this->phpize($value));
4146
}
4247
}
4348
}
@@ -49,4 +54,33 @@ public function supports($resource, $type = null)
4954
{
5055
return is_string($resource) && 'ini' === pathinfo($resource, PATHINFO_EXTENSION);
5156
}
57+
58+
/**
59+
* Note that the following features are not supported:
60+
* * strings with escaped quotes are not supported "foo\"bar";
61+
* * string concatenation ("foo" "bar").
62+
*/
63+
private function phpize($value)
64+
{
65+
// trim on the right as comments removal keep whitespaces
66+
$value = rtrim($value);
67+
$lowercaseValue = strtolower($value);
68+
69+
switch (true) {
70+
case defined($value):
71+
return constant($value);
72+
case 'yes' === $lowercaseValue || 'on' === $lowercaseValue:
73+
return true;
74+
case 'no' === $lowercaseValue || 'off' === $lowercaseValue || 'none' === $lowercaseValue:
75+
return false;
76+
case isset($value[1]) && (
77+
("'" === $value[0] && "'" === $value[strlen($value) - 1]) ||
78+
('"' === $value[0] && '"' === $value[strlen($value) - 1])
79+
):
80+
// quoted string
81+
return substr($value, 1, -1);
82+
default:
83+
return XmlUtils::phpize($value);
84+
}
85+
}
5286
}

‎src/Symfony/Component/DependencyInjection/Tests/Fixtures/ini/almostvalid.ini

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
foo = '
2+
bar = bar

‎src/Symfony/Component/DependencyInjection/Tests/Fixtures/ini/types.ini

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
[parameters]
2+
true = true
3+
true_comment = true ; comment
4+
false = false
5+
null = null
6+
on = on
7+
off = off
8+
yes = yes
9+
no = no
10+
none = none
11+
constant = PHP_VERSION
12+
12 = 12
13+
12_string = '12'
14+
12_comment = 12 ; comment
15+
12_string_comment = '12' ; comment
16+
12_string_comment_again = "12" ; comment
17+
-12 = -12
18+
0 = 0
19+
1 = 1
20+
0b0110 = 0b0110
21+
11112222333344445555 = 1111,2222,3333,4444,5555
22+
0777 = 0777
23+
255 = 0xFF
24+
100.0 = 1e2
25+
-120.0 = -1.2E2
26+
-10100.1 = -10100.1
27+
-10,100.1 = -10,100.1

‎src/Symfony/Component/DependencyInjection/Tests/Loader/IniFileLoaderTest.php

Lines changed: 73 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -17,20 +17,13 @@
1717

1818
class IniFileLoaderTest extends \PHPUnit_Framework_TestCase
1919
{
20-
protected static $fixturesPath;
21-
2220
protected $container;
2321
protected $loader;
2422

25-
public static function setUpBeforeClass()
26-
{
27-
self::$fixturesPath = realpath(__DIR__.'/../Fixtures/');
28-
}
29-
3023
protected function setUp()
3124
{
3225
$this->container = new ContainerBuilder();
33-
$this->loader = new IniFileLoader($this->container, new FileLocator(self::$fixturesPath.'/ini'));
26+
$this->loader = new IniFileLoader($this->container, new FileLocator(realpath(__DIR__.'/../Fixtures/').'/ini'));
3427
}
3528

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

35+
/**
36+
* @dataProvider getTypeConversions
37+
*/
38+
public function testTypeConversions($key, $value, $supported)
39+
{
40+
$this->loader->load('types.ini');
41+
$parameters = $this->container->getParameterBag()->all();
42+
$this->assertSame($value, $parameters[$key], '->load() converts values to PHP types');
43+
}
44+
45+
/**
46+
* @dataProvider getTypeConversions
47+
* @requires PHP 5.6.1
48+
* This test illustrates where our conversions differs from INI_SCANNER_TYPED introduced in PHP 5.6.1
49+
*/
50+
public function testTypeConversionsWithNativePhp($key, $value, $supported)
51+
{
52+
if (defined('HHVM_VERSION_ID')) {
53+
return $this->markTestSkipped();
54+
}
55+
56+
if (!$supported) {
57+
return;
58+
}
59+
60+
$this->loader->load('types.ini');
61+
$expected = parse_ini_file(__DIR__.'/../Fixtures/ini/types.ini', true, INI_SCANNER_TYPED);
62+
$this->assertSame($value, $expected['parameters'][$key], '->load() converts values to PHP types');
63+
}
64+
65+
public function getTypeConversions()
66+
{
67+
return array(
68+
array('true_comment', true, true),
69+
array('true', true, true),
70+
array('false', false, true),
71+
array('on', true, true),
72+
array('off', false, true),
73+
array('yes', true, true),
74+
array('no', false, true),
75+
array('none', false, true),
76+
array('null', null, true),
77+
array('constant', PHP_VERSION, true),
78+
array('12', 12, true),
79+
array('12_string', '12', true),
80+
array('12_comment', 12, true),
81+
array('12_string_comment', '12', true),
82+
array('12_string_comment_again', '12', true),
83+
array('-12', -12, true),
84+
array('1', 1, true),
85+
array('0', 0, true),
86+
array('0b0110', bindec('0b0110'), false), // not supported by INI_SCANNER_TYPED
87+
array('11112222333344445555', '1111,2222,3333,4444,5555', true),
88+
array('0777', 0777, false), // not supported by INI_SCANNER_TYPED
89+
array('255', 0xFF, false), // not supported by INI_SCANNER_TYPED
90+
array('100.0', 1e2, false), // not supported by INI_SCANNER_TYPED
91+
array('-120.0', -1.2E2, false), // not supported by INI_SCANNER_TYPED
92+
array('-10100.1', -10100.1, false), // not supported by INI_SCANNER_TYPED
93+
array('-10,100.1', '-10,100.1', true),
94+
);
95+
}
96+
4297
/**
4398
* @expectedException \InvalidArgumentException
4499
* @expectedExceptionMessage The file "foo.ini" does not exist (in:
@@ -49,14 +104,23 @@ public function testExceptionIsRaisedWhenIniFileDoesNotExist()
49104
}
50105

51106
/**
52-
* @expectedException \InvalidArgumentException
107+
F438 * @expectedException \Symfony\Component\DependencyInjection\Exception\InvalidArgumentException
53108
* @expectedExceptionMessage The "nonvalid.ini" file is not valid.
54109
*/
55110
public function testExceptionIsRaisedWhenIniFileCannotBeParsed()
56111
{
57112
@$this->loader->load('nonvalid.ini');
58113
}
59114

115+
/**
116+
* @expectedException \Symfony\Component\DependencyInjection\Exception\InvalidArgumentException
117+
* @expectedExceptionMessage The "almostvalid.ini" file is not valid.
118+
*/
119+
public function testExceptionIsRaisedWhenIniFileIsAlmostValid()
120+
{
121+
@$this->loader->load('almostvalid.ini');
122+
}
123+
60124
public function testSupports()
61125
{
62126
$loader = new IniFileLoader(new ContainerBuilder(), new FileLocator());

‎src/Symfony/Component/DependencyInjection/Tests/Loader/XmlFileLoaderTest.php

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -164,6 +164,7 @@ public function testLoadImports()
164164
);
165165

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

168169
// Bad import throws no exception due to ignore_errors value.
169170
$loader->load('services4_bad_import.xml');

‎src/Symfony/Component/DependencyInjection/Tests/Loader/YamlFileLoaderTest.php

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -115,6 +115,7 @@ public function testLoadImports()
115115
$actual = $container->getParameterBag()->all();
116116
$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);
117117
$this->assertEquals(array_keys($expected), array_keys($actual), '->load() imports and merges imported files');
118+
$this->assertTrue($actual['imported_from_ini']);
118119

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

0 commit comments

Comments
 (0)
0