8000 [Routing] fix greediness of trailing slash · symfony/symfony@eb6aee6 · GitHub
[go: up one dir, main page]
Skip to content

Commit eb6aee6

Browse files
nicolas-grekasnicolas-grekas
committed
[Routing] fix greediness of trailing slash
1 parent bce7748 commit eb6aee6

File tree

15 files changed

+241
-106
lines changed
  • url_matcher2.php
  • url_matcher3.php
  • url_matcher4.php
  • url_matcher5.php
  • url_matcher6.php
  • url_matcher7.php
  • url_matcher8.php
  • Matcher

    • 15 files changed

    +241
    -106
    lines changed

    ‎src/Symfony/Component/Routing/Matcher/Dumper/PhpMatcherDumper.php

    Lines changed: 41 additions & 18 deletions
    Original file line numberDiff line numberDiff line change
    @@ -550,16 +550,23 @@ private function compileStaticPrefixCollection(StaticPrefixCollection $tree, \st
    550550
    private function compileSwitchDefault(bool $hasVars, bool $matchHost): string
    551551
    {
    552552
    $code = sprintf("
    553-
    if ('/' !== \$pathinfo) {
    554-
    if (!\$hasTrailingSlash && '/' === \$pathinfo[-1]%s) {
    555-
    %s;
    556-
    }
    557-
    if (\$hasTrailingSlash && '/' !== \$pathinfo[-1]) {
    558-
    %2\$s;
    553+
    if ('/' !== \$pathinfo) {%s
    554+
    if (\$hasTrailingSlash !== ('/' === \$pathinfo[-1])) {%s
    555+
    break;
    559556
    }
    560557
    }\n",
    561-
    $hasVars ? ' && preg_match($regex, substr($pathinfo, 0, -1), $n) && $m === (int) $n[\'MARK\']' : '',
    562-
    $this->supportsRedirections ? 'return null' : 'break'
    558+
    $hasVars ? "
    559+
    if ('/' === \$pathinfo[-1]) {
    560+
    if (preg_match(\$regex, substr(\$pathinfo, 0, -1), \$n) && \$m === (int) \$n['MARK']) {
    561+
    \$matches = \$n;
    562+
    } else {
    563+
    \$hasTrailingSlash = true;
    564+
    }
    565+
    }\n" : '',
    566+
    $this->supportsRedirections ? "
    567+
    if (!\$requiredMethods || isset(\$requiredMethods['GET'])) {
    568+
    return null;
    569+
    }" : ''
    563570
    );
    564571

    565572
    if ($hasVars) {
    @@ -616,26 +623,42 @@ private function compileSwitchDefault(bool $hasVars, bool $matchHost): string
    616623
    */
    617624
    private function compileRoute(Route $route, string $name, bool $checkHost, bool $hasTrailingSlash): string
    618625
    {
    626+
    $compiledRoute = $route->compile();
    627+
    $conditions = array();
    628+
    $matches = (bool) $compiledRoute->getPathVariables();
    629+
    $hostMatches = (bool) $compiledRoute->getHostVariables();
    630+
    $methods = array_flip($route->getMethods());
    619631
    $code = " // $name";
    620632

    621-
    if ('/' !== $route->getPath()) {
    633+
    if ('/' === $route->getPath()) {
    634+
    $code .= "\n";
    635+
    } elseif (!$matches) {
    622636
    $code .= sprintf("
    623637
    if ('/' !== \$pathinfo && '/' %s \$pathinfo[-1]) {
    624638
    %s;
    625-
    }\n",
    639+
    }\n\n",
    626640
    $hasTrailingSlash ? '!==' : '===',
    627-
    $this->supportsRedirections ? 'return null' : 'break'
    641+
    $this->supportsRedirections && (!$methods || isset($methods['GET'])) ? 'return null' : 'break'
    642+
    );
    643+
    } elseif ($hasTrailingSlash) {
    644+
    $code .= sprintf("
    645+
    if ('/' !== \$pathinfo[-1]) {
    646+
    %s;
    647+
    }
    648+
    if ('/' !== \$pathinfo && preg_match(\$regex, substr(\$pathinfo, 0, -1), \$n) && \$m === (int) \$n['MARK']) {
    649+
    \$matches = \$n;
    650+
    }\n\n",
    651+
    $this->supportsRedirections && (!$methods || isset($methods['GET'])) ? 'return null' : 'break'
    628652
    );
    629653
    } else {
    630-
    $code .= "\n";
    654+
    $code .= sprintf("
    655+
    if ('/' !== \$pathinfo && '/' === \$pathinfo[-1] && preg_match(\$regex, substr(\$pathinfo, 0, -1), \$n) && \$m === (int) \$n['MARK']) {
    656+
    %s;
    657+
    }\n\n",
    658+
    $this->supportsRedirections && (!$methods || isset($methods['GET'])) ? 'return null' : 'break'
    659+
    );
    631660
    }
    632661

    633-
    $compiledRoute = $route->compile();
    634-
    $conditions = array();
    635-
    $matches = (bool) $compiledRoute->getPathVariables();
    636-
    $hostMatches = (bool) $compiledRoute->getHostVariables();
    637-
    $methods = array_flip($route->getMethods());
    638-
    639662
    if ($route->getCondition()) {
    640663
    $expression = $this->getExpressionLanguage()->compile($route->getCondition(), array('context', 'request'));
    641664

    ‎src/Symfony/Component/Routing/Matcher/UrlMatcher.php

    Lines changed: 14 additions & 6 deletions
    Original file line numberDiff line numberDiff line change
    @@ -135,11 +135,12 @@ protected function matchCollection($pathinfo, RouteCollection $routes)
    135135
    foreach ($routes as $name => $route) {
    136136
    $compiledRoute = $route->compile();
    137137
    $staticPrefix = $compiledRoute->getStaticPrefix();
    138+
    $requiredMethods = $route->getMethods();
    138139

    139140
    // check the static prefix of the URL first. Only use the more expensive preg_match when it matches
    140141
    if ('' === $staticPrefix || 0 === strpos($pathinfo, $staticPrefix)) {
    141142
    // no-op
    142-
    } elseif (!$supportsTrailingSlash) {
    143+
    } elseif (!$supportsTrailingSlash || ($requiredMethods && !\in_array('GET', $requiredMethods))) {
    143144
    continue;
    144145
    } elseif ('/' === $staticPrefix[-1] && substr($staticPrefix, 0, -1) === $pathinfo) {
    145146
    return;
    @@ -161,11 +162,18 @@ protected function matchCollection($pathinfo, RouteCollection $routes)
    161162
    }
    162163

    163164
    if ($supportsTrailingSlash) {
    164-
    if (!$hasTrailingSlash && '/' === $pathinfo[-1] && preg_match($regex, substr($pathinfo, 0, -1))) {
    165-
    return;
    165+
    if ('/' === $pathinfo[-1]) {
    166+
    if (preg_match($regex, substr($pathinfo, 0, -1), $m)) {
    167+
    $matches = $m;
    168+
    } else {
    169+
    $hasTrailingSlash = true;
    170+
    }
    166171
    }
    167-
    if ($hasTrailingSlash && '/' !== $pathinfo[-1]) {
    168-
    return;
    172+
    if ($hasTrailingSlash !== ('/' === $pathinfo[-1])) {
    173+
    if (!$requiredMethods || \in_array('GET', $requiredMethods)) {
    174+
    return;
    175+
    }
    176+
    continue;
    169177
    }
    170178
    }
    171179

    @@ -181,7 +189,7 @@ protected function matchCollection($pathinfo, RouteCollection $routes)
    181189
    }
    182190

    183191
    $hasRequiredScheme = !$route->getSchemes() || $route->hasScheme($this->context->getScheme());
    184-
    if ($requiredMethods = $route->getMethods()) {
    192+
    if ($requiredMethods) {
    185193
    // HEAD and GET are equivalent as per RFC
    186194
    if ('HEAD' === $method = $this->context->getMethod()) {
    187195
    $method = 'GET';

    ‎src/Symfony/Component/Routing/Tests/Fixtures/dumper/url_matcher1.php

    Lines changed: 30 additions & 13 deletions
    Original file line numberDiff line numberDiff line change
    @@ -55,10 +55,7 @@ public function match($rawPathinfo)
    5555
    list($ret, $requiredHost, $requiredMethods, $requiredSchemes, $hasTrailingSlash) = $routes[$trimmedPathinfo];
    5656

    5757
    if ('/' !== $pathinfo) {
    58-
    if (!$hasTrailingSlash && '/' === $pathinfo[-1]) {
    59-
    break;
    60-
    }
    61-
    if ($hasTrailingSlash && '/' !== $pathinfo[-1]) {
    58+
    if ($hasTrailingSlash !== ('/' === $pathinfo[-1])) {
    6259
    break;
    6360
    }
    6461
    }
    @@ -145,15 +142,23 @@ public function match($rawPathinfo)
    145142
    $matches = array('foo' => $matches[1] ?? null);
    146143

    147144
    // baz4
    148-
    if ('/' !== $pathinfo && '/' !== $pathinfo[-1]) {
    145+
    if ('/' !== $pathinfo[-1]) {
    149146
    break;
    150147
    }
    148+
    if ('/' !== $pathinfo && preg_match($regex, substr($pathinfo, 0, -1), $n) && $m === (int) $n['MARK']) {
    149+
    $matches = $n;
    150+
    }
    151+
    151152
    return $this->mergeDefaults(array('_route' => 'baz4') + $matches, array());
    152153

    153154
    // baz5
    154-
    if ('/' !== $pathinfo && '/' !== $pathinfo[-1]) {
    155+
    if ('/' !== $pathinfo[-1]) {
    155156
    break;
    156157
    }
    158+
    if ('/' !== $pathinfo && preg_match($regex, substr($pathinfo, 0, -1), $n) && $m === (int) $n['MARK']) {
    159+
    $matches = $n;
    160+
    }
    161+
    157162
    $ret = $this->mergeDefaults(array('_route' => 'baz5') + $matches, array());
    158163
    if (!isset(($a = array('POST' => 0))[$requestMethod])) {
    159164
    $allow += $a;
    @@ -164,9 +169,13 @@ public function match($rawPathinfo)
    164169
    not_baz5:
    165170

    166171
    // baz.baz6
    167-
    if ('/' !== $pathinfo && '/' !== $pathinfo[-1]) {
    172+
    if ('/' !== $pathinfo[-1]) {
    168173
    break;
    169174
    }
    175+
    if ('/' !== $pathinfo && preg_match($regex, substr($pathinfo, 0, -1), $n) && $m === (int) $n['MARK']) {
    176+
    $matches = $n;
    177+
    }
    178+
    170179
    $ret = $this->mergeDefaults(array('_route' => 'baz.baz6') + $matches, array());
    171180
    if (!isset(($a = array('PUT' => 0))[$requestMethod])) {
    172181
    $allow += $a;
    @@ -181,9 +190,10 @@ public function match($rawPathinfo)
    181190
    $matches = array('foo' => $matches[1] ?? null);
    182191

    183192
    // foo1
    184-
    if ('/' !== $pathinfo && '/' === $pathinfo[-1]) {
    193+
    if ('/' !== $pathinfo && '/' === $pathinfo[-1] && preg_match($regex, substr($pathinfo, 0, -1), $n) && $m === (int) $n['MARK']) {
    185194
    break;
    186195
    }
    196+
    187197
    $ret = $this->mergeDefaults(array('_route' => 'foo1') + $matches, array());
    188198
    if (!isset(($a = array('PUT' => 0))[$requestMethod])) {
    189199
    $allow += $a;
    @@ -198,19 +208,21 @@ public function match($rawPathinfo)
    198208
    $matches = array('foo1' => $matches[1] ?? null);
    199209

    200210
    // foo2
    201-
    if ('/' !== $pathinfo && '/' === $pathinfo[-1]) {
    211+
    if ('/' !== $pathinfo && '/' === $pathinfo[-1] && preg_match($regex, substr($pathinfo, 0, -1), $n) && $m === (int) $n['MARK']) {
    202212
    break;
    203213
    }
    214+
    204215
    return $this->mergeDefaults(array('_route' => 'foo2') + $matches, array());
    205216

    206217
    break;
    207218
    case 279:
    208219
    $matches = array('_locale' => $matches[1] ?? null, 'foo' => $matches[2] ?? null);
    209220

    210221
    // foo3
    211-
    if ('/' !== $pathinfo && '/' === $pathinfo[-1]) {
    222+
    if ('/' !== $pathinfo && '/' === $pathinfo[-1] && preg_match($regex, substr($pathinfo, 0, -1), $n) && $m === (int) $n['MARK']) {
    212223
    break;
    213224
    }
    225+
    214226
    return $this->mergeDefaults(array('_route' => 'foo3') + $matches, array());
    215227

    216228
    break;
    @@ -238,10 +250,15 @@ public function match($rawPathinfo)
    238250
    list($ret, $vars, $requiredMethods, $requiredSchemes, $hasTrailingSlash) = $routes[$m];
    239251

    240252
    if ('/' !== $pathinfo) {
    241-
    if (!$hasTrailingSlash && '/' === $pathinfo[-1] && preg_match($regex, substr($pathinfo, 0, -1), $n) && $m === (int) $n['MARK']) {
    242-
    break;
    253+
    if ('/' === $pathinfo[-1]) {
    254+
    if (preg_match($regex, substr($pathinfo, 0, -1), $n) && $m === (int) $n['MARK']) {
    255+
    $matches = $n;
    256+
    } else {
    257+
    $hasTrailingSlash = true;
    258+
    }
    243259
    }
    244-
    if ($hasTrailingSlash && '/' !== $pathinfo[-1]) {
    260+
    261+
    if ($hasTrailingSlash !== ('/' === $pathinfo[-1])) {
    245262
    break;
    246263
    }
    247264
    }

    ‎src/Symfony/Component/Routing/Tests/Fixtures/dumper/url_matcher10.php

    Lines changed: 8 additions & 3 deletions
    Original file line numberDiff line numberDiff line change
    @@ -2794,10 +2794,15 @@ public function match($rawPathinfo)
    27942794
    list($ret, $vars, $requiredMethods, $requiredSchemes, $hasTrailingSlash) = $routes[$m];
    27952795

    27962796
    if ('/' !== $pathinfo) {
    2797-
    if (!$hasTrailingSlash && '/' === $pathinfo[-1] && preg_match($regex, substr($pathinfo, 0, -1), $n) && $m === (int) $n['MARK']) {
    2798-
    break;
    2797+
    if ('/' === $pathinfo[-1]) {
    2798+
    if (preg_match($regex, substr($pathinfo, 0, -1), $n) && $m === (int) $n['MARK']) {
    2799+
    $matches = $n;
    2800+
    } else {
    2801+
    $hasTrailingSlash = true;
    2802+
    }
    27992803
    }
    2800-
    if ($hasTrailingSlash && '/' !== $pathinfo[-1]) {
    2804+
    2805+
    if ($hasTrailingSlash !== ('/' === $pathinfo[-1])) {
    28012806
    break;
    28022807
    }
    28032808
    }

    ‎src/Symfony/Component/Routing/Tests/Fixtures/dumper/url_matcher11.php

    Lines changed: 12 additions & 4 deletions
    Original file line numberDiff line numberDiff line change
    @@ -119,11 +119,19 @@ private function doMatch(string $rawPathinfo, array &$allow = array(), array &$a
    119119
    list($ret, $vars, $requiredMethods, $requiredSchemes, $hasTrailingSlash) = $routes[$m];
    120120

    121121
    if ('/' !== $pathinfo) {
    122-
    if (!$hasTrailingSlash && '/' === $pathinfo[-1] && preg_match($regex, substr($pathinfo, 0, -1), $n) && $m === (int) $n['MARK']) {
    123-
    return null;
    122+
    if ('/' === $pathinfo[-1]) {
    123+
    if (preg_match($regex, substr($pathinfo, 0, -1), $n) && $m === (int) $n['MARK']) {
    124+
    $matches = $n;
    125+
    } else {
    126+
    $hasTrailingSlash = true;
    127+
    }
    124128
    }
    125-
    if ($hasTrailingSlash && '/' !== $pathinfo[-1]) {
    126-
    return null;
    129+
    130+
    if ($hasTrailingSlash !== ('/' === $pathinfo[-1])) {
    131+
    if (!$requiredMethods || isset($requiredMethods['GET'])) {
    132+
    return null;
    133+
    }
    134+
    break;
    127135
    }
    128136
    }
    129137

    ‎src/Symfony/Component/Routing/Tests/Fixtures/dumper/url_matcher12.php

    Lines changed: 8 additions & 3 deletions
    Original file line numberDiff line numberDiff line change
    @@ -64,10 +64,15 @@ public function match($rawPathinfo)
    6464
    list($ret, $vars, $requiredMethods, $requiredSchemes, $hasTrailingSlash) = $routes[$m];
    6565

    6666
    if ('/' !== $pathinfo) {
    67-
    if (!$hasTrailingSlash && '/' === $pathinfo[-1] && preg_match($regex, substr($pathinfo, 0, -1), $n) && $m === (int) $n['MARK']) {
    68-
    break;
    67+
    if ('/' === $pathinfo[-1]) {
    68+
    if (preg_match($regex, substr($pathinfo, 0, -1), $n) && $m === (int) $n['MARK']) {
    69+
    $matches = $n;
    70+
    } else {
    71+
    $hasTrailingSlash = true;
    72+
    }
    6973
    }
    70-
    if ($hasTrailingSlash && '/' !== $pathinfo[-1]) {
    74+
    75+
    if ($hasTrailingSlash !== ('/' === $pathinfo[-1])) {
    7176
    break;
    7277
    }
    7378
    }

    ‎src/Symfony/Component/Routing/Tests/Fixtures/dumper/url_matcher13.php

    Lines changed: 4 additions & 2 deletions
    Original file line numberDiff line numberDiff line change
    @@ -45,15 +45,17 @@ public function match($rawPathinfo)
    4545
    $matches = array('foo' => $matches[1] ?? null, 'foo' => $matches[2] ?? null);
    4646

    4747
    // r1
    48-
    if ('/' !== $pathinfo && '/' === $pathinfo[-1]) {
    48+
    if ('/' !== $pathinfo && '/' === $pathinfo[-1] && preg_match($regex, substr($pathinfo, 0, -1), $n) && $m === (int) $n['MARK']) {
    4949
    break;
    5050
    }
    51+
    5152
    return $this->mergeDefaults(array('_route' => 'r1') + $matches, array());
    5253

    5354
    // r2
    54-
    if ('/' !== $pathinfo && '/' === $pathinfo[-1]) {
    55+
    if ('/' !== $pathinfo && '/' === $pathinfo[-1] && preg_match($regex, substr($pathinfo, 0, -1), $n) && $m === (int) $n['MARK']) {
    5556
    break;
    5657
    }
    58+
    5759
    return $this->mergeDefaults(array('_route' => 'r2') + $matches, array());
    5860

    5961
    break;

    0 commit comments

    Comments
     (0)
    0