Skip to content

Commit 6cf7a0c

Browse files
nicolas-grekasnicolas-grekas
committed
[Routing] fix too much greediness in host-matching regex
1 parent b38b979 commit 6cf7a0c

File tree

4 files changed

+101
-91
lines changed

4 files changed

+101
-91
lines changed

‎Matcher/Dumper/PhpMatcherDumper.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -379,7 +379,7 @@ private function compileDynamicRoutes(RouteCollection $collection, bool $matchHo
379379
$hostRegex = '(?i:'.preg_replace_callback('#\?P<([^>]++)>#', $state->getVars, $rx[1]).')\.';
380380
$state->hostVars = $state->vars;
381381
} else {
382-
$hostRegex = '(?:(?:[^.]*+\.)++)';
382+
$hostRegex = '(?:(?:[^./]*+\.)++)';
383383
$state->hostVars = array();
384384
}
385385
$state->mark += strlen($rx = ($prev ? ')' : '')."|{$hostRegex}(?");

‎Tests/Fixtures/dumper/url_matcher1.php‎

Lines changed: 45 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -82,47 +82,47 @@ public function match($rawPathinfo)
8282
$matchedPathinfo = $host.'.'.$pathinfo;
8383
$regexList = array(
8484
0 => '{^(?'
85-
.'|(?:(?:[^.]*+\\.)++)(?'
86-
.'|/foo/(baz|symfony)(*:46)'
85+
.'|(?:(?:[^./]*+\\.)++)(?'
86+
.'|/foo/(baz|symfony)(*:47)'
8787
.'|/bar(?'
88-
.'|/([^/]++)(*:69)'
89-
.'|head/([^/]++)(*:89)'
88+
.'|/([^/]++)(*:70)'
89+
.'|head/([^/]++)(*:90)'
9090
.')'
9191
.'|/test/([^/]++)/(?'
92-
.'|(*:115)'
92+
.'|(*:116)'
9393
.')'
94-
.'|/([\']+)(*:131)'
94+
.'|/([\']+)(*:132)'
9595
.'|/a/(?'
9696
.'|b\'b/([^/]++)(?'
97-
.'|(*:160)'
98-
.'|(*:168)'
97+
.'|(*:161)'
98+
.'|(*:169)'
9999
.')'
100-
.'|(.*)(*:181)'
100+
.'|(.*)(*:182)'
101101
.'|b\'b/([^/]++)(?'
102-
.'|(*:204)'
103-
.'|(*:212)'
102+
.'|(*:205)'
103+
.'|(*:213)'
104104
.')'
105105
.')'
106-
.'|/multi/hello(?:/([^/]++))?(*:248)'
106+
.'|/multi/hello(?:/([^/]++))?(*:249)'
107107
.'|/([^/]++)/b/([^/]++)(?'
108-
.'|(*:279)'
109-
.'|(*:287)'
108+
.'|(*:280)'
109+
.'|(*:288)'
110110
.')'
111-
.'|/aba/([^/]++)(*:309)'
111+
.'|/aba/([^/]++)(*:310)'
112112
.')|(?i:([^\\.]++)\\.example\\.com)\\.(?'
113113
.'|/route1(?'
114-
.'|3/([^/]++)(*:371)'
115-
.'|4/([^/]++)(*:389)'
114+
.'|3/([^/]++)(*:372)'
115+
.'|4/([^/]++)(*:390)'
116116
.')'
117117
.')|(?i:c\\.example\\.com)\\.(?'
118-
.'|/route15/([^/]++)(*:441)'
119-
.')|(?:(?:[^.]*+\\.)++)(?'
120-
.'|/route16/([^/]++)(*:488)'
118+
.'|/route15/([^/]++)(*:442)'
119+
.')|(?:(?:[^./]*+\\.)++)(?'
120+
.'|/route16/([^/]++)(*:490)'
121121
.'|/a/(?'
122-
.'|a\\.\\.\\.(*:509)'
122+
.'|a\\.\\.\\.(*:511)'
123123
.'|b/(?'
124-
.'|([^/]++)(*:530)'
125-
.'|c/([^/]++)(*:548)'
124+
.'|([^/]++)(*:532)'
125+
.'|c/([^/]++)(*:550)'
126126
.')'
127127
.')'
128128
.')'
@@ -132,7 +132,7 @@ public function match($rawPathinfo)
132132
foreach ($regexList as $offset => $regex) {
133133
while (preg_match($regex, $matchedPathinfo, $matches)) {
134134
switch ($m = (int) $matches['MARK']) {
135-
case 115:
135+
case 116:
136136
$matches = array('foo' => $matches[1] ?? null);
137137

138138
// baz4
@@ -159,7 +159,7 @@ public function match($rawPathinfo)
159159
not_bazbaz6:
160160

161161
break;
162-
case 160:
162+
case 161:
163163
$matches = array('foo' => $matches[1] ?? null);
164164

165165
// foo1
@@ -173,14 +173,14 @@ public function match($rawPathinfo)
173173
not_foo1:
174174

175175
break;
176-
case 204:
176+
case 205:
177177
$matches = array('foo1' => $matches[1] ?? null);
178178

179179
// foo2
180180
return $this->mergeDefaults(array('_route' => 'foo2') + $matches, array());
181181

182182
break;
183-
case 279:
183+
case 280:
184184
$matches = array('_locale' => $matches[1] ?? null, 'foo' => $matches[2] ?? null);
185185

186186
// foo3
@@ -189,23 +189,23 @@ public function match($rawPathinfo)
189189
break;
190190
default:
191191
$routes = array(
192-
46 => array(array('_route' => 'foo', 'def' => 'test'), array('bar'), null, null),
193-
69 => array(array('_route' => 'bar'), array('foo'), array('GET' => 0, 'HEAD' => 1), null),
194-
89 => array(array('_route' => 'barhead'), array('foo'), array('GET' => 0), null),
195-
131 => array(array('_route' => 'quoter'), array('quoter'), null, null),
196-
168 => array(array('_route' => 'bar1'), array('bar'), null, null),
197-
181 => array(array('_route' => 'overridden'), array('var'), null, null),
198-
212 => array(array('_route' => 'bar2'), array('bar1'), null, null),
199-
248 => array(array('_route' => 'helloWorld', 'who' => 'World!'), array('who'), null, null),
200-
287 => array(array('_route' => 'bar3'), array('_locale', 'bar'), null, null),
201-
309 => array(array('_route' => 'foo4'), array('foo'), null, null),
202-
371 => array(array('_route' => 'route13'), array('var1', 'name'), null, null),
203-
389 => array(array('_route' => 'route14', 'var1' => 'val'), array('var1', 'name'), null, null),
204-
441 => array(array('_route' => 'route15'), array('name'), null, null),
205-
488 => array(array('_route' => 'route16', 'var1' => 'val'), array('name'), null, null),
206-
509 => array(array('_route' => 'a'), array(), null, null),
207-
530 => array(array('_route' => 'b'), array('var'), null, null),
208-
548 => array(array('_route' => 'c'), array('var'), null, null),
192+
47 => array(array('_route' => 'foo', 'def' => 'test'), array('bar'), null, null),
193+
70 => array(array('_route' => 'bar'), array('foo'), array('GET' => 0, 'HEAD' => 1), null),
194+
90 => array(array('_route' => 'barhead'), array('foo'), array('GET' => 0), null),
195+
132 => array(array('_route' => 'quoter'), array('quoter'), null, null),
196+
169 => array(array('_route' => 'bar1'), array('bar'), null, null),
197+
182 => array(array('_route' => 'overridden'), array('var'), null, null),
198+
213 => array(array('_route' => 'bar2'), array('bar1'), null, null),
199+
249 => array(array('_route' => 'helloWorld', 'who' => 'World!'), array('who'), null, null),
200+
288 => array(array('_route' => 'bar3'), array('_locale', 'bar'), null, null),
201+
310 => array(array('_route' => 'foo4'), array('foo'), null, null),
202+
372 => array(array('_route' => 'route13'), array('var1', 'name'), null, null),
203+
390 => array(array('_route' => 'route14', 'var1' => 'val'), array('var1', 'name'), null, null),
204+
442 => array(array('_route' => 'route15'), array('name'), null, null),
205+
490 => array(array('_route' => 'route16', 'var1' => 'val'), array('name'), null, null),
206+
511 => array(array('_route' => 'a'), array(), null, null),
207+
532 => array(array('_route' => 'b'), array('var'), null, null),
208+
550 => array(array('_route' => 'c'), array('var'), null, null),
209209
);
210210

211211
list($ret, $vars, $requiredMethods, $requiredSchemes) = $routes[$m];
@@ -231,7 +231,7 @@ public function match($rawPathinfo)
231231
return $ret;
232232
}
233233

234-
if (548 === $m) {
234+
if (550 === $m) {
235235
break;
236236
}
237237
$regex = substr_replace($regex, 'F', $m - $offset, 1 + strlen($m));

‎Tests/Fixtures/dumper/url_matcher2.php‎

Lines changed: 45 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -119,47 +119,47 @@ private function doMatch(string $rawPathinfo, array &$allow = array(), array &$a
119119
$matchedPathinfo = $host.'.'.$pathinfo;
120120
$regexList = array(
121121
0 => '{^(?'
122-
.'|(?:(?:[^.]*+\\.)++)(?'
123-
.'|/foo/(baz|symfony)(*:46)'
122+
.'|(?:(?:[^./]*+\\.)++)(?'
123+
.'|/foo/(baz|symfony)(*:47)'
124124
.'|/bar(?'
125-
.'|/([^/]++)(*:69)'
126-
.'|head/([^/]++)(*:89)'
125+
.'|/([^/]++)(*:70)'
126+
.'|head/([^/]++)(*:90)'
127127
.')'
128128
.'|/test/([^/]++)/(?'
129-
.'|(*:115)'
129+
.'|(*:116)'
130130
.')'
131-
.'|/([\']+)(*:131)'
131+
.'|/([\']+)(*:132)'
132132
.'|/a/(?'
133133
.'|b\'b/([^/]++)(?'
134-
.'|(*:160)'
135-
.'|(*:168)'
134+
.'|(*:161)'
135+
.'|(*:169)'
136136
.')'
137-
.'|(.*)(*:181)'
137+
.'|(.*)(*:182)'
138138
.'|b\'b/([^/]++)(?'
139-
.'|(*:204)'
140-
.'|(*:212)'
139+
.'|(*:205)'
140+
.'|(*:213)'
141141
.')'
142142
.')'
143-
.'|/multi/hello(?:/([^/]++))?(*:248)'
143+
.'|/multi/hello(?:/([^/]++))?(*:249)'
144144
.'|/([^/]++)/b/([^/]++)(?'
145-
.'|(*:279)'
146-
.'|(*:287)'
145+
.'|(*:280)'
146+
.'|(*:288)'
147147
.')'
148-
.'|/aba/([^/]++)(*:309)'
148+
.'|/aba/([^/]++)(*:310)'
149149
.')|(?i:([^\\.]++)\\.example\\.com)\\.(?'
150150
.'|/route1(?'
151-
.'|3/([^/]++)(*:371)'
152-
.'|4/([^/]++)(*:389)'
151+
.'|3/([^/]++)(*:372)'
152+
.'|4/([^/]++)(*:390)'
153153
.')'
154154
.')|(?i:c\\.example\\.com)\\.(?'
155-
.'|/route15/([^/]++)(*:441)'
156-
.')|(?:(?:[^.]*+\\.)++)(?'
157-
.'|/route16/([^/]++)(*:488)'
155+
.'|/route15/([^/]++)(*:442)'
156+
.')|(?:(?:[^./]*+\\.)++)(?'
157+
.'|/route16/([^/]++)(*:490)'
158158
.'|/a/(?'
159-
.'|a\\.\\.\\.(*:509)'
159+
.'|a\\.\\.\\.(*:511)'
160160
.'|b/(?'
161-
.'|([^/]++)(*:530)'
162-
.'|c/([^/]++)(*:548)'
161+
.'|([^/]++)(*:532)'
162+
.'|c/([^/]++)(*:550)'
163163
.')'
164164
.')'
165165
.')'
@@ -169,7 +169,7 @@ private function doMatch(string $rawPathinfo, array &$allow = array(), array &$a
169169
foreach ($regexList as $offset => $regex) {
170170
while (preg_match($regex, $matchedPathinfo, $matches)) {
171171
switch ($m = (int) $matches['MARK']) {
172-
case 115:
172+
case 116:
173173
$matches = array('foo' => $matches[1] ?? null);
174174

175175
// baz4
@@ -196,7 +196,7 @@ private function doMatch(string $rawPathinfo, array &$allow = array(), array &$a
196196
not_bazbaz6:
197197

198198
break;
199-
case 160:
199+
case 161:
200200
$matches = array('foo' => $matches[1] ?? null);
201201

202202
// foo1
@@ -210,14 +210,14 @@ private function doMatch(string $rawPathinfo, array &$allow = array(), array &$a
210210
not_foo1:
211211

212212
break;
213-
case 204:
213+
case 205:
214214
$matches = array('foo1' => $matches[1] ?? null);
215215

216216
// foo2
217217
return $this->mergeDefaults(array('_route' => 'foo2') + $matches, array());
218218

219219
break;
220-
case 279:
220+
case 280:
221221
$matches = array('_locale' => $matches[1] ?? null, 'foo' => $matches[2] ?? null);
222222

223223
// foo3
@@ -226,23 +226,23 @@ private function doMatch(string $rawPathinfo, array &$allow = array(), array &$a
226226
break;
227227
default:
228228
$routes = array(
229-
46 => array(array('_route' => 'foo', 'def' => 'test'), array('bar'), null, null),
230-
69 => array(array('_route' => 'bar'), array('foo'), array('GET' => 0, 'HEAD' => 1), null),
231-
89 => array(array('_route' => 'barhead'), array('foo'), array('GET' => 0), null),
232-
131 => array(array('_route' => 'quoter'), array('quoter'), null, null),
233-
168 => array(array('_route' => 'bar1'), array('bar'), null, null),
234-
181 => array(array('_route' => 'overridden'), array('var'), null, null),
235-
212 => array(array('_route' => 'bar2'), array('bar1'), null, null),
236-
248 => array(array('_route' => 'helloWorld', 'who' => 'World!'), array('who'), null, null),
237-
287 => array(array('_route' => 'bar3'), array('_locale', 'bar'), null, null),
238-
309 => array(array('_route' => 'foo4'), array('foo'), null, null),
239-
371 => array(array('_route' => 'route13'), array('var1', 'name'), null, null),
240-
389 => array(array('_route' => 'route14', 'var1' => 'val'), array('var1', 'name'), null, null),
241-
441 => array(array('_route' => 'route15'), array('name'), null, null),
242-
488 => array(array('_route' => 'route16', 'var1' => 'val'), array('name'), null, null),
243-
509 => array(array('_route' => 'a'), array(), null, null),
244-
530 => array(array('_route' => 'b'), array('var'), null, null),
245-
548 => array(array('_route' => 'c'), array('var'), null, null),
229+
47 => array(array('_route' => 'foo', 'def' => 'test'), array('bar'), null, null),
230+
70 => array(array('_route' => 'bar'), array('foo'), array('GET' => 0, 'HEAD' => 1), null),
231+
90 => array(array('_route' => 'barhead'), array('foo'), array('GET' => 0), null),
232+
132 => array(array('_route' => 'quoter'), array('quoter'), null, null),
233+
169 => array(array('_route' => 'bar1'), array('bar'), null, null),
234+
182 => array(array('_route' => 'overridden'), array('var'), null, null),
235+
213 => array(array('_route' => 'bar2'), array('bar1'), null, null),
236+
249 => array(array('_route' => 'helloWorld', 'who' => 'World!'), array('who'), null, null),
237+
288 => array(array('_route' => 'bar3'), array('_locale', 'bar'), null, null),
238+
310 => array(array('_route' => 'foo4'), array('foo'), null, null),
239+
372 => array(array('_route' => 'route13'), array('var1', 'name'), null, null),
240+
390 => array(array('_route' => 'route14', 'var1' => 'val'), array('var1', 'name'), null, null),
241+
442 => array(array('_route' => 'route15'), array('name'), null, null),
242+
490 => array(array('_route' => 'route16', 'var1' => 'val'), array('name'), null, null),
243+
511 => array(array('_route' => 'a'), array(), null, null),
244+
532 => array(array('_route' => 'b'), array('var'), null, null),
245+
550 => array(array('_route' => 'c'), array('var'), null, null),
246246
);
247247

248248
list($ret, $vars, $requiredMethods, $requiredSchemes) = $routes[$m];
@@ -268,7 +268,7 @@ private function doMatch(string $rawPathinfo, array &$allow = array(), array &$a
268268
return $ret;
269269
}
270270

271-
if (548 === $m) {
271+
if (550 === $m) {
272272
break;
273273
}
274274
$regex = substr_replace($regex, 'F', $m - $offset, 1 + strlen($m));

‎Tests/Matcher/UrlMatcherTest.php‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -670,6 +670,16 @@ public function testUtf8AndMethodMatching()
670670
$this->assertEquals('c', $matcher->match('/admin/api/package.json')['_route']);
671671
}
672672

673+
public function testHostWithDot()
674+
{
675+
$coll = new RouteCollection();
676+
$coll->add('a', new Route('/foo', array(), array(), array(), 'foo.example.com'));
677+
$coll->add('b', new Route('/bar/{baz}'));
678+
679+
$matcher = $this->getUrlMatcher($coll);
680+
$this->assertEquals('b', $matcher->match('/bar/abc.123')['_route']);
681+
}
682+
673683
protected function getUrlMatcher(RouteCollection $routes, RequestContext $context = null)
674684
{
675685
return new UrlMatcher($routes, $context ?: new RequestContext());

0 commit comments

Comments
 (0)