Skip to content

Commit 44c5d7f

Browse files
nicolas-grekasnicolas-grekas
committed
Merge branch '2.7' into 2.8
* 2.7: fixed CS fixed CS [Security] Namespace generated CSRF tokens depending of the current scheme ensure that submitted data are uploaded files [Console] remove dead code bumped Symfony version to 2.7.38 updated VERSION for 2.7.37 updated CHANGELOG for 2.7.37 [Security] Validate redirect targets using the session cookie domain prevent bundle readers from breaking out of paths
2 parents db6da75 + b67b807 commit 44c5d7f

File tree

26 files changed

+605
-138
lines changed

26 files changed

+605
-138
lines changed

‎CHANGELOG-2.7.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,11 @@ in 2.7 minor versions.
77
To get the diff for a specific change, go to https://github.com/symfony/symfony/commit/XXX where XXX is the change hash
88
To get the diff between two versions, go to https://github.com/symfony/symfony/compare/v2.7.0...v2.7.1
99

10+
* 2.7.37 (2017-11-13)
11+
12+
* bug #24952 [HttpFoundation] Fix session-related BC break (nicolas-grekas, sroze)
13+
* bug #24929 [Console] Fix traversable autocomplete values (ro0NL)
14+
1015
* 2.7.36 (2017-11-10)
1116

1217
* bug #24888 [FrameworkBundle] Specifically inject the debug dispatcher in the collector (ogizanagi)

‎UPGRADE-2.7.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,8 @@ Router
3939
Form
4040
----
4141

42+
* the `isFileUpload()` method was added to the `RequestHandlerInterface`
43+
4244
* In form types and extension overriding the "setDefaultOptions" of the
4345
AbstractType or AbstractTypeExtension has been deprecated in favor of
4446
overriding the new "configureOptions" method.

‎src/Symfony/Bundle/FrameworkBundle/Resources/config/security_csrf.xml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020
<service id="security.csrf.token_manager" class="%security.csrf.token_manager.class%">
2121
<argument type="service" id="security.csrf.token_generator" />
2222
<argument type="service" id="security.csrf.token_storage" />
23+
<argument type="service" id="request_stack" on-invalid="ignore" />
2324
</service>
2425
</services>
2526
</container>

‎src/Symfony/Bundle/SecurityBundle/DependencyInjection/Compiler/AddSessionDomainConstraintPass.php

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
<?php
2+
3+
/*
4+
* This file is part of the Symfony package.
5+
*
6+
* (c) Fabien Potencier <[email protected]>
7+
*
8+
* For the full copyright and license information, please view the LICENSE
9+
* file that was distributed with this source code.
10+
*/
11+
12+
namespace Symfony\Bundle\SecurityBundle\DependencyInjection\Compiler;
13+
14+
use Symfony\Component\DependencyInjection\ContainerBuilder;
15+
use Symfony\Component\DependencyInjection\Compiler\CompilerPassInterface;
16+
17+
/**
18+
* Uses the session domain to restrict allowed redirection targets.
19+
*
20+
* @author Nicolas Grekas <[email protected]>
21+
*/
22+
class AddSessionDomainConstraintPass implements CompilerPassInterface
23+
{
24+
/**
25+
* {@inheritdoc}
26+
*/
27+
public function process(ContainerBuilder $container)
28+
{
29+
if (!$container->hasParameter('session.storage.options') || !$container->has('security.http_utils')) {
30+
return;
31+
}
32+
33+
$sessionOptions = $container->getParameter('session.storage.options');
34+
$domainRegexp = empty($sessionOptions['cookie_domain']) ? '%s' : sprintf('(?:%%s|(?:.+\.)?%s)', preg_quote(trim($sessionOptions['cookie_domain'], '.')));
35+
$domainRegexp = (empty($sessionOptions['cookie_secure']) ? 'https?://' : 'https://').$domainRegexp;
36+
37+
$container->findDefinition('security.http_utils')->addArgument(sprintf('{^%s$}i', $domainRegexp));
38+
}
39+
}

‎src/Symfony/Bundle/SecurityBundle/SecurityBundle.php

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,8 +12,10 @@
1212
namespace Symfony\Bundle\SecurityBundle;
1313

1414
use Symfony\Component\HttpKernel\Bundle\Bundle;
15+
use Symfony\Component\DependencyInjection\Compiler\PassConfig;
1516
use Symfony\Component\DependencyInjection\ContainerBuilder;
1617
use Symfony\Bundle\SecurityBundle\DependencyInjection\Compiler\AddSecurityVotersPass;
18+
use Symfony\Bundle\SecurityBundle\DependencyInjection\Compiler\AddSessionDomainConstraintPass;
1719
use Symfony\Bundle\SecurityBundle\DependencyInjection\Security\Factory\FormLoginFactory;
1820
use Symfony\Bundle\SecurityBundle\DependencyInjection\Security\Factory\FormLoginLdapFactory;
1921
use Symfony\Bundle\SecurityBundle\DependencyInjection\Security\Factory\HttpBasicFactory;
@@ -55,5 +57,6 @@ public function build(ContainerBuilder $container)
5557
$extension->addUserProviderFactory(new InMemoryFactory());
5658
$extension->addUserProviderFactory(new LdapFactory());
5759
$container->addCompilerPass(new AddSecurityVotersPass());
60+
$container->addCompilerPass(new AddSessionDomainConstraintPass(), PassConfig::TYPE_AFTER_REMOVING);
5861
}
5962
}

‎src/Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/Compiler/AddSessionDomainConstraintPassTest.php

Lines changed: 131 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,131 @@
1+
<?php
2+
3+
/*
4+
* This file is part of the Symfony package.
5+
*
6+
* (c) Fabien Potencier <[email protected]>
7+
*
8+
* For the full copyright and license information, please view the LICENSE
9+
* file that was distributed with this source code.
10+
*/
11+
12+
namespace Symfony\Bundle\SecurityBundle\Tests\DependencyInjection\Compiler;
13+
14+
use PHPUnit\Framework\TestCase;
15+
use Symfony\Bundle\FrameworkBundle\DependencyInjection\FrameworkExtension;
16+
use Symfony\Bundle\SecurityBundle\DependencyInjection\Compiler\AddSessionDomainConstraintPass;
17+
use Symfony\Bundle\SecurityBundle\DependencyInjection\SecurityExtension;
18+
use Symfony\Component\DependencyInjection\ContainerBuilder;
19+
use Symfony\Component\HttpFoundation\Request;
20+
21+
class AddSessionDomainConstraintPassTest extends TestCase
22+
{
23+
public function testSessionCookie()
24+
{
25+
$container = $this->createContainer(array('cookie_domain' => '.symfony.com.', 'cookie_secure' => true));
26+
27+
$utils = $container->get('security.http_utils');
28+
$request = Request::create('/', 'get');
29+
30+
$this->assertTrue($utils->createRedirectResponse($request, 'https://symfony.com/blog')->isRedirect('https://symfony.com/blog'));
31+
$this->assertTrue($utils->createRedirectResponse($request, 'https://www.symfony.com/blog')->isRedirect('https://www.symfony.com/blog'));
32+
$this->assertTrue($utils->createRedirectResponse($request, 'https://localhost/foo')->isRedirect('https://localhost/foo'));
33+
$this->assertTrue($utils->createRedirectResponse($request, 'https://www.localhost/foo')->isRedirect('http://localhost/'));
34+
$this->assertTrue($utils->createRedirectResponse($request, 'http://symfony.com/blog')->isRedirect('http://localhost/'));
35+
$this->assertTrue($utils->createRedirectResponse($request, 'http://pirate.com/foo')->isRedirect('http://localhost/'));
36+
}
37+
38+
public function testSessionNoDomain()
39+
{
40+
$container = $this->createContainer(array('cookie_secure' => true));
41+
42+
$utils = $container->get('security.http_utils');
43+
$request = Request::create('/', 'get');
44+
45+
$this->assertTrue($utils->createRedirectResponse($request, 'https://symfony.com/blog')->isRedirect('http://localhost/'));
46+
$this->assertTrue($utils->createRedirectResponse($request, 'https://www.symfony.com/blog')->isRedirect('http://localhost/'));
47+
$this->assertTrue($utils->createRedirectResponse($request, 'https://localhost/foo')->isRedirect('https://localhost/foo'));
48+
$this->assertTrue($utils->createRedirectResponse($request, 'https://www.localhost/foo')->isRedirect('http://localhost/'));
49+
$this->assertTrue($utils->createRedirectResponse($request, 'http://symfony.com/blog')->isRedirect('http://localhost/'));
50+
$this->assertTrue($utils->createRedirectResponse($request, 'http://pirate.com/foo')->isRedirect('http://localhost/'));
51+
}
52+
53+
public function testSessionNoSecure()
54+
{
55+
$container = $this->createContainer(array('cookie_domain' => '.symfony.com.'));
56+
57+
$utils = $container->get('security.http_utils');
58+
$request = Request::create('/', 'get');
59+
60+
$this->assertTrue($utils->createRedirectResponse($request, 'https://symfony.com/blog')->isRedirect('https://symfony.com/blog'));
61+
$this->assertTrue($utils->createRedirectResponse($request, 'https://www.symfony.com/blog')->isRedirect('https://www.symfony.com/blog'));
62+
$this->assertTrue($utils->createRedirectResponse($request, 'https://localhost/foo')->isRedirect('https://localhost/foo'));
63+
$this->assertTrue($utils->createRedirectResponse($request, 'https://www.localhost/foo')->isRedirect('http://localhost/'));
64+
$this->assertTrue($utils->createRedirectResponse($request, 'http://symfony.com/blog')->isRedirect('http://symfony.com/blog'));
65+
$this->assertTrue($utils->createRedirectResponse($request, 'http://pirate.com/foo')->isRedirect('http://localhost/'));
66+
}
67+
68+
public function testSessionNoSecureAndNoDomain()
69+
{
70+
$container = $this->createContainer(array());
71+
72+
$utils = $container->get('security.http_utils');
73+
$request = Request::create('/', 'get');
74+
75+
$this->assertTrue($utils->createRedirectResponse($request, 'https://symfony.com/blog')->isRedirect('http://localhost/'));
76+
$this->assertTrue($utils->createRedirectResponse($request, 'https://www.symfony.com/blog')->isRedirect('http://localhost/'));
77+
$this->assertTrue($utils->createRedirectResponse($request, 'https://localhost/foo')->isRedirect('https://localhost/foo'));
78+
$this->assertTrue($utils->createRedirectResponse($request, 'http://localhost/foo')->isRedirect('http://localhost/foo'));
79+
$this->assertTrue($utils->createRedirectResponse($request, 'https://www.localhost/foo')->isRedirect('http://localhost/'));
80+
$this->assertTrue($utils->createRedirectResponse($request, 'http://symfony.com/blog')->isRedirect('http://localhost/'));
81+
$this->assertTrue($utils->createRedirectResponse($request, 'http://pirate.com/foo')->isRedirect('http://localhost/'));
82+
}
83+
84+
public function testNoSession()
85+
{
86+
$container = $this->createContainer(null);
87+
88+
$utils = $container->get('security.http_utils');
89+
$request = Request::create('/', 'get');
90+
91+
$this->assertTrue($utils->createRedirectResponse($request, 'https://symfony.com/blog')->isRedirect('https://symfony.com/blog'));
92+
$this->assertTrue($utils->createRedirectResponse($request, 'https://www.symfony.com/blog')->isRedirect('https://www.symfony.com/blog'));
93+
$this->assertTrue($utils->createRedirectResponse($request, 'https://localhost/foo')->isRedirect('https://localhost/foo'));
94+
$this->assertTrue($utils->createRedirectResponse($request, 'https://www.localhost/foo')->isRedirect('https://www.localhost/foo'));
95+
$this->assertTrue($utils->createRedirectResponse($request, 'http://symfony.com/blog')->isRedirect('http://symfony.com/blog'));
96+
$this->assertTrue($utils->createRedirectResponse($request, 'http://pirate.com/foo')->isRedirect('http://pirate.com/foo'));
97+
}
98+
99+
private function createContainer($sessionStorageOptions)
100+
{
101+
$container = new ContainerBuilder();
102+
$container->setParameter('kernel.cache_dir', __DIR__);
103+
$container->setParameter('kernel.charset', 'UTF-8');
104+
$container->setParameter('kernel.container_class', 'cc');
105+
$container->setParameter('kernel.debug', true);
106+
$container->setParameter('kernel.root_dir', __DIR__);
107+
$container->setParameter('kernel.secret', __DIR__);
108+
if (null !== $sessionStorageOptions) {
109+
$container->setParameter('session.storage.options', $sessionStorageOptions);
110+
}
111+
$container->setParameter('request_listener.http_port', 80);
112+
$container->setParameter('request_listener.https_port', 443);
113+
114+
$config = array(
115+
'security' => array(
116+
'providers' => array('some_provider' => array('id' => 'foo')),
117+
'firewalls' => array('some_firewall' => array('security' => false)),
118+
),
119+
);
120+
121+
$ext = new FrameworkExtension();
122+
$ext->load(array(), $container);
123+
124+
$ext = new SecurityExtension();
125+
$ext->load($config, $container);
126+
127+
(new AddSessionDomainConstraintPass())->process($container);
128+
129+
return $container;
130+
}
131+
}

‎src/Symfony/Component/Console/Application.php

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -115,15 +115,13 @@ public function run(InputInterface $input = null, OutputInterface $output = null
115115
try {
116116
$e = null;
117117
$exitCode = $this->doRun($input, $output);
118-
} catch (\Exception $x) {
119-
$e = $x;
120-
} catch (\Throwable $x) {
121-
$e = new FatalThrowableError($x);
118+
} catch (\Exception $e) {
119+
} catch (\Throwable $e) {
122120
}
123121

124122
if (null !== $e) {
125-
if (!$this->catchExceptions || !$x instanceof \Exception) {
126-
throw $x;
123+
if (!$this->catchExceptions || !$e instanceof \Exception) {
124+
throw $e;
127125
}
128126

129127
if ($output instanceof ConsoleOutputInterface) {

‎src/Symfony/Component/Form/CHANGELOG.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,11 @@ CHANGELOG
1212
* moved data trimming logic of TrimListener into StringUtil
1313
* [BC BREAK] When registering a type extension through the DI extension, the tag alias has to match the actual extended type.
1414

15+
2.7.38
16+
------
17+
18+
* [BC BREAK] the `isFileUpload()` method was added to the `RequestHandlerInterface`
19+
1520
2.7.0
1621
-----
1722

‎src/Symfony/Component/Form/Extension/Core/Type/FileType.php

Lines changed: 23 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -27,20 +27,35 @@ class FileType extends AbstractType
2727
*/
2828
public function buildForm(FormBuilderInterface $builder, array $options)
2929
{
30-
if ($options['multiple']) {
31-
$builder->addEventListener(FormEvents::PRE_SUBMIT, function (FormEvent $event) {
32-
$form = $event->getForm();
33-
$data = $event->getData();
30+
$builder->addEventListener(FormEvents::PRE_SUBMIT, function (FormEvent $event) use ($options) {
31+
$form = $event->getForm();
32+
$requestHandler = $form->getConfig()->getRequestHandler();
33+
$data = null;
34+
35+
if ($options['multiple']) {
36+
$data = array();
37+
38+
foreach ($event->getData() as $file) {
39+
if ($requestHandler->isFileUpload($file)) {
40+
$data[] = $file;
41+
}
42+
}
3443

3544
// submitted data for an input file (not required) without choosing any file
36-
if (array(null) === $data) {
45+
if (array(null) === $data || array() === $data) {
3746
$emptyData = $form->getConfig()->getEmptyData();
3847

3948
$data = is_callable($emptyData) ? call_user_func($emptyData, $form, $data) : $emptyData;
40-
$event->setData($data);
4149
}
42-
});
43-
}
50+
51+
$event->setData($data);
52+
} elseif (!$requestHandler->isFileUpload($event->getData())) {
53+
$emptyData = $form->getConfig()->getEmptyData();
54+
55+
$data = is_callable($emptyData) ? call_user_func($emptyData, $form, $data) : $emptyData;
56+
$event->setData($data);
57+
}
58+
});
4459
}
4560

4661
/**

‎src/Symfony/Component/Form/Extension/HttpFoundation/HttpFoundationRequestHandler.php

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
use Symfony\Component\Form\FormInterface;
1717
use Symfony\Component\Form\RequestHandlerInterface;
1818
use Symfony\Component\Form\Util\ServerParams;
19+
use Symfony\Component\HttpFoundation\File\File;
1920
use Symfony\Component\HttpFoundation\Request;
2021

2122
/**
@@ -28,9 +29,6 @@ class HttpFoundationRequestHandler implements RequestHandlerInterface
2829
{
2930
private $serverParams;
3031

31-
/**
32-
* {@inheritdoc}
33-
*/
3432
public function __construct(ServerParams $serverParams = null)
3533
{
3634
$this->serverParams = $serverParams ?: new ServerParams();
@@ -109,4 +107,9 @@ public function handleRequest(FormInterface $form, $request = null)
109107

110108
$form->submit($data, 'PATCH' !== $method);
111109
}
110+
111+
public function isFileUpload($data)
112+
{
113+
return $data instanceof File;
114+
}
112115
}

0 commit comments

Comments
 (0)