Skip to content

Commit 369dd6f

Browse files
nicolas-grekasnicolas-grekas
committed
[Routing] fix trailing slash redirections involving a trailing var
1 parent 5b52dd4 commit 369dd6f

18 files changed

+1507
-1431
lines changed

‎Matcher/Dumper/PhpMatcherDumper.php

Lines changed: 128 additions & 83 deletions
Original file line numberDiff line numberDiff line change
@@ -112,7 +112,8 @@ private function generateMatchMethod(): string
112112
$code = <<<EOF
113113
{
114114
\$allow = \$allowSchemes = array();
115-
\$pathinfo = rawurldecode(\$rawPathinfo) ?: '/';
115+
\$pathinfo = rawurldecode(\$pathinfo) ?: '/';
116+
\$trimmedPathinfo = rtrim(\$pathinfo, '/') ?: '/';
116117
\$context = \$this->context;
117118
\$requestMethod = \$canonicalMethod = \$context->getMethod();
118119
{$fetchHost}
@@ -148,8 +149,8 @@ public function match($pathinfo)
148149
} finally {
149150
$this->context->setScheme($scheme);
150151
}
151-
} elseif ('/' !== $pathinfo) {
152-
$pathinfo = '/' !== $pathinfo[-1] ? $pathinfo.'/' : substr($pathinfo, 0, -1);
152+
} elseif ('/' !== $trimmedPathinfo = rtrim($pathinfo, '/') ?: '/') {
153+
$pathinfo = $trimmedPathinfo === $pathinfo ? $pathinfo.'/' : $trimmedPathinfo;
153154
if ($ret = $this->doMatch($pathinfo, $allow, $allowSchemes)) {
154155
return $this->redirect($pathinfo, $ret['_route']) + $ret;
155156
}
@@ -161,13 +162,13 @@ public function match($pathinfo)
161162
throw new ResourceNotFoundException();
162163
}
163164
164-
private function doMatch(string $rawPathinfo, array &$allow = array(), array &$allowSchemes = array()): array
165+
private function doMatch(string $pathinfo, array &$allow = array(), array &$allowSchemes = array()): array
165166

166167
EOF
167168
.$code."\n return array();\n }";
168169
}
169170

170-
return " public function match(\$rawPathinfo)\n".$code."\n throw \$allow ? new MethodNotAllowedException(array_keys(\$allow)) : new ResourceNotFoundException();\n }";
171+
return " public function match(\$pathinfo)\n".$code."\n throw \$allow ? new MethodNotAllowedException(array_keys(\$allow)) : new ResourceNotFoundException();\n }";
171172
}
172173

173174
/**
@@ -304,7 +305,7 @@ private function compileStaticRoutes(array $staticRoutes, bool $matchHost): stri
304305
EOF;
305306
}
306307

307-
return sprintf(" switch (\$trimmedPathinfo = '/' !== \$pathinfo && '/' === \$pathinfo[-1] ? substr(\$pathinfo, 0, -1) : \$pathinfo) {\n%s }\n\n", $this->indent($code));
308+
return sprintf(" switch (\$trimmedPathinfo) {\n%s }\n\n", $this->indent($code));
308309
}
309310

310311
/**
@@ -408,8 +409,9 @@ private function compileDynamicRoutes(RouteCollection $collection, bool $matchHo
408409
if ($hasTrailingSlash = '/' !== $regex && '/' === $regex[-1]) {
409410
$regex = substr($regex, 0, -1);
410411
}
412+
$hasTrailingVar = (bool) preg_match('#\{\w+\}/?$#', $route->getPath());
411413

412-
$tree->addRoute($regex, array($name, $regex, $state->vars, $route, $hasTrailingSlash));
414+
$tree->addRoute($regex, array($name, $regex, $state->vars, $route, $hasTrailingSlash, $hasTrailingVar));
413415
}
414416

415417
$code .= $this->compileStaticPrefixCollection($tree, $state);
@@ -418,7 +420,7 @@ private function compileDynamicRoutes(RouteCollection $collection, bool $matchHo
418420
$code .= "\n .')'";
419421
$state->regex .= ')';
420422
}
421-
$rx = ")(?:/?)$}{$modifiers}";
423+
$rx = ")/?$}{$modifiers}";
422424
$code .= "\n .'{$rx}',";
423425
$state->regex .= $rx;
424426
$state->markTail = 0;
@@ -438,7 +440,7 @@ private function compileDynamicRoutes(RouteCollection $collection, bool $matchHo
438440
\$routes = array(
439441
{$this->indent($state->default, 4)} );
440442
441-
list(\$ret, \$vars, \$requiredMethods, \$requiredSchemes, \$hasTrailingSlash) = \$routes[\$m];
443+
list(\$ret, \$vars, \$requiredMethods, \$requiredSchemes, \$hasTrailingSlash, \$hasTrailingVar) = \$routes[\$m];
442444
{$this->compileSwitchDefault(true, $matchHost)}
443445
EOF;
444446
}
@@ -493,11 +495,11 @@ private function compileStaticPrefixCollection(StaticPrefixCollection $tree, \st
493495
continue;
494496
}
495497

496-
list($name, $regex, $vars, $route, $hasTrailingSlash) = $route;
498+
list($name, $regex, $vars, $route, $hasTrailingSlash, $hasTrailingVar) = $route;
497499
$compiledRoute = $route->compile();
498500

499501
if ($compiledRoute->getRegex() === $prevRegex) {
500-
$state->switch = substr_replace($state->switch, $this->compileRoute($route, $name, false, $hasTrailingSlash)."\n", -19, 0);
502+
$state->switch = substr_replace($state->switch, $this->compileRoute($route, $name, false, $hasTrailingSlash, $hasTrailingVar)."\n", -19, 0);
501503
continue;
502504
}
503505

@@ -516,20 +518,21 @@ private function compileStaticPrefixCollection(StaticPrefixCollection $tree, \st
516518
unset($defaults['_canonical_route']);
517519
}
518520
$state->default .= sprintf(
519-
"%s => array(%s, %s, %s, %s, %s),\n",
521+
"%s => array(%s, %s, %s, %s, %s, %s),\n",
520522
$state->mark,
521523
self::export(array('_route' => $name) + $defaults),
522524
self::export($vars),
523525
self::export(array_flip($route->getMethods()) ?: null),
524526
self::export(array_flip($route->getSchemes()) ?: null),
525-
self::export($hasTrailingSlash)
527+
self::export($hasTrailingSlash),
528+
self::export($hasTrailingVar)
526529
);
527530
} else {
528531
$prevRegex = $compiledRoute->getRegex();
529532

530533
$state->switch .= <<<EOF
531534
case {$state->mark}:
532-
{$this->compileRoute($route, $name, false, $hasTrailingSlash, $vars)}
535+
{$this->compileRoute($route, $name, false, $hasTrailingSlash, $hasTrailingVar, $vars)}
533536
break;
534537
535538
EOF;
@@ -544,67 +547,84 @@ private function compileStaticPrefixCollection(StaticPrefixCollection $tree, \st
544547
*/
545548
private function compileSwitchDefault(bool $hasVars, bool $matchHost): string
546549
{
547-
$code = sprintf("
548-
if ('/' !== \$pathinfo) {%s
549-
if (\$hasTrailingSlash !== ('/' === \$pathinfo[-1])) {%s
550-
break;
550+
if ($this->supportsRedirections) {
551+
$code = <<<'EOF'
552+
553+
if ('GET' === $canonicalMethod && (!$requiredMethods || isset($requiredMethods['GET']))) {
554+
return $allow = $allowSchemes = array();
551555
}
552-
}\n",
553-
$hasVars ? "
554-
if ('/' === \$pathinfo[-1]) {
555-
if (preg_match(\$regex, substr(\$pathinfo, 0, -1), \$n) && \$m === (int) \$n['MARK']) {
556-
\$matches = \$n;
557-
} else {
558-
\$hasTrailingSlash = true;
559-
}
560-
}\n" : '',
561-
$this->supportsRedirections ? "
562-
if ((!\$requiredMethods || isset(\$requiredMethods['GET'])) && 'GET' === \$canonicalMethod) {
563-
return \$allow = \$allowSchemes = array();
564-
}" : ''
556+
EOF;
557+
} else {
558+
$code = '';
559+
}
560+
561+
$code .= $hasVars ? '
562+
if ($trimmedPathinfo === $pathinfo || !$hasTrailingVar) {
563+
break;
564+
}' : '
565+
break;';
566+
567+
$code = sprintf(<<<'EOF'
568+
if ('/' !== $pathinfo && $hasTrailingSlash === ($trimmedPathinfo === $pathinfo)) {%s
569+
}
570+
571+
EOF
572+
,
573+
$code
565574
);
566575

567576
if ($hasVars) {
568-
$code .= <<<EOF
577+
$code = <<<'EOF'
578+
579+
if ($trimmedPathinfo === $pathinfo || !$hasTrailingVar) {
580+
// no-op
581+
} elseif (preg_match($regex, $trimmedPathinfo, $n) && $m === (int) $n['MARK']) {
582+
$matches = $n;
583+
} else {
584+
$hasTrailingSlash = true;
585+
}
569586

570-
foreach (\$vars as \$i => \$v) {
571-
if (isset(\$matches[1 + \$i])) {
572-
\$ret[\$v] = \$matches[1 + \$i];
587+
EOF
588+
.$code.<<<'EOF'
589+
590+
foreach ($vars as $i => $v) {
591+
if (isset($matches[1 + $i])) {
592+
$ret[$v] = $matches[1 + $i];
573593
}
574594
}
575595

576596
EOF;
577597
} elseif ($matchHost) {
578-
$code .= <<<EOF
598+
$code .= <<<'EOF'
579599
580-
if (\$requiredHost) {
581-
if ('#' !== \$requiredHost[0] ? \$requiredHost !== \$host : !preg_match(\$requiredHost, \$host, \$hostMatches)) {
600+
if ($requiredHost) {
601+
if ('#' !== $requiredHost[0] ? $requiredHost !== $host : !preg_match($requiredHost, $host, $hostMatches)) {
582602
break;
583603
}
584-
if ('#' === \$requiredHost[0] && \$hostMatches) {
585-
\$hostMatches['_route'] = \$ret['_route'];
586-
\$ret = \$this->mergeDefaults(\$hostMatches, \$ret);
604+
if ('#' === $requiredHost[0] && $hostMatches) {
605+
$hostMatches['_route'] = $ret['_route'];
606+
$ret = $this->mergeDefaults($hostMatches, $ret);
587607
}
588608
}
589609

590610
EOF;
591611
}
592612

593-
$code .= <<<EOF
613+
$code .= <<<'EOF'
594614
595-
\$hasRequiredScheme = !\$requiredSchemes || isset(\$requiredSchemes[\$context->getScheme()]);
596-
if (\$requiredMethods && !isset(\$requiredMethods[\$canonicalMethod]) && !isset(\$requiredMethods[\$requestMethod])) {
597-
if (\$hasRequiredScheme) {
598-
\$allow += \$requiredMethods;
615+
$hasRequiredScheme = !$requiredSchemes || isset($requiredSchemes[$context->getScheme()]);
616+
if ($requiredMethods && !isset($requiredMethods[$canonicalMethod]) && !isset($requiredMethods[$requestMethod])) {
617+
if ($hasRequiredScheme) {
618+
$allow += $requiredMethods;
599619
}
600620
break;
601621
}
602-
if (!\$hasRequiredScheme) {
603-
\$allowSchemes += \$requiredSchemes;
622+
if (!$hasRequiredScheme) {
623+
$allowSchemes += $requiredSchemes;
604624
break;
605625
}
606626
607-
return \$ret;
627+
return $ret;
608628

609629
EOF;
610630

@@ -616,52 +636,77 @@ private function compileSwitchDefault(bool $hasVars, bool $matchHost): string
616636
*
617637
* @throws \LogicException
618638
*/
619-
private function compileRoute(Route $route, string $name, bool $checkHost, bool $hasTrailingSlash, array $vars = null): string
639+
private function compileRoute(Route $route, string $name, bool $checkHost, bool $hasTrailingSlash, bool $hasTrailingVar = false, array $vars = null): string
620640
{
621641
$compiledRoute = $route->compile();
622642
$conditions = array();
623643
$matches = (bool) $compiledRoute->getPathVariables();
624644
$hostMatches = (bool) $compiledRoute->getHostVariables();
625645
$methods = array_flip($route->getMethods());
626646
$gotoname = 'not_'.preg_replace('/[^A-Za-z0-9_]/', '', $name);
627-
$code = " // $name";
647+
$code = " // $name\n";
628648

629649
if ('/' === $route->getPath()) {
630-
$code .= "\n";
631-
} elseif (!$matches) {
632-
$code .= sprintf("
633-
if ('/' !== \$pathinfo && '/' %s \$pathinfo[-1]) {%s
634-
goto $gotoname;
635-
}\n\n",
636-
$hasTrailingSlash ? '!==' : '===',
637-
$this->supportsRedirections && (!$methods || isset($methods['GET'])) ? "
638-
if ('GET' === \$canonicalMethod) {
639-
return \$allow = \$allowSchemes = array();
640-
}" : ''
650+
// no-op
651+
} elseif (!$hasTrailingVar) {
652+
$code .= sprintf(<<<'EOF'
653+
if ('/' !== $pathinfo && $trimmedPathinfo %s $pathinfo) {%%s
654+
goto %%s;
655+
}
656+
EOF
657+
,
658+
$hasTrailingSlash ? '===' : '!=='
641659
);
642660
} elseif ($hasTrailingSlash) {
643-
$code .= sprintf("
644-
if ('/' !== \$pathinfo[-1]) {%s
645-
goto $gotoname;
646-
}
647-
if ('/' !== \$pathinfo && preg_match(\$regex, substr(\$pathinfo, 0, -1), \$n) && \$m === (int) \$n['MARK']) {
648-
\$matches = \$n;
649-
}\n\n",
650-
$this->supportsRedirections && (!$methods || isset($methods['GET'])) ? "
651-
if ('GET' === \$canonicalMethod) {
652-
return \$allow = \$allowSchemes = array();
653-
}" : ''
654-
);
661+
$code .= <<<'EOF'
662+
if ('/' !== $pathinfo && $trimmedPathinfo === $pathinfo) {%s
663+
goto %s;
664+
}
665+
if ('/' !== $pathinfo && preg_match($regex, $trimmedPathinfo, $n) && $m === (int) $n['MARK']) {
666+
$matches = $n;
667+
}
668+
EOF;
669+
} elseif ($this->supportsRedirections && (!$methods || isset($methods['GET']))) {
670+
$code .= <<<'EOF'
671+
$hasTrailingSlash = false;
672+
if ($trimmedPathinfo === $pathinfo) {
673+
// no-op
674+
} elseif (preg_match($regex, $trimmedPathinfo, $n) && $m === (int) $n['MARK']) {
675+
$matches = $n;
655676
} else {
656-
$code .= sprintf("
657-
if ('/' !== \$pathinfo && '/' === \$pathinfo[-1] && preg_match(\$regex, substr(\$pathinfo, 0, -1), \$n) && \$m === (int) \$n['MARK']) {%s
658-
goto $gotoname;
659-
}\n\n",
660-
$this->supportsRedirections && (!$methods || isset($methods['GET'])) ? "
661-
if ('GET' === \$canonicalMethod) {
662-
return \$allow = \$allowSchemes = array();
663-
}" : ''
664-
);
677+
$hasTrailingSlash = true;
678+
}
679+
680+
if ('/' !== $pathinfo && $hasTrailingSlash === ($trimmedPathinfo === $pathinfo)) {%s
681+
if ($trimmedPathinfo === $pathinfo) {
682+
goto %s;
683+
}
684+
}
685+
EOF;
686+
} else {
687+
$code .= <<<'EOF'
688+
if ($trimmedPathinfo === $pathinfo) {
689+
// no-op
690+
} elseif (preg_match($regex, $trimmedPathinfo, $n) && $m === (int) $n['MARK']) {
691+
$matches = $n;
692+
} elseif ('/' !== $pathinfo) {
693+
goto %2$s;
694+
}
695+
EOF;
696+
}
697+
698+
if ($this->supportsRedirections && (!$methods || isset($methods['GET']))) {
699+
$code = sprintf($code, <<<'EOF'
700+
701+
if ('GET' === $canonicalMethod) {
702+
return $allow = $allowSchemes = array();
703+
}
704+
EOF
705+
,
706+
$gotoname
707+
)."\n\n";
708+
} elseif ('/' !== $route->getPath()) {
709+
$code = sprintf($code, '', $gotoname)."\n\n";
665710
}
666711

667712
if ($vars) {

‎Matcher/RedirectableUrlMatcher.php

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -44,11 +44,11 @@ public function match($pathinfo)
4444
} finally {
4545
$this->context->setScheme($scheme);
4646
}
47-
} elseif ('/' === $pathinfo) {
47+
} elseif ('/' === $trimmedPathinfo = rtrim($pathinfo, '/') ?: '/') {
4848
throw $e;
4949
} else {
5050
try {
51-
$pathinfo = '/' !== $pathinfo[-1] ? $pathinfo.'/' : substr($pathinfo, 0, -1);
51+
$pathinfo = $trimmedPathinfo === $pathinfo ? $pathinfo.'/' : $trimmedPathinfo;
5252
$ret = parent::match($pathinfo);
5353

5454
return $this->redirect($pathinfo, $ret['_route'] ?? null) + $ret;

0 commit comments

Comments
 (0)