8000 [Routing] Fixes to handle spaces in route pattern · symfony/symfony@6465a69 · GitHub
[go: up one dir, main page]
Skip to content

Commit 6465a69

Browse files
vicbvicb
committed
[Routing] Fixes to handle spaces in route pattern
- The route compiler does not add extra space or line-feed, - The generated regex does not use the 'x' modified any more, - The PHP and apache matchers do not need to strip any chars (vs space and line feed before), - The space characters are escaped according to the apache format
1 parent e4ebffb commit 6465a69

File tree

9 files changed

+177
-70
lines changed

9 files changed

+177
-70
lines changed

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

Lines changed: 44 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,8 @@ class ApacheMatcherDumper extends MatcherDumper
3131
* @param array $options An array of options
3232
*
3333
* @return string A string to be used as Apache rewrite rules
34+
*
35+
* @throws \LogicException When the route regex is invalid
3436
*/
3537
public function dump(array $options = array())
3638
{
@@ -39,15 +41,23 @@ public function dump(array $options = array())
3941
'base_uri' => '',
4042
), $options);
4143

44+
$options['script_name'] = self::escape($options['script_name'], ' ', '\\');
45+
4246
$rules = array("# skip \"real\" requests\nRewriteCond %{REQUEST_FILENAME} -f\nRewriteRule .* - [QSA,L]");
4347
$methodVars = array();
4448

4549
foreach ($this->getRoutes()->all() as $name => $route) {
4650
$compiledRoute = $route->compile();
4751

4852
// prepare the apache regex
49-
$regex = preg_replace('/\?P<.+?>/', '', substr(str_replace(array("\n", ' '), '', $compiledRoute->getRegex()), 1, -3));
50-
$regex = '^'.preg_quote($options['base_uri']).substr($regex, 1);
53+
$regex = $compiledRoute->getRegex();
54+
$delimiter = $regex[0];
55+
$regexPatternEnd = strrpos($regex, $delimiter);
56+
if (strlen($regex) < 2 || 0 === $regexPatternEnd) {
57+
throw new \LogicException('The "%s" route regex "%s" is invalid', $name, $regex);
58+
}
59+
$regex = preg_replace('/\?P<.+?>/', '', substr($regex, 1, $regexPatternEnd - 1));
60+
$regex = '^'.self::escape(preg_quote($options['base_uri']).substr($regex, 1), ' ', '\\');
5161

5262
$hasTrailingSlash = '/$' == substr($regex, -2) && '^/$' != $regex;
5363

@@ -56,7 +66,6 @@ public function dump(array $options = array())
5666
$variables[] = 'E=_ROUTING_'.$variable.':%'.($i + 1);
5767
}
5868
foreach ($route->getDefaults() as $key => $value) {
59-
// todo: a more legit way to escape the value?
6069
$variables[] = 'E=_ROUTING_'.$key.':'.strtr($value, array(
6170
':' => '\\:',
6271
'=' => '\\=',
@@ -112,4 +121,36 @@ public function dump(array $options = array())
112121

113122
return implode("\n\n", $rules)."\n";
114123
}
124+
125+
/**
126+
* Escapes a string.
127+
*
128+
* @param string $string The string to be escaped
129+
* @param string $char The character to be escaped
130+
* @param string $with The character to be used for escaping
131+
*
132+
* @return string The escaped string
133+
*/
134+
static private function escape($string, $char, $with)
135+
{
136+
$escaped = false;
137+
$output = '';
138+
foreach(str_split($string) as $symbol) {
139+
if ($escaped) {
140+
$output .= $symbol;
141+
$escaped = false;
142+
continue;
143+
}
144+
if ($symbol === $char) {
145+
$output .= $with.$char;
146+
continue;
147+
}
148+
if ($symbol === $with) {
149+
$escaped = true;
150+
}
151+
$output .= $symbol;
152+
}
153+
154+
return $output;
155+
}
115156
}

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -148,7 +148,7 @@ private function compileRoute(Route $route, $name, $supportsRedirections, $paren
148148
$conditions = array();
149149
$hasTrailingSlash = false;
150150
$matches = false;
151-
if (!count($compiledRoute->getVariables()) && false !== preg_match('#^(.)\^(?P<url>.*?)\$\1#', str_replace(array("\n", ' '), '', $compiledRoute->getRegex()), $m)) {
151+
if (!count($compiledRoute->getVariables()) && false !== preg_match('#^(.)\^(?P<url>.*?)\$\1#', $compiledRoute->getRegex(), $m)) {
152152
if ($supportsRedirections && substr($m['url'], -1) === '/') {
153153
$conditions[] = sprintf("rtrim(\$pathinfo, '/') === %s", var_export(rtrim(str_replace('\\', '', $m['url']), '/'), true));
154154
$hasTrailingSlash = true;
@@ -160,7 +160,7 @@ private function compileRoute(Route $route, $name, $supportsRedirections, $paren
160160
$conditions[] = sprintf("0 === strpos(\$pathinfo, %s)", var_export($compiledRoute->getStaticPrefix(), true));
161161
}
162162

163-
$regex = str_replace(array("\n", ' '), '', $compiledRoute->getRegex());
163+
$regex = $compiledRoute->getRegex();
164164
if ($supportsRedirections && $pos = strpos($regex, '/$')) {
165165
$regex = substr($regex, 0, $pos).'/?$'.substr($regex, $pos + 2);
166166
$hasTrailingSlash = true;

‎src/Symfony/Component/Routing/RouteCompiler.php

Lines changed: 42 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -66,44 +66,62 @@ public function compile(Route $route)
6666
// find the first optional token
6767
$firstOptional = INF;
6868
for ($i = count($tokens) - 1; $i >= 0; $i--) {
69-
if ('variable' === $tokens[$i][0] && $route->hasDefault($tokens[$i][3])) {
69+
$token = $tokens[$i];
70+
if ('variable' === $token[0] && $route->hasDefault($token[3])) {
7071
$firstOptional = $i;
7172
} else {
7273
break;
7374
}
7475
}
7576

7677
// compute the matching regexp
77-
$regex = '';
78-
$indent = 1;
79-
if (1 === count($tokens) && 0 === $firstOptional) {
80-
$token = $tokens[0];
81-
++$indent;
82-
$regex .= str_repeat(' ', $indent * 4).sprintf("%s(?:\n", preg_quote($token[1], '#'));
83-
$regex .= str_repeat(' ', $indent * 4).sprintf("(?P<%s>%s)\n", $token[3], $token[2]);
84-
} else {
85-
foreach ($tokens as $i => $token) {
86-
if ('text' === $token[0]) {
87-
$regex .= str_repeat(' ', $indent * 4).preg_quote($token[1], '#')."\n";
88-
} else {
89-
if ($i >= $firstOptional) {
90-
$regex .= str_repeat(' ', $indent * 4)."(?:\n";
91-
++$indent;
92-
}
93-
$regex .= str_repeat(' ', $indent * 4).sprintf("%s(?P<%s>%s)\n", preg_quote($token[1], '#'), $token[3], $token[2]);
94-
}
95-
}
96-
}
97-
while (--$indent) {
98-
$regex .= str_repeat(' ', $indent * 4).")?\n";
78+
$regexp = '';
79+
for ($i = 0, $nbToken = count($tokens); $i < $nbToken; $i++) {
80+
$regexp .= $this->computeRegexp($tokens, $i, $firstOptional);
9981
}
10082

10183
return new CompiledRoute(
10284
$route,
10385
'text' === $tokens[0][0] ? $tokens[0][1] : '',
104-
sprintf("#^\n%s$#xs", $regex),
86+
sprintf("#^%s$#s", $regexp),
10587
array_reverse($tokens),
10688
$variables
10789
);
10890
}
91+
92+
/**
93+
* Computes the regexp used to match the token.
94+
*
95+
* @param array $tokens The route tokens
96+
* @param integer $index The index of the current token
97+
* @param integer $firstOptional The index of the first optional token
98+
*
99+
* @return string The regexp
100+
*/
101+
private function computeRegexp(array $tokens, $index, $firstOptional)
102+
{
103+
$token = $tokens[$index];
104+
if('text' === $token[0]) {
105+
// Text tokens
106+
return preg_quote($token[1], '#');
107+
} else {
108+
// Variable tokens
109+
if (0 === $index && 0 === $firstOptional && 1 == count($tokens)) {
110+
// When the only token is an optional variable token, the separator is required
111+
return sprintf('%s(?P<%s>%s)?', preg_quote($token[1], '#'), $token[3], $token[2]);
112+
} else {
113+
$nbTokens = count($tokens);
114+
$regexp = sprintf('%s(?P<%s>%s)', preg_quote($token[1], '#'), $token[3], $token[2]);
115+
if ($index >= $firstOptional) {
116+
// Enclose each optional tokens in a subpattern to make it optional
117+
$regexp = "(?:$regexp";
118+
if ($nbTokens - 1 == $index) {
119+
// Close the optional subpatterns
120+
$regexp .= str_repeat(")?", $nbTokens - $firstOptional);
121+
}
122+
}
123+
return $regexp;
124+
}
125+
}
126+
}
109127
}

‎tests/Symfony/Tests/Component/Routing/Fixtures/dumper/url_matcher1.apache

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,10 @@ RewriteRule .* app.php [QSA,L,E=_ROUTING__route:baz5,E=_ROUTING_foo:%1]
5353
RewriteCond %{REQUEST_URI} ^/test/baz$
5454
RewriteRule .* app.php [QSA,L,E=_ROUTING__route:baz6,E=_ROUTING_foo:bar\ baz]
5555

56+
# baz7
57+
RewriteCond %{REQUEST_URI} ^/te\ st/baz$
58+
RewriteRule .* app.php [QSA,L,E=_ROUTING__route:baz7]
59+
5660
# 405 Method Not Allowed
5761
RewriteCond %{_ROUTING__allow_GET} !-z [OR]
5862
RewriteCond %{_ROUTING__allow_HEAD} !-z [OR]

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

Lines changed: 19 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -26,12 +26,12 @@ public function match($pathinfo)
2626
$pathinfo = urldecode($pathinfo);
2727

2828
// foo
29-
if (0 === strpos($pathinfo, '/foo') && preg_match('#^/foo/(?P<bar>baz|symfony)$#xs', $pathinfo, $matches)) {
29+
if (0 === strpos($pathinfo, '/foo') && preg_match('#^/foo/(?P<bar>baz|symfony)$#s', $pathinfo, $matches)) {
3030
return array_merge($this->mergeDefaults($matches, array ( 'def' => 'test',)), array('_route' => 'foo'));
3131
}
3232

3333
// bar
34-
if (0 === strpos($pathinfo, '/bar') && preg_match('#^/bar/(?P<foo>[^/]+?)$#xs', $pathinfo, $matches)) {
34+
if (0 === strpos($pathinfo, '/bar') && preg_match('#^/bar/(?P<foo>[^/]+?)$#s', $pathinfo, $matches)) {
3535
if (!in_array($this->context->getMethod(), array('GET', 'HEAD'))) {
3636
$allow = array_merge($allow, array('GET', 'HEAD'));
3737
goto not_bar;
@@ -42,7 +42,7 @@ public function match($pathinfo)
4242
not_bar:
4343

4444
// barhead
45-
if (0 === strpos($pathinfo, '/barhead') && preg_match('#^/barhead/(?P<foo>[^/]+?)$#xs', $pathinfo, $matches)) {
45+
if (0 === strpos($pathinfo, '/barhead') && preg_match('#^/barhead/(?P<foo>[^/]+?)$#s', $pathinfo, $matches)) {
4646
if (!in_array($this->context->getMethod(), array('GET', 'HEAD'))) {
4747
$allow = array_merge($allow, array('GET', 'HEAD'));
4848
goto not_barhead;
@@ -68,13 +68,13 @@ public function match($pathinfo)
6868
}
6969

7070
// baz4
71-
if (0 === strpos($pathinfo, '/test') && preg_match('#^/test/(?P<foo>[^/]+?)/$#xs', $pathinfo, $matches)) {
71+
if (0 === strpos($pathinfo, '/test') && preg_match('#^/test/(?P<foo>[^/]+?)/$#s', $pathinfo, $matches)) {
7272
$matches['_route'] = 'baz4';
7373
return $matches;
7474
}
7575

7676
// baz5
77-
if (0 === strpos($pathinfo, '/test') && preg_match('#^/test/(?P<foo>[^/]+?)/$#xs', $pathinfo, $matches)) {
77+
if (0 === strpos($pathinfo, '/test') && preg_match('#^/test/(?P<foo>[^/]+?)/$#s', $pathinfo, $matches)) {
7878
if ($this->context->getMethod() != 'POST') {
7979
$allow[] = 'POST';
8080
goto not_baz5;
@@ -85,7 +85,7 @@ public function match($pathinfo)
8585
not_baz5:
8686

8787
// baz.baz6
88-
if (0 === strpos($pathinfo, '/test') && preg_match('#^/test/(?P<foo>[^/]+?)/$#xs', $pathinfo, $matches)) {
88+
if (0 === strpos($pathinfo, '/test') && preg_match('#^/test/(?P<foo>[^/]+?)/$#s', $pathinfo, $matches)) {
8989
if ($this->context->getMethod() != 'PUT') {
9090
$allow[] = 'PUT';
9191
goto not_bazbaz6;
@@ -101,33 +101,38 @@ public function match($pathinfo)
101101
}
102102

103103
// quoter
104-
if (preg_match('#^/(?P<quoter>[\']+)$#xs', $pathinfo, $matches)) {
104+
if (preg_match('#^/(?P<quoter>[\']+)$#s', $pathinfo, $matches)) {
105105
$matches['_route'] = 'quoter';
106106
return $matches;
107107
}
108108

109+
// space
110+
if ($pathinfo === '/spa ce') {
111+
return array('_route' => 'space');
112+
}
113+
109114
if (0 === strpos($pathinfo, '/a')) {
110115
if (0 === strpos($pathinfo, '/a/b\'b')) {
111116
// foo1
112-
if (preg_match('#^/a/b\'b/(?P<foo>[^/]+?)$#xs', $pathinfo, $matches)) {
117+
if (preg_match('#^/a/b\'b/(?P<foo>[^/]+?)$#s', $pathinfo, $matches)) {
113118
$matches['_route'] = 'foo1';
114119
return $matches;
115120
}
116121

117122
// bar1
118-
if (preg_match('#^/a/b\'b/(?P<bar>[^/]+?)$#xs', $pathinfo, $matches)) {
123+
if (preg_match('#^/a/b\'b/(?P<bar>[^/]+?)$#s', $pathinfo, $matches)) {
119124
$matches['_route'] = 'bar1';
120125
return $matches;
121126
}
122127

123128
// foo2
124-
if (preg_match('#^/a/b\'b/(?P<foo1>[^/]+?)$#xs', $pathinfo, $matches)) {
129+
if (preg_match('#^/a/b\'b/(?P<foo1>[^/]+?)$#s', $pathinfo, $matches)) {
125130
$matches['_route'] = 'foo2';
126131
return $matches;
127132
}
128133

129134
// bar2
130-
if (preg_match('#^/a/b\'b/(?P<bar1>[^/]+?)$#xs', $pathinfo, $matches)) {
135+
if (preg_match('#^/a/b\'b/(?P<bar1>[^/]+?)$#s', $pathinfo, $matches)) {
131136
$matches['_route'] = 'bar2';
132137
return $matches;
133138
}
@@ -145,21 +150,21 @@ public function match($pathinfo)
145150
}
146151

147152
// foo4
148-
if (preg_match('#^/aba/(?P<foo>[^/]+?)$#xs', $pat BD94 hinfo, $matches)) {
153+
if (preg_match('#^/aba/(?P<foo>[^/]+?)$#s', $pathinfo, $matches)) {
149154
$matches['_route'] = 'foo4';
150155
return $matches;
151156
}
152157

153158
}
154159

155160
// foo3
156-
if (preg_match('#^/(?P<_locale>[^/]+?)/b/(?P<foo>[^/]+?)$#xs', $pathinfo, $matches)) {
161+
if (preg_match('#^/(?P<_locale>[^/]+?)/b/(?P<foo>[^/]+?)$#s', $pathinfo, $matches)) {
157162
$matches['_route'] = 'foo3';
158163
return $matches;
159164
}
160165

161166
// bar3
162-
if (preg_match('#^/(?P<_locale>[^/]+?)/b/(?P<bar>[^/]+?)$#xs', $pathinfo, $matches)) {
167+
if (preg_match('#^/(?P<_locale>[^/]+?)/b/(?P<bar>[^/]+?)$#s', $pathinfo, $matches)) {
163168
$matches['_route'] = 'bar3';
164169
return $matches;
165170
}

0 commit comments

Comments
 (0)
0