8000 merged branch vicb/routing-ok (PR #3313) · symfony/symfony@803fba8 · GitHub
[go: up one dir, main page]
Skip to content

Commit 803fba8

Browse files
fabpotfabpot
committed
merged branch vicb/routing-ok (PR #3313)
Commits ------- 9d6eb82 [Routing] Fix a bug in the TraceableUrlMatcher 9fc8d28 [FrameworkBundle] Fix a bug in the RedirectableUrlMatcher 4fcf9ef [Routing] Small optimization in the UrlMatcher abc2141 [Routing] Added a missing property declaration d86e1eb [Routing] Remove a weird dependency Discussion ---------- [Routing] Remove a dependency on a derived class, fixes, optim Subset of #3296 which should be acceptable. Travis is happy. The side effect of removing the dependency is that the `UrlMatcher` does not throw an exception any more when the scheme does not match the required scheme. I think it is better because: * it removes a dependency on a derived class, * it was an undocumented "feature", * other thrown excs are component specific while this one was raw SPL. --------------------------------------------------------------------------- by vicb at 2012-02-09T14:43:02Z let me know what should go in 2.0 as well.
2 parents b8322b3 + 9d6eb82 commit 803fba8

File tree

7 files changed

+129
-29
lines changed

7 files changed

+129
-29
lines changed

‎CHANGELOG-2.1.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -279,6 +279,7 @@ To get the diff between two versions, go to https://github.com/symfony/symfony/c
279279

280280
### Routing
281281

282+
* the UrlMatcher does not throw a \LogicException any more when the required scheme is not the current one
282283
* added a TraceableUrlMatcher
283284
* added the possibility to define default values and requirements for placeholders in prefix, including imported routes
284285
* added RouterInterface::getRouteCollection

‎src/Symfony/Bundle/FrameworkBundle/Routing/RedirectableUrlMatcher.php

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -11,13 +11,12 @@
1111

1212
namespace Symfony\Bundle\FrameworkBundle\Routing;
1313

14-
use Symfony\Component\Routing\Matcher\UrlMatcher;
15-
use Symfony\Component\Routing\Matcher\RedirectableUrlMatcherInterface;
14+
use Symfony\Component\Routing\Matcher\RedirectableUrlMatcher as BaseMatcher;
1615

1716
/**
1817
* @author Fabien Potencier <fabien@symfony.com>
1918
*/
20-
class RedirectableUrlMatcher extends UrlMatcher implements RedirectableUrlMatcherInterface
19+
class RedirectableUrlMatcher extends BaseMatcher
2120
{
2221
/**
2322
* Redirects the user to another URL.

‎src/Symfony/Bundle/FrameworkBundle/Tests/Routing/RedirectableUrlMatcherTest.php

Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,61 @@
1+
<?php
2+
3+
/*
4+
* This file is part of the Symfony framework.
5+
*
6+
* (c) Fabien Potencier <fabien@symfony.com>
7+
*
8+
* This source file is subject to the MIT license that is bundled
9+
* with this source code in the file LICENSE.
10+
*/
11+
12+
namespace Symfony\Bundle\FrameworkBundle\Tests\Routing;
13+
14+
use Symfony\Bundle\FrameworkBundle\Routing\Router;
15+
use Symfony\Component\Routing\Route;
16+
use Symfony\Component\Routing\RouteCollection;
17+
use Symfony\Bundle\FrameworkBundle\Routing\RedirectableUrlMatcher;
18+
use Symfony\Component\Routing\RequestContext;
19+
20+
class RedirectableUrlMatcherTest extends \PHPUnit_Framework_TestCase
21+
{
22+
public function testRedirectWhenNoSlash()
23+
{
24+
$coll = new RouteCollection();
25+
$coll->add('foo', new Route('/foo/'));
26+
27+
$matcher = new RedirectableUrlMatcher($coll, $context = new RequestContext());
28+
29+
$this->assertEquals(array(
30+
'_controller' => 'Symfony\Bundle\FrameworkBundle\Controller\RedirectController::urlRedirectAction',
31+
'path' => '/foo/',
32+
'permanent' => true,
33+
'scheme' => null,
34+
'httpPort' => $context->getHttpPort(),
35+
'httpsPort' => $context->getHttpsPort(),
36+
'_route' => null,
37+
),
38+
$matcher->match('/foo')
39+
);
40+
}
41+
42+
public function testSchemeRedirect()
43+
{
44+
$coll = new RouteCollection();
45+
$coll->add('foo', new Route('/foo', array(), array('_scheme' => 'https')));
46+
47+
$matcher = new RedirectableUrlMatcher($coll, $context = new RequestContext());
48+
49+
$this->assertEquals(array(
50+
'_controller' => 'Symfony\Bundle\FrameworkBundle\Controller\RedirectController::urlRedirectAction',
51+
'path' => '/foo',
52+
'permanent' => true,
53+
'scheme' => 'https',
54+
'httpPort' => $context->getHttpPort(),
55+
'httpsPort' => $context->getHttpsPort(),
56+
'_route' => 'foo',
57+
),
58+
$matcher->match('/foo')
59+
);
60+
}
61+
}

‎src/Symfony/Component/Routing/Matcher/RedirectableUrlMatcher.php

Lines changed: 19 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
namespace Symfony\Component\Routing\Matcher;
1313

1414
use Symfony\Component\Routing\Exception\ResourceNotFoundException;
15+
use Symfony\Component\Routing\Route;
1516

1617
/**
1718
* @author Fabien Potencier <fabien@symfony.com>
@@ -20,8 +21,6 @@
2021
*/
2122
abstract class RedirectableUrlMatcher extends UrlMatcher implements RedirectableUrlMatcherInterface
2223
{
23-
private $trailingSlashTest = false;
24-
2524
/**
2625
* @see UrlMatcher::match()
2726
*
@@ -36,18 +35,28 @@ public function match($pathinfo)
3635
throw $e;
3736
}
3837

39-
// try with a / at the end
40-
$this->trailingSlashTest = true;
41-
42-
return $this->match($pathinfo.'/');
38+
try {
39+
parent::match($pathinfo.'/');
40+
return $this->redirect($pathinfo.'/', null);
41+
} catch (ResourceNotFoundException $e2) {
42+
throw $e;
43+
}
4344
}
4445

45-
if ($this->trailingSlashTest) {
46-
$this->trailingSlashTest = false;
46+
return $parameters;
47+
}
4748

48-
return $this->redirect($pathinfo, null);
49+
/**
50+
* {@inheritDoc}
51+
*/
52+
protected function handleRouteRequirements($pathinfo, $name, Route $route)
53+
{
54+
// check HTTP scheme requirement
55+
$scheme = $route->getRequirement('_scheme');
56+
if ($scheme && $this->context->getScheme() !== $scheme) {
57+
return array(self::ROUTE_MATCH, $this->redirect($pathinfo, $name, $scheme));
4958
}
5059

51-
return $parameters;
60+
return array(self::REQUIREMENT_MATCH, null);
5261
}
5362
}

‎src/Symfony/Component/Routing/Matcher/TraceableUrlMatcher.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -73,7 +73,7 @@ protected function matchCollection($pathinfo, RouteCollection $routes)
7373
if (in_array($n, $cr->getVariables()) && !preg_match($cr->getRegex(), $pathinfo)) {
7474
$this->addTrace(sprintf('Requirement for "%s" does not match (%s)', $n, $regex), self::ROUTE_ALMOST_MATCHES, $name, $route);
7575

76-
continue;
76+
continue 2;
7777
}
7878
}
7979

‎src/Symfony/Component/Routing/Matcher/UrlMatcher.php

Lines changed: 43 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@
1515
use Symfony\Component\Routing\Exception\ResourceNotFoundException;
1616
use Symfony\Component\Routing\RouteCollection;
1717
use Symfony\Component\Routing\RequestContext;
18-
use Symfony\Component\Routing\Matcher\RedirectableUrlMatcherInterface;
18+
use Symfony\Component\Routing\Route;
1919

2020
/**
2121
* UrlMatcher matches URL based on a set of routes.
@@ -26,7 +26,12 @@
2626
*/
2727
class UrlMatcher implements UrlMatcherInterface
2828
{
29+
const REQUIREMENT_MATCH = 0;
30+
const REQUIREMENT_MISMATCH = 1;
31+
const ROUTE_MATCH = 2;
32+
2933
protected $context;
34+
protected $allow;
3035

3136
private $routes;
3237

@@ -75,7 +80,7 @@ public function match($pathinfo)
7580
{
7681
$this->allow = array();
7782

78-
if ($ret = $this->matchCollection($pathinfo, $this->routes)) {
83+
if ($ret = $this->matchCollection(urldecode($pathinfo), $this->routes)) {
7984
return $ret;
8085
}
8186

@@ -84,10 +89,19 @@ public function match($pathinfo)
8489
: new ResourceNotFoundException();
8590
}
8691

92+
/**
93+
* Tries to match a URL with a set of routes.
94+
*
95+
* @param string $pathinfo The path info to be parsed
96+
* @param RouteCollection $routes The set of routes
97+
*
98+
* @return array An array of parameters
99+
*
100+
* @throws ResourceNotFoundException If the resource could not be found
101+
* @throws MethodNotAllowedException If the resource was found but the request method is not allowed
102+
*/
87103
protected function matchCollection($pathinfo, RouteCollection $routes)
88104
{
89-
$pathinfo = urldecode($pathinfo);
90-
91105
foreach ($routes as $name => $route) {
92106
if ($route instanceof RouteCollection) {
93107
if (false === strpos($route->getPrefix(), '{') && $route->getPrefix() !== substr($pathinfo, 0, strlen($route->getPrefix()))) {
@@ -126,21 +140,38 @@ protected function matchCollection($pathinfo, RouteCollection $routes)
126140
}
127141
}
128142

129-
// check HTTP scheme requirement
130-
if ($scheme = $route->getRequirement('_scheme')) {
131-
if (!$this instanceof RedirectableUrlMatcherInterface) {
132-
throw new \LogicException('The "_scheme" requirement is only supported for URL matchers that implement RedirectableUrlMatcherInterface.');
133-
}
143+
$status = $this->handleRouteRequirements($pathinfo, $name, $route);
134144

135-
if ($this->context->getScheme() !== $scheme) {
136-
return $this->redirect($pathinfo, $name, $scheme);
137-
}
145+
if (self::ROUTE_MATCH === $status[0]) {
146+
return $status[1];
147+
}
148+
149+
if (self::REQUIREMENT_MISMATCH === $status[0]) {
150+
continue;
138151
}
139152

140153
return array_merge($this->mergeDefaults($matches, $route->getDefaults()), array('_route' => $name));
141154
}
142155
}
143156

157+
/**
158+
* Handles specific route requirements.
159+
*
160+
* @param string $pathinfo The path
161+
* @param string $name The route name
162+
* @param string $route The route
163+
*
164+
* @return array The first element represents the status, the second contains additional information
165+
*/
166+
protected function handleRouteRequirements($pathinfo, $name, Route $route)
167+
{
168+
// check HTTP scheme requirement
169+
$scheme = $route->getRequirement('_scheme');
170+
$status = $scheme && $scheme !== $this->context->getScheme() ? self::REQUIREMENT_MISMATCH : self::REQUIREMENT_MATCH;
171+
172+
return array($status, null);
173+
}
174+
144175
protected function mergeDefaults($params, $defaults)
145176
{
146177
$parameters = $defaults;

‎tests/Symfony/Tests/Component/Routing/Matcher/UrlMatcherTest.php

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -207,14 +207,13 @@ public function testMatchRegression()
207207
}
208208

209209
/**
210-
* @expectedException \LogicException
210+
* @expectedException Symfony\Component\Routing\Exception\ResourceNotFoundException
211211
*/
212212
public function testSchemeRequirement()
213213
{
214214
$coll = new RouteCollection();
215215
$coll->add('foo', new Route('/foo', array(), array('_scheme' => 'https')));
216-
217216
$matcher = new UrlMatcher($coll, new RequestContext());
218217
$matcher->match('/foo');
219218
}
220-
}
219+
}

0 commit comments

Comments
 (0)
0