Skip to content

Commit 0a0370d

Browse files
nicolas-grekasnicolas-grekas
committed
Merge branch '2.8' into 3.4
* 2.8: [appveyor] Workaround GitHub disabling of low versions of TLS [Routing] Don't throw 405 when scheme requirement doesn't match [Routing] Revert throwing 405 on missed slash/scheme redirections Fix ArrayInput::toString() for InputArgument::IS_ARRAY args [Routing] fix CS
2 parents 43738be + 23cf699 commit 0a0370d

File tree

10 files changed

+60
-43
lines changed

10 files changed

+60
-43
lines changed

‎Matcher/Dumper/PhpMatcherDumper.php‎

Lines changed: 25 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -309,7 +309,6 @@ private function compileRoute(Route $route, $name, $supportsRedirections, $paren
309309
if ('/' === substr(\$pathinfo, -1)) {
310310
// no-op
311311
} elseif ('GET' !== \$canonicalMethod) {
312-
\$allow[] = 'GET';
313312
goto $gotoname;
314313
} else {
315314
return array_replace(\$ret, \$this->redirect(\$rawPathinfo.'/', '$name'));
@@ -324,11 +323,19 @@ private function compileRoute(Route $route, $name, $supportsRedirections, $paren
324323
throw new \LogicException('The "schemes" requirement is only supported for URL matchers that implement RedirectableUrlMatcherInterface.');
325324
}
326325
$schemes = str_replace("\n", '', var_export(array_flip($schemes), true));
327-
$code .= <<<EOF
326+
if ($methods) {
327+
$methods = implode("', '", $methods);
328+
$code .= <<<EOF
328329
\$requiredSchemes = $schemes;
329-
if (!isset(\$requiredSchemes[\$context->getScheme()])) {
330+
\$hasRequiredScheme = isset(\$requiredSchemes[\$context->getScheme()]);
331+
if (!in_array(\$context->getMethod(), array('$methods'))) {
332+
if (\$hasRequiredScheme) {
333+
\$allow = array_merge(\$allow, array('$methods'));
334+
}
335+
goto $gotoname;
336+
}
337+
if (!\$hasRequiredScheme) {
330338
if ('GET' !== \$canonicalMethod) {
331-
\$allow[] = 'GET';
332339
goto $gotoname;
333340
}
334341
@@ -337,9 +344,21 @@ private function compileRoute(Route $route, $name, $supportsRedirections, $paren
337344
338345
339346
EOF;
340-
}
347+
} else {
348+
$code .= <<<EOF
349+
\$requiredSchemes = $schemes;
350+
if (!isset(\$requiredSchemes[\$this->context->getScheme()])) {
351+
if ('GET' !== \$canonicalMethod) {
352+
goto $gotoname;
353+
}
354+
355+
return array_replace(\$ret, \$this->redirect(\$rawPathinfo, '$name', key(\$requiredSchemes)));
356+
}
357+
341358
342-
if ($methods) {
359+
EOF;
360+
}
361+
} elseif ($methods) {
343362
if (1 === count($methods)) {
344363
if ('HEAD' === $methods[0]) {
345364
$code .= <<<EOF

‎Matcher/UrlMatcher.php‎

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -135,6 +135,12 @@ protected function matchCollection($pathinfo, RouteCollection $routes)
135135
continue;
136136
}
137137

138+
$status = $this->handleRouteRequirements($pathinfo, $name, $route);
139+
140+
if (self::REQUIREMENT_MISMATCH === $status[0]) {
141+
continue;
142+
}
143+
138144
// check HTTP method requirement
139145
if ($requiredMethods = $route->getMethods()) {
140146
// HEAD and GET are equivalent as per RFC
@@ -143,18 +149,14 @@ protected function matchCollection($pathinfo, RouteCollection $routes)
143149
}
144150

145151
if (!in_array($method, $requiredMethods)) {
146-
$this->allow = array_merge($this->allow, $requiredMethods);
152+
if (self::REQUIREMENT_MATCH === $status[0]) {
153+
$this->allow = array_merge($this->allow, $requiredMethods);
154+
}
147155

148156
continue;
149157
}
150158
}
151159

152-
$status = $this->handleRouteRequirements($pathinfo, $name, $route);
153-
154-
if (self::REQUIREMENT_MISMATCH === $status[0]) {
155-
continue;
156-
}
157-
158160
return $this->getAttributes($route, $name, array_replace($matches, $hostMatches, isset($status[1]) ? $status[1] : array()));
159161
}
160162
}

‎RouteCollection.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -117,7 +117,7 @@ public function remove($name)
117117
* Adds a route collection at the end of the current set by appending all
118118
* routes of the added collection.
119119
*/
120-
public function addCollection(RouteCollection $collection)
120+
public function addCollection(self $collection)
121121
{
122122
// we need to remove all routes with the same names first because just replacing them
123123
// would not place the new route at the end of the merged array

‎Tests/Fixtures/dumper/url_matcher2.php‎

Lines changed: 2 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -86,7 +86,6 @@ public function match($rawPathinfo)
8686
if ('/' === substr($pathinfo, -1)) {
8787
// no-op
8888
} elseif ('GET' !== $canonicalMethod) {
89-
$allow[] = 'GET';
9089
goto not_baz3;
9190
} else {
9291
return array_replace($ret, $this->redirect($rawPathinfo.'/', 'baz3'));
@@ -104,7 +103,6 @@ public function match($rawPathinfo)
104103
if ('/' === substr($pathinfo, -1)) {
105104
// no-op
106105
} elseif ('GET' !== $canonicalMethod) {
107-
$allow[] = 'GET';
108106
goto not_baz4;
109107
} else {
110108
return array_replace($ret, $this->redirect($rawPathinfo.'/', 'baz4'));
@@ -196,7 +194,6 @@ public function match($rawPathinfo)
196194
if ('/' === substr($pathinfo, -1)) {
197195
// no-op
198196
} elseif ('GET' !== $canonicalMethod) {
199-
$allow[] = 'GET';
200197
goto not_hey;
201198
} else {
202199
return array_replace($ret, $this->redirect($rawPathinfo.'/', 'hey'));
@@ -346,9 +343,8 @@ public function match($rawPathinfo)
346343
if ('/secure' === $pathinfo) {
347344
$ret = array('_route' => 'secure');
348345
$requiredSchemes = array ( 'https' => 0,);
349-
if (!isset($requiredSchemes[$context->getScheme()])) {
346+
if (!isset($requiredSchemes[$this->context->getScheme()])) {
350347
if ('GET' !== $canonicalMethod) {
351-
$allow[] = 'GET';
352348
goto not_secure;
353349
}
354350

@@ -363,9 +359,8 @@ public function match($rawPathinfo)
363359
if ('/nonsecure' === $pathinfo) {
364360
$ret = array('_route' => 'nonsecure');
365361
$requiredSchemes = array ( 'http' => 0,);
366-
if (!isset($requiredSchemes[$context->getScheme()])) {
362+
if (!isset($requiredSchemes[$this->context->getScheme()])) {
367363
if ('GET' !== $canonicalMethod) {
368-
$allow[] = 'GET';
369364
goto not_nonsecure;
370365
}
371366

‎Tests/Fixtures/dumper/url_matcher5.php‎

Lines changed: 0 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,6 @@ public function match($rawPathinfo)
5858
if ('/' === substr($pathinfo, -1)) {
5959
// no-op
6060
} elseif ('GET' !== $canonicalMethod) {
61-
$allow[] = 'GET';
6261
goto not_a_fourth;
6362
} else {
6463
return array_replace($ret, $this->redirect($rawPathinfo.'/', 'a_fourth'));
@@ -74,7 +73,6 @@ public function match($rawPathinfo)
7473
if ('/' === substr($pathinfo, -1)) {
7574
// no-op
7675
} elseif ('GET' !== $canonicalMethod) {
77-
$allow[] = 'GET';
7876
goto not_a_fifth;
7977
} else {
8078
return array_replace($ret, $this->redirect($rawPathinfo.'/', 'a_fifth'));
@@ -90,7 +88,6 @@ public function match($rawPathinfo)
9088
if ('/' === substr($pathinfo, -1)) {
9189
// no-op
9290
} elseif ('GET' !== $canonicalMethod) {
93-
$allow[] = 'GET';
9491
goto not_a_sixth;
9592
} else {
9693
return array_replace($ret, $this->redirect($rawPathinfo.'/', 'a_sixth'));
@@ -114,7 +111,6 @@ public function match($rawPathinfo)
114111
if ('/' === substr($pathinfo, -1)) {
115112
// no-op
116113
} elseif ('GET' !== $canonicalMethod) {
117-
$allow[] = 'GET';
118114
goto not_nested_a;
119115
} else {
120116
return array_replace($ret, $this->redirect($rawPathinfo.'/', 'nested_a'));
@@ -130,7 +126,6 @@ public function match($rawPathinfo)
130126
if ('/' === substr($pathinfo, -1)) {
131127
// no-op
132128
} elseif ('GET' !== $canonicalMethod) {
133-
$allow[] = 'GET';
134129
goto not_nested_b;
135130
} else {
136131
return array_replace($ret, $this->redirect($rawPathinfo.'/', 'nested_b'));
@@ -146,7 +141,6 @@ public function match($rawPathinfo)
146141
if ('/' === substr($pathinfo, -1)) {
147142
// no-op
148143
} elseif ('GET' !== $canonicalMethod) {
149-
$allow[] = 'GET';
150144
goto not_nested_c;
151145
} else {
152146
return array_replace($ret, $this->redirect($rawPathinfo.'/', 'nested_c'));
@@ -165,7 +159,6 @@ public function match($rawPathinfo)
165159
if ('/' === substr($pathinfo, -1)) {
166160
// no-op
167161
} elseif ('GET' !== $canonicalMethod) {
168-
$allow[] = 'GET';
169162
goto not_slashed_a;
170163
} else {
171164
return array_replace($ret, $this->redirect($rawPathinfo.'/', 'slashed_a'));
@@ -181,7 +174,6 @@ public function match($rawPathinfo)
181174
if ('/' === substr($pathinfo, -1)) {
182175
// no-op
183176
} elseif ('GET' !== $canonicalMethod) {
184-
$allow[] = 'GET';
185177
goto not_slashed_b;
186178
} else {
187179
return array_replace($ret, $this->redirect($rawPathinfo.'/', 'slashed_b'));
@@ -197,7 +189,6 @@ public function match($rawPathinfo)
197189
if ('/' === substr($pathinfo, -1)) {
198190
// no-op
199191
} elseif ('GET' !== $canonicalMethod) {
200-
$allow[] = 'GET';
201192
goto not_slashed_c;
202193
} else {
203194
return array_replace($ret, $this->redirect($rawPathinfo.'/', 'slashed_c'));

‎Tests/Fixtures/dumper/url_matcher7.php‎

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,6 @@ public function match($rawPathinfo)
3535
if ('/' === substr($pathinfo, -1)) {
3636
// no-op
3737
} elseif ('GET' !== $canonicalMethod) {
38-
$allow[] = 'GET';
3938
goto not_simple_trailing_slash_no_methods;
4039
} else {
4140
return array_replace($ret, $this->redirect($rawPathinfo.'/', 'simple_trailing_slash_no_methods'));
@@ -51,7 +50,6 @@ public function match($rawPathinfo)
5150
if ('/' === substr($pathinfo, -1)) {
5251
// no-op
5352
} elseif ('GET' !== $canonicalMethod) {
54-
$allow[] = 'GET';
5553
goto not_simple_trailing_slash_GET_method;
5654
} else {
5755
return array_replace($ret, $this->redirect($rawPathinfo.'/', 'simple_trailing_slash_GET_method'));
@@ -99,7 +97,6 @@ public function match($rawPathinfo)
9997
if ('/' === substr($pathinfo, -1)) {
10098
// no-op
10199
} elseif ('GET' !== $canonicalMethod) {
102-
$allow[] = 'GET';
103100
goto not_regex_trailing_slash_no_methods;
104101
} else {
105102
return array_replace($ret, $this->redirect($rawPathinfo.'/', 'regex_trailing_slash_no_methods'));
@@ -115,7 +112,6 @@ public function match($rawPathinfo)
115112
if ('/' === substr($pathinfo, -1)) {
116113
// no-op
117114
} elseif ('GET' !== $canonicalMethod) {
118-
$allow[] = 'GET';
119115
goto not_regex_trailing_slash_GET_method;
120116
} else {
121117
return array_replace($ret, $this->redirect($rawPathinfo.'/', 'regex_trailing_slash_GET_method'));

‎Tests/Matcher/DumpedRedirectableUrlMatcherTest.php‎

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -19,14 +19,6 @@
1919

2020
class DumpedRedirectableUrlMatcherTest extends RedirectableUrlMatcherTest
2121
{
22-
/**
23-
* @expectedException \Symfony\Component\Routing\Exception\MethodNotAllowedException
24-
*/
25-
public function testRedirectWhenNoSlashForNonSafeMethod()
26-
{
27-
parent::testRedirectWhenNoSlashForNonSafeMethod();
28-
}
29-
3022
protected function getUrlMatcher(RouteCollection $routes, RequestContext $context = null)
3123
{
3224
static $i = 0;

‎Tests/Matcher/DumpedUrlMatcherTest.php‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,15 @@ public function testSchemeRequirement()
2626
parent::testSchemeRequirement();
2727
}
2828

29+
/**
30+
* @expectedException \LogicException
31+
* @expectedExceptionMessage The "schemes" requirement is only supported for URL matchers that implement RedirectableUrlMatcherInterface.
32+
*/
33+
public function testSchemeAndMethodMismatch()
34+
{
35+
parent::testSchemeRequirement();
36+
}
37+
2938
protected function getUrlMatcher(RouteCollection $routes, RequestContext $context = null)
3039
{
3140
static $i = 0;

‎Tests/Matcher/UrlMatcherTest.php‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -475,6 +475,18 @@ public function testNestedCollections()
475475
$this->assertEquals(array('_route' => 'buz'), $matcher->match('/prefix/buz'));
476476
}
477477

478+
/**
479+
* @expectedException \Symfony\Component\Routing\Exception\ResourceNotFoundException
480+
*/
481+
public function testSchemeAndMethodMismatch()
482+
{
483+
$coll = new RouteCollection();
484+
$coll->add('foo', new Route('/', array(), array(), array(), null, array('https'), array('POST')));
485+
486+
$matcher = $this->getUrlMatcher($coll);
487+
$matcher->match('/');
488+
}
489+
478490
protected function getUrlMatcher(RouteCollection $routes, RequestContext $context = null)
479491
{
480492
return new UrlMatcher($routes, $context ?: new RequestContext());

‎Tests/RouterTest.php‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
namespace Symfony\Component\Routing\Tests;
1313

1414
use PHPUnit\Framework\TestCase;
15+
use Symfony\Component\Routing\RouteCollection;
1516
use Symfony\Component\Routing\Router;
1617
use Symfony\Component\HttpFoundation\Request;
1718

@@ -83,7 +84,7 @@ public function testThatRouteCollectionIsLoaded()
8384
{
8485
$this->router->setOption('resource_type', 'ResourceType');
8586

86-
$routeCollection = $this->getMockBuilder('Symfony\Component\Routing\RouteCollection')->getMock();
87+
$routeCollection = new RouteCollection();
8788

8889
$this->loader->expects($this->once())
8990
->method('load')->with('routing.yml', 'ResourceType')

0 commit comments

Comments
 (0)