From d86e1eb71c403459335e92635f6c944521aca6b8 Mon Sep 17 00:00:00 2001 From: Victor Berchet Date: Tue, 7 Feb 2012 17:15:28 +0100 Subject: [PATCH 1/5] [Routing] Remove a weird dependency --- CHANGELOG-2.1.md | 1 + .../Matcher/RedirectableUrlMatcher.php | 33 +++++++----- .../Component/Routing/Matcher/UrlMatcher.php | 50 +++++++++++++++---- .../Routing/Matcher/UrlMatcherTest.php | 5 +- 4 files changed, 65 insertions(+), 24 deletions(-) diff --git a/CHANGELOG-2.1.md b/CHANGELOG-2.1.md index 8caeaacbb8..322899d9d5 100644 --- a/CHANGELOG-2.1.md +++ b/CHANGELOG-2.1.md @@ -242,6 +242,7 @@ To get the diff between two versions, go to https://github.com/symfony/symfony/c ### Routing + * the UrlMatcher does not throw a \LogicException any more when the required scheme is not the current one * added a TraceableUrlMatcher * added the possibility to define default values and requirements for placeholders in prefix, including imported routes * added RouterInterface::getRouteCollection diff --git a/src/Symfony/Component/Routing/Matcher/RedirectableUrlMatcher.php b/src/Symfony/Component/Routing/Matcher/RedirectableUrlMatcher.php index fcd588073b..825eefe22a 100644 --- a/src/Symfony/Component/Routing/Matcher/RedirectableUrlMatcher.php +++ b/src/Symfony/Component/Routing/Matcher/RedirectableUrlMatcher.php @@ -12,6 +12,7 @@ namespace Symfony\Component\Routing\Matcher; use Symfony\Component\Routing\Exception\ResourceNotFoundException; +use Symfony\Component\Routing\Route; /** * @author Fabien Potencier @@ -20,8 +21,6 @@ use Symfony\Component\Routing\Exception\ResourceNotFoundException; */ abstract class RedirectableUrlMatcher extends UrlMatcher implements RedirectableUrlMatcherInterface { - private $trailingSlashTest = false; - /** * @see UrlMatcher::match() * @@ -36,18 +35,28 @@ abstract class RedirectableUrlMatcher extends UrlMatcher implements Redirectable throw $e; } - // try with a / at the end - $this->trailingSlashTest = true; - - return $this->match($pathinfo.'/'); - } - - if ($this->trailingSlashTest) { - $this->trailingSlashTest = false; - - return $this->redirect($pathinfo, null); + try { + parent::match($pathinfo.'/'); + return $this->redirect($pathinfo.'/', null); + } catch (ResourceNotFoundException $e2) { + throw $e; + } } return $parameters; } + + /** + * {@inheritDoc} + */ + protected function handleRouteRequirements($pathinfo, $name, Route $route) + { + // check HTTP scheme requirement + $scheme = $route->getRequirement('_scheme'); + if ($scheme && $this->context->getScheme() !== $scheme) { + return array(self::ROUTE_MATCH, $this->redirect($pathinfo, $name, $scheme)); + } + + return array(self::REQUIREMENT_MATCH, null); + } } diff --git a/src/Symfony/Component/Routing/Matcher/UrlMatcher.php b/src/Symfony/Component/Routing/Matcher/UrlMatcher.php index 5d30ae9bfb..a9a2443e6d 100644 --- a/src/Symfony/Component/Routing/Matcher/UrlMatcher.php +++ b/src/Symfony/Component/Routing/Matcher/UrlMatcher.php @@ -15,7 +15,7 @@ use Symfony\Component\Routing\Exception\MethodNotAllowedException; use Symfony\Component\Routing\Exception\ResourceNotFoundException; use Symfony\Component\Routing\RouteCollection; use Symfony\Component\Routing\RequestContext; -use Symfony\Component\Routing\Matcher\RedirectableUrlMatcherInterface; +use Symfony\Component\Routing\Route; /** * UrlMatcher matches URL based on a set of routes. @@ -26,6 +26,10 @@ use Symfony\Component\Routing\Matcher\RedirectableUrlMatcherInterface; */ class UrlMatcher implements UrlMatcherInterface { + const REQUIREMENT_MATCH = 0; + const REQUIREMENT_MISMATCH = 1; + const ROUTE_MATCH = 2; + protected $context; private $routes; @@ -84,6 +88,17 @@ class UrlMatcher implements UrlMatcherInterface : new ResourceNotFoundException(); } + /** + * Tries to match a URL with a set of routes. + * + * @param string $pathinfo The path info to be parsed + * @param RouteCollection $routes The set of routes + * + * @return array An array of parameters + * + * @throws ResourceNotFoundException If the resource could not be found + * @throws MethodNotAllowedException If the resource was found but the request method is not allowed + */ protected function matchCollection($pathinfo, RouteCollection $routes) { $pathinfo = urldecode($pathinfo); @@ -126,21 +141,38 @@ class UrlMatcher implements UrlMatcherInterface } } - // check HTTP scheme requirement - if ($scheme = $route->getRequirement('_scheme')) { - if (!$this instanceof RedirectableUrlMatcherInterface) { - throw new \LogicException('The "_scheme" requirement is only supported for URL matchers that implement RedirectableUrlMatcherInterface.'); - } + $status = $this->handleRouteRequirements($pathinfo, $name, $route); - if ($this->context->getScheme() !== $scheme) { - return $this->redirect($pathinfo, $name, $scheme); - } + if (self::ROUTE_MATCH === $status[0]) { + return $status[1]; + } + + if (self::REQUIREMENT_MISMATCH === $status[0]) { + continue; } return array_merge($this->mergeDefaults($matches, $route->getDefaults()), array('_route' => $name)); } } + /** + * Handles specific route requirements. + * + * @param string $pathinfo The path + * @param string $name The route name + * @param string $route The route + * + * @return array The first element represents the status, the second contains additional information + */ + protected function handleRouteRequirements($pathinfo, $name, Route $route) + { + // check HTTP scheme requirement + $scheme = $route->getRequirement('_scheme'); + $status = $scheme && $scheme !== $this->context->getScheme() ? self::REQUIREMENT_MISMATCH : self::REQUIREMENT_MATCH; + + return array($status, null); + } + protected function mergeDefaults($params, $defaults) { $parameters = $defaults; diff --git a/tests/Symfony/Tests/Component/Routing/Matcher/UrlMatcherTest.php b/tests/Symfony/Tests/Component/Routing/Matcher/UrlMatcherTest.php index 3eb13e07fe..4852a483a6 100644 --- a/tests/Symfony/Tests/Component/Routing/Matcher/UrlMatcherTest.php +++ b/tests/Symfony/Tests/Component/Routing/Matcher/UrlMatcherTest.php @@ -207,14 +207,13 @@ class UrlMatcherTest extends \PHPUnit_Framework_TestCase } /** - * @expectedException \LogicException + * @expectedException Symfony\Component\Routing\Exception\ResourceNotFoundException */ public function testSchemeRequirement() { $coll = new RouteCollection(); $coll->add('foo', new Route('/foo', array(), array('_scheme' => 'https'))); - $matcher = new UrlMatcher($coll, new RequestContext()); $matcher->match('/foo'); } -} +} \ No newline at end of file From abc2141d5b3f1223fb0470440e400a10fe2f0b2e Mon Sep 17 00:00:00 2001 From: Victor Berchet Date: Tue, 7 Feb 2012 17:43:55 +0100 Subject: [PATCH 2/5] [Routing] Added a missing property declaration --- src/Symfony/Component/Routing/Matcher/UrlMatcher.php | 1 + 1 file changed, 1 insertion(+) diff --git a/src/Symfony/Component/Routing/Matcher/UrlMatcher.php b/src/Symfony/Component/Routing/Matcher/UrlMatcher.php index a9a2443e6d..7029356ece 100644 --- a/src/Symfony/Component/Routing/Matcher/UrlMatcher.php +++ b/src/Symfony/Component/Routing/Matcher/UrlMatcher.php @@ -31,6 +31,7 @@ class UrlMatcher implements UrlMatcherInterface const ROUTE_MATCH = 2; protected $context; + protected $allow; private $routes; From 4fcf9efe65a5c329ae09feac560615c70223807e Mon Sep 17 00:00:00 2001 From: Victor Berchet Date: Tue, 7 Feb 2012 17:52:32 +0100 Subject: [PATCH 3/5] [Routing] Small optimization in the UrlMatcher --- src/Symfony/Component/Routing/Matcher/UrlMatcher.php | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/Symfony/Component/Routing/Matcher/UrlMatcher.php b/src/Symfony/Component/Routing/Matcher/UrlMatcher.php index 7029356ece..eaae027392 100644 --- a/src/Symfony/Component/Routing/Matcher/UrlMatcher.php +++ b/src/Symfony/Component/Routing/Matcher/UrlMatcher.php @@ -80,7 +80,7 @@ class UrlMatcher implements UrlMatcherInterface { $this->allow = array(); - if ($ret = $this->matchCollection($pathinfo, $this->routes)) { + if ($ret = $this->matchCollection(urldecode($pathinfo), $this->routes)) { return $ret; } @@ -102,8 +102,6 @@ class UrlMatcher implements UrlMatcherInterface */ protected function matchCollection($pathinfo, RouteCollection $routes) { - $pathinfo = urldecode($pathinfo); - foreach ($routes as $name => $route) { if ($route instanceof RouteCollection) { if (false === strpos($route->getPrefix(), '{') && $route->getPrefix() !== substr($pathinfo, 0, strlen($route->getPrefix()))) { From 9fc8d284be21ff3c8988756bc008461c6c9653bb Mon Sep 17 00:00:00 2001 From: Victor Berchet Date: Tue, 7 Feb 2012 19:06:41 +0100 Subject: [PATCH 4/5] [FrameworkBundle] Fix a bug in the RedirectableUrlMatcher --- .../Routing/RedirectableUrlMatcher.php | 5 +- .../Routing/RedirectableUrlMatcherTest.php | 61 +++++++++++++++++++ 2 files changed, 63 insertions(+), 3 deletions(-) create mode 100644 src/Symfony/Bundle/FrameworkBundle/Tests/Routing/RedirectableUrlMatcherTest.php diff --git a/src/Symfony/Bundle/FrameworkBundle/Routing/RedirectableUrlMatcher.php b/src/Symfony/Bundle/FrameworkBundle/Routing/RedirectableUrlMatcher.php index c08badac42..8a3607d397 100644 --- a/src/Symfony/Bundle/FrameworkBundle/Routing/RedirectableUrlMatcher.php +++ b/src/Symfony/Bundle/FrameworkBundle/Routing/RedirectableUrlMatcher.php @@ -11,13 +11,12 @@ namespace Symfony\Bundle\FrameworkBundle\Routing; -use Symfony\Component\Routing\Matcher\UrlMatcher; -use Symfony\Component\Routing\Matcher\RedirectableUrlMatcherInterface; +use Symfony\Component\Routing\Matcher\RedirectableUrlMatcher as BaseMatcher; /** * @author Fabien Potencier */ -class RedirectableUrlMatcher extends UrlMatcher implements RedirectableUrlMatcherInterface +class RedirectableUrlMatcher extends BaseMatcher { /** * Redirects the user to another URL. diff --git a/src/Symfony/Bundle/FrameworkBundle/Tests/Routing/RedirectableUrlMatcherTest.php b/src/Symfony/Bundle/FrameworkBundle/Tests/Routing/RedirectableUrlMatcherTest.php new file mode 100644 index 0000000000..55c0125ed7 --- /dev/null +++ b/src/Symfony/Bundle/FrameworkBundle/Tests/Routing/RedirectableUrlMatcherTest.php @@ -0,0 +1,61 @@ + + * + * This source file is subject to the MIT license that is bundled + * with this source code in the file LICENSE. + */ + +namespace Symfony\Bundle\FrameworkBundle\Tests\Routing; + +use Symfony\Bundle\FrameworkBundle\Routing\Router; +use Symfony\Component\Routing\Route; +use Symfony\Component\Routing\RouteCollection; +use Symfony\Bundle\FrameworkBundle\Routing\RedirectableUrlMatcher; +use Symfony\Component\Routing\RequestContext; + +class RedirectableUrlMatcherTest extends \PHPUnit_Framework_TestCase +{ + public function testRedirectWhenNoSlash() + { + $coll = new RouteCollection(); + $coll->add('foo', new Route('/foo/')); + + $matcher = new RedirectableUrlMatcher($coll, $context = new RequestContext()); + + $this->assertEquals(array( + '_controller' => 'Symfony\Bundle\FrameworkBundle\Controller\RedirectController::urlRedirectAction', + 'path' => '/foo/', + 'permanent' => true, + 'scheme' => null, + 'httpPort' => $context->getHttpPort(), + 'httpsPort' => $context->getHttpsPort(), + '_route' => null, + ), + $matcher->match('/foo') + ); + } + + public function testSchemeRedirect() + { + $coll = new RouteCollection(); + $coll->add('foo', new Route('/foo', array(), array('_scheme' => 'https'))); + + $matcher = new RedirectableUrlMatcher($coll, $context = new RequestContext()); + + $this->assertEquals(array( + '_controller' => 'Symfony\Bundle\FrameworkBundle\Controller\RedirectController::urlRedirectAction', + 'path' => '/foo', + 'permanent' => true, + 'scheme' => 'https', + 'httpPort' => $context->getHttpPort(), + 'httpsPort' => $context->getHttpsPort(), + '_route' => 'foo', + ), + $matcher->match('/foo') + ); + } +} \ No newline at end of file From 9d6eb8216ea719e6e284f4fda84d73f11d0b7539 Mon Sep 17 00:00:00 2001 From: Victor Berchet Date: Thu, 9 Feb 2012 15:07:10 +0100 Subject: [PATCH 5/5] [Routing] Fix a bug in the TraceableUrlMatcher --- src/Symfony/Component/Routing/Matcher/TraceableUrlMatcher.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Symfony/Component/Routing/Matcher/TraceableUrlMatcher.php b/src/Symfony/Component/Routing/Matcher/TraceableUrlMatcher.php index ec00380c61..df8e89ee2c 100644 --- a/src/Symfony/Component/Routing/Matcher/TraceableUrlMatcher.php +++ b/src/Symfony/Component/Routing/Matcher/TraceableUrlMatcher.php @@ -73,7 +73,7 @@ class TraceableUrlMatcher extends UrlMatcher if (in_array($n, $cr->getVariables()) && !preg_match($cr->getRegex(), $pathinfo)) { $this->addTrace(sprintf('Requirement for "%s" does not match (%s)', $n, $regex), self::ROUTE_ALMOST_MATCHES, $name, $route); - continue; + continue 2; } }