Permalink
Browse files

[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
  • Loading branch information...
1 parent e4ebffb commit 6465a6987a8bfedc2c8d983eef7e30bbb4bebdce @vicb vicb committed Apr 10, 2012
@@ -31,6 +31,8 @@ class ApacheMatcherDumper extends MatcherDumper
* @param array $options An array of options
*
* @return string A string to be used as Apache rewrite rules
+ *
+ * @throws \LogicException When the route regex is invalid
*/
public function dump(array $options = array())
{
@@ -39,15 +41,23 @@ public function dump(array $options = array())
'base_uri' => '',
), $options);
+ $options['script_name'] = self::escape($options['script_name'], ' ', '\\');
+
$rules = array("# skip \"real\" requests\nRewriteCond %{REQUEST_FILENAME} -f\nRewriteRule .* - [QSA,L]");
$methodVars = array();
foreach ($this->getRoutes()->all() as $name => $route) {
$compiledRoute = $route->compile();
// prepare the apache regex
- $regex = preg_replace('/\?P<.+?>/', '', substr(str_replace(array("\n", ' '), '', $compiledRoute->getRegex()), 1, -3));
- $regex = '^'.preg_quote($options['base_uri']).substr($regex, 1);
+ $regex = $compiledRoute->getRegex();
+ $delimiter = $regex[0];
+ $regexPatternEnd = strrpos($regex, $delimiter);
+ if (strlen($regex) < 2 || 0 === $regexPatternEnd) {
+ throw new \LogicException('The "%s" route regex "%s" is invalid', $name, $regex);
+ }
+ $regex = preg_replace('/\?P<.+?>/', '', substr($regex, 1, $regexPatternEnd - 1));
+ $regex = '^'.self::escape(preg_quote($options['base_uri']).substr($regex, 1), ' ', '\\');
$hasTrailingSlash = '/$' == substr($regex, -2) && '^/$' != $regex;
@@ -56,7 +66,6 @@ public function dump(array $options = array())
$variables[] = 'E=_ROUTING_'.$variable.':%'.($i + 1);
}
foreach ($route->getDefaults() as $key => $value) {
- // todo: a more legit way to escape the value?
$variables[] = 'E=_ROUTING_'.$key.':'.strtr($value, array(
':' => '\\:',
'=' => '\\=',
@@ -112,4 +121,36 @@ public function dump(array $options = array())
return implode("\n\n", $rules)."\n";
}
+
+ /**
+ * Escapes a string.
+ *
+ * @param string $string The string to be escaped
+ * @param string $char The character to be escaped
+ * @param string $with The character to be used for escaping
+ *
+ * @return string The escaped string
+ */
+ static private function escape($string, $char, $with)
+ {
+ $escaped = false;
+ $output = '';
+ foreach(str_split($string) as $symbol) {
+ if ($escaped) {
+ $output .= $symbol;
+ $escaped = false;
+ continue;
+ }
+ if ($symbol === $char) {
+ $output .= $with.$char;
+ continue;
+ }
+ if ($symbol === $with) {
+ $escaped = true;
+ }
+ $output .= $symbol;
+ }
+
+ return $output;
+ }
}
@@ -148,7 +148,7 @@ private function compileRoute(Route $route, $name, $supportsRedirections, $paren
$conditions = array();
$hasTrailingSlash = false;
$matches = false;
- if (!count($compiledRoute->getVariables()) && false !== preg_match('#^(.)\^(?P<url>.*?)\$\1#', str_replace(array("\n", ' '), '', $compiledRoute->getRegex()), $m)) {
+ if (!count($compiledRoute->getVariables()) && false !== preg_match('#^(.)\^(?P<url>.*?)\$\1#', $compiledRoute->getRegex(), $m)) {
if ($supportsRedirections && substr($m['url'], -1) === '/') {
$conditions[] = sprintf("rtrim(\$pathinfo, '/') === %s", var_export(rtrim(str_replace('\\', '', $m['url']), '/'), true));
$hasTrailingSlash = true;
@@ -160,7 +160,7 @@ private function compileRoute(Route $route, $name, $supportsRedirections, $paren
$conditions[] = sprintf("0 === strpos(\$pathinfo, %s)", var_export($compiledRoute->getStaticPrefix(), true));
}
- $regex = str_replace(array("\n", ' '), '', $compiledRoute->getRegex());
+ $regex = $compiledRoute->getRegex();
if ($supportsRedirections && $pos = strpos($regex, '/$')) {
$regex = substr($regex, 0, $pos).'/?$'.substr($regex, $pos + 2);
$hasTrailingSlash = true;
@@ -66,44 +66,62 @@ public function compile(Route $route)
// find the first optional token
$firstOptional = INF;
for ($i = count($tokens) - 1; $i >= 0; $i--) {
- if ('variable' === $tokens[$i][0] && $route->hasDefault($tokens[$i][3])) {
+ $token = $tokens[$i];
+ if ('variable' === $token[0] && $route->hasDefault($token[3])) {
$firstOptional = $i;
} else {
break;
}
}
// compute the matching regexp
- $regex = '';
- $indent = 1;
- if (1 === count($tokens) && 0 === $firstOptional) {
- $token = $tokens[0];
- ++$indent;
- $regex .= str_repeat(' ', $indent * 4).sprintf("%s(?:\n", preg_quote($token[1], '#'));
- $regex .= str_repeat(' ', $indent * 4).sprintf("(?P<%s>%s)\n", $token[3], $token[2]);
- } else {
- foreach ($tokens as $i => $token) {
- if ('text' === $token[0]) {
- $regex .= str_repeat(' ', $indent * 4).preg_quote($token[1], '#')."\n";
- } else {
- if ($i >= $firstOptional) {
- $regex .= str_repeat(' ', $indent * 4)."(?:\n";
- ++$indent;
- }
- $regex .= str_repeat(' ', $indent * 4).sprintf("%s(?P<%s>%s)\n", preg_quote($token[1], '#'), $token[3], $token[2]);
- }
- }
- }
- while (--$indent) {
- $regex .= str_repeat(' ', $indent * 4).")?\n";
+ $regexp = '';
+ for ($i = 0, $nbToken = count($tokens); $i < $nbToken; $i++) {
+ $regexp .= $this->computeRegexp($tokens, $i, $firstOptional);
}
return new CompiledRoute(
$route,
'text' === $tokens[0][0] ? $tokens[0][1] : '',
- sprintf("#^\n%s$#xs", $regex),
+ sprintf("#^%s$#s", $regexp),
array_reverse($tokens),
$variables
);
}
+
+ /**
+ * Computes the regexp used to match the token.
+ *
+ * @param array $tokens The route tokens
+ * @param integer $index The index of the current token
+ * @param integer $firstOptional The index of the first optional token
+ *
+ * @return string The regexp
+ */
+ private function computeRegexp(array $tokens, $index, $firstOptional)
+ {
+ $token = $tokens[$index];
+ if('text' === $token[0]) {
+ // Text tokens
+ return preg_quote($token[1], '#');
+ } else {
+ // Variable tokens
+ if (0 === $index && 0 === $firstOptional && 1 == count($tokens)) {
+ // When the only token is an optional variable token, the separator is required
+ return sprintf('%s(?P<%s>%s)?', preg_quote($token[1], '#'), $token[3], $token[2]);
+ } else {
+ $nbTokens = count($tokens);
+ $regexp = sprintf('%s(?P<%s>%s)', preg_quote($token[1], '#'), $token[3], $token[2]);
+ if ($index >= $firstOptional) {
+ // Enclose each optional tokens in a subpattern to make it optional
+ $regexp = "(?:$regexp";
+ if ($nbTokens - 1 == $index) {
+ // Close the optional subpatterns
+ $regexp .= str_repeat(")?", $nbTokens - $firstOptional);
+ }
+ }
+ return $regexp;
+ }
+ }
+ }
}
@@ -53,6 +53,10 @@ RewriteRule .* app.php [QSA,L,E=_ROUTING__route:baz5,E=_ROUTING_foo:%1]
RewriteCond %{REQUEST_URI} ^/test/baz$
RewriteRule .* app.php [QSA,L,E=_ROUTING__route:baz6,E=_ROUTING_foo:bar\ baz]
+# baz7
+RewriteCond %{REQUEST_URI} ^/te\ st/baz$
+RewriteRule .* app.php [QSA,L,E=_ROUTING__route:baz7]
+
# 405 Method Not Allowed
RewriteCond %{_ROUTING__allow_GET} !-z [OR]
RewriteCond %{_ROUTING__allow_HEAD} !-z [OR]
@@ -26,12 +26,12 @@ public function match($pathinfo)
$pathinfo = urldecode($pathinfo);
// foo
- if (0 === strpos($pathinfo, '/foo') && preg_match('#^/foo/(?P<bar>baz|symfony)$#xs', $pathinfo, $matches)) {
+ if (0 === strpos($pathinfo, '/foo') && preg_match('#^/foo/(?P<bar>baz|symfony)$#s', $pathinfo, $matches)) {
return array_merge($this->mergeDefaults($matches, array ( 'def' => 'test',)), array('_route' => 'foo'));
}
// bar
- if (0 === strpos($pathinfo, '/bar') && preg_match('#^/bar/(?P<foo>[^/]+?)$#xs', $pathinfo, $matches)) {
+ if (0 === strpos($pathinfo, '/bar') && preg_match('#^/bar/(?P<foo>[^/]+?)$#s', $pathinfo, $matches)) {
if (!in_array($this->context->getMethod(), array('GET', 'HEAD'))) {
$allow = array_merge($allow, array('GET', 'HEAD'));
goto not_bar;
@@ -42,7 +42,7 @@ public function match($pathinfo)
not_bar:
// barhead
- if (0 === strpos($pathinfo, '/barhead') && preg_match('#^/barhead/(?P<foo>[^/]+?)$#xs', $pathinfo, $matches)) {
+ if (0 === strpos($pathinfo, '/barhead') && preg_match('#^/barhead/(?P<foo>[^/]+?)$#s', $pathinfo, $matches)) {
if (!in_array($this->context->getMethod(), array('GET', 'HEAD'))) {
$allow = array_merge($allow, array('GET', 'HEAD'));
goto not_barhead;
@@ -68,13 +68,13 @@ public function match($pathinfo)
}
// baz4
- if (0 === strpos($pathinfo, '/test') && preg_match('#^/test/(?P<foo>[^/]+?)/$#xs', $pathinfo, $matches)) {
+ if (0 === strpos($pathinfo, '/test') && preg_match('#^/test/(?P<foo>[^/]+?)/$#s', $pathinfo, $matches)) {
$matches['_route'] = 'baz4';
return $matches;
}
// baz5
- if (0 === strpos($pathinfo, '/test') && preg_match('#^/test/(?P<foo>[^/]+?)/$#xs', $pathinfo, $matches)) {
+ if (0 === strpos($pathinfo, '/test') && preg_match('#^/test/(?P<foo>[^/]+?)/$#s', $pathinfo, $matches)) {
if ($this->context->getMethod() != 'POST') {
$allow[] = 'POST';
goto not_baz5;
@@ -85,7 +85,7 @@ public function match($pathinfo)
not_baz5:
// baz.baz6
- if (0 === strpos($pathinfo, '/test') && preg_match('#^/test/(?P<foo>[^/]+?)/$#xs', $pathinfo, $matches)) {
+ if (0 === strpos($pathinfo, '/test') && preg_match('#^/test/(?P<foo>[^/]+?)/$#s', $pathinfo, $matches)) {
if ($this->context->getMethod() != 'PUT') {
$allow[] = 'PUT';
goto not_bazbaz6;
@@ -101,33 +101,38 @@ public function match($pathinfo)
}
// quoter
- if (preg_match('#^/(?P<quoter>[\']+)$#xs', $pathinfo, $matches)) {
+ if (preg_match('#^/(?P<quoter>[\']+)$#s', $pathinfo, $matches)) {
$matches['_route'] = 'quoter';
return $matches;
}
+ // space
+ if ($pathinfo === '/spa ce') {
+ return array('_route' => 'space');
+ }
+
if (0 === strpos($pathinfo, '/a')) {
if (0 === strpos($pathinfo, '/a/b\'b')) {
// foo1
- if (preg_match('#^/a/b\'b/(?P<foo>[^/]+?)$#xs', $pathinfo, $matches)) {
+ if (preg_match('#^/a/b\'b/(?P<foo>[^/]+?)$#s', $pathinfo, $matches)) {
$matches['_route'] = 'foo1';
return $matches;
}
// bar1
- if (preg_match('#^/a/b\'b/(?P<bar>[^/]+?)$#xs', $pathinfo, $matches)) {
+ if (preg_match('#^/a/b\'b/(?P<bar>[^/]+?)$#s', $pathinfo, $matches)) {
$matches['_route'] = 'bar1';
return $matches;
}
// foo2
- if (preg_match('#^/a/b\'b/(?P<foo1>[^/]+?)$#xs', $pathinfo, $matches)) {
+ if (preg_match('#^/a/b\'b/(?P<foo1>[^/]+?)$#s', $pathinfo, $matches)) {
$matches['_route'] = 'foo2';
return $matches;
}
// bar2
- if (preg_match('#^/a/b\'b/(?P<bar1>[^/]+?)$#xs', $pathinfo, $matches)) {
+ if (preg_match('#^/a/b\'b/(?P<bar1>[^/]+?)$#s', $pathinfo, $matches)) {
$matches['_route'] = 'bar2';
return $matches;
}
@@ -145,21 +150,21 @@ public function match($pathinfo)
}
// foo4
- if (preg_match('#^/aba/(?P<foo>[^/]+?)$#xs', $pathinfo, $matches)) {
+ if (preg_match('#^/aba/(?P<foo>[^/]+?)$#s', $pathinfo, $matches)) {
$matches['_route'] = 'foo4';
return $matches;
}
}
// foo3
- if (preg_match('#^/(?P<_locale>[^/]+?)/b/(?P<foo>[^/]+?)$#xs', $pathinfo, $matches)) {
+ if (preg_match('#^/(?P<_locale>[^/]+?)/b/(?P<foo>[^/]+?)$#s', $pathinfo, $matches)) {
$matches['_route'] = 'foo3';
return $matches;
}
// bar3
- if (preg_match('#^/(?P<_locale>[^/]+?)/b/(?P<bar>[^/]+?)$#xs', $pathinfo, $matches)) {
+ if (preg_match('#^/(?P<_locale>[^/]+?)/b/(?P<bar>[^/]+?)$#s', $pathinfo, $matches)) {
$matches['_route'] = 'bar3';
return $matches;
}
Oops, something went wrong.

0 comments on commit 6465a69

Please sign in to comment.