Skip to content

Commit 469668e

Browse files
nicolas-grekasnicolas-grekas
committed
[Routing] Redirect from trailing slash to no-slash when possible
1 parent 896ca07 commit 469668e

20 files changed

+387
-428
lines changed

‎Matcher/Dumper/PhpMatcherDumper.php‎

Lines changed: 72 additions & 100 deletions
Large diffs are not rendered by default.

‎Matcher/Dumper/StaticPrefixCollection.php‎

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -180,12 +180,6 @@ private function getCommonPrefix(string $prefix, string $anotherPrefix): array
180180
break;
181181
}
182182
}
183-
if (1 < $i && '/' === $prefix[$i - 1]) {
184-
--$i;
185-
}
186-
if (null !== $staticLength && 1 < $staticLength && '/' === $prefix[$staticLength - 1]) {
187-
--$staticLength;
188-
}
189183

190184
return array(substr($prefix, 0, $i), substr($prefix, 0, $staticLength ?? $i));
191185
}

‎Matcher/RedirectableUrlMatcher.php‎

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -25,22 +25,21 @@ abstract class RedirectableUrlMatcher extends UrlMatcher implements Redirectable
2525
public function match($pathinfo)
2626
{
2727
try {
28-
$parameters = parent::match($pathinfo);
28+
return parent::match($pathinfo);
2929
} catch (ResourceNotFoundException $e) {
30-
if ('/' === substr($pathinfo, -1) || !in_array($this->context->getMethod(), array('HEAD', 'GET'))) {
30+
if ('/' === $pathinfo || !\in_array($this->context->getMethod(), array('HEAD', 'GET'), true)) {
3131
throw $e;
3232
}
3333

3434
try {
35-
$parameters = parent::match($pathinfo.'/');
35+
$pathinfo = '/' !== $pathinfo[-1] ? $pathinfo.'/' : substr($pathinfo, 0, -1);
36+
$ret = parent::match($pathinfo);
3637

37-
return array_replace($parameters, $this->redirect($pathinfo.'/', isset($parameters['_route']) ? $parameters['_route'] : null));
38+
return $this->redirect($pathinfo, $ret['_route'] ?? null) + $ret;
3839
} catch (ResourceNotFoundException $e2) {
3940
throw $e;
4041
}
4142
}
42-
43-
return $parameters;
4443
}
4544

4645
/**

‎Tests/Fixtures/dumper/url_matcher0.php‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,6 @@ public function match($rawPathinfo)
1919
{
2020
$allow = array();
2121
$pathinfo = rawurldecode($rawPathinfo);
22-
$trimmedPathinfo = rtrim($pathinfo, '/');
2322
$context = $this->context;
2423
$requestMethod = $canonicalMethod = $context->getMethod();
2524

‎Tests/Fixtures/dumper/url_matcher1.php‎

Lines changed: 42 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,6 @@ public function match($rawPathinfo)
1919
{
2020
$allow = array();
2121
$pathinfo = rawurldecode($rawPathinfo);
22-
$trimmedPathinfo = rtrim($pathinfo, '/');
2322
$context = $this->context;
2423
$requestMethod = $canonicalMethod = $context->getMethod();
2524
$host = strtolower($context->getHost());
@@ -82,41 +81,41 @@ public function match($rawPathinfo)
8281
.'|/([^/]++)(*:57)'
8382
.'|head/([^/]++)(*:77)'
8483
.')'
85-
.'|/test/([^/]++)(?'
86-
.'|/(*:103)'
84+
.'|/test/([^/]++)/(?'
85+
.'|(*:103)'
8786
.')'
8887
.'|/([\']+)(*:119)'
89-
.'|/a(?'
90-
.'|/b\'b/([^/]++)(?'
88+
.'|/a/(?'
89+
.'|b\'b/([^/]++)(?'
9190
.'|(*:148)'
9291
.'|(*:156)'
9392
.')'
94-
.'|/(.*)(*:170)'
95-
.'|/b\'b/([^/]++)(?'
96-
.'|(*:194)'
97-
.'|(*:202)'
93+
.'|(.*)(*:169)'
94+
.'|b\'b/([^/]++)(?'
95+
.'|(*:192)'
96+
.'|(*:200)'
9897
.')'
9998
.')'
100-
.'|/multi/hello(?:/([^/]++))?(*:238)'
99+
.'|/multi/hello(?:/([^/]++))?(*:236)'
101100
.'|/([^/]++)/b/([^/]++)(?'
102-
.'|(*:269)'
103-
.'|(*:277)'
101+
.'|(*:267)'
102+
.'|(*:275)'
104103
.')'
105-
.'|/aba/([^/]++)(*:299)'
104+
.'|/aba/([^/]++)(*:297)'
106105
.')|(?i:([^\\.]++)\\.example\\.com)(?'
107106
.'|/route1(?'
108-
.'|3/([^/]++)(*:359)'
109-
.'|4/([^/]++)(*:377)'
107+
.'|3/([^/]++)(*:357)'
108+
.'|4/([^/]++)(*:375)'
110109
.')'
111110
.')|(?i:c\\.example\\.com)(?'
112-
.'|/route15/([^/]++)(*:427)'
111+
.'|/route15/([^/]++)(*:425)'
113112
.')|[^/]*+(?'
114-
.'|/route16/([^/]++)(*:462)'
115-
.'|/a(?'
116-
.'|/a\\.\\.\\.(*:483)'
117-
.'|/b(?'
118-
.'|/([^/]++)(*:505)'
119-
.'|/c/([^/]++)(*:524)'
113+
.'|/route16/([^/]++)(*:460)'
114+
.'|/a/(?'
115+
.'|a\\.\\.\\.(*:481)'
116+
.'|b/(?'
117+
.'|([^/]++)(*:502)'
118+
.'|c/([^/]++)(*:520)'
120119
.')'
121120
.')'
122121
.')'
@@ -130,10 +129,10 @@ public function match($rawPathinfo)
130129
$matches = array('foo' => $matches[1] ?? null);
131130

132131
// baz4
133-
return $this->mergeDefaults(array_replace($matches, array('_route' => 'baz4')), array());
132+
return $this->mergeDefaults(array('_route' => 'baz4') + $matches, array());
134133

135134
// baz5
136-
$ret = $this->mergeDefaults(array_replace($matches, array('_route' => 'baz5')), array());
135+
$ret = $this->mergeDefaults(array('_route' => 'baz5') + $matches, array());
137136
if (!isset(($a = array('POST' => 0))[$requestMethod])) {
138137
$allow += $a;
139138
goto not_baz5;
@@ -143,7 +142,7 @@ public function match($rawPathinfo)
143142
not_baz5:
144143

145144
// baz.baz6
146-
$ret = $this->mergeDefaults(array_replace($matches, array('_route' => 'baz.baz6')), array());
145+
$ret = $this->mergeDefaults(array('_route' => 'baz.baz6') + $matches, array());
147146
if (!isset(($a = array('PUT' => 0))[$requestMethod])) {
148147
$allow += $a;
149148
goto not_bazbaz6;
@@ -157,7 +156,7 @@ public function match($rawPathinfo)
157156
$matches = array('foo' => $matches[1] ?? null);
158157

159158
// foo1
160-
$ret = $this->mergeDefaults(array_replace($matches, array('_route' => 'foo1')), array());
159+
$ret = $this->mergeDefaults(array('_route' => 'foo1') + $matches, array());
161160
if (!isset(($a = array('PUT' => 0))[$requestMethod])) {
162161
$allow += $a;
163162
goto not_foo1;
@@ -167,18 +166,18 @@ public function match($rawPathinfo)
167166
not_foo1:
168167

169168
break;
170-
case 194:
169+
case 192:
171170
$matches = array('foo1' => $matches[1] ?? null);
172171

173172
// foo2
174-
return $this->mergeDefaults(array_replace($matches, array('_route' => 'foo2')), array());
173+
return $this->mergeDefaults(array('_route' => 'foo2') + $matches, array());
175174

176175
break;
177-
case 269:
176+
case 267:
178177
$matches = array('_locale' => $matches[1] ?? null, 'foo' => $matches[2] ?? null);
179178

180179
// foo3
181-
return $this->mergeDefaults(array_replace($matches, array('_route' => 'foo3')), array());
180+
return $this->mergeDefaults(array('_route' => 'foo3') + $matches, array());
182181

183182
break;
184183
default:
@@ -188,18 +187,18 @@ public function match($rawPathinfo)
188187
77 => array(array('_route' => 'barhead'), array('foo'), array('GET' => 0), null),
189188
119 => array(array('_route' => 'quoter'), array('quoter'), null, null),
190189
156 => array(array('_route' => 'bar1'), array('bar'), null, null),
191-
170 => array(array('_route' => 'overridden'), array('var'), null, null),
192-
202 => array(array('_route' => 'bar2'), array('bar1'), null, null),
193-
238 => array(array('_route' => 'helloWorld', 'who' => 'World!'), array('who'), null, null),
194-
277 => array(array('_route' => 'bar3'), array('_locale', 'bar'), null, null),
195-
299 => array(array('_route' => 'foo4'), array('foo'), null, null),
196-
359 => array(array('_route' => 'route13'), array('var1', 'name'), null, null),
197-
377 => array(array('_route' => 'route14', 'var1' => 'val'), array('var1', 'name'), null, null),
198-
427 => array(array('_route' => 'route15'), array('name'), null, null),
199-
462 => array(array('_route' => 'route16', 'var1' => 'val'), array('name'), null, null),
200-
483 => array(array('_route' => 'a'), array(), null, null),
201-
505 => array(array('_route' => 'b'), array('var'), null, null),
202-
524 => array(array('_route' => 'c'), array('var'), null, null),
190+
169 => array(array('_route' => 'overridden'), array('var'), null, null),
191+
200 => array(array('_route' => 'bar2'), array('bar1'), null, null),
192+
236 => array(array('_route' => 'helloWorld', 'who' => 'World!'), array('who'), null, null),
193+
275 => array(array('_route' => 'bar3'), array('_locale', 'bar'), null, null),
194+
297 => array(array('_route' => 'foo4'), array('foo'), null, null),
195+
357 => array(array('_route' => 'route13'), array('var1', 'name'), null, null),
196+
375 => array(array('_route' => 'route14', 'var1' => 'val'), array('var1', 'name'), null, null),
197+
425 => array(array('_route' => 'route15'), array('name'), null, null),
198+
460 => array(array('_route' => 'route16', 'var1' => 'val'), array('name'), null, null),
199+
481 => array(array('_route' => 'a'), array(), null, null),
200+
502 => array(array('_route' => 'b'), array('var'), null, null),
201+
520 => array(array('_route' => 'c'), array('var'), null, null),
203202
);
204203

205204
list($ret, $vars, $requiredMethods, $requiredSchemes) = $routes[$m];
@@ -218,7 +217,7 @@ public function match($rawPathinfo)
218217
return $ret;
219218
}
220219

221-
if (524 === $m) {
220+
if (520 === $m) {
222221
break;
223222
}
224223
$regex = substr_replace($regex, 'F', $m - $offset, 1 + strlen($m));

‎Tests/Fixtures/dumper/url_matcher10.php‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,6 @@ public function match($rawPathinfo)
1919
{
2020
$allow = array();
2121
$pathinfo = rawurldecode($rawPathinfo);
22-
$trimmedPathinfo = rtrim($pathinfo, '/');
2322
$context = $this->context;
2423
$requestMethod = $canonicalMethod = $context->getMethod();
2524

‎Tests/Fixtures/dumper/url_matcher11.php‎

Lines changed: 55 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -15,11 +15,26 @@ public function __construct(RequestContext $context)
1515
$this->context = $context;
1616
}
1717

18-
public function match($rawPathinfo)
18+
public function match($pathinfo)
19+
{
20+
$allow = array();
21+
if ($ret = $this->doMatch($pathinfo, $allow)) {
22+
return $ret;
23+
}
24+
if ('/' !== $pathinfo && in_array($this->context->getMethod(), array('HEAD', 'GET'), true)) {
25+
$pathinfo = '/' !== $pathinfo[-1] ? $pathinfo.'/' : substr($pathinfo, 0, -1);
26+
if ($ret = $this->doMatch($pathinfo)) {
27+
return $this->redirect($pathinfo, $ret['_route']) + $ret;
28+
}
29+
}
30+
31+
throw $allow ? new MethodNotAllowedException(array_keys($allow)) : new ResourceNotFoundException();
32+
}
33+
34+
private function doMatch(string $rawPathinfo, array &$allow = array()): ?array
1935
{
2036
$allow = array();
2137
$pathinfo = rawurldecode($rawPathinfo);
22-
$trimmedPathinfo = rtrim($pathinfo, '/');
2338
$context = $this->context;
2439
$requestMethod = $canonicalMethod = $context->getMethod();
2540

@@ -30,32 +45,34 @@ public function match($rawPathinfo)
3045
$matchedPathinfo = $pathinfo;
3146
$regexList = array(
3247
0 => '{^(?'
33-
.'|/(en|fr)(?'
34-
.'|/admin/post(?'
35-
.'|/?(*:34)'
36-
.'|/new(*:45)'
37-
.'|/(\\d+)(?'
38-
.'|(*:61)'
39-
.'|/edit(*:73)'
40-
.'|/delete(*:87)'
48+
.'|/(en|fr)/(?'
49+
.'|admin/post/(?'
50+
.'|(*:33)'
51+
.'|new(*:43)'
52+
.'|(\\d+)(?'
53+
.'|(*:58)'
54+
.'|/(?'
55+
.'|edit(*:73)'
56+
.'|delete(*:86)'
57+
.')'
4158
.')'
4259
.')'
43-
.'|/blog(?'
44-
.'|/?(*:106)'
45-
.'|/rss\\.xml(*:123)'
46-
.'|/p(?'
47-
.'|age/([^/]++)(*:148)'
48-
.'|osts/([^/]++)(*:169)'
60+
.'|blog/(?'
61+
.'|(*:104)'
62+
.'|rss\\.xml(*:120)'
63+
.'|p(?'
64+
.'|age/([^/]++)(*:144)'
65+
.'|osts/([^/]++)(*:165)'
4966
.')'
50-
.'|/comments/(\\d+)/new(*:197)'
51-
.'|/search(*:212)'
67+
.'|comments/(\\d+)/new(*:192)'
68+
.'|search(*:206)'
5269
.')'
53-
.'|/log(?'
54-
.'|in(*:230)'
55-
.'|out(*:241)'
70+
.'|log(?'
71+
.'|in(*:223)'
72+
.'|out(*:234)'
5673
.')'
5774
.')'
58-
.'|/(en|fr)?(*:260)'
75+
.'|/(en|fr)?(*:253)'
5976
.')$}sD',
6077
);
6178

@@ -64,20 +81,20 @@ public function match($rawPathinfo)
6481
switch ($m = (int) $matches['MARK']) {
6582
default:
6683
$routes = array(
67-
34 => array(array('_route' => 'a', '_locale' => 'en'), array('_locale'), null, null, true),
68-
45 => array(array('_route' => 'b', '_locale' => 'en'), array('_locale'), null, null),
69-
61 => array(array('_route' => 'c', '_locale' => 'en'), array('_locale', 'id'), null, null),
84+
33 => array(array('_route' => 'a', '_locale' => 'en'), array('_locale'), null, null),
85+
43 => array(array('_route' => 'b', '_locale' => 'en'), array('_locale'), null, null),
86+
58 => array(array('_route' => 'c', '_locale' => 'en'), array('_locale', 'id'), null, null),
7087
73 => array(array('_route' => 'd', '_locale' => 'en'), array('_locale', 'id'), null, null),
71-
87 => array(array('_route' => 'e', '_locale' => 'en'), array('_locale', 'id'), null, null),
72-
106 => array(array('_route' => 'f', '_locale' => 'en'), array('_locale'), null, null, true),
73-
123 => array(array('_route' => 'g', '_locale' => 'en'), array('_locale'), null, null),
74-
148 => array(array('_route' => 'h', '_locale' => 'en'), array('_locale', 'page'), null, null),
75-
169 => array(array('_route' => 'i', '_locale' => 'en'), array('_locale', 'page'), null, null),
76-
197 => array(array('_route' => 'j', '_locale' => 'en'), array('_locale', 'id'), null, null),
77-
212 => array(array('_route' => 'k', '_locale' => 'en'), array('_locale'), null, null),
78-
230 => array(array('_route' => 'l', '_locale' => 'en'), array('_locale'), null, null),
79-
241 => array(array('_route' => 'm', '_locale' => 'en'), array('_locale'), null, null),
80-
260 => array(array('_route' => 'n', '_locale' => 'en'), array('_locale'), null, null),
88+
86 => array(array('_route' => 'e', '_locale' => 'en'), array('_locale', 'id'), null, null),
89+
104 => array(array('_route' => 'f', '_locale' => 'en'), array('_locale'), null, null),
90+
120 => array(array('_route' => 'g', '_locale' => 'en'), array('_locale'), null, null),
91+
144 => array(array('_route' => 'h', '_locale' => 'en'), array('_locale', 'page'), null, null),
92+
165 => array(array('_route' => 'i', '_locale' => 'en'), array('_locale', 'page'), null, null),
93+
192 => array(array('_route' => 'j', '_locale' => 'en'), array('_locale', 'id'), null, null),
94+
206 => array(array('_route' => 'k', '_locale' => 'en'), array('_locale'), null, null),
95+
223 => array(array('_route' => 'l', '_locale' => 'en'), array('_locale'), null, null),
96+
234 => array(array('_route' => 'm', '_locale' => 'en'), array('_locale'), null, null),
97+
253 => array(array('_route' => 'n', '_locale' => 'en'), array('_locale'), null, null),
8198
);
8299

83100
list($ret, $vars, $requiredMethods, $requiredSchemes) = $routes[$m];
@@ -88,22 +105,13 @@ public function match($rawPathinfo)
88105
}
89106
}
90107

91-
if (empty($routes[$m][4]) || '/' === $pathinfo[-1]) {
92-
// no-op
93-
} elseif ('GET' !== $canonicalMethod) {
94-
$allow['GET'] = 'GET';
95-
break;
96-
} else {
97-
return array_replace($ret, $this->redirect($rawPathinfo.'/', $ret['_route']));
98-
}
99-
100108
if ($requiredSchemes && !isset($requiredSchemes[$context->getScheme()])) {
101109
if ('GET' !== $canonicalMethod) {
102110
$allow['GET'] = 'GET';
103111
break;
104112
}
105113

106-
return array_replace($ret, $this->redirect($rawPathinfo, $ret['_route'], key($requiredSchemes)));
114+
return $this->redirect($rawPathinfo, $ret['_route'], key($requiredSchemes)) + $ret;
107115
}
108116

109117
if ($requiredMethods && !isset($requiredMethods[$canonicalMethod]) && !isset($requiredMethods[$requestMethod])) {
@@ -114,14 +122,14 @@ public function match($rawPathinfo)
114122
return $ret;
115123
}
116124

117-
if (260 === $m) {
125+
if (253 === $m) {
118126
break;
119127
}
120128
$regex = substr_replace($regex, 'F', $m - $offset, 1 + strlen($m));
121129
$offset += strlen($m);
122130
}
123131
}
124132

125-
throw $allow ? new MethodNotAllowedException(array_keys($allow)) : new ResourceNotFoundException();
133+
return null;
126134
}
127135
}

0 commit comments

Comments
 (0)