8000 bug #9601 [Routing] Remove usage of deprecated _scheme requirement (D… · symfony/symfony@0af3d19 · GitHub
[go: up one dir, main page]
Skip to content

Commit 0af3d19

Browse files
fabpotfabpot
committed
bug #9601 [Routing] Remove usage of deprecated _scheme requirement (Danez)
This PR was merged into the 2.3 branch. Discussion ---------- [Routing] Remove usage of deprecated _scheme requirement **This is exact the same commit as it was in #9585, which was not merged due to my fault. Sorry for the noise.** | Q | A | ------------- | --- | Bug fix? | yes | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | #8898, #8176 | License | MIT | Doc PR | I removed all usages of the deprecated _scheme requirement inside the Routing Component. Most parts were pretty easy and after multiple refactorings I came up with the solution to have a Route::hasScheme() method and check against this method. I also checked for performance and after trying in_array, arra_flip+isset and foreach, the last one was clearly the winner. https://gist.github.com/Danez/7609898#file-test_performance-php I also adjusted all tests that test '_scheme' to also check the new schemes-requirement. Commits ------- 557dfaa Remove usage of deprecated _scheme in Routing Component
2 parents d56cc4b + 557dfaa commit 0af3d19

File tree

14 files changed

+219
-34
lines changed

14 files changed

+219
-34
lines changed

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

Lines changed: 21 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@ public function testRedirectWhenNoSlash()
3838
);
3939
}
4040

41-
public function testSchemeRedirect()
41+
public function testSchemeRedirectBC()
4242
{
4343
$coll = new RouteCollection();
4444
$coll->add('foo', new Route('/foo', array(), array('_scheme' => 'https')));
@@ -57,4 +57,24 @@ public function testSchemeRedirect()
5757
$matcher->match('/foo')
5858
);
5959
}
60+
61+
public function testSchemeRedirect()
62+
{
63+
$coll = new RouteCollection();
64+
$coll->add('foo', new Route('/foo', array(), array(), array(), '', array('https')));
65+
66+
$matcher = new RedirectableUrlMatcher($coll, $context = new RequestContext());
67+
68+
$this->assertEquals(array(
69+
'_controller' => 'Symfony\Bundle\FrameworkBundle\Controller\RedirectController::urlRedirectAction',
70+
'path' => '/foo',
71+
'permanent' => true,
72+
'scheme' => 'https',
73+
'httpPort' => $context->getHttpPort(),
74+
'httpsPort' => $context->getHttpsPort(),
75+
'_route' => 'foo',
76+
),
77+
$matcher->match('/foo')
78+
);
79+
}
6080
}

‎src/Symfony/Component/Routing/Generator/Dumper/PhpGeneratorDumper.php

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line number< 10000 /th>Diff line change
@@ -92,6 +92,7 @@ private function generateDeclaredRoutes()
9292
$properties[] = $route->getRequirements();
9393
$properties[] = $compiledRoute->getTokens();
9494
$properties[] = $compiledRoute->getHostTokens();
95+
$properties[] = $route->getSchemes();
9596

9697
$routes .= sprintf(" '%s' => %s,\n", $name, str_replace("\n", '', var_export($properties, true)));
9798
}
@@ -114,9 +115,9 @@ public function generate(\$name, \$parameters = array(), \$referenceType = self:
114115
throw new RouteNotFoundException(sprintf('Unable to generate a URL for the named route "%s" as such route does not exist.', \$name));
115116
}
116117
117-
list(\$variables, \$defaults, \$requirements, \$tokens, \$hostTokens) = self::\$declaredRoutes[\$name];
118+
list(\$variables, \$defaults, \$requirements, \$tokens, \$hostTokens, \$requiredSchemes) = self::\$declaredRoutes[\$name];
118119
119-
return \$this->doGenerate(\$variables, \$defaults, \$requirements, \$tokens, \$parameters, \$name, \$referenceType, \$hostTokens);
120+
return \$this->doGenerate(\$variables, \$defaults, \$requirements, \$tokens, \$parameters, \$name, \$referenceType, \$hostTokens, \$requiredSchemes);
120121
}
121122
EOF;
122123
}

‎src/Symfony/Component/Routing/Generator/UrlGenerator.php

Lines changed: 20 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -137,15 +137,15 @@ public function generate($name, $parameters = array(), $referenceType = self::AB
137137
// the Route has a cache of its own and is not recompiled as long as it does not get modified
138138
$compiledRoute = $route->compile();
139139

140-
return $this->doGenerate($compiledRoute->getVariables(), $route->getDefaults(), $route->getRequirements(), $compiledRoute->getTokens(), $parameters, $name, $referenceType, $compiledRoute->getHostTokens());
140+
return $this->doGenerate($compiledRoute->getVariables(), $route->getDefaults(), $route->getRequirements(), $compiledRoute->getTokens(), $parameters, $name, $referenceType, $compiledRoute->getHostTokens(), $route->getSchemes());
141141
}
142142

143143
/**
144144
* @throws MissingMandatoryParametersException When some parameters are missing that are mandatory for the route
145145
* @throws InvalidParameterException When a parameter value for a placeholder is not correct because
146146
* it does not match the requirement
147147
*/
148-
protected function doGenerate($variables, $defaults, $requirements, $tokens, $parameters, $name, $referenceType, $hostTokens)
148+
protected function doGenerate($variables, $defaults, $requirements, $tokens, $parameters, $name, $referenceType, $hostTokens, array $requiredSchemes = array())
149149
{
150150
$variables = array_flip($variables);
151151
$mergedParams = array_replace($defaults, $this->context->getParameters(), $parameters);
@@ -204,7 +204,24 @@ protected function doGenerate($variables, $defaults, $requirements, $tokens, $pa
204204
$schemeAuthority = '';
205205
if ($host = $this->context->getHost()) {
206206
$scheme = $this->context->getScheme();
207-
if (isset($requirements['_scheme']) && ($req = strtolower($requirements['_scheme'])) && $scheme !== $req) {
207+
208+
if ($requiredSchemes) {
209+
$schemeMatched = false;
210+
foreach ($requiredSchemes as $requiredScheme) {
211+
if ($scheme === $requiredScheme) {
212+
$schemeMatched = true;
213+
214+
break;
215+
}
216+
}
217+
218+
if (!$schemeMatched) {
219+
$referenceType = self::ABSOLUTE_URL;
220+
$scheme = current($requiredSchemes);
221+
}
222+
223+
} elseif (isset($requirements['_scheme']) && ($req = strtolower($requirements['_scheme'])) && $scheme !== $req) {
224+
// We do this for BC; to be removed if _scheme is not supported anymore
208225
$referenceType = self::ABSOLUTE_URL;
209226
$scheme = $req;
210227
}

‎src/Symfony/Component/Routing/Matcher/Dumper/PhpMatcherDumper.php

Lines changed: 11 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -280,14 +280,15 @@ private function compileRoute(Route $route, $name, $supportsRedirections, $paren
280280
EOF;
281281
}
282282

283-
if ($scheme = $route->getRequirement('_scheme')) {
283+
if ($schemes = $route->getSchemes()) {
284284
if (!$supportsRedirections) {
285-
throw new \LogicException('The "_scheme" requirement is only supported for URL matchers that implement RedirectableUrlMatcherInterface.');
285+
throw new \LogicException('The "schemes" requirement is only supported for URL matchers that implement RedirectableUrlMatcherInterface.');
286286
}
287-
287+
$schemes = str_replace("\n", '', var_export(array_flip($schemes), true));
288288
$code .= <<<EOF
289-
if (\$this->context->getScheme() !== '$scheme') {
290-
return \$this->redirect(\$pathinfo, '$name', '$scheme');
289+
\$requiredSchemes = $schemes;
290+
if (!isset(\$requiredSchemes[\$this->context->getScheme()])) {
291+
return \$this->redirect(\$pathinfo, '$name', key(\$requiredSchemes));
291292
}
292293
293294
@@ -305,8 +306,11 @@ private function compileRoute(Route $route, $name, $supportsRedirections, $paren
305306
}
306307
$vars[] = "array('_route' => '$name')";
307308

308-
$code .= sprintf(" return \$this->mergeDefaults(array_replace(%s), %s);\n"
309-
, implode(', ', $vars), str_replace("\n", '', var_export($route->getDefaults(), true)));
309+
$code .= sprintf(
310+
" return \$this->mergeDefaults(array_replace(%s), %s);\n",
311+
implode(', ', $vars),
312+
str_replace("\n", '', var_export($route->getDefaults(), true))
313+
);
310314

311315
} elseif ($route->getDefaults()) {
312316
$code .= sprintf(" return %s;\n", str_replace("\n", '', var_export(array_replace($route->getDefaults(), array('_route' => $name)), true)));

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

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -51,9 +51,10 @@ public function match($pathinfo)
5151
protected function handleRouteRequirements($pathinfo, $name, Route $route)
5252
{
5353
// check HTTP scheme requirement
54-
$scheme = $route->getRequirement('_scheme');
55-
if ($scheme && $this->context->getScheme() !== $scheme) {
56-
return array(self::ROUTE_MATCH, $this->redirect($pathinfo, $name, $scheme));
54+
$scheme = $this->context->getScheme();
55+
$schemes = $route->getSchemes();
56+
if ($schemes && !$route->hasScheme($scheme)) {
57+
return array(self::ROUTE_MATCH, $this->redirect($pathinfo, $name, current($schemes)));
5758
}
5859

5960
return array(self::REQUIREMENT_MATCH, null);

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

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -95,9 +95,11 @@ protected function matchCollection($pathinfo, RouteCollection $routes)
9595
}
9696

9797
// check HTTP scheme requirement
98-
if ($scheme = $route->getRequirement('_scheme')) {
99-
if ($this->context->getScheme() !== $scheme) {
100-
$this->addTrace(sprintf('Scheme "%s" does not match the requirement ("%s"); the user will be redirected', $this->context->getScheme(), $scheme), self::ROUTE_ALMOST_MATCHES, $name, $route);
98+
if ($requiredSchemes = $route->getSchemes()) {
99+
$scheme = $this->context->getScheme();
100+
101+
if (!$route->hasScheme($scheme)) {
102+
$this->addTrace(sprintf('Scheme "%s" does not match any of the required schemes ("%s"); the user will be redirected to first required scheme', $scheme, implode(', ', $requiredSchemes)), self::ROUTE_ALMOST_MATCHES, $name, $route);
101103

102104
return true;
103105
}

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -181,8 +181,8 @@ protected function getAttributes(Route $route, $name, array $attributes)
181181
protected function handleRouteRequirements($pathinfo, $name, Route $route)
182182
{
183183
// check HTTP scheme requirement
184-
$scheme = $route->getRequirement('_scheme');
185-
$status = $scheme && $scheme !== $this->context->getScheme() ? self::REQUIREMENT_MISMATCH : self::REQUIREMENT_MATCH;
184+
$scheme = $this->context->getScheme();
185+
$status = $route->getSchemes() && !$route->hasScheme($scheme) ? self::REQUIREMENT_MISMATCH : self::REQUIREMENT_MATCH;
186186

187187
return array($status, null);
188188
}

‎src/Symfony/Component/Routing/Route.php

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -241,6 +241,25 @@ public function setSchemes($schemes)
241241
return $this;
242242
}
243243

244+
/**
245+
* Checks if a scheme requirement has been set.
246+
*
247+
* @param string $scheme
248+
*
249+
* @return Boolean true if the scheme requirement exists, otherwise false
250+
*/
251+
public function hasScheme($scheme)
252+
{
253+
$scheme = strtolower($scheme);
254+
foreach ($this->schemes as $requiredScheme) {
255+
if ($scheme === $requiredScheme) {
256+
return true;
257+
}
258+
}
259+
260+
return false;
261+
}
262+
244263
/**
245264
* Returns the uppercased HTTP methods this route is restricted to.
246265
* So an empty array means that any method is allowed.

‎src/Symfony/Component/Routing/Tests/Fixtures/dumper/url_matcher2.php

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -319,17 +319,19 @@ public function match($pathinfo)
319319

320320
// secure
321321
if ($pathinfo === '/secure') {
322-
if ($this->context->getScheme() !== 'https') {
323-
return $this->redirect($pathinfo, 'secure', 'https');
322+
$requiredSchemes = array ( 'https' => 0,);
323+
if (!isset($requiredSchemes[$this->context->getScheme()])) {
324+
return $this->redirect($pathinfo, 'secure', key($requiredSchemes));
324325
}
325326

326327
return array('_route' => 'secure');
327328
}
328329

329330
// nonsecure
330331
if ($pathinfo === '/nonsecure') {
331-
if ($this->context->getScheme() !== 'http') {
332-
return $this->redirect($pathinfo, 'nonsecure', 'http');
332+
$requiredSchemes = array ( 'http' => 0,);
333+
if (!isset($requiredSchemes[$this->context->getScheme()])) {
334+
return $this->redirect($pathinfo, 'nonsecure', key($requiredSchemes));
333335
}
334336

335337
return array('_route' => 'nonsecure');

‎src/Symfony/Component/Routing/Tests/Generator/Dumper/PhpGeneratorDumperTest.php

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -114,4 +114,37 @@ public function testDumpForRouteWithDefaults()
114114

115115
$this->assertEquals($url, '/testing');
116116
}
117+
118+
public function testDumpWithSchemeRequirement()
119+
{
120+
$this->routeCollection->add('Test1', new Route('/testing', array(), array(), array(), '', array('ftp', 'https')));
121+
$this->routeCollection->add('Test2', new Route('/testing_bc', array(), array('_scheme' => 'https'))); // BC
122+
123+
file_put_contents($this->testTmpFilepath, $this->generatorDumper->dump(array('class' => 'SchemeUrlGenerator')));
124+
include ($this->testTmpFilepath);
125+
126+
$projectUrlGenerator = new \SchemeUrlGenerator(new RequestContext('/app.php'));
127+
128+
$absoluteUrl = $projectUrlGenerator->generate('Test1', array(), true);
129+
$absoluteUrlBC = $projectUrlGenerator->generate('Test2', array(), true);
130+
$relativeUrl = $projectUrlGenerator->generate('Test1', array(), false);
131+
$relativeUrlBC = $projectUrlGenerator->generate('Test2', array(), false);
132+
133+
$this->assertEquals($absoluteUrl, 'ftp://localhost/app.php/testing');
134+
$this->assertEquals($absoluteUrlBC, 'https://localhost/app.php/testing_bc');
135+
$this->assertEquals($relativeUrl, 'ftp://localhost/app.php/testing');
136+
$this->assertEquals($relativeUrlBC, 'https://localhost/app.php/testing_bc');
137+
138+
$projectUrlGenerator = new \SchemeUrlGenerator(new RequestContext('/app.php', 'GET', 'localhost', 'https'));
139+
140+
$absoluteUrl = $projectUrlGenerator->generate('Test1', array(), true);
141+
$absoluteUrlBC = $projectUrlGenerator->generate('Test2', array(), true);
142+
$relativeUrl = $projectUrlGenerator->generate('Test1', array(), false);
143+
$relativeUrlBC = $projectUrlGenerator->generate('Test2', array(), false);
144+
145+
$this->assertEquals($absoluteUrl, 'https://localhost/app.php/testing');
146+
$this->assertEquals($absoluteUrlBC, 'https://localhost/app.php/testing_bc');
147+
$this->assertEquals($relativeUrl, '/app.php/testing');
148+
$this->assertEquals($relativeUrlBC, '/app.php/testing_bc');
149+
}
117150
}

‎src/Symfony/Component/Routing/Tests/Generator/UrlGeneratorTest.php

Lines changed: 47 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -248,22 +248,40 @@ public function testRequiredParamAndEmptyPassed()
248248

249249
public function testSchemeRequirementDoesNothingIfSameCurrentScheme()
250250
{
251-
$routes = $this->getRoutes('test', new Route('/', array(), array('_scheme' => 'http')));
251+
$routes = $this->getRoutes('test', new Route('/', array(), array('_scheme' => 'http'))); // BC
252252
$this->assertEquals('/app.php/', $this->getGenerator($routes)->generate('test'));
253253

254-
$routes = $this->getRoutes('test', new Route('/', array(), array('_scheme' => 'https')));
254+
$routes = $this->getRoutes('test', new Route('/', array(), array('_scheme' => 'https'))); // BC
255+
$this->assertEquals('/app.php/', $this->getGenerator($routes, array('scheme' => 'https'))->generate('test'));
256+
257+
$routes = $this->getRoutes('test', new Route('/', array(), array(), array(), '', array('http')));
258+
$this->assertEquals('/app.php/', $this->getGenerator($routes)->generate('test'));
259+
260+
$routes = $this->getRoutes('test', new Route('/', array(), array(), array(), '', array('https')));
255261
$this->assertEquals('/app.php/', $this->getGenerator($routes, array('scheme' => 'https'))->generate('test'));
256262
}
257263

258264
public function testSchemeRequirementForcesAbsoluteUrl()
259265
{
260-
$routes = $this->getRoutes('test', new Route('/', array(), array('_scheme' => 'https')));
266+
$routes = $this->getRoutes('test', new Route('/', array(), array('_scheme' => 'https'))); // BC
267+
$this->assertEquals('https://localhost/app.php/', $this->getGenerator($routes)->generate('test'));
268+
269+
$routes = $this->getRoutes('test', new Route('/', array(), array('_scheme' => 'http'))); // BC
270+
$this->assertEquals('http://localhost/app.php/', $this->getGenerator($routes, array('scheme' => 'https'))->generate('test'));
271+
272+
$routes = $this->getRoutes('test', new Route('/', array(), array(), array(), '', array('https')));
261273
$this->assertEquals('https://localhost/app.php/', $this->getGenerator($routes)->generate('test'));
262274

263-
$routes = $this->getRoutes('test', new Route('/', array(), array('_scheme' => 'http')));
275+
$routes = $this->getRoutes('test', new Route('/', array(), array(), array(), '', array('http')));
264276
$this->assertEquals('http://localhost/app.php/', $this->getGenerator($routes, array('scheme' => 'https'))->generate('test'));
265277
}
266278

279+
public function testSchemeRequirementCreatesUrlForFirstRequiredScheme()
280+
{
281+
$routes = $this->getRoutes('test', new Route('/', array(), array(), array(), '', array('Ftp', 'https')));
282+
$this->assertEquals('ftp://localhost/app.php/', $this->getGenerator($routes)->generate('test'));
283+
}
284+
267285
public function testPathWithTwoStartingSlashes()
268286
{
269287
$routes = $this->getRoutes('test', new Route('//path-and-not-domain'));
@@ -447,7 +465,7 @@ public function testUrlWithInvalidParameterInHostInNonStrictMode()
447465
$this->assertNull($generator->generate('test', array('foo' => 'baz'), false));
448466
}
449467

450-
public function testGenerateNetworkPath()
468+
public function testGenerateNetworkPathBC()
451469
{
452470
$routes = $this->getRoutes('test', new Route('/{name}', array(), array('_scheme' => 'http'), array(), '{locale}.example.com'));
453471

@@ -465,13 +483,32 @@ public function testGenerateNetworkPath()
465483
);
466484
}
 10000
467485

486+
public function testGenerateNetworkPath()
487+
{
488+
$routes = $this->getRoutes('test', new Route('/{name}', array(), array(), array(), '{locale}.example.com', array('http')));
489+
490+
$this->assertSame('//fr.example.com/app.php/Fabien', $this->getGenerator($routes)->generate('test',
491+
array('name' =>'Fabien', 'locale' => 'fr'), UrlGeneratorInterface::NETWORK_PATH), 'network path with different host'
492+
);
493+
$this->assertSame('//fr.example.com/app.php/Fabien?query=string', $this->getGenerator($routes, array('host' => 'fr.example.com'))->generate('test',
494+
array('name' =>'Fabien', 'locale' => 'fr', 'query' => 'string'), UrlGeneratorInterface::NETWORK_PATH), 'network path although host same as context'
495+
);
496+
$this->assertSame('http://fr.example.com/app.php/Fabien', $this->getGenerator($routes, array('scheme' => 'https'))->generate('test',
497+
array('name' =>'Fabien', 'locale' => 'fr'), UrlGeneratorInterface::NETWORK_PATH), 'absolute URL because scheme requirement does not match context'
498+
);
499+
$this->assertSame('http://fr.example.com/app.php/Fabien', $this->getGenerator($routes)->generate('test',
500+
array('name' =>'Fabien', 'locale' => 'fr'), UrlGeneratorInterface::ABSOLUTE_URL), 'absolute URL with same scheme because it is requested'
501+
);
502+
}
503+
468504
public function testGenerateRelativePath()
469505
{
470506
$routes = new RouteCollection();
471507
$routes->add('article', new Route('/{author}/{article}/'));
472508
$routes->add('comments', new Route('/{author}/{article}/comments'));
473509
$routes->add('host', new Route('/{article}', array(), array(), array(), '{author}.example.com'));
474-
$routes->add('scheme', new Route('/{author}', array(), array('_scheme' => 'https')));
510+
$routes->add('schemeBC', new Route('/{author}', array(), array('_scheme' => 'https'))); // BC
511+
$routes->add('scheme', new Route('/{author}/blog', array(), array(), array(), '', array('https')));
475512
$routes->add('unrelated', new Route('/about'));
476513

477514
$generator = $this->getGenerator($routes, array('host' => 'example.com', 'pathInfo' => '/fabien/symfony-is-great/'));
@@ -491,9 +528,12 @@ public function testGenerateRelativePath()
491528
$this->assertSame('//bernhard.example.com/app.php/forms-are-great', $generator->generate('host',
492529
array('author' =>'bernhard', 'article' => 'forms-are-great'), UrlGeneratorInterface::RELATIVE_PATH)
493530
);
494-
$this->assertSame('https://example.com/app.php/bernhard', $generator->generate('scheme',
531+
$this->assertSame('https://example.com/app.php/bernhard', $generator->generate('schemeBC',
495532
array('author' =>'bernhard'), UrlGeneratorInterface::RELATIVE_PATH)
496533
);
534+
$this->assertSame('https://example.com/app.php/bernhard/blog', $generator->generate('scheme',
535+
array('author' =>'bernhard'), UrlGeneratorInterface::RELATIVE_PATH)
536+
);
497537
$this->assertSame('../../about', $generator->generate('unrelated',
498538
array(), UrlGeneratorInterface::RELATIVE_PATH)
499539
);

0 commit comments

Comments
 (0)
0