Permalink
Browse files

merged branch bamarni/2.0 (PR #6154)

This PR was merged into the 2.0 branch.

Commits
-------

f0743b1 Merge pull request #1 from pylebecq/2.0
555e777 [FrameworkBundle] Added tests for trusted_proxies configuration.
a0e2391 [FrameworkBundle] used the new method for trusted proxies

Discussion
----------

[FrameworkBundle] used the new method for trusted proxies

This makes the framework bundle using the new method from the request class.

---------------------------------------------------------------------------

by fabpot at 2012-12-05T10:38:20Z

As this is a sensitive issue, can you add some tests? Thanks.

---------------------------------------------------------------------------

by bamarni at 2012-12-06T13:00:24Z

Well I don't know why it fails on travis, I can't run the full test suite locally because of a segfault but ```phpunit src/Symfony/Bundle/``` marks all the tests as passing.

---------------------------------------------------------------------------

by fabpot at 2012-12-06T13:08:11Z

But it looks like the failing tests come from what you've changed.

---------------------------------------------------------------------------

by bamarni at 2012-12-06T13:29:33Z

Yes, I'm not saying it's not my fault but I can't reproduce this as locally it tells me they pass, I'll try to fix this this evening.

---------------------------------------------------------------------------

by bamarni at 2012-12-06T17:49:28Z

Apparently it fails only when running the whole testsuite, looking at other travis builds I can see this one on 2.0 : https://travis-ci.org/symfony/symfony/jobs/3495511 which fails in a similar way than here (https://travis-ci.org/symfony/symfony/jobs/3530928). Because of a place trying to access an undefined $_SERVER key : ```PHP Notice:  Undefined index: SCRIPT_NAME ...``` but I can't find where, and the stack trace references some phpunit classes.

I'd be happy if someone could give me some pointers in here as I don't have any clue about how to fix this..

---------------------------------------------------------------------------

by bamarni at 2012-12-06T18:00:57Z

As a consulsion I'd say I can't run the whole testsuite locally (it fails even when I revert my commit), so there is no reliable way for me to fix this, if anyone is up for continuing this feel free.

---------------------------------------------------------------------------

by fabpot at 2012-12-11T09:47:48Z

@bamarni Can you just update this PR with the code change and no tests at all? I will then finish the PR. Thanks.

---------------------------------------------------------------------------

by bamarni at 2012-12-11T16:58:17Z

@fabpot: thanks for helping me out on this, hope you won't run into the same issue!
  • Loading branch information...
2 parents da98371 + f0743b1 commit c0fc33df44bb3bdc80828f8af4c9b52beecbc093 @fabpot fabpot committed Dec 15, 2012
@@ -47,7 +47,15 @@ public function getConfigTreeBuilder()
$rootNode
->children()
->scalarNode('charset')->end()
- ->scalarNode('trust_proxy_headers')->defaultFalse()->end()
+ ->arrayNode('trusted_proxies')
+ ->prototype('scalar')
+ ->validate()
+ ->ifTrue(function($v) { return !filter_var($v, FILTER_VALIDATE_IP); })
+ ->thenInvalid('Invalid proxy IP "%s"')
+ ->end()
+ ->end()
+ ->end()
+ ->scalarNode('trust_proxy_headers')->defaultFalse()->end() // @deprecated, to be removed in 2.3
->scalarNode('secret')->isRequired()->end()
->scalarNode('ide')->defaultNull()->end()
->booleanNode('test')->end()
@@ -62,6 +62,9 @@ public function load(array $configs, ContainerBuilder $container)
}
$container->setParameter('kernel.secret', $config['secret']);
+ $container->setParameter('kernel.trusted_proxies', $config['trusted_proxies']);
+
+ // @deprecated, to be removed in 2.3
$container->setParameter('kernel.trust_proxy_headers', $config['trust_proxy_headers']);
if (!empty($config['test'])) {
@@ -37,8 +37,10 @@ class FrameworkBundle extends Bundle
{
public function boot()
{
- if ($this->container->getParameter('kernel.trust_proxy_headers')) {
- Request::trustProxyData();
+ if ($trustedProxies = $this->container->getParameter('kernel.trusted_proxies')) {
+ Request::setTrustedProxies($trustedProxies);
+ } elseif ($this->container->getParameter('kernel.trust_proxy_headers')) {
+ Request::trustProxyData(); // @deprecated, to be removed in 2.3
}
}
@@ -0,0 +1,78 @@
+<?php
+
+/*
+ * This file is part of the Symfony package.
+ *
+ * (c) Fabien Potencier <fabien@symfony.com>
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+namespace Symfony\Bundle\FrameworkBundle\Tests\DependencyInjection;
+
+use Symfony\Bundle\FrameworkBundle\DependencyInjection\Configuration;
+use Symfony\Component\Config\Definition\Processor;
+
+class ConfigurationTest extends \PHPUnit_Framework_TestCase
+{
+ /**
+ * @dataProvider getTestConfigTreeData
+ */
+ public function testConfigTree($options, $results)
+ {
+ $processor = new Processor();
+ $configuration = new Configuration(array());
+ $config = $processor->processConfiguration($configuration, array($options));
+
+ $this->assertEquals($results, $config);
+ }
+
+ public function getTestConfigTreeData()
+ {
+ return array(
+ array(array('secret' => 's3cr3t'), array('secret' => 's3cr3t', 'trusted_proxies' => array(), 'trust_proxy_headers' => false, 'ide' => NULL, 'annotations' => array('cache' => 'file', 'file_cache_dir' => '%kernel.cache_dir%/annotations', 'debug' => false))),
+ );
+ }
+
+ /**
+ * @dataProvider getTestValidTrustedProxiesData
+ */
+ public function testValidTrustedProxies($options, $results)
+ {
+ $processor = new Processor();
+ $configuration = new Configuration(array());
+ $config = $processor->processConfiguration($configuration, array($options));
+
+ $this->assertEquals($results, $config);
+ }
+
+ public function getTestValidTrustedProxiesData()
+ {
+ return array(
+ array(array('secret' => 's3cr3t', 'trusted_proxies' => array('127.0.0.1')), array('secret' => 's3cr3t', 'trusted_proxies' => array('127.0.0.1'), 'trust_proxy_headers' => false, 'ide' => NULL, 'annotations' => array('cache' => 'file', 'file_cache_dir' => '%kernel.cache_dir%/annotations', 'debug' => false))),
+ array(array('secret' => 's3cr3t', 'trusted_proxies' => array('::1')), array('secret' => 's3cr3t', 'trusted_proxies' => array('::1'), 'trust_proxy_headers' => false, 'ide' => NULL, 'annotations' => array('cache' => 'file', 'file_cache_dir' => '%kernel.cache_dir%/annotations', 'debug' => false))),
+ array(array('secret' => 's3cr3t', 'trusted_proxies' => array('127.0.0.1', '::1')), array('secret' => 's3cr3t', 'trusted_proxies' => array('127.0.0.1', '::1'), 'trust_proxy_headers' => false, 'ide' => NULL, 'annotations' => array('cache' => 'file', 'file_cache_dir' => '%kernel.cache_dir%/annotations', 'debug' => false))),
+ );
+ }
+
+ /**
+ * @expectedException Symfony\Component\Config\Definition\Exception\InvalidTypeException
+ */
+ public function testInvalidTypeTrustedProxies()
+ {
+ $processor = new Processor();
+ $configuration = new Configuration(array());
+ $config = $processor->processConfiguration($configuration, array(array('secret' => 's3cr3t', 'trusted_proxies' => 'Not an IP address')));
+ }
+
+ /**
+ * @expectedException Symfony\Component\Config\Definition\Exception\InvalidConfigurationException
+ */
+ public function testInvalidValueTrustedProxies()
+ {
+ $processor = new Processor();
+ $configuration = new Configuration(array());
+ $config = $processor->processConfiguration($configuration, array(array('secret' => 's3cr3t', 'trusted_proxies' => array('Not an IP address'))));
+ }
+}

0 comments on commit c0fc33d

Please sign in to comment.