Skip to content

Commit 4baad7d

Browse files
nicolas-grekasnicolas-grekas
committed
bug #25373 Use the PCRE_DOLLAR_ENDONLY modifier in route regexes (mpdude)
This PR was squashed before being merged into the 2.7 branch (closes #25373). Discussion ---------- Use the PCRE_DOLLAR_ENDONLY modifier in route regexes | Q | A | ------------- | --- | Branch? | 2.7 | Bug fix? | yes | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | | License | MIT | Doc PR | `UrlMatcher::match($pathinfo)` applies `rawurldecode()` to the `$pathinfo` before trying to match it against the routes. If the URL contains a percent-encoded trailing newline (like in `/foo%0a`), the default PHP PCRE will still consider `#^/foo$#` a match, as the `$` metacharacter will also match *immediately before* the final character *if it is a newline*. This behavior can be changed by applying the [`PCRE_DOLLAR_ENDONLY` modifier](http://php.net/manual/en/reference.pcre.pattern.modifiers.php). Without this change, URLs with trailing `%0a` lead to weird notices further down the road, for example when the `RedirectableUrlMatcher` or its equivalent in `PhpMatcherDumper` kick in, look at the last character (this time actually the newline), append a `/` and try to redirect to the resulting URL. Ultimately, PHP will complain with `Warning: Header may not contain more than a single header, new line detected` when sending the `Location` header. Commits ------- f713a3e Use the PCRE_DOLLAR_ENDONLY modifier in route regexes
2 parents 4eab2fb + 4ac81a5 commit 4baad7d

File tree

7 files changed

+93
-81
lines changed

7 files changed

+93
-81
lines changed

‎RouteCompiler.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -177,7 +177,7 @@ private static function compilePattern(Route $route, $pattern, $isHost)
177177

178178
return array(
179179
'staticPrefix' => 'text' === $tokens[0][0] ? $tokens[0][1] : '',
180-
'regex' => self::REGEX_DELIMITER.'^'.$regexp.'$'.self::REGEX_DELIMITER.'s'.($isHost ? 'i' : ''),
180+
'regex' => self::REGEX_DELIMITER.'^'.$regexp.'$'.self::REGEX_DELIMITER.'sD'.($isHost ? 'i' : ''),
181181
'tokens' => array_reverse($tokens),
182182
'variables' => $variables,
183183
);

‎Tests/Fixtures/dumper/url_matcher1.php‎

Lines changed: 28 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -23,13 +23,13 @@ public function match($rawPathinfo)
2323
$request = $this->request ?: $this->createRequest($pathinfo);
2424

2525
// foo
26-
if (0 === strpos($pathinfo, '/foo') && preg_match('#^/foo/(?P<bar>baz|symfony)$#s', $pathinfo, $matches)) {
26+
if (0 === strpos($pathinfo, '/foo') && preg_match('#^/foo/(?P<bar>baz|symfony)$#sD', $pathinfo, $matches)) {
2727
return $this->mergeDefaults(array_replace($matches, array('_route' => 'foo')), array ( 'def' => 'test',));
2828
}
2929

3030
if (0 === strpos($pathinfo, '/bar')) {
3131
// bar
32-
if (preg_match('#^/bar/(?P<foo>[^/]++)$#s', $pathinfo, $matches)) {
32+
if (preg_match('#^/bar/(?P<foo>[^/]++)$#sD', $pathinfo, $matches)) {
3333
if (!in_array($this->context->getMethod(), array('GET', 'HEAD'))) {
3434
$allow = array_merge($allow, array('GET', 'HEAD'));
3535
goto not_bar;
@@ -40,7 +40,7 @@ public function match($rawPathinfo)
4040
not_bar:
4141

4242
// barhead
43-
if (0 === strpos($pathinfo, '/barhead') && preg_match('#^/barhead/(?P<foo>[^/]++)$#s', $pathinfo, $matches)) {
43+
if (0 === strpos($pathinfo, '/barhead') && preg_match('#^/barhead/(?P<foo>[^/]++)$#sD', $pathinfo, $matches)) {
4444
if (!in_array($this->context->getMethod(), array('GET', 'HEAD'))) {
4545
$allow = array_merge($allow, array('GET', 'HEAD'));
4646
goto not_barhead;
@@ -72,12 +72,12 @@ public function match($rawPathinfo)
7272
}
7373

7474
// baz4
75-
if (preg_match('#^/test/(?P<foo>[^/]++)/$#s', $pathinfo, $matches)) {
75+
if (preg_match('#^/test/(?P<foo>[^/]++)/$#sD', $pathinfo, $matches)) {
7676
return $this->mergeDefaults(array_replace($matches, array('_route' => 'baz4')), array ());
7777
}
7878

7979
// baz5
80-
if (preg_match('#^/test/(?P<foo>[^/]++)/$#s', $pathinfo, $matches)) {
80+
if (preg_match('#^/test/(?P<foo>[^/]++)/$#sD', $pathinfo, $matches)) {
8181
if ($this->context->getMethod() != 'POST') {
8282
$allow[] = 'POST';
8383
goto not_baz5;
@@ -88,7 +88,7 @@ public function match($rawPathinfo)
8888
not_baz5:
8989

9090
// baz.baz6
91-
if (preg_match('#^/test/(?P<foo>[^/]++)/$#s', $pathinfo, $matches)) {
91+
if (preg_match('#^/test/(?P<foo>[^/]++)/$#sD', $pathinfo, $matches)) {
9292
if ($this->context->getMethod() != 'PUT') {
9393
$allow[] = 'PUT';
9494
goto not_bazbaz6;
@@ -106,7 +106,7 @@ public function match($rawPathinfo)
106106
}
107107

108108
// quoter
109-
if (preg_match('#^/(?P<quoter>[\']+)$#s', $pathinfo, $matches)) {
109+
if (preg_match('#^/(?P<quoter>[\']+)$#sD', $pathinfo, $matches)) {
110110
return $this->mergeDefaults(array_replace($matches, array('_route' => 'quoter')), array ());
111111
}
112112

@@ -118,30 +118,30 @@ public function match($rawPathinfo)
118118
if (0 === strpos($pathinfo, '/a')) {
119119
if (0 === strpos($pathinfo, '/a/b\'b')) {
120120
// foo1
121-
if (preg_match('#^/a/b\'b/(?P<foo>[^/]++)$#s', $pathinfo, $matches)) {
121+
if (preg_match('#^/a/b\'b/(?P<foo>[^/]++)$#sD', $pathinfo, $matches)) {
122122
return $this->mergeDefaults(array_replace($matches, array('_route' => 'foo1')), array ());
123123
}
124124

125125
// bar1
126-
if (preg_match('#^/a/b\'b/(?P<bar>[^/]++)$#s', $pathinfo, $matches)) {
126+
if (preg_match('#^/a/b\'b/(?P<bar>[^/]++)$#sD', $pathinfo, $matches)) {
127127
return $this->mergeDefaults(array_replace($matches, array('_route' => 'bar1')), array ());
128128
}
129129

130130
}
131131

132132
// overridden
133-
if (preg_match('#^/a/(?P<var>.*)$#s', $pathinfo, $matches)) {
133+
if (preg_match('#^/a/(?P<var>.*)$#sD', $pathinfo, $matches)) {
134134
return $this->mergeDefaults(array_replace($matches, array('_route' => 'overridden')), array ());
135135
}
136136

137137
if (0 === strpos($pathinfo, '/a/b\'b')) {
138138
// foo2
139-
if (preg_match('#^/a/b\'b/(?P<foo1>[^/]++)$#s', $pathinfo, $matches)) {
139+
if (preg_match('#^/a/b\'b/(?P<foo1>[^/]++)$#sD', $pathinfo, $matches)) {
140140
return $this->mergeDefaults(array_replace($matches, array('_route' => 'foo2')), array ());
141141
}
142142

143143
// bar2
144-
if (preg_match('#^/a/b\'b/(?P<bar1>[^/]++)$#s', $pathinfo, $matches)) {
144+
if (preg_match('#^/a/b\'b/(?P<bar1>[^/]++)$#sD', $pathinfo, $matches)) {
145145
return $this->mergeDefaults(array_replace($matches, array('_route' => 'bar2')), array ());
146146
}
147147

@@ -151,7 +151,7 @@ public function match($rawPathinfo)
151151

152152
if (0 === strpos($pathinfo, '/multi')) {
153153
// helloWorld
154-
if (0 === strpos($pathinfo, '/multi/hello') && preg_match('#^/multi/hello(?:/(?P<who>[^/]++))?$#s', $pathinfo, $matches)) {
154+
if (0 === strpos($pathinfo, '/multi/hello') && preg_match('#^/multi/hello(?:/(?P<who>[^/]++))?$#sD', $pathinfo, $matches)) {
155155
return $this->mergeDefaults(array_replace($matches, array('_route' => 'helloWorld')), array ( 'who' => 'World!',));
156156
}
157157

@@ -168,12 +168,12 @@ public function match($rawPathinfo)
168168
}
169169

170170
// foo3
171-
if (preg_match('#^/(?P<_locale>[^/]++)/b/(?P<foo>[^/]++)$#s', $pathinfo, $matches)) {
171+
if (preg_match('#^/(?P<_locale>[^/]++)/b/(?P<foo>[^/]++)$#sD', $pathinfo, $matches)) {
172172
return $this->mergeDefaults(array_replace($matches, array('_route' => 'foo3')), array ());
173173
}
174174

175175
// bar3
176-
if (preg_match('#^/(?P<_locale>[^/]++)/b/(?P<bar>[^/]++)$#s', $pathinfo, $matches)) {
176+
if (preg_match('#^/(?P<_locale>[^/]++)/b/(?P<bar>[^/]++)$#sD', $pathinfo, $matches)) {
177177
return $this->mergeDefaults(array_replace($matches, array('_route' => 'bar3')), array ());
178178
}
179179

@@ -184,15 +184,15 @@ public function match($rawPathinfo)
184184
}
185185

186186
// foo4
187-
if (preg_match('#^/aba/(?P<foo>[^/]++)$#s', $pathinfo, $matches)) {
187+
if (preg_match('#^/aba/(?P<foo>[^/]++)$#sD', $pathinfo, $matches)) {
188188
return $this->mergeDefaults(array_replace($matches, array('_route' => 'foo4')), array ());
189189
}
190190

191191
}
192192

193193
$host = $this->context->getHost();
194194

195-
if (preg_match('#^a\\.example\\.com$#si', $host, $hostMatches)) {
195+
if (preg_match('#^a\\.example\\.com$#sDi', $host, $hostMatches)) {
196196
// route1
197197
if ('/route1' === $pathinfo) {
198198
return array('_route' => 'route1');
@@ -205,23 +205,23 @@ public function match($rawPathinfo)
205205

206206
}
207207

208-
if (preg_match('#^b\\.example\\.com$#si', $host, $hostMatches)) {
208+
if (preg_match('#^b\\.example\\.com$#sDi', $host, $hostMatches)) {
209209
// route3
210210
if ('/c2/route3' === $pathinfo) {
211211
return array('_route' => 'route3');
212212
}
213213

214214
}
215215

216-
if (preg_match('#^a\\.example\\.com$#si', $host, $hostMatches)) {
216+
if (preg_match('#^a\\.example\\.com$#sDi', $host, $hostMatches)) {
217217
// route4
218218
if ('/route4' === $pathinfo) {
219219
return array('_route' => 'route4');
220220
}
221221

222222
}
223223

224-
if (preg_match('#^c\\.example\\.com$#si', $host, $hostMatches)) {
224+
if (preg_match('#^c\\.example\\.com$#sDi', $host, $hostMatches)) {
225225
// route5
226226
if ('/route5' === $pathinfo) {
227227
return array('_route' => 'route5');
@@ -234,7 +234,7 @@ public function match($rawPathinfo)
234234
return array('_route' => 'route6');
235235
}
236236

237-
if (preg_match('#^(?P<var1>[^\\.]++)\\.example\\.com$#si', $host, $hostMatches)) {
237+
if (preg_match('#^(?P<var1>[^\\.]++)\\.example\\.com$#sDi', $host, $hostMatches)) {
238238
if (0 === strpos($pathinfo, '/route1')) {
239239
// route11
240240
if ('/route11' === $pathinfo) {
@@ -247,30 +247,30 @@ public function match($rawPathinfo)
247247
}
248248

249249
// route13
250-
if (0 === strpos($pathinfo, '/route13') && preg_match('#^/route13/(?P<name>[^/]++)$#s', $pathinfo, $matches)) {
250+
if (0 === strpos($pathinfo, '/route13') && preg_match('#^/route13/(?P<name>[^/]++)$#sD', $pathinfo, $matches)) {
251251
return $this->mergeDefaults(array_replace($hostMatches, $matches, array('_route' => 'route13')), array ());
252252
}
253253

254254
// route14
255-
if (0 === strpos($pathinfo, '/route14') && preg_match('#^/route14/(?P<name>[^/]++)$#s', $pathinfo, $matches)) {
255+
if (0 === strpos($pathinfo, '/route14') && preg_match('#^/route14/(?P<name>[^/]++)$#sD', $pathinfo, $matches)) {
256256
return $this->mergeDefaults(array_replace($hostMatches, $matches, array('_route' => 'route14')), array ( 'var1' => 'val',));
257257
}
258258

259259
}
260260

261261
}
262262

263-
if (preg_match('#^c\\.example\\.com$#si', $host, $hostMatches)) {
263+
if (preg_match('#^c\\.example\\.com$#sDi', $host, $hostMatches)) {
264264
// route15
265-
if (0 === strpos($pathinfo, '/route15') && preg_match('#^/route15/(?P<name>[^/]++)$#s', $pathinfo, $matches)) {
265+
if (0 === strpos($pathinfo, '/route15') && preg_match('#^/route15/(?P<name>[^/]++)$#sD', $pathinfo, $matches)) {
266266
return $this->mergeDefaults(array_replace($matches, array('_route' => 'route15')), array ());
267267
}
268268

269269
}
270270

271271
if (0 === strpos($pathinfo, '/route1')) {
272272
// route16
273-
if (0 === strpos($pathinfo, '/route16') && preg_match('#^/route16/(?P<name>[^/]++)$#s', $pathinfo, $matches)) {
273+
if (0 === strpos($pathinfo, '/route16') && preg_match('#^/route16/(?P<name>[^/]++)$#sD', $pathinfo, $matches)) {
274274
return $this->mergeDefaults(array_replace($matches, array('_route' => 'route16')), array ( 'var1' => 'val',));
275275
}
276276

@@ -289,12 +289,12 @@ public function match($rawPathinfo)
289289

290290
if (0 === strpos($pathinfo, '/a/b')) {
291291
// b
292-
if (preg_match('#^/a/b/(?P<var>[^/]++)$#s', $pathinfo, $matches)) {
292+
if (preg_match('#^/a/b/(?P<var>[^/]++)$#sD', $pathinfo, $matches)) {
293293
return $this->mergeDefaults(array_replace($matches, array('_route' => 'b')), array ());
294294
}
295295

296296
// c
297-
if (0 === strpos($pathinfo, '/a/b/c') && preg_match('#^/a/b/c/(?P<var>[^/]++)$#s', $pathinfo, $matches)) {
297+
if (0 === strpos($pathinfo, '/a/b/c') && preg_match('#^/a/b/c/(?P<var>[^/]++)$#sD', $pathinfo, $matches)) {
298298
return $this->mergeDefaults(array_replace($matches, array('_route' => 'c')), array ());
299299
}
300300

0 commit comments

Comments
 (0)