Skip to content

Commit

Permalink
[Routing] Changing the _method route requirement to be a regular expr…
Browse files Browse the repository at this point in the history
…ession so that it's consistent with all other requirements.

Unlike all other requirements, the _method regex requirement is case-insensitive.
  • Loading branch information
weaverryan authored and fabpot committed Nov 27, 2010
1 parent acb977a commit 739ebf9
Show file tree
Hide file tree
Showing 10 changed files with 31 additions and 26 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -61,8 +61,9 @@ public function dump(array $options = array())
$variables = implode(',', $variables);

$conditions = array();
foreach ((array) $route->getRequirement('_method') as $method) {
$conditions[] = sprintf('RewriteCond %%{REQUEST_METHOD} =%s', strtoupper($method));
if ($req = $route->getRequirement('_method'))
{
$conditions[] = sprintf('RewriteCond %%{REQUEST_METHOD} ^(%s) [NC]', $req);
}

$conditions = count($conditions) ? implode(" [OR]\n", $conditions)."\n" : '';
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -57,9 +57,7 @@ protected function addMatcher()
$conditions = array();

if ($req = $route->getRequirement('_method')) {
$req = array_map('strtolower', (array) $req);

$conditions[] = sprintf("isset(\$this->context['method']) && in_array(strtolower(\$this->context['method']), %s)", str_replace("\n", '', var_export($req, true)));
$conditions[] = sprintf("isset(\$this->context['method']) && preg_match('#^(%s)$#xi', \$this->context['method'])", $req);
}

if ($compiledRoute->getStaticPrefix()) {
Expand Down
3 changes: 2 additions & 1 deletion src/Symfony/Component/Routing/Matcher/UrlMatcher.php
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,8 @@ public function match($url)
$compiledRoute = $route->compile();

// check HTTP method requirement
if (isset($this->context['method']) && (($req = $route->getRequirement('_method')) && !in_array(strtolower($this->context['method']), array_map('strtolower', (array) $req)))) {

if (isset($this->context['method']) && (($req = $route->getRequirement('_method')) && !preg_match(sprintf('#^(%s)$#xi', $req), $this->context['method']))) {
continue;
}

Expand Down
18 changes: 10 additions & 8 deletions src/Symfony/Component/Routing/Route.php
Original file line number Diff line number Diff line change
Expand Up @@ -193,14 +193,16 @@ public function setRequirements(array $requirements)
{
$this->requirements = array();
foreach ($requirements as $key => $regex) {
if (!is_array($regex)) {
if ('^' == $regex[0]) {
$regex = substr($regex, 1);
}

if ('$' == substr($regex, -1)) {
$regex = substr($regex, 0, -1);
}
if (is_array($regex)) {
throw new \InvalidArgumentException(sprintf('Routing requirements must be a string, array given for "%s"', $key));
}

if ('^' == $regex[0]) {
$regex = substr($regex, 1);
}

if ('$' == substr($regex, -1)) {
$regex = substr($regex, 0, -1);
}

$this->requirements[$key] = $regex;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
RewriteCond %{PATH_INFO} ^/foo/(baz|symfony)$
RewriteRule .* app.php [QSA,L,E=_ROUTING__route:foo,E=_ROUTING_bar:%1,E=_ROUTING_def:test]

RewriteCond %{REQUEST_METHOD} =GET [OR]
RewriteCond %{REQUEST_METHOD} =HEAD
RewriteCond %{REQUEST_METHOD} ^(GET|head) [NC]
RewriteCond %{PATH_INFO} ^/bar/([^/\.]+?)$
RewriteRule .* app.php [QSA,L,E=_ROUTING__route:bar,E=_ROUTING_foo:%1]
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ public function match($url)
return array_merge($this->mergeDefaults($matches, array ( 'def' => 'test',)), array('_route' => 'foo'));
}

if (isset($this->context['method']) && in_array(strtolower($this->context['method']), array ( 0 => 'get', 1 => 'head',)) && 0 === strpos($url, '/bar') && preg_match('#^/bar/(?P<foo>[^/\.]+?)$#x', $url, $matches)) {
if (isset($this->context['method']) && preg_match('#^(GET|head)$#xi', $this->context['method']) && 0 === strpos($url, '/bar') && preg_match('#^/bar/(?P<foo>[^/\.]+?)$#x', $url, $matches)) {
return array_merge($this->mergeDefaults($matches, array ()), array('_route' => 'bar'));
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,15 +28,15 @@ public function testDump()
{
$collection = new RouteCollection();

$collection->addRoute('foo', new Route(
$collection->add('foo', new Route(
'/foo/:bar',
array('def' => 'test'),
array('bar' => 'baz|symfony')
));
$collection->addRoute('bar', new Route(
$collection->add('bar', new Route(
'/bar/:foo',
array(),
array('_method' => array('GET', 'HEAD'))
array('_method' => 'GET|head')
));

$dumper = new ApacheMatcherDumper($collection);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,15 +28,15 @@ public function testDump()
{
$collection = new RouteCollection();

$collection->addRoute('foo', new Route(
$collection->add('foo', new Route(
'/foo/:bar',
array('def' => 'test'),
array('bar' => 'baz|symfony')
));
$collection->addRoute('bar', new Route(
$collection->add('bar', new Route(
'/bar/:foo',
array(),
array('_method' => array('GET', 'HEAD'))
array('_method' => 'GET|head')
));
$dumper = new PhpMatcherDumper($collection);
$this->assertStringEqualsFile(self::$fixturesPath.'/dumper/url_matcher1.php', $dumper->dump(), '->dump() dumps basic routes to the correct PHP file.');
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,20 +34,20 @@ public function testMatch()
{
// test the patterns are matched are parameters are returned
$collection = new RouteCollection();
$collection->addRoute('foo', new Route('/foo/:bar'));
$collection->add('foo', new Route('/foo/:bar'));
$matcher = new UrlMatcher($collection, array(), array());
$this->assertEquals(false, $matcher->match('/no-match'));
$this->assertEquals(array('_route' => 'foo', 'bar' => 'baz'), $matcher->match('/foo/baz'));

// test that defaults are merged
$collection = new RouteCollection();
$collection->addRoute('foo', new Route('/foo/:bar', array('def' => 'test')));
$collection->add('foo', new Route('/foo/:bar', array('def' => 'test')));
$matcher = new UrlMatcher($collection, array(), array());
$this->assertEquals(array('_route' => 'foo', 'bar' => 'baz', 'def' => 'test'), $matcher->match('/foo/baz'));

// test that route "metod" is ignore if no method is given in the context
$collection = new RouteCollection();
$collection->addRoute('foo', new Route('/foo', array(), array('_method' => array('GET', 'HEAD'))));
$collection->add('foo', new Route('/foo', array(), array('_method' => 'GET|head')));

// route matches with no context
$matcher = new UrlMatcher($collection, array(), array());
Expand Down
4 changes: 4 additions & 0 deletions tests/Symfony/Tests/Component/Routing/RouteTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,10 @@ public function testRequirements()
$route->setRequirements(array('foo' => '^\d+$'));
$this->assertEquals('\d+', $route->getRequirement('foo'), '->getRequirement() removes ^ and $ from the pattern');
$this->assertEquals($route, $route->setRequirements(array()), '->setRequirements() implements a fluent interface');

// test that an array requirement throws an exception
$this->setExpectedException('InvalidArgumentException');
$route->setRequirements(array('foo' => array('bar', 'baz')));
}

public function testCompile()
Expand Down

0 comments on commit 739ebf9

Please sign in to comment.