Skip to content

Commit 5965917

Browse files
Tobionnicolas-grekas
Tobion
authored and
nicolas-grekas
committed
[Routing] support scheme requirement without redirectable dumped matcher
1 parent f45e489 commit 5965917

21 files changed

+381
-202
lines changed

‎Matcher/Dumper/PhpMatcherDumper.php‎

Lines changed: 37 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,6 @@ class PhpMatcherDumper extends MatcherDumper
2929
{
3030
private $expressionLanguage;
3131
private $signalingException;
32-
private $supportsRedirections;
3332

3433
/**
3534
* @var ExpressionFunctionProviderInterface[]
@@ -57,7 +56,7 @@ public function dump(array $options = array())
5756

5857
// trailing slash support is only enabled if we know how to redirect the user
5958
$interfaces = class_implements($options['base_class']);
60-
$this->supportsRedirections = isset($interfaces[RedirectableUrlMatcherInterface::class]);
59+
$supportsRedirections = isset($interfaces[RedirectableUrlMatcherInterface::class]);
6160

6261
return <<<EOF
6362
<?php
@@ -77,7 +76,7 @@ public function __construct(RequestContext \$context)
7776
\$this->context = \$context;
7877
}
7978
80-
{$this->generateMatchMethod()}
79+
{$this->generateMatchMethod($supportsRedirections)}
8180
}
8281
8382
EOF;
@@ -91,7 +90,7 @@ public function addExpressionLanguageProvider(ExpressionFunctionProviderInterfac
9190
/**
9291
* Generates the code for the match method implementing UrlMatcherInterface.
9392
*/
94-
private function generateMatchMethod(): string
93+
private function generateMatchMethod(bool $supportsRedirections): string
9594
{
9695
// Group hosts by same-suffix, re-order when possible
9796
$matchHost = false;
@@ -111,7 +110,7 @@ private function generateMatchMethod(): string
111110

112111
$code = <<<EOF
113112
{
114-
\$allow = array();
113+
\$allow = \$allowSchemes = array();
115114
\$pathinfo = rawurldecode(\$rawPathinfo);
116115
\$context = \$this->context;
117116
\$requestMethod = \$canonicalMethod = \$context->getMethod();
@@ -124,25 +123,44 @@ private function generateMatchMethod(): string
124123
125124
EOF;
126125

127-
if ($this->supportsRedirections) {
126+
if ($supportsRedirections) {
128127
return <<<'EOF'
129128
public function match($pathinfo)
130129
{
131-
$allow = array();
132-
if ($ret = $this->doMatch($pathinfo, $allow)) {
130+
$allow = $allowSchemes = array();
131+
if ($ret = $this->doMatch($pathinfo, $allow, $allowSchemes)) {
133132
return $ret;
134133
}
135-
if ('/' !== $pathinfo && in_array($this->context->getMethod(), array('HEAD', 'GET'), true)) {
134+
if ($allow) {
135+
throw new MethodNotAllowedException(array_keys($allow));
136+
}
137+
if (!in_array($this->context->getMethod(), array('HEAD', 'GET'), true)) {
138+
// no-op
139+
} elseif ($allowSchemes) {
140+
redirect_scheme:
141+
$scheme = $this->context->getScheme();
142+
$this->context->setScheme(key($allowSchemes));
143+
try {
144+
if ($ret = $this->doMatch($pathinfo)) {
145+
return $this->redirect($pathinfo, $ret['_route'], $this->context->getScheme()) + $ret;
146+
}
147+
} finally {
148+
$this->context->setScheme($scheme);
149+
}
150+
} elseif ('/' !== $pathinfo) {
136151
$pathinfo = '/' !== $pathinfo[-1] ? $pathinfo.'/' : substr($pathinfo, 0, -1);
137-
if ($ret = $this->doMatch($pathinfo)) {
152+
if ($ret = $this->doMatch($pathinfo, $allow, $allowSchemes)) {
138153
return $this->redirect($pathinfo, $ret['_route']) + $ret;
139154
}
155+
if ($allowSchemes) {
156+
goto redirect_scheme;
157+
}
140158
}
141159
142-
throw $allow ? new MethodNotAllowedException(array_keys($allow)) : new ResourceNotFoundException();
160+
throw new ResourceNotFoundException();
143161
}
144162
145-
private function doMatch(string $rawPathinfo, array &$allow = array()): ?array
163+
private function doMatch(string $rawPathinfo, array &$allow = array(), array &$allowSchemes = array()): ?array
146164

147165
EOF
148166
.$code."\n return null;\n }";
@@ -238,9 +256,6 @@ private function compileStaticRoutes(array $staticRoutes, bool $matchHost): stri
238256
}
239257

240258
if (!$route->getCondition()) {
241-
if (!$this->supportsRedirections && $route->getSchemes()) {
242-
throw new \LogicException('The "schemes" requirement is only supported for URL matchers that implement RedirectableUrlMatcherInterface.');
243-
}
244259
$default .= sprintf(
245260
"%s => array(%s, %s, %s, %s),\n",
246261
self::export($url),
@@ -535,8 +550,8 @@ private function compileSwitchDefault(bool $hasVars, bool $matchHost): string
535550
} else {
536551
$code = '';
537552
}
538-
if ($this->supportsRedirections) {
539-
$code .= <<<EOF
553+
554+
$code .= <<<EOF
540555
541556
\$hasRequiredScheme = !\$requiredSchemes || isset(\$requiredSchemes[\$context->getScheme()]);
542557
if (\$requiredMethods && !isset(\$requiredMethods[\$canonicalMethod]) && !isset(\$requiredMethods[\$requestMethod])) {
@@ -546,28 +561,13 @@ private function compileSwitchDefault(bool $hasVars, bool $matchHost): string
546561
break;
547562
}
548563
if (!\$hasRequiredScheme) {
549-
if ('GET' !== \$canonicalMethod) {
550-
break;
551-
}
552-
553-
return \$this->redirect(\$rawPathinfo, \$ret['_route'], key(\$requiredSchemes)) + \$ret;
554-
}
555-
556-
return \$ret;
557-
558-
EOF;
559-
} else {
560-
$code .= <<<EOF
561-
562-
if (\$requiredMethods && !isset(\$requiredMethods[\$canonicalMethod]) && !isset(\$requiredMethods[\$requestMethod])) {
563-
\$allow += \$requiredMethods;
564+
\$allowSchemes += \$requiredSchemes;
564565
break;
565566
}
566567
567568
return \$ret;
568569
569570
EOF;
570-
}
571571

572572
return $code;
573573
}
@@ -647,9 +647,6 @@ private function compileRoute(Route $route, string $name, bool $checkHost): stri
647647
}
648648

649649
if ($schemes = $route->getSchemes()) {
650-
if (!$this->supportsRedirections) {
651-
throw new \LogicException('The "schemes" requirement is only supported for URL matchers that implement RedirectableUrlMatcherInterface.');
652-
}
653650
$schemes = self::export(array_flip($schemes));
654651
if ($methods) {
655652
$code .= <<<EOF
@@ -662,11 +659,8 @@ private function compileRoute(Route $route, string $name, bool $checkHost): stri
662659
goto $gotoname;
663660
}
664661
if (!\$hasRequiredScheme) {
665-
if ('GET' !== \$canonicalMethod) {
666-
goto $gotoname;
667-
}
668-
669-
return \$this->redirect(\$rawPathinfo, '$name', key(\$requiredSchemes)) + \$ret;
662+
\$allowSchemes += \$requiredSchemes;
663+
goto $gotoname;
670664
}
671665
672666
@@ -675,11 +669,8 @@ private function compileRoute(Route $route, string $name, bool $checkHost): stri
675669
$code .= <<<EOF
676670
\$requiredSchemes = $schemes;
677671
if (!isset(\$requiredSchemes[\$context->getScheme()])) {
678-
if ('GET' !== \$canonicalMethod) {
679-
goto $gotoname;
680-
}
681-
682-
return \$this->redirect(\$rawPathinfo, '$name', key(\$requiredSchemes)) + \$ret;
672+
\$allowSchemes += \$requiredSchemes;
673+
goto $gotoname;
683674
}
684675
685676

‎Matcher/RedirectableUrlMatcher.php‎

Lines changed: 27 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -11,8 +11,8 @@
1111

1212
namespace Symfony\Component\Routing\Matcher;
1313

14+
use Symfony\Component\Routing\Exception\ExceptionInterface;
1415
use Symfony\Component\Routing\Exception\ResourceNotFoundException;
15-
use Symfony\Component\Routing\Route;
1616

1717
/**
1818
* @author Fabien Potencier <fabien@symfony.com>
@@ -27,38 +27,38 @@ public function match($pathinfo)
2727
try {
2828
return parent::match($pathinfo);
2929
} catch (ResourceNotFoundException $e) {
30-
if ('/' === $pathinfo || !\in_array($this->context->getMethod(), array('HEAD', 'GET'), true)) {
30+
if (!\in_array($this->context->getMethod(), array('HEAD', 'GET'), true)) {
3131
throw $e;
3232
}
3333

34-
try {
35-
$pathinfo = '/' !== $pathinfo[-1] ? $pathinfo.'/' : substr($pathinfo, 0, -1);
36-
$ret = parent::match($pathinfo);
34+
if ($this->allowSchemes) {
35+
redirect_scheme:
36+
$scheme = $this->context->getScheme();
37+
$this->context->setScheme(current($this->allowSchemes));
38+
try {
39+
$ret = parent::match($pathinfo);
3740

38-
return $this->redirect($pathinfo, $ret['_route'] ?? null) + $ret;
39-
} catch (ResourceNotFoundException $e2) {
41+
return $this->redirect($pathinfo, $ret['_route'] ?? null, $this->context->getScheme()) + $ret;
42+
} catch (ExceptionInterface $e2) {
43+
throw $e;
44+
} finally {
45+
$this->context->setScheme($scheme);
46+
}
47+
} elseif ('/' === $pathinfo) {
4048
throw $e;
41-
}
42-
}
43-
}
49+
} else {
50+
try {
51+
$pathinfo = '/' !== $pathinfo[-1] ? $pathinfo.'/' : substr($pathinfo, 0, -1);
52+
$ret = parent::match($pathinfo);
4453

45-
/**
46-
* {@inheritdoc}
47-
*/
48-
protected function handleRouteRequirements($pathinfo, $name, Route $route)
49-
{
50-
// expression condition
51-
if ($route->getCondition() && !$this->getExpressionLanguage()->evaluate($route->getCondition(), array('context' => $this->context, 'request' => $this->request ?: $this->createRequest($pathinfo)))) {
52-
return array(self::REQUIREMENT_MISMATCH, null);
53-
}
54-
55-
// check HTTP scheme requirement
56-
$scheme = $this->context->getScheme();
57-
$schemes = $route->getSchemes();
58-
if ($schemes && !$route->hasScheme($scheme)) {
59-
return array(self::ROUTE_MATCH, $this->redirect($pathinfo, $name, current($schemes)));
54+
return $this->redirect($pathinfo, $ret['_route'] ?? null) + $ret;
55+
} catch (ExceptionInterface $e2) {
56+
if ($this->allowSchemes) {
57+
goto redirect_scheme;
58+
}
59+
throw $e;
60+
}
61+
}
6062
}
61-
62-
return array(self::REQUIREMENT_MATCH, null);
6363
}
6464
}

‎Matcher/UrlMatcher.php‎

Lines changed: 22 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,19 @@ class UrlMatcher implements UrlMatcherInterface, RequestMatcherInterface
3333
const ROUTE_MATCH = 2;
3434

3535
protected $context;
36+
37+
/**
38+
* Collects HTTP methods that would be allowed for the request.
39+
*/
3640
protected $allow = array();
41+
42+
/**
43+
* Collects URI schemes that would be allowed for the request.
44+
*
45+
* @internal
46+
*/
47+
protected $allowSchemes = array();
48+
3749
protected $routes;
3850
protected $request;
3951
protected $expressionLanguage;
@@ -70,7 +82,7 @@ public function getContext()
7082
*/
7183
public function match($pathinfo)
7284
{
73-
$this->allow = array();
85+
$this->allow = $this->allowSchemes = array();
7486

7587
if ($ret = $this->matchCollection(rawurldecode($pathinfo), $this->routes)) {
7688
return $ret;
@@ -141,22 +153,28 @@ protected function matchCollection($pathinfo, RouteCollection $routes)
141153
continue;
142154
}
143155

144-
// check HTTP method requirement
156+
$hasRequiredScheme = !$route->getSchemes() || $route->hasScheme($this->context->getScheme());
145157
if ($requiredMethods = $route->getMethods()) {
146158
// HEAD and GET are equivalent as per RFC
147159
if ('HEAD' === $method = $this->context->getMethod()) {
148160
$method = 'GET';
149161
}
150162

151163
if (!in_array($method, $requiredMethods)) {
152-
if (self::REQUIREMENT_MATCH === $status[0]) {
164+
if ($hasRequiredScheme) {
153165
$this->allow = array_merge($this->allow, $requiredMethods);
154166
}
155167

156168
continue;
157169
}
158170
}
159171

172+
if (!$hasRequiredScheme) {
173+
$this->allowSchemes = array_merge($this->allowSchemes, $route->getSchemes());
174+
175+
continue;
176+
}
177+
160178
return $this->getAttributes($route, $name, array_replace($matches, $hostMatches, isset($status[1]) ? $status[1] : array()));
161179
}
162180
}
@@ -197,11 +215,7 @@ protected function handleRouteRequirements($pathinfo, $name, Route $route)
197215
return array(self::REQUIREMENT_MISMATCH, null);
198216
}
199217

200-
// check HTTP scheme requirement
201-
$scheme = $this->context->getScheme();
202-
$status = $route->getSchemes() && !$route->hasScheme($scheme) ? self::REQUIREMENT_MISMATCH : self::REQUIREMENT_MATCH;
203-
204-
return array($status, null);
218+
return array(self::REQUIREMENT_MATCH, null);
205219
}
206220

207221
/**

‎Tests/Fixtures/dumper/url_matcher0.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ public function __construct(RequestContext $context)
1717

1818
public function match($rawPathinfo)
1919
{
20-
$allow = array();
20+
$allow = $allowSchemes = array();
2121
$pathinfo = rawurldecode($rawPathinfo);
2222
$context = $this->context;
2323
$requestMethod = $canonicalMethod = $context->getMethod();

‎Tests/Fixtures/dumper/url_matcher1.php‎

Lines changed: 17 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ public function __construct(RequestContext $context)
1717

1818
public function match($rawPathinfo)
1919
{
20-
$allow = array();
20+
$allow = $allowSchemes = array();
2121
$pathinfo = rawurldecode($rawPathinfo);
2222
$context = $this->context;
2323
$requestMethod = $canonicalMethod = $context->getMethod();
@@ -64,8 +64,15 @@ public function match($rawPathinfo)
6464
}
6565
}
6666

67+
$hasRequiredScheme = !$requiredSchemes || isset($requiredSchemes[$context->getScheme()]);
6768
if ($requiredMethods && !isset($requiredMethods[$canonicalMethod]) && !isset($requiredMethods[$requestMethod])) {
68-
$allow += $requiredMethods;
69+
if ($hasRequiredScheme) {
70+
$allow += $requiredMethods;
71+
}
72+
break;
73+
}
74+
if (!$hasRequiredScheme) {
75+
$allowSchemes += $requiredSchemes;
6976
break;
7077
}
7178

@@ -209,8 +216,15 @@ public function match($rawPathinfo)
209216
}
210217
}
211218

219+
$hasRequiredScheme = !$requiredSchemes || isset($requiredSchemes[$context->getScheme()]);
212220
if ($requiredMethods && !isset($requiredMethods[$canonicalMethod]) && !isset($requiredMethods[$requestMethod])) {
213-
$allow += $requiredMethods;
221+
if ($hasRequiredScheme) {
222+
$allow += $requiredMethods;
223+
}
224+
break;
225+
}
226+
if (!$hasRequiredScheme) {
227+
$allowSchemes += $requiredSchemes;
214228
break;
215229
}
216230

0 commit comments

Comments
 (0)