From 8b2c17f80377582287a78e0b521497e039dd6b0d Mon Sep 17 00:00:00 2001 From: Tobias Schultze Date: Fri, 14 Dec 2012 23:08:21 +0100 Subject: [PATCH 1/7] fix double-decoding in the routing system --- .../Bundle/FrameworkBundle/EventListener/RouterListener.php | 5 ++++- src/Symfony/Component/Security/Http/HttpUtils.php | 4 ++-- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/src/Symfony/Bundle/FrameworkBundle/EventListener/RouterListener.php b/src/Symfony/Bundle/FrameworkBundle/EventListener/RouterListener.php index 5cd959e73c..1c8ea585ca 100644 --- a/src/Symfony/Bundle/FrameworkBundle/EventListener/RouterListener.php +++ b/src/Symfony/Bundle/FrameworkBundle/EventListener/RouterListener.php @@ -70,7 +70,10 @@ class RouterListener // add attributes based on the path info (routing) try { - $parameters = $this->router->match($request->getPathInfo()); + // The path is returned in decoded form from the request, so we need to + // encode it again as the router applies its own decoding. This prevents + // double-decoding. + $parameters = $this->router->match(urlencode($request->getPathInfo())); if (null !== $this->logger) { $this->logger->info(sprintf('Matched route "%s" (parameters: %s)', $parameters['_route'], $this->parametersToString($parameters))); diff --git a/src/Symfony/Component/Security/Http/HttpUtils.php b/src/Symfony/Component/Security/Http/HttpUtils.php index cac130ed97..78bfb85475 100644 --- a/src/Symfony/Component/Security/Http/HttpUtils.php +++ b/src/Symfony/Component/Security/Http/HttpUtils.php @@ -107,7 +107,7 @@ class HttpUtils { if ('/' !== $path[0]) { try { - $parameters = $this->router->match($request->getPathInfo()); + $parameters = $this->router->match(urlencode($request->getPathInfo())); return $path === $parameters['_route']; } catch (MethodNotAllowedException $e) { @@ -129,7 +129,7 @@ class HttpUtils } try { - $parameters = $this->router->match($request->getPathInfo()); + $parameters = $this->router->match(urlencode($request->getPathInfo())); if (isset($parameters['_locale'])) { $context->setParameter('_locale', $parameters['_locale']); From e713bb4e7e86b6bb4f12e807d3f58e2ebb77d028 Mon Sep 17 00:00:00 2001 From: Bernhard Schussek Date: Sat, 15 Dec 2012 18:04:13 +0100 Subject: [PATCH 2/7] Fixed failing test --- .../DateTimeToStringTransformer.php | 22 +++++++++---------- .../Form/Tests/AbstractLayoutTest.php | 6 +++-- 2 files changed, 15 insertions(+), 13 deletions(-) diff --git a/src/Symfony/Component/Form/Extension/Core/DataTransformer/DateTimeToStringTransformer.php b/src/Symfony/Component/Form/Extension/Core/DataTransformer/DateTimeToStringTransformer.php index cbaac96859..f7c4eb4798 100644 --- a/src/Symfony/Component/Form/Extension/Core/DataTransformer/DateTimeToStringTransformer.php +++ b/src/Symfony/Component/Form/Extension/Core/DataTransformer/DateTimeToStringTransformer.php @@ -140,6 +140,17 @@ class DateTimeToStringTransformer extends BaseDateTimeTransformer $outputTz = new \DateTimeZone($this->outputTimezone); $dateTime = \DateTime::createFromFormat($this->parseFormat, $value, $outputTz); + $lastErrors = \DateTime::getLastErrors(); + + if (0 < $lastErrors['warning_count'] || 0 < $lastErrors['error_count']) { + throw new TransformationFailedException( + implode(', ', array_merge( + array_values($lastErrors['warnings']), + array_values($lastErrors['errors']) + )) + ); + } + // On PHP versions < 5.3.8 we need to emulate the pipe operator // and reset parts not given in the format to their equivalent // of the UNIX base timestamp. @@ -204,17 +215,6 @@ class DateTimeToStringTransformer extends BaseDateTimeTransformer } } - $lastErrors = \DateTime::getLastErrors(); - - if (0 < $lastErrors['warning_count'] || 0 < $lastErrors['error_count']) { - throw new TransformationFailedException( - implode(', ', array_merge( - array_values($lastErrors['warnings']), - array_values($lastErrors['errors']) - )) - ); - } - if ($this->inputTimezone !== $this->outputTimezone) { $dateTime->setTimeZone(new \DateTimeZone($this->inputTimezone)); } diff --git a/src/Symfony/Component/Form/Tests/AbstractLayoutTest.php b/src/Symfony/Component/Form/Tests/AbstractLayoutTest.php index 1f296a81df..aa3d46bf2e 100644 --- a/src/Symfony/Component/Form/Tests/AbstractLayoutTest.php +++ b/src/Symfony/Component/Form/Tests/AbstractLayoutTest.php @@ -876,8 +876,10 @@ abstract class AbstractLayoutTest extends FormIntegrationTestCase public function testDateTimeWithEmptyValueOnTime() { - $form = $this->factory->createNamed('name', 'datetime', '2011-02-03', array( - 'input' => 'string', + $data = array('year' => '2011', 'month' => '2', 'day' => '3', 'hour' => '', 'minute' => ''); + + $form = $this->factory->createNamed('name', 'datetime', $data, array( + 'input' => 'array', 'empty_value' => array('hour' => 'Change&Me', 'minute' => 'Change&Me'), 'required' => false, )); From ab64da5671ec161ab73aba07fc011bc8de56ae41 Mon Sep 17 00:00:00 2001 From: Fabien Potencier Date: Sat, 15 Dec 2012 18:28:15 +0100 Subject: [PATCH 3/7] [Locale] fixed a test --- src/Symfony/Component/Locale/Tests/Stub/StubLocaleTest.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Symfony/Component/Locale/Tests/Stub/StubLocaleTest.php b/src/Symfony/Component/Locale/Tests/Stub/StubLocaleTest.php index 568ee1ead3..112812b122 100644 --- a/src/Symfony/Component/Locale/Tests/Stub/StubLocaleTest.php +++ b/src/Symfony/Component/Locale/Tests/Stub/StubLocaleTest.php @@ -67,7 +67,7 @@ class StubLocaleTest extends LocaleTestCase public function testGetCurrenciesData() { - $symbol = $this->isSameAsIcuVersion('4.8') ? 'BR$' : 'R$'; + $symbol = $this->isGreaterOrEqualThanIcuVersion('4.8') ? 'BR$' : 'R$'; $currencies = StubLocale::getCurrenciesData('en'); $this->assertEquals($symbol, $currencies['BRL']['symbol']); From df0623f98f761e63bbe6a565276f130532ff6152 Mon Sep 17 00:00:00 2001 From: Drak Date: Sat, 15 Dec 2012 21:54:38 +0000 Subject: [PATCH 4/7] Making it easier to grab the PR template. --- CONTRIBUTING.md | 4 +++- README.md | 4 +++- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 6ecca78e05..383bec81f6 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -3,7 +3,9 @@ Contributing Symfony2 is an open source, community-driven project. If you'd like to contribute, please read the [Contributing Code][1] part of the documentation. If you're submitting -a pull request, please follow the guidelines in the [Submitting a Patch][2] section. +a pull request, please follow the guidelines in the [Submitting a Patch][2] section +and use the [Pull Request Template][3]. [1]: http://symfony.com/doc/current/contributing/code/index.html [2]: http://symfony.com/doc/current/contributing/code/patches.html#check-list +[3]: http://symfony.com/doc/current/contributing/code/patches.html#make-a-pull-request diff --git a/README.md b/README.md index 9e49f11a73..d5baf98f80 100644 --- a/README.md +++ b/README.md @@ -35,10 +35,12 @@ Contributing Symfony2 is an open source, community-driven project. If you'd like to contribute, please read the [Contributing Code][4] part of the documentation. If you're submitting -a pull request, please follow the guidelines in the [Submitting a Patch][5] section. +a pull request, please follow the guidelines in the [Submitting a Patch][5] section +and use [Pull Request Template][6]. [1]: http://symfony.com/download [2]: http://symfony.com/get_started [3]: http://symfony.com/doc/current/ [4]: http://symfony.com/doc/current/contributing/code/index.html [5]: http://symfony.com/doc/current/contributing/code/patches.html#check-list +[6]: http://symfony.com/doc/current/contributing/code/patches.html#make-a-pull-request From 2cd43da120dd08eb86611afe09e3624778824b27 Mon Sep 17 00:00:00 2001 From: Pierre Tachoire Date: Tue, 18 Dec 2012 22:02:51 +0100 Subject: [PATCH 5/7] [Process] Allow non-blocking start with PhpProcess --- src/Symfony/Component/Process/PhpProcess.php | 13 +++--------- .../Process/Tests/PhpProcessTest.php | 21 +++++++++++++++++++ 2 files changed, 24 insertions(+), 10 deletions(-) create mode 100644 src/Symfony/Component/Process/Tests/PhpProcessTest.php diff --git a/src/Symfony/Component/Process/PhpProcess.php b/src/Symfony/Component/Process/PhpProcess.php index 2bd5600451..5db6e134e2 100644 --- a/src/Symfony/Component/Process/PhpProcess.php +++ b/src/Symfony/Component/Process/PhpProcess.php @@ -55,16 +55,9 @@ class PhpProcess extends Process } /** - * Runs the process. - * - * @param Closure|string|array $callback A PHP callback to run whenever there is some - * output available on STDOUT or STDERR - * - * @return integer The exit status code - * - * @api + * {@inheritdoc} */ - public function run($callback = null) + public function start($callback = null) { if (null === $this->getCommandLine()) { if (false === $php = $this->executableFinder->find()) { @@ -73,6 +66,6 @@ class PhpProcess extends Process $this->setCommandLine($php); } - return parent::run($callback); + return parent::start($callback); } } diff --git a/src/Symfony/Component/Process/Tests/PhpProcessTest.php b/src/Symfony/Component/Process/Tests/PhpProcessTest.php new file mode 100644 index 0000000000..7bcdd2f05d --- /dev/null +++ b/src/Symfony/Component/Process/Tests/PhpProcessTest.php @@ -0,0 +1,21 @@ +start(); + $process->wait(); + $this->assertEquals($expected, $process->getOutput()); + } +} From 1f8c501b998368db204176294b078df1484a10ed Mon Sep 17 00:00:00 2001 From: Fabien Potencier Date: Wed, 19 Dec 2012 09:12:13 +0100 Subject: [PATCH 6/7] [FrameworkBundle] restricted the type of controllers that can be executed by InternalController --- .../Controller/InternalController.php | 29 +++++++++++ .../Controller/InternalControllerTest.php | 52 +++++++++++++++++++ 2 files changed, 81 insertions(+) create mode 100644 src/Symfony/Bundle/FrameworkBundle/Tests/Controller/InternalControllerTest.php diff --git a/src/Symfony/Bundle/FrameworkBundle/Controller/InternalController.php b/src/Symfony/Bundle/FrameworkBundle/Controller/InternalController.php index 641deb7683..ce674b190c 100644 --- a/src/Symfony/Bundle/FrameworkBundle/Controller/InternalController.php +++ b/src/Symfony/Bundle/FrameworkBundle/Controller/InternalController.php @@ -31,6 +31,35 @@ class InternalController extends ContainerAware */ public function indexAction($path, $controller) { + // safeguard + if (!is_string($controller)) { + throw new \RuntimeException('A Controller must be a string.'); + } + + // check that the controller looks like a controller + if (false === strpos($controller, '::')) { + $count = substr_count($controller, ':'); + if (2 == $count) { + // the convention already enforces the Controller suffix + } elseif (1 == $count) { + // controller in the service:method notation + list($service, $method) = explode(':', $controller, 2); + $class = get_class($this->container->get($service)); + + if (!preg_match('/Controller$/', $class)) { + throw new \RuntimeException('A Controller class name must end with Controller.'); + } + } else { + throw new \LogicException('Unable to parse the Controller name.'); + } + } else { + list($class, $method) = explode('::', $controller, 2); + + if (!preg_match('/Controller$/', $class)) { + throw new \RuntimeException('A Controller class name must end with Controller.'); + } + } + $request = $this->container->get('request'); $attributes = $request->attributes; diff --git a/src/Symfony/Bundle/FrameworkBundle/Tests/Controller/InternalControllerTest.php b/src/Symfony/Bundle/FrameworkBundle/Tests/Controller/InternalControllerTest.php new file mode 100644 index 0000000000..2b35ff1424 --- /dev/null +++ b/src/Symfony/Bundle/FrameworkBundle/Tests/Controller/InternalControllerTest.php @@ -0,0 +1,52 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace Symfony\Bundle\FrameworkBundle\Tests\Controller; + +use Symfony\Bundle\FrameworkBundle\Controller\InternalController; +use Symfony\Bundle\FrameworkBundle\Tests\TestCase; +use Symfony\Component\HttpFoundation\Request; + +class InternalControllerTest extends TestCase +{ + /** + * @expectedException \RuntimeException + * @expectedExceptionMessage A Controller class name must end with Controller. + */ + public function testWithAClassMethodController() + { + $container = $this->getMock('Symfony\Component\DependencyInjection\ContainerInterface'); + + $controller = new InternalController(); + $controller->setContainer($container); + + $controller->indexAction('/', 'Symfony\Component\HttpFoundation\Request::getPathInfo'); + } + + /** + * @expectedException \RuntimeException + * @expectedExceptionMessage A Controller class name must end with Controller. + */ + public function testWithAServiceController() + { + $container = $this->getMock('Symfony\Component\DependencyInjection\ContainerInterface'); + $container + ->expects($this->once()) + ->method('get') + ->will($this->returnValue(new Request())) + ; + + $controller = new InternalController(); + $controller->setContainer($container); + + $controller->indexAction('/', 'service:method'); + } +} From 532cc9a0e656a6ebecf82d0f64d7dad6a365e1c8 Mon Sep 17 00:00:00 2001 From: Fabien Potencier Date: Wed, 19 Dec 2012 09:13:27 +0100 Subject: [PATCH 7/7] [FrameworkBundle] added support for URIs as an argument to HttpKernel::render() --- src/Symfony/Bundle/FrameworkBundle/HttpKernel.php | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/src/Symfony/Bundle/FrameworkBundle/HttpKernel.php b/src/Symfony/Bundle/FrameworkBundle/HttpKernel.php index 2b3e645fec..2551530ce2 100644 --- a/src/Symfony/Bundle/FrameworkBundle/HttpKernel.php +++ b/src/Symfony/Bundle/FrameworkBundle/HttpKernel.php @@ -125,8 +125,13 @@ class HttpKernel extends BaseHttpKernel $request = $this->container->get('request'); - // controller or URI? - if (0 === strpos($controller, '/')) { + // controller or URI or path? + if (0 === strpos($controller, 'http://') || 0 === strpos($controller, 'https://')) { + $subRequest = Request::create($controller, 'get', array(), $request->cookies->all(), array(), $request->server->all()); + if ($session = $request->getSession()) { + $subRequest->setSession($session); + } + } elseif (0 === strpos($controller, '/')) { $subRequest = Request::create($request->getUriForPath($controller), 'get', array(), $request->cookies->all(), array(), $request->server->all()); if ($session = $request->getSession()) { $subRequest->setSession($session);