Skip to content

Commit

Permalink
merged branch vicb/routingmatcher (PR #4170)
Browse files Browse the repository at this point in the history 
Commits
-------

a196ca0 [Routing] Compiler: remove lazy quantifiers with no effect
8232aa1 [Routing] Compiler: fix in the computing of the segment separators

Discussion
----------

[Routing] Fix the matching process

This PR is based on the PR #3678, #4139.

[![Build Status](https://secure.travis-ci.org/vicb/symfony.png?branch=routingmatcher)](http://travis-ci.org/vicb/symfony)

**The spec**

A pattern is composed of both text and variable segments: `/{variable}-test/{other_variable}`.

A variable segment will match anything until a separator is encountered. The separator is the character following the variable segment when available or preceding the variable otherwise (i.e. at the end of the pattern).

That is:

* the separator is `-` for the `variable`,
* the separator is `/` for the `other_variable`.

*Note: This default matching behavior can be overridden if a requirement is specified for a variable)*

**Fixes**

* The current behavior is to consider booth the preceding and following characters as separators (considering availability),
* The "preceding" separator of the first variable is always set to `/` whatever the preceding character is (due to `$pos = 0` for the first iteration).

**Todo**

Update the doc once this is merged
  • Loading branch information
@fabpot
fabpot committed May 7, 2012
2 parents ff7c475 + a196ca0 commit cc85a6e
Show file tree
Hide file tree
Showing 6 changed files with 74 additions and 63 deletions.
14 changes: 8 additions & 6 deletions src/Symfony/Component/Routing/RouteCompiler.php
View file
Expand Up @@ -26,6 +26,8 @@ class RouteCompiler implements RouteCompilerInterface
* @param Route $route A Route instance
*
* @return CompiledRoute A CompiledRoute instance
*
* @throws \LogicException If a variable is referenced more than once
*/
public function compile(Route $route)
{
Expand All @@ -34,22 +36,22 @@ public function compile(Route $route)
$tokens = array();
$variables = array();
$pos = 0;
preg_match_all('#.\{([\w\d_]+)\}#', $pattern, $matches, PREG_OFFSET_CAPTURE | PREG_SET_ORDER);
preg_match_all('#.\{(\w+)\}#', $pattern, $matches, PREG_OFFSET_CAPTURE | PREG_SET_ORDER);
foreach ($matches as $match) {
if ($text = substr($pattern, $pos, $match[0][1] - $pos)) {
$tokens[] = array('text', $text);
}
$seps = array($pattern[$pos]);

$pos = $match[0][1] + strlen($match[0][0]);
$var = $match[1][0];

if ($req = $route->getRequirement($var)) {
$regexp = $req;
} else {
if ($pos !== $len) {
$seps[] = $pattern[$pos];
}
$regexp = sprintf('[^%s]+?', preg_quote(implode('', array_unique($seps)), self::REGEX_DELIMITER));
// Use the character following the variable as the separator when available
// Use the character preceding the variable otherwise
$separator = $pos !== $len ? $pattern[$pos] : $match[0][0][0];
$regexp = sprintf('[^%s]+', preg_quote($separator, self::REGEX_DELIMITER));
}

$tokens[] = array('variable', $match[0][0][0], $regexp, $var);
Expand Down
22 changes: 11 additions & 11 deletions src/Symfony/Component/Routing/Tests/Fixtures/dumper/url_matcher1.apache
View file
Expand Up @@ -7,17 +7,17 @@ RewriteCond %{REQUEST_URI} ^/foo/(baz|symfony)$
RewriteRule .* app.php [QSA,L,E=_ROUTING__route:foo,E=_ROUTING_bar:%1,E=_ROUTING_def:test]

# bar
RewriteCond %{REQUEST_URI} ^/bar/([^/]+?)$
RewriteCond %{REQUEST_URI} ^/bar/([^/]+)$
RewriteCond %{REQUEST_METHOD} !^(GET|HEAD)$ [NC]
RewriteRule .* - [S=1,E=_ROUTING__allow_GET:1,E=_ROUTING__allow_HEAD:1]
RewriteCond %{REQUEST_URI} ^/bar/([^/]+?)$
RewriteCond %{REQUEST_URI} ^/bar/([^/]+)$
RewriteRule .* app.php [QSA,L,E=_ROUTING__route:bar,E=_ROUTING_foo:%1]

# baragain
RewriteCond %{REQUEST_URI} ^/baragain/([^/]+?)$
RewriteCond %{REQUEST_URI} ^/baragain/([^/]+)$
RewriteCond %{REQUEST_METHOD} !^(GET|POST|HEAD)$ [NC]
RewriteRule .* - [S=1,E=_ROUTING__allow_GET:1,E=_ROUTING__allow_POST:1,E=_ROUTING__allow_HEAD:1]
RewriteCond %{REQUEST_URI} ^/baragain/([^/]+?)$
RewriteCond %{REQUEST_URI} ^/baragain/([^/]+)$
RewriteRule .* app.php [QSA,L,E=_ROUTING__route:baragain,E=_ROUTING_foo:%1]

# baz
Expand All @@ -35,25 +35,25 @@ RewriteCond %{REQUEST_URI} ^/test/baz3/$
RewriteRule .* app.php [QSA,L,E=_ROUTING__route:baz3]

# baz4
RewriteCond %{REQUEST_URI} ^/test/([^/]+?)$
RewriteCond %{REQUEST_URI} ^/test/([^/]+)$
RewriteRule .* $0/ [QSA,L,R=301]
RewriteCond %{REQUEST_URI} ^/test/([^/]+?)/$
RewriteCond %{REQUEST_URI} ^/test/([^/]+)/$
RewriteRule .* app.php [QSA,L,E=_ROUTING__route:baz4,E=_ROUTING_foo:%1]

# baz5
RewriteCond %{REQUEST_URI} ^/test/([^/]+?)/$
RewriteCond %{REQUEST_URI} ^/test/([^/]+)/$
RewriteCond %{REQUEST_METHOD} !^(GET|HEAD)$ [NC]
RewriteRule .* - [S=2,E=_ROUTING__allow_GET:1,E=_ROUTING__allow_HEAD:1]
RewriteCond %{REQUEST_URI} ^/test/([^/]+?)$
RewriteCond %{REQUEST_URI} ^/test/([^/]+)$
RewriteRule .* $0/ [QSA,L,R=301]
RewriteCond %{REQUEST_URI} ^/test/([^/]+?)/$
RewriteCond %{REQUEST_URI} ^/test/([^/]+)/$
RewriteRule .* app.php [QSA,L,E=_ROUTING__route:baz5,E=_ROUTING_foo:%1]

# baz5unsafe
RewriteCond %{REQUEST_URI} ^/testunsafe/([^/]+?)/$
RewriteCond %{REQUEST_URI} ^/testunsafe/([^/]+)/$
RewriteCond %{REQUEST_METHOD} !^(POST)$ [NC]
RewriteRule .* - [S=1,E=_ROUTING__allow_POST:1]
RewriteCond %{REQUEST_URI} ^/testunsafe/([^/]+?)/$
RewriteCond %{REQUEST_URI} ^/testunsafe/([^/]+)/$
RewriteRule .* app.php [QSA,L,E=_ROUTING__route:baz5unsafe,E=_ROUTING_foo:%1]

# baz6
Expand Down
30 changes: 15 additions & 15 deletions src/Symfony/Component/Routing/Tests/Fixtures/dumper/url_matcher1.php
View file
Expand Up @@ -31,7 +31,7 @@ public function match($pathinfo)
}

// bar
if (0 === strpos($pathinfo, '/bar') && preg_match('#^/bar/(?<foo>[^/]+?)$#s', $pathinfo, $matches)) {
if (0 === strpos($pathinfo, '/bar') && preg_match('#^/bar/(?<foo>[^/]+)$#s', $pathinfo, $matches)) {
if (!in_array($this->context->getMethod(), array('GET', 'HEAD'))) {
$allow = array_merge($allow, array('GET', 'HEAD'));
goto not_bar;
Expand All @@ -44,7 +44,7 @@ public function match($pathinfo)
not_bar:

// barhead
if (0 === strpos($pathinfo, '/barhead') && preg_match('#^/barhead/(?<foo>[^/]+?)$#s', $pathinfo, $matches)) {
if (0 === strpos($pathinfo, '/barhead') && preg_match('#^/barhead/(?<foo>[^/]+)$#s', $pathinfo, $matches)) {
if (!in_array($this->context->getMethod(), array('GET', 'HEAD'))) {
$allow = array_merge($allow, array('GET', 'HEAD'));
goto not_barhead;
Expand Down Expand Up @@ -72,14 +72,14 @@ public function match($pathinfo)
}

// baz4
if (0 === strpos($pathinfo, '/test') && preg_match('#^/test/(?<foo>[^/]+?)/$#s', $pathinfo, $matches)) {
if (0 === strpos($pathinfo, '/test') && preg_match('#^/test/(?<foo>[^/]+)/$#s', $pathinfo, $matches)) {
$matches['_route'] = 'baz4';

return $matches;
}

// baz5
if (0 === strpos($pathinfo, '/test') && preg_match('#^/test/(?<foo>[^/]+?)/$#s', $pathinfo, $matches)) {
if (0 === strpos($pathinfo, '/test') && preg_match('#^/test/(?<foo>[^/]+)/$#s', $pathinfo, $matches)) {
if ($this->context->getMethod() != 'POST') {
$allow[] = 'POST';
goto not_baz5;
Expand All @@ -92,7 +92,7 @@ public function match($pathinfo)
not_baz5:

// baz.baz6
if (0 === strpos($pathinfo, '/test') && preg_match('#^/test/(?<foo>[^/]+?)/$#s', $pathinfo, $matches)) {
if (0 === strpos($pathinfo, '/test') && preg_match('#^/test/(?<foo>[^/]+)/$#s', $pathinfo, $matches)) {
if ($this->context->getMethod() != 'PUT') {
$allow[] = 'PUT';
goto not_bazbaz6;
Expand Down Expand Up @@ -124,14 +124,14 @@ public function match($pathinfo)
if (0 === strpos($pathinfo, '/a')) {
if (0 === strpos($pathinfo, '/a/b\'b')) {
// foo1
if (preg_match('#^/a/b\'b/(?<foo>[^/]+?)$#s', $pathinfo, $matches)) {
if (preg_match('#^/a/b\'b/(?<foo>[^/]+)$#s', $pathinfo, $matches)) {
$matches['_route'] = 'foo1';

return $matches;
}

// bar1
if (preg_match('#^/a/b\'b/(?<bar>[^/]+?)$#s', $pathinfo, $matches)) {
if (preg_match('#^/a/b\'b/(?<bar>[^/]+)$#s', $pathinfo, $matches)) {
$matches['_route'] = 'bar1';

return $matches;
Expand All @@ -148,14 +148,14 @@ public function match($pathinfo)

if (0 === strpos($pathinfo, '/a/b\'b')) {
// foo2
if (preg_match('#^/a/b\'b/(?<foo1>[^/]+?)$#s', $pathinfo, $matches)) {
if (preg_match('#^/a/b\'b/(?<foo1>[^/]+)$#s', $pathinfo, $matches)) {
$matches['_route'] = 'foo2';

return $matches;
}

// bar2
if (preg_match('#^/a/b\'b/(?<bar1>[^/]+?)$#s', $pathinfo, $matches)) {
if (preg_match('#^/a/b\'b/(?<bar1>[^/]+)$#s', $pathinfo, $matches)) {
$matches['_route'] = 'bar2';

return $matches;
Expand All @@ -167,7 +167,7 @@ public function match($pathinfo)

if (0 === strpos($pathinfo, '/multi')) {
// helloWorld
if (0 === strpos($pathinfo, '/multi/hello') && preg_match('#^/multi/hello(?:/(?<who>[^/]+?))?$#s', $pathinfo, $matches)) {
if (0 === strpos($pathinfo, '/multi/hello') && preg_match('#^/multi/hello(?:/(?<who>[^/]+))?$#s', $pathinfo, $matches)) {
return array_merge($this->mergeDefaults($matches, array ( 'who' => 'World!',)), array('_route' => 'helloWorld'));
}

Expand All @@ -184,14 +184,14 @@ public function match($pathinfo)
}

// foo3
if (preg_match('#^/(?<_locale>[^/]+?)/b/(?<foo>[^/]+?)$#s', $pathinfo, $matches)) {
if (preg_match('#^/(?<_locale>[^/]+)/b/(?<foo>[^/]+)$#s', $pathinfo, $matches)) {
$matches['_route'] = 'foo3';

return $matches;
}

// bar3
if (preg_match('#^/(?<_locale>[^/]+?)/b/(?<bar>[^/]+?)$#s', $pathinfo, $matches)) {
if (preg_match('#^/(?<_locale>[^/]+)/b/(?<bar>[^/]+)$#s', $pathinfo, $matches)) {
$matches['_route'] = 'bar3';

return $matches;
Expand All @@ -203,7 +203,7 @@ public function match($pathinfo)
}

// foo4
if (0 === strpos($pathinfo, '/aba') && preg_match('#^/aba/(?<foo>[^/]+?)$#s', $pathinfo, $matches)) {
if (0 === strpos($pathinfo, '/aba') && preg_match('#^/aba/(?<foo>[^/]+)$#s', $pathinfo, $matches)) {
$matches['_route'] = 'foo4';

return $matches;
Expand All @@ -217,14 +217,14 @@ public function match($pathinfo)

if (0 === strpos($pathinfo, '/a/b')) {
// b
if (preg_match('#^/a/b/(?<var>[^/]+?)$#s', $pathinfo, $matches)) {
if (preg_match('#^/a/b/(?<var>[^/]+)$#s', $pathinfo, $matches)) {
$matches['_route'] = 'b';

return $matches;
}

// c
if (0 === strpos($pathinfo, '/a/b/c') && preg_match('#^/a/b/c/(?<var>[^/]+?)$#s', $pathinfo, $matches)) {
if (0 === strpos($pathinfo, '/a/b/c') && preg_match('#^/a/b/c/(?<var>[^/]+)$#s', $pathinfo, $matches)) {
$matches['_route'] = 'c';

return $matches;
Expand Down
30 changes: 15 additions & 15 deletions src/Symfony/Component/Routing/Tests/Fixtures/dumper/url_matcher2.php
View file
Expand Up @@ -31,7 +31,7 @@ public function match($pathinfo)
}

// bar
if (0 === strpos($pathinfo, '/bar') && preg_match('#^/bar/(?<foo>[^/]+?)$#s', $pathinfo, $matches)) {
if (0 === strpos($pathinfo, '/bar') && preg_match('#^/bar/(?<foo>[^/]+)$#s', $pathinfo, $matches)) {
if (!in_array($this->context->getMethod(), array('GET', 'HEAD'))) {
$allow = array_merge($allow, array('GET', 'HEAD'));
goto not_bar;
Expand All @@ -44,7 +44,7 @@ public function match($pathinfo)
not_bar:

// barhead
if (0 === strpos($pathinfo, '/barhead') && preg_match('#^/barhead/(?<foo>[^/]+?)$#s', $pathinfo, $matches)) {
if (0 === strpos($pathinfo, '/barhead') && preg_match('#^/barhead/(?<foo>[^/]+)$#s', $pathinfo, $matches)) {
if (!in_array($this->context->getMethod(), array('GET', 'HEAD'))) {
$allow = array_merge($allow, array('GET', 'HEAD'));
goto not_barhead;
Expand Down Expand Up @@ -76,7 +76,7 @@ public function match($pathinfo)
}

// baz4
if (0 === strpos($pathinfo, '/test') && preg_match('#^/test/(?<foo>[^/]+?)/?$#s', $pathinfo, $matches)) {
if (0 === strpos($pathinfo, '/test') && preg_match('#^/test/(?<foo>[^/]+)/?$#s', $pathinfo, $matches)) {
if (substr($pathinfo, -1) !== '/') {
return $this->redirect($pathinfo.'/', 'baz4');
}
Expand All @@ -87,7 +87,7 @@ public function match($pathinfo)
}

// baz5
if (0 === strpos($pathinfo, '/test') && preg_match('#^/test/(?<foo>[^/]+?)/$#s', $pathinfo, $matches)) {
if (0 === strpos($pathinfo, '/test') && preg_match('#^/test/(?<foo>[^/]+)/$#s', $pathinfo, $matches)) {
if ($this->context->getMethod() != 'POST') {
$allow[] = 'POST';
goto not_baz5;
Expand All @@ -100,7 +100,7 @@ public function match($pathinfo)
not_baz5:

// baz.baz6
if (0 === strpos($pathinfo, '/test') && preg_match('#^/test/(?<foo>[^/]+?)/$#s', $pathinfo, $matches)) {
if (0 === strpos($pathinfo, '/test') && preg_match('#^/test/(?<foo>[^/]+)/$#s', $pathinfo, $matches)) {
if ($this->context->getMethod() != 'PUT') {
$allow[] = 'PUT';
goto not_bazbaz6;
Expand Down Expand Up @@ -132,14 +132,14 @@ public function match($pathinfo)
if (0 === strpos($pathinfo, '/a')) {
if (0 === strpos($pathinfo, '/a/b\'b')) {
// foo1
if (preg_match('#^/a/b\'b/(?<foo>[^/]+?)$#s', $pathinfo, $matches)) {
if (preg_match('#^/a/b\'b/(?<foo>[^/]+)$#s', $pathinfo, $matches)) {
$matches['_route'] = 'foo1';

return $matches;
}

// bar1
if (preg_match('#^/a/b\'b/(?<bar>[^/]+?)$#s', $pathinfo, $matches)) {
if (preg_match('#^/a/b\'b/(?<bar>[^/]+)$#s', $pathinfo, $matches)) {
$matches['_route'] = 'bar1';

return $matches;
Expand All @@ -156,14 +156,14 @@ public function match($pathinfo)

if (0 === strpos($pathinfo, '/a/b\'b')) {
// foo2
if (preg_match('#^/a/b\'b/(?<foo1>[^/]+?)$#s', $pathinfo, $matches)) {
if (preg_match('#^/a/b\'b/(?<foo1>[^/]+)$#s', $pathinfo, $matches)) {
$matches['_route'] = 'foo2';

return $matches;
}

// bar2
if (preg_match('#^/a/b\'b/(?<bar1>[^/]+?)$#s', $pathinfo, $matches)) {
if (preg_match('#^/a/b\'b/(?<bar1>[^/]+)$#s', $pathinfo, $matches)) {
$matches['_route'] = 'bar2';

return $matches;
Expand All @@ -175,7 +175,7 @@ public function match($pathinfo)

if (0 === strpos($pathinfo, '/multi')) {
// helloWorld
if (0 === strpos($pathinfo, '/multi/hello') && preg_match('#^/multi/hello(?:/(?<who>[^/]+?))?$#s', $pathinfo, $matches)) {
if (0 === strpos($pathinfo, '/multi/hello') && preg_match('#^/multi/hello(?:/(?<who>[^/]+))?$#s', $pathinfo, $matches)) {
return array_merge($this->mergeDefaults($matches, array ( 'who' => 'World!',)), array('_route' => 'helloWorld'));
}

Expand All @@ -196,14 +196,14 @@ public function match($pathinfo)
}

// foo3
if (preg_match('#^/(?<_locale>[^/]+?)/b/(?<foo>[^/]+?)$#s', $pathinfo, $matches)) {
if (preg_match('#^/(?<_locale>[^/]+)/b/(?<foo>[^/]+)$#s', $pathinfo, $matches)) {
$matches['_route'] = 'foo3';

return $matches;
}

// bar3
if (preg_match('#^/(?<_locale>[^/]+?)/b/(?<bar>[^/]+?)$#s', $pathinfo, $matches)) {
if (preg_match('#^/(?<_locale>[^/]+)/b/(?<bar>[^/]+)$#s', $pathinfo, $matches)) {
$matches['_route'] = 'bar3';

return $matches;
Expand All @@ -215,7 +215,7 @@ public function match($pathinfo)
}

// foo4
if (0 === strpos($pathinfo, '/aba') && preg_match('#^/aba/(?<foo>[^/]+?)$#s', $pathinfo, $matches)) {
if (0 === strpos($pathinfo, '/aba') && preg_match('#^/aba/(?<foo>[^/]+)$#s', $pathinfo, $matches)) {
$matches['_route'] = 'foo4';

return $matches;
Expand All @@ -229,14 +229,14 @@ public function match($pathinfo)

if (0 === strpos($pathinfo, '/a/b')) {
// b
if (preg_match('#^/a/b/(?<var>[^/]+?)$#s', $pathinfo, $matches)) {
if (preg_match('#^/a/b/(?<var>[^/]+)$#s', $pathinfo, $matches)) {
$matches['_route'] = 'b';

return $matches;
}

// c
if (0 === strpos($pathinfo, '/a/b/c') && preg_match('#^/a/b/c/(?<var>[^/]+?)$#s', $pathinfo, $matches)) {
if (0 === strpos($pathinfo, '/a/b/c') && preg_match('#^/a/b/c/(?<var>[^/]+)$#s', $pathinfo, $matches)) {
$matches['_route'] = 'c';

return $matches;
Expand Down
2 changes: 1 addition & 1 deletion src/Symfony/Component/Routing/Tests/Fixtures/dumper/url_matcher3.php
View file
Expand Up @@ -32,7 +32,7 @@ public function match($pathinfo)
}

// dynamic
if (preg_match('#^/rootprefix/(?<var>[^/]+?)$#s', $pathinfo, $matches)) {
if (preg_match('#^/rootprefix/(?<var>[^/]+)$#s', $pathinfo, $matches)) {
$matches['_route'] = 'dynamic';

return $matches;
Expand Down

0 comments on commit cc85a6e

Please sign in to comment.