Skip to content

Commit 1b04d20

Browse files
author
Robin Chalas
committed
Merge branch '4.2'
* 4.2: Fix url matcher edge cases with trailing slash [Form] Fix author tag + exception messages [TwigBridge] Fix deprecation on twig 2.9 Fix left-associative ternary deprecation warnings for PHP 7.4 [Validator] Fixed imprecise translations [Validator] Add Dutch translations [Security] Cleanup "Digest nonce has expired." translation Intercept redirections only for HTML format [PhpUnitBridge] fix reading phpunit.xml on bootstrap resolve class name parameters Fix name and phpdoc of ContainerBuilder::removeBindings [Intl] Update the ICU data to 64.2
2 parents 665c743 + f4e43bb commit 1b04d20

File tree

4 files changed

+170
-20
lines changed

4 files changed

+170
-20
lines changed

‎Matcher/Dumper/CompiledUrlMatcherDumper.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -222,7 +222,7 @@ private function compileStaticRoutes(array $staticRoutes, array &$conditions): a
222222
foreach ($staticRoutes as $url => $routes) {
223223
$compiledRoutes[$url] = [];
224224
foreach ($routes as $name => list($route, $hasTrailingSlash)) {
225-
$compiledRoutes[$url][] = $this->compileRoute($route, $name, !$route->compile()->getHostVariables() ? $route->getHost() : $route->compile()->getHostRegex() ?: null, $hasTrailingSlash, false, $conditions);
225+
$compiledRoutes[$url][] = $this->compileRoute($route, $name, (!$route->compile()->getHostVariables() ? $route->getHost() : $route->compile()->getHostRegex()) ?: null, $hasTrailingSlash, false, $conditions);
226226
}
227227
}
228228

‎Matcher/Dumper/CompiledUrlMatcherTrait.php

Lines changed: 11 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -15,11 +15,14 @@
1515
use Symfony\Component\Routing\Exception\NoConfigurationException;
1616
use Symfony\Component\Routing\Exception\ResourceNotFoundException;
1717
use Symfony\Component\Routing\Matcher\RedirectableUrlMatcherInterface;
18+
use Symfony\Component\Routing\RequestContext;
1819

1920
/**
2021
* @author Nicolas Grekas <[email protected]>
2122
*
2223
* @internal
24+
*
25+
* @property RequestContext $context
2326
*/
2427
trait CompiledUrlMatcherTrait
2528
{
@@ -89,13 +92,6 @@ private function doMatch(string $pathinfo, array &$allow = [], array &$allowSche
8992
continue;
9093
}
9194

92-
if ('/' !== $pathinfo && $hasTrailingSlash === ($trimmedPathinfo === $pathinfo)) {
93-
if ($supportsRedirections && (!$requiredMethods || isset($requiredMethods['GET']))) {
94-
return $allow = $allowSchemes = [];
95-
}
96-
continue;
97-
}
98-
9995
if ($requiredHost) {
10096
if ('#' !== $requiredHost[0] ? $requiredHost !== $host : !preg_match($requiredHost, $host, $hostMatches)) {
10197
continue;
@@ -106,13 +102,21 @@ private function doMatch(string $pathinfo, array &$allow = [], array &$allowSche
106102
}
107103
}
108104

105+
if ('/' !== $pathinfo && $hasTrailingSlash === ($trimmedPathinfo === $pathinfo)) {
106+
if ($supportsRedirections && (!$requiredMethods || isset($requiredMethods['GET']))) {
107+
return $allow = $allowSchemes = [];
108+
}
109+
continue;
110+
}
111+
109112
$hasRequiredScheme = !$requiredSchemes || isset($requiredSchemes[$context->getScheme()]);
110113
if ($requiredMethods && !isset($requiredMethods[$canonicalMethod]) && !isset($requiredMethods[$requestMethod])) {
111114
if ($hasRequiredScheme) {
112115
$allow += $requiredMethods;
113116
}
114117
continue;
115118
}
119+
116120
if (!$hasRequiredScheme) {
117121
$allowSchemes += $requiredSchemes;
118122
continue;

‎Matcher/UrlMatcher.php

Lines changed: 9 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@ class UrlMatcher implements UrlMatcherInterface, RequestMatcherInterface
3232
const REQUIREMENT_MISMATCH = 1;
3333
const ROUTE_MATCH = 2;
3434

35+
/** @var RequestContext */
3536
protected $context;
3637

3738
/**
@@ -166,14 +167,6 @@ protected function matchCollection($pathinfo, RouteCollection $routes)
166167
}
167168
}
168169

169-
if ('/' !== $pathinfo && !$hasTrailingVar && $hasTrailingSlash === ($trimmedPathinfo === $pathinfo)) {
170-
if ($supportsTrailingSlash && (!$requiredMethods || \in_array('GET', $requiredMethods))) {
171-
return $this->allow = $this->allowSchemes = [];
172-
}
173-
174-
continue;
175-
}
176-
177170
$hostMatches = [];
178171
if ($compiledRoute->getHostRegex() && !preg_match($compiledRoute->getHostRegex(), $this->context->getHost(), $hostMatches)) {
179172
continue;
@@ -185,6 +178,14 @@ protected function matchCollection($pathinfo, RouteCollection $routes)
185178
continue;
186179
}
187180

181+
if ('/' !== $pathinfo && !$hasTrailingVar && $hasTrailingSlash === ($trimmedPathinfo === $pathinfo)) {
182+
if ($supportsTrailingSlash && (!$requiredMethods || \in_array('GET', $requiredMethods))) {
183+
return $this->allow = $this->allowSchemes = [];
184+
}
185+
186+
continue;
187+
}
188+
188189
$hasRequiredScheme = !$route->getSchemes() || $route->hasScheme($this->context->getScheme());
189190
if ($requiredMethods) {
190191
if (!\in_array($method, $requiredMethods)) {

‎Tests/Matcher/UrlMatcherTest.php

Lines changed: 149 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -85,9 +85,8 @@ public function testMethodNotAllowedAggregatesAllowedMethods()
8585
}
8686
}
8787

88-
public function testMatch()
88+
public function testPatternMatchAndParameterReturn()
8989
{
90-
// test the patterns are matched and parameters are returned
9190
$collection = new RouteCollection();
9291
$collection->add('foo', new Route('/foo/{bar}'));
9392
$matcher = $this->getUrlMatcher($collection);
@@ -96,14 +95,21 @@ public function testMatch()
9695
$this->fail();
9796
} catch (ResourceNotFoundException $e) {
9897
}
98+
9999
$this->assertEquals(['_route' => 'foo', 'bar' => 'baz'], $matcher->match('/foo/baz'));
100+
}
100101

102+
public function testDefaultsAreMerged()
103+
{
101104
// test that defaults are merged
102105
$collection = new RouteCollection();
103106
$collection->add('foo', new Route('/foo/{bar}', ['def' => 'test']));
104107
$matcher = $this->getUrlMatcher($collection);
105108
$this->assertEquals(['_route' => 'foo', 'bar' => 'baz', 'def' => 'test'], $matcher->match('/foo/baz'));
109+
}
106110

111+
public function testMethodIsIgnoredIfNoMethodGiven()
112+
{
107113
// test that route "method" is ignored if no method is given in the context
108114
$collection = new RouteCollection();
109115
$collection->add('foo', new Route('/foo', [], [], [], '', [], ['get', 'head']));
@@ -123,8 +129,10 @@ public function testMatch()
123129
$this->assertInternalType('array', $matcher->match('/foo'));
124130
$matcher = $this->getUrlMatcher($collection, new RequestContext('', 'head'));
125131
$this->assertInternalType('array', $matcher->match('/foo'));
132+
}
126133

127-
// route with an optional variable as the first segment
134+
public function testRouteWithOptionalVariableAsFirstSegment()
135+
{
128136
$collection = new RouteCollection();
129137
$collection->add('bar', new Route('/{bar}/foo', ['bar' => 'bar'], ['bar' => 'foo|bar']));
130138
$matcher = $this->getUrlMatcher($collection);
@@ -136,8 +144,10 @@ public function testMatch()
136144
$matcher = $this->getUrlMatcher($collection);
137145
$this->assertEquals(['_route' => 'bar', 'bar' => 'foo'], $matcher->match('/foo'));
138146
$this->assertEquals(['_route' => 'bar', 'bar' => 'bar'], $matcher->match('/'));
147+
}
139148

140-
// route with only optional variables
149+
public function testRouteWithOnlyOptionalVariables()
150+
{
141151
$collection = new RouteCollection();
142152
$collection->add('bar', new Route('/{foo}/{bar}', ['foo' => 'foo', 'bar' => 'bar'], []));
143153
$matcher = $this->getUrlMatcher($collection);
@@ -532,6 +542,141 @@ public function testWithHostOnRouteCollection()
532542
$this->assertEquals(['foo' => 'bar', '_route' => 'bar', 'locale' => 'en'], $matcher->match('/bar/bar'));
533543
}
534544

545+
public function testVariationInTrailingSlashWithHosts()
546+
{
547+
$coll = new RouteCollection();
548+
$coll->add('foo', new Route('/foo/', [], [], [], 'foo.example.com'));
549+
$coll->add('bar', new Route('/foo', [], [], [], 'bar.example.com'));
550+
551+
$matcher = $this->getUrlMatcher($coll, new RequestContext('', 'GET', 'foo.example.com'));
552+
$this->assertEquals(['_route' => 'foo'], $matcher->match('/foo/'));
553+
554+
$matcher = $this->getUrlMatcher($coll, new RequestContext('', 'GET', 'bar.example.com'));
555+
$this->assertEquals(['_route' => 'bar'], $matcher->match('/foo'));
556+
}
557+
558+
public function testVariationInTrailingSlashWithHostsInReverse()
559+
{
560+
// The order should not matter
561+
$coll = new RouteCollection();
562+
$coll->add('bar', new Route('/foo', [], [], [], 'bar.example.com'));
563+
$coll->add('foo', new Route('/foo/', [], [], [], 'foo.example.com'));
564+
565+
$matcher = $this->getUrlMatcher($coll, new RequestContext('', 'GET', 'foo.example.com'));
566+
$this->assertEquals(['_route' => 'foo'], $matcher->match('/foo/'));
567+
568+
$matcher = $this->getUrlMatcher($coll, new RequestContext('', 'GET', 'bar.example.com'));
569+
$this->assertEquals(['_route' => 'bar'], $matcher->match('/foo'));
570+
}
571+
572+
public function testVariationInTrailingSlashWithHostsAndVariable()
573+
{
574+
$coll = new RouteCollection();
575+
$coll->add('foo', new Route('/{foo}/', [], [], [], 'foo.example.com'));
576+
$coll->add('bar', new Route('/{foo}', [], [], [], 'bar.example.com'));
577+
578+
$matcher = $this->getUrlMatcher($coll, new RequestContext('', 'GET', 'foo.example.com'));
579+
$this->assertEquals(['foo' => 'bar', '_route' => 'foo'], $matcher->match('/bar/'));
580+
581+
$matcher = $this->getUrlMatcher($coll, new RequestContext('', 'GET', 'bar.example.com'));
582+
$this->assertEquals(['foo' => 'bar', '_route' => 'bar'], $matcher->match('/bar'));
583+
}
584+
585+
public function testVariationInTrailingSlashWithHostsAndVariableInReverse()
586+
{
587+
// The order should not matter
588+
$coll = new RouteCollection();
589+
$coll->add('bar', new Route('/{foo}', [], [], [], 'bar.example.com'));
590+
$coll->add('foo', new Route('/{foo}/', [], [], [], 'foo.example.com'));
591+
592+
$matcher = $this->getUrlMatcher($coll, new RequestContext('', 'GET', 'foo.example.com'));
593+
$this->assertEquals(['foo' => 'bar', '_route' => 'foo'], $matcher->match('/bar/'));
594+
595+
$matcher = $this->getUrlMatcher($coll, new RequestContext('', 'GET', 'bar.example.com'));
596+
$this->assertEquals(['foo' => 'bar', '_route' => 'bar'], $matcher->match('/bar'));
597+
}
598+
599+
public function testVariationInTrailingSlashWithMethods()
600+
{
601+
$coll = new RouteCollection();
602+
$coll->add('foo', new Route('/foo/', [], [], [], '', [], ['POST']));
603+
$coll->add('bar', new Route('/foo', [], [], [], '', [], ['GET']));
604+
605+
$matcher = $this->getUrlMatcher($coll, new RequestContext('', 'POST'));
606+
$this->assertEquals(['_route' => 'foo'], $matcher->match('/foo/'));
607+
608+
$matcher = $this->getUrlMatcher($coll, new RequestContext('', 'GET'));
609+
$this->assertEquals(['_route' => 'bar'], $matcher->match('/foo'));
610+
}
611+
612+
public function testVariationInTrailingSlashWithMethodsInReverse()
613+
{
614+
// The order should not matter
615+
$coll = new RouteCollection();
616+
$coll->add('bar', new Route('/foo', [], [], [], '', [], ['GET']));
617+
$coll->add('foo', new Route('/foo/', [], [], [], '', [], ['POST']));
618+
619+
$matcher = $this->getUrlMatcher($coll, new RequestContext('', 'POST'));
620+
$this->assertEquals(['_route' => 'foo'], $matcher->match('/foo/'));
621+
622+
$matcher = $this->getUrlMatcher($coll, new RequestContext('', 'GET'));
623+
$this->assertEquals(['_route' => 'bar'], $matcher->match('/foo'));
624+
}
625+
626+
public function testVariableVariationInTrailingSlashWithMethods()
627+
{
628+
$coll = new RouteCollection();
629+
$coll->add('foo', new Route('/{foo}/', [], [], [], '', [], ['POST']));
630+
$coll->add('bar', new Route('/{foo}', [], [], [], '', [], ['GET']));
631+
632+
$matcher = $this->getUrlMatcher($coll, new RequestContext('', 'POST'));
633+
$this->assertEquals(['foo' => 'bar', '_route' => 'foo'], $matcher->match('/bar/'));
634+
635+
$matcher = $this->getUrlMatcher($coll, new RequestContext('', 'GET'));
636+
$this->assertEquals(['foo' => 'bar', '_route' => 'bar'], $matcher->match('/bar'));
637+
}
638+
639+
public function testVariableVariationInTrailingSlashWithMethodsInReverse()
640+
{
641+
// The order should not matter
642+
$coll = new RouteCollection();
643+
$coll->add('bar', new Route('/{foo}', [], [], [], '', [], ['GET']));
644+
$coll->add('foo', new Route('/{foo}/', [], [], [], '', [], ['POST']));
645+
646+
$matcher = $this->getUrlMatcher($coll, new RequestContext('', 'POST'));
647+
$this->assertEquals(['foo' => 'bar', '_route' => 'foo'], $matcher->match('/bar/'));
648+
649+
$matcher = $this->getUrlMatcher($coll, new RequestContext('', 'GET'));
650+
$this->assertEquals(['foo' => 'bar', '_route' => 'bar'], $matcher->match('/bar'));
651+
}
652+
653+
public function testMixOfStaticAndVariableVariationInTrailingSlashWithHosts()
654+
{
655+
$coll = new RouteCollection();
656+
$coll->add('foo', new Route('/foo/', [], [], [], 'foo.example.com'));
657+
$coll->add('bar', new Route('/{foo}', [], [], [], 'bar.example.com'));
658+
659+
$matcher = $this->getUrlMatcher($coll, new RequestContext('', 'GET', 'foo.example.com'));
660+
$this->assertEquals(['_route' => 'foo'], $matcher->match('/foo/'));
661+
662+
$matcher = $this->getUrlMatcher($coll, new RequestContext('', 'GET', 'bar.example.com'));
663+
$this->assertEquals(['foo' => 'bar', '_route' => 'bar'], $matcher->match('/bar'));
664+
}
665+
666+
public function testMixOfStaticAndVariableVariationInTrailingSlashWithMethods()
667+
{
668+
$coll = new RouteCollection();
669+
$coll->add('foo', new Route('/foo/', [], [], [], '', [], ['POST']));
670+
$coll->add('bar', new Route('/{foo}', [], [], [], '', [], ['GET']));
671+
672+
$matcher = $this->getUrlMatcher($coll, new RequestContext('', 'POST'));
673+
$this->assertEquals(['_route' => 'foo'], $matcher->match('/foo/'));
674+
675+
$matcher = $this->getUrlMatcher($coll, new RequestContext('', 'GET'));
676+
$this->assertEquals(['foo' => 'bar', '_route' => 'bar'], $matcher->match('/bar'));
677+
$this->assertEquals(['foo' => 'foo', '_route' => 'bar'], $matcher->match('/foo'));
678+
}
679+
535680
/**
536681
* @expectedException \Symfony\Component\Routing\Exception\ResourceNotFoundException
537682
*/

0 commit comments

Comments
 (0)