10000 Merge branch '3.3' into 3.4 · symfony/symfony@caa10ae · GitHub
[go: up one dir, main page]
Skip to content

Commit caa10ae

Browse files
nicolas-grekasnicolas-grekas
committed
Merge branch '3.3' into 3.4
* 3.3: 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 3.3.13 updated VERSION for 3.3.12 updated CHANGELOG for 3.3.12 bumped Symfony version to 2.8.31 updated VERSION for 2.8.30 updated CHANGELOG for 2.8.30 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 ecad1c4 + ea2447f commit caa10ae

File tree

25 files changed

+604
-138
lines changed

25 files changed

+604
-138
lines changed

‎CHANGELOG-3.3.md

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,12 @@ in 3.3 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/v3.3.0...v3.3.1
99

10+
* 3.3.12 (2017-11-13)
11+
12+
* bug #24954 [DI] Fix dumping with custom base class (nicolas-grekas)
13+
* bug #24952 [HttpFoundation] Fix session-related BC break (nicolas-grekas, sroze)
14+
* bug #24929 [Console] Fix traversable autocomplete values (ro0NL)
15+
1016
* 3.3.11 (2017-11-10)
1117

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

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
<service id="security.csrf.token_manager" class="Symfony\Component\Security\Csrf\CsrfTokenManager" public="true">
1919
<argument type="service" id="security.csrf.token_generator" />
2020
<argument type="service" id="security.csrf.token_storage" />
21+
<argument type="service" id="request_stack" on-invalid="ignore" />
2122
</service>
2223
<service id="Symfony\Component\Security\Csrf\CsrfTokenManagerInterface" alias="security.csrf.token_manager" />
2324
</services>

‎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 <fabien@symfony.com>
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 <p@tchwork.com>
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
@@ -14,8 +14,10 @@
1414
use Symfony\Bundle\SecurityBundle\DependencyInjection\Security\Factory\JsonLoginFactory;
1515
use Symfony\Component\Console\Application;
1616
use Symfony\Component\HttpKernel\Bundle\Bundle;
17+
use Symfony\Component\DependencyInjection\Compiler\PassConfig;
1718
use Symfony\Component\DependencyInjection\ContainerBuilder;
1819
use Symfony\Bundle\SecurityBundle\DependencyInjection\Compiler\AddSecurityVotersPass;
20+
use Symfony\Bundle\SecurityBundle\DependencyInjection\Compiler\AddSessionDomainConstraintPass;
1921
use Symfony\Bundle\SecurityBundle\DependencyInjection\Security\Factory\FormLoginFactory;
2022
use Symfony\Bundle\SecurityBundle\DependencyInjection\Security\Factory\FormLoginLdapFactory;
2123
use Symfony\Bundle\SecurityBundle\DependencyInjection\Security\Factory\HttpBasicFactory;
@@ -58,6 +60,7 @@ public function build(ContainerBuilder $container)
5860
$extension->addUserProviderFactory(new InMemoryFactory());
5961
$extension->addUserProviderFactory(new LdapFactory());
6062
$container->addCompilerPass(new AddSecurityVotersPass());
63+
$container->addCompilerPass(new AddSessionDomainConstraintPass(), PassConfig::TYPE_AFTER_REMOVING);
6164
}
6265

6366
public function registerCommands(Application $application)

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

Lines changed: 131 additions & 0 deletions
< 2851 td data-grid-cell-id="diff-564bb88b7f36d7f570795cedf514d0b25ea2acf57dc21ede418a1c295a3f8397-empty-13-0" data-selected="false" role="gridcell" style="background-color:var(--diffBlob-additionNum-bgColor, var(--diffBlob-addition-bgColor-num));text-align:center" tabindex="-1" valign="top" class="focusable-grid-cell diff-line-number position-relative left-side">
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 <fabien@symfony.com>
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
@@ -127,15 +127,13 @@ public function run(InputInterface $input = null, OutputInterface $output = null
127127
try {
128128
$e = null;
129129
$exitCode = $this->doRun($input, $output);
130-
} catch (\Exception $x) {
131-
$e = $x;
132-
} catch (\Throwable $x) {
133-
$e = new FatalThrowableError($x);
130+
} catch (\Exception $e) {
131+
} catch (\Throwable $e) {
134132
}
135133

136134
if (null !== $e) {
137-
if (!$this->catchExceptions || !$x instanceof \Exception) {
138-
throw $x;
135+
if (!$this->catchExceptions || !$e instanceof \Exception) {
136+
throw $e;
139137
}
140138

141139
if ($output instanceof ConsoleOutputInterface) {

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

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

60+
2.7.38
61+
------
62+
63+
* [BC BREAK] the `isFileUpload()` method was added to the `RequestHandlerInterface`
64+
6065
2.7.0
6166
-----
6267

‎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
}

‎src/Symfony/Component/Form/NativeRequestHandler.php

Lines changed: 13 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -23,14 +23,6 @@ class NativeRequestHandler implements RequestHandlerInterface
2323
{
2424
private $serverParams;
2525

26-
/**
27-
* {@inheritdoc}
28-
*/
29-
public function __construct(ServerParams $params = null)
30-
{
31-
$this->serverParams = $params ?: new ServerParams();
32-
}
33-
3426
/**
3527
* The allowed keys of the $_FILES array.
3628
*/
@@ -42,6 +34,11 @@ public function __construct(ServerParams $params = null)
4234
'type',
4335
);
4436

37+
public function __construct(ServerParams $params = null)
38+
{
39+
$this->serverParams = $params ?: new ServerParams();
40+
}
41+
4542
/**
4643
* {@inheritdoc}
4744
*/
@@ -121,6 +118,14 @@ public function handleRequest(FormInterface $form, $request = null)
121118
$form->submit($data, 'PATCH' !== $method);
122119
}
123120

121+
public function isFileUpload($data)
122+
{
123+
// POST data will always be strings or arrays of strings. Thus, we can be sure
124+
// that the submitted data is a file upload if the "error" value is an integer
125+
// (this value must have been injected by PHP itself).
126+
return is_array($data) && isset($data['error']) && is_int($data['error']);
127+
}
128+
124129
/**
125130
* Returns the method used to submit the request to the server.
126131
*

‎src/Symfony/Component/Form/RequestHandlerInterface.php

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,4 +25,11 @@ interface RequestHandlerInterface
2525
* @param mixed $request The current request
2626
*/
2727
public function handleRequest(FormInterface $form, $request = null);
28+
29+
/**
30+
* @param mixed $data
31+
*
32+
* @return bool
33+
*/
34+
public function isFileUpload($data);
2835
}

‎src/Symfony/Component/Form/Tests/AbstractRequestHandlerTest.php

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -353,12 +353,24 @@ public function getPostMaxSizeFixtures()
353353
);
354354
}
355355

356+
public function testUploadedFilesAreAccepted()
357+
{
358+
$this->assertTrue($this->requestHandler->isFileUpload($this->getMockFile()));
359+
}
360+
361+
public function testInvalidFilesAreRejected()
362+
{
363+
$this->assertFalse($this->requestHandler->isFileUpload($this->getInvalidFile()));
364+
}
365+
356366
abstract protected function setRequestData($method, $data, $files = array());
357367

358368
abstract protected function getRequestHandler();
359369

360370
abstract protected function getMockFile($suffix = '');
361371

372+
abstract protected function getInvalidFile();
373+
362374
protected function getMockForm($name, $method = null, $compound = true)
363375
{
364376
$config = $this->getMockBuilder('Symfony\Component\Form\FormConfigInterface')->getMock();

0 commit comments

Comments
 (0)
0