Skip to content

Commit

Permalink
Fixing issues where route parameters that overlapped could cause rout…
Browse files Browse the repository at this point in the history
…e compilation errors. Tests added. Fixes #565
  • Loading branch information
markstory committed Apr 10, 2010
1 parent 1c5898d commit fbaabad
Show file tree
Hide file tree
Showing 2 changed files with 22 additions and 8 deletions.
15 changes: 7 additions & 8 deletions cake/libs/router.php
Expand Up @@ -1348,35 +1348,34 @@ function _writeRoute() {
return;
}
$route = $this->template;
$names = $replacements = $search = array();
$names = $routeParams = array();
$parsed = preg_quote($this->template, '#');

preg_match_all('#:([A-Za-z0-9_-]+[A-Z0-9a-z])#', $route, $namedElements);
foreach ($namedElements[1] as $i => $name) {
$search = '\\' . $namedElements[0][$i];
if (isset($this->options[$name])) {
$option = null;
if ($name !== 'plugin' && array_key_exists($name, $this->defaults)) {
$option = '?';
}
$slashParam = '/\\' . $namedElements[0][$i];
if (strpos($parsed, $slashParam) !== false) {
$replacements[] = '(?:/(?P<' . $name . '>' . $this->options[$name] . ')' . $option . ')' . $option;
$search[] = $slashParam;
$routeParams[$slashParam] = '(?:/(?P<' . $name . '>' . $this->options[$name] . ')' . $option . ')' . $option;
} else {
$search[] = '\\' . $namedElements[0][$i];
$replacements[] = '(?:(?P<' . $name . '>' . $this->options[$name] . ')' . $option . ')' . $option;
$routeParams[$search] = '(?:(?P<' . $name . '>' . $this->options[$name] . ')' . $option . ')' . $option;
}
} else {
$replacements[] = '(?:(?P<' . $name . '>[^/]+))';
$search[] = '\\' . $namedElements[0][$i];
$routeParams[$search] = '(?:(?P<' . $name . '>[^/]+))';
}
$names[] = $name;
}
if (preg_match('#\/\*$#', $route, $m)) {
$parsed = preg_replace('#/\\\\\*$#', '(?:/(?P<_args_>.*))?', $parsed);
$this->_greedy = true;
}
$parsed = str_replace($search, $replacements, $parsed);
krsort($routeParams);
$parsed = str_replace(array_keys($routeParams), array_values($routeParams), $parsed);
$this->_compiledRoute = '#^' . $parsed . '[/]*$#';
$this->keys = $names;
}
Expand Down
15 changes: 15 additions & 0 deletions cake/tests/cases/libs/router.test.php
Expand Up @@ -2119,6 +2119,21 @@ function testBasicRouteCompiling() {
$this->assertPattern($result, '/test_plugin/posts/edit/5/name:value/nick:name');
}

/**
* test that route parameters that overlap don't cause errors.
*
* @return void
*/
function testRouteParameterOverlap() {
$route =& new CakeRoute('/invoices/add/:idd/:id', array('controller' => 'invoices', 'action' => 'add'));
$result = $route->compile();
$this->assertPattern($result, '/invoices/add/1/3');

$route =& new CakeRoute('/invoices/add/:id/:idd', array('controller' => 'invoices', 'action' => 'add'));
$result = $route->compile();
$this->assertPattern($result, '/invoices/add/1/3');
}

/**
* test compiling routes with keys that have patterns
*
Expand Down

0 comments on commit fbaabad

Please sign in to comment.