Skip to content

Commit 23cf699

Browse files
nicolas-grekasnicolas-grekas
committed
Merge branch '2.7' into 2.8
* 2.7: [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 39d54f5 + 109e8ad commit 23cf699

File tree

8 files changed

+58
-28
lines changed

8 files changed

+58
-28
lines changed

‎Matcher/Dumper/PhpMatcherDumper.php

Lines changed: 25 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -264,7 +264,6 @@ private function compileRoute(Route $route, $name, $supportsRedirections, $paren
264264
if ('/' === substr(\$pathinfo, -1)) {
265265
// no-op
266266
} elseif (!in_array(\$this->context->getMethod(), array('HEAD', 'GET'))) {
267-
\$allow[] = 'GET';
268267
goto $gotoname;
269268
} else {
270269
return \$this->redirect(\$rawPathinfo.'/', '$name');
@@ -279,11 +278,19 @@ private function compileRoute(Route $route, $name, $supportsRedirections, $paren
279278
throw new \LogicException('The "schemes" requirement is only supported for URL matchers that implement RedirectableUrlMatcherInterface.');
280279
}
281280
$schemes = str_replace("\n", '', var_export(array_flip($schemes), true));
282-
$code .= <<<EOF
281+
if ($methods) {
282+
$methods = implode("', '", $methods);
283+
$code .= <<<EOF
283284
\$requiredSchemes = $schemes;
284-
if (!isset(\$requiredSchemes[\$this->context->getScheme()])) {
285+
\$hasRequiredScheme = isset(\$requiredSchemes[\$this->context->getScheme()]);
286+
if (!in_array(\$this->context->getMethod(), array('$methods'))) {
287+
if (\$hasRequiredScheme) {
288+
\$allow = array_merge(\$allow, array('$methods'));
289+
}
290+
goto $gotoname;
291+
}
292+
if (!\$hasRequiredScheme) {
285293
if (!in_array(\$this->context->getMethod(), array('HEAD', 'GET'))) {
286-
\$allow[] = 'GET';
287294
goto $gotoname;
288295
}
289296
@@ -292,9 +299,21 @@ private function compileRoute(Route $route, $name, $supportsRedirections, $paren
292299
293300
294301
EOF;
295-
}
302+
} else {
303+
$code .= <<<EOF
304+
\$requiredSchemes = $schemes;
305+
if (!isset(\$requiredSchemes[\$this->context->getScheme()])) {
306+
if (!in_array(\$this->context->getMethod(), array('HEAD', 'GET'))) {
307+
goto $gotoname;
308+
}
296309
297-
if ($methods) {
310+
return \$this->redirect(\$rawPathinfo, '$name', key(\$requiredSchemes));
311+
}
312+
313+
314+
EOF;
315+
}
316+
} elseif ($methods) {
298317
if (1 === count($methods)) {
299318
$code .= <<<EOF
300319
if (\$this->context->getMethod() != '$methods[0]') {

‎Matcher/UrlMatcher.php

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -129,6 +129,12 @@ protected function matchCollection($pathinfo, RouteCollection $routes)
129129
continue;
130130
}
131131

132+
$status = $this->handleRouteRequirements($pathinfo, $name, $route);
133+
134+
if (self::REQUIREMENT_MISMATCH === $status[0]) {
135+
continue;
136+
}
137+
132138
// check HTTP method requirement
133139
if ($requiredMethods = $route->getMethods()) {
134140
// HEAD and GET are equivalent as per RFC
@@ -137,22 +143,18 @@ protected function matchCollection($pathinfo, RouteCollection $routes)
137143
}
138144

139145
if (!in_array($method, $requiredMethods)) {
140-
$this->allow = array_merge($this->allow, $requiredMethods);
146+
if (self::REQUIREMENT_MATCH === $status[0]) {
147+
$this->allow = array_merge($this->allow, $requiredMethods);
148+
}
141149

142150
continue;
143151
}
144152
}
145153

146-
$status = $this->handleRouteRequirements($pathinfo, $name, $route);
147-
148154
if (self::ROUTE_MATCH === $status[0]) {
149155
return $status[1];
150156
}
151157

152-
if (self::REQUIREMENT_MISMATCH === $status[0]) {
153-
continue;
154-
}
155-
156158
return $this->getAttributes($route, $name, array_replace($matches, $hostMatches));
157159
}
158160
}

‎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: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,6 @@ public function match($rawPathinfo)
6969
if ('/' === substr($pathinfo, -1)) {
7070
// no-op
7171
} elseif (!in_array($this->context->getMethod(), array('HEAD', 'GET'))) {
72-
$allow[] = 'GET';
7372
goto not_baz3;
7473
} else {
7574
return $this->redirect($rawPathinfo.'/', 'baz3');
@@ -86,7 +85,6 @@ public function match($rawPathinfo)
8685
if ('/' === substr($pathinfo, -1)) {
8786
// no-op
8887
} elseif (!in_array($this->context->getMethod(), array('HEAD', 'GET'))) {
89-
$allow[] = 'GET';
9088
goto not_baz4;
9189
} else {
9290
return $this->redirect($rawPathinfo.'/', 'baz4');
@@ -185,7 +183,6 @@ public function match($rawPathinfo)
185183
if ('/' === substr($pathinfo, -1)) {
186184
// no-op
187185
} elseif (!in_array($this->context->getMethod(), array('HEAD', 'GET'))) {
188-
$allow[] = 'GET';
189186
goto not_hey;
190187
} else {
191188
return $this->redirect($rawPathinfo.'/', 'hey');
@@ -337,7 +334,6 @@ public function match($rawPathinfo)
337334
$requiredSchemes = array ( 'https' => 0,);
338335
if (!isset($requiredSchemes[$this->context->getScheme()])) {
339336
if (!in_array($this->context->getMethod(), array('HEAD', 'GET'))) {
340-
$allow[] = 'GET';
341337
goto not_secure;
342338
}
343339

@@ -353,7 +349,6 @@ public function match($rawPathinfo)
353349
$requiredSchemes = array ( 'http' => 0,);
354350
if (!isset($requiredSchemes[$this->context->getScheme()])) {
355351
if (!in_array($this->context->getMethod(), array('HEAD', 'GET'))) {
356-
$allow[] = 'GET';
357352
goto not_nonsecure;
358353
}
359354

‎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
@@ -464,6 +464,18 @@ public function testNestedCollections()
464464
$this->assertEquals(array('_route' => 'buz'), $matcher->match('/prefix/buz'));
465465
}
466466

467+
/**
468+
* @expectedException \Symfony\Component\Routing\Exception\ResourceNotFoundException
469+
*/
470+
public function testSchemeAndMethodMismatch()
471+
{
472+
$coll = new RouteCollection();
473+
$coll->add('foo', new Route('/', array(), array(), array(), null, array('https'), array('POST')));
474+
475+
$matcher = $this->getUrlMatcher($coll);
476+
$matcher->match('/');
477+
}
478+
467479
protected function getUrlMatcher(RouteCollection $routes, RequestContext $context = null)
468480
{
469481
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)