From bd194b51e5e0082b217719d77cb6a4750d317032 Mon Sep 17 00:00:00 2001
From: nate <nate@cakephp.org>
Date: Mon, 20 Aug 2007 13:09:11 +0000
Subject: [PATCH] Routing fixes for invalid null value matching and named
argument handling (Ticket #3083, #3085, #3086, #3093). Thanks biesbjerg for
all your help!
git-svn-id: https://svn.cakephp.org/repo/branches/1.2.x.x@5554 3807eeeb-6ff5-0310-8944-8be069107fe0
---
cake/libs/router.php | 104 ++++++----------
cake/libs/set.php | 7 +-
cake/libs/view/helpers/paginator.php | 2 +-
cake/tests/cases/libs/router.test.php | 112 ++++++++++++++----
.../libs/view/helpers/paginator.test.php | 1 +
5 files changed, 132 insertions(+), 94 deletions(-)
diff --git a/cake/libs/router.php b/cake/libs/router.php
index aecb7d96e..21b456f23 100644
--- a/cake/libs/router.php
+++ b/cake/libs/router.php
@@ -157,13 +157,6 @@ class Router extends Object {
* @access private
*/
var $__paths = array();
-/**
- * Maintains the mapped elements for array based urls
- *
- * @var array
- * @access private
- */
- var $__mapped = array();
/**
* Keeps Router state to determine if default routes have already been connected
*
@@ -171,22 +164,6 @@ class Router extends Object {
* @access private
*/
var $__defaultsMapped = false;
-/**
- * Initialize the Router object
- *
- */
- function __construct() {
- if (defined('CAKE_ADMIN')) {
- $admin = CAKE_ADMIN;
- if (!empty($admin)) {
- $this->__admin = array(
- '/:' . $admin . '/:controller/:action/*',
- '/^(?:\/(?:(' . $admin . ')(?:\\/([a-zA-Z0-9_\\-\\.\\;\\:]+)(?:\\/([a-zA-Z0-9_\\-\\.\\;\\:]+)(?:[\\/\\?](.*))?)?)?))[\/]*$/',
- array($admin, 'controller', 'action'), array()
- );
- }
- }
- }
/**
* Gets a reference to the Router object instance
*
@@ -648,6 +625,7 @@ class Router extends Object {
function url($url = null, $full = false) {
$_this =& Router::getInstance();
$defaults = $params = array('plugin' => null, 'controller' => null, 'action' => 'index');
+ $admin = Configure::read('Routing.admin');
if (!empty($_this->__params)) {
if (isset($this) && !isset($this->params['requested'])) {
@@ -688,16 +666,16 @@ class Router extends Object {
$url['action'] = 'index';
}
}
- $url = am(array('controller' => $params['controller'], 'plugin' => $params['plugin']), $url);
+ $url = am(array('controller' => $params['controller'], 'plugin' => $params['plugin']), Set::filter($url, true));
if (isset($url['ext'])) {
$extension = '.' . $url['ext'];
+ unset($url['ext']);
}
- if ($admin = Configure::read('Routing.admin')) {
+ if ($admin) {
if (!isset($url[$admin]) && isset($params[$admin])) {
$url[$admin] = true;
- $url['action'] = str_replace("{$admin}_", '', $url['action']);
- } elseif ($admin && isset($url[$admin]) && $url[$admin] == false) {
+ } elseif ($admin && array_key_exists($admin, $url) && !$url[$admin]) {
unset($url[$admin]);
}
}
@@ -705,29 +683,23 @@ class Router extends Object {
foreach ($_this->routes as $route) {
if ($match = $_this->mapRouteElements($route, $url)) {
- list($output, $url) = $match;
- if (strpos($output, '/') === 0) {
- $output = substr($output, 1);
- }
+ $output = trim($match, '/');
+ $url = array();
break;
}
}
$named = $args = array();
- $skip = am(array_keys($_this->__mapped), array('bare', 'action', 'controller', 'plugin', 'ext', '?', '#'));
+ $skip = array('bare', 'action', 'controller', 'plugin', 'ext', '?', '#');
- $_this->__mapped = array();
$keys = array_values(array_diff(array_keys($url), $skip));
$count = count($keys);
+ // Remove this once parsed URL parameters can be inserted into 'pass'
for ($i = 0; $i < $count; $i++) {
if ($i == 0 && is_numeric($keys[$i]) && in_array('id', $keys)) {
$args[0] = $url[$keys[$i]];
} elseif (is_numeric($keys[$i]) || $keys[$i] == 'id') {
$args[] = $url[$keys[$i]];
- } elseif (!empty($path['namedArgs']) && in_array($keys[$i], array_keys($path['namedArgs'])) && !empty($url[$keys[$i]])) {
- $named[] = $keys[$i] . $_this->__argSeparator . $url[$keys[$i]];
- } elseif (!empty($url[$keys[$i]]) || is_numeric($url[$keys[$i]])) {
- $named[] = $keys[$i] . $_this->__argSeparator . $url[$keys[$i]];
}
}
@@ -742,20 +714,15 @@ class Router extends Object {
if ($url['plugin'] == $url['controller']) {
array_shift($urlOut);
}
- if (defined('CAKE_ADMIN') && isset($url[CAKE_ADMIN]) && $url[CAKE_ADMIN]) {
- array_unshift($urlOut, CAKE_ADMIN);
- }
$output = join('/', $urlOut) . '/';
}
- foreach (array('args', 'named') as $var) {
- if (!empty(${$var})) {
- ${$var} = join('/', ${$var});
- if ($output{strlen($output) - 1} != '/') {
- ${$var} = '/'. ${$var};
- }
- $output .= ${$var};
+ if (!empty($args)) {
+ $args = join('/', $args);
+ if ($output{strlen($output) - 1} != '/') {
+ $args = '/'. $args;
}
+ $output .= $args;
}
$output = str_replace('//', '/', $base . '/' . $output);
} else {
@@ -765,12 +732,12 @@ class Router extends Object {
if (empty($url)) {
return $path['here'];
- } elseif ($url{0} == '/') {
+ } elseif (substr($url, 0, 1) == '/') {
$output = $base . $url;
} else {
$output = $base . '/';
- if (defined('CAKE_ADMIN') && isset($params[CAKE_ADMIN])) {
- $output .= CAKE_ADMIN . '/';
+ if ($admin && isset($params[$admin])) {
+ $output .= $admin . '/';
}
if (!empty($params['plugin'])) {
$output .= Inflector::underscore($params['plugin']) . '/';
@@ -798,12 +765,15 @@ class Router extends Object {
*/
function mapRouteElements($route, $url) {
$_this =& Router::getInstance();
- unset($route[3]['prefix']);
+ if (isset($route[3]['prefix'])) {
+ $prefix = $route[3]['prefix'];
+ unset($route[3]['prefix']);
+ }
$pass = array();
$defaults = $route[3];
- $params = Set::diff($url, $defaults);
$routeParams = $route[2];
+ $params = Set::diff($url, $defaults);
foreach ($params as $key => $value) {
if (is_int($key)) {
@@ -817,14 +787,13 @@ class Router extends Object {
return false;
}
if (!empty($params)) {
+ if (array_diff(array_keys($params), $routeParams) != array()) {
+ return false;
+ }
$required = array_diff(array_keys($defaults), array_keys($url));
}
-
- krsort($defaults);
- krsort($url);
-
if (empty($params)) {
- return array(Router::__mapRoute($route, am($url, compact('pass', 'named'))), array());
+ return Router::__mapRoute($route, am($url, compact('pass', 'named', 'prefix')));
} elseif (!empty($routeParams) && !empty($route[3])) {
if (!empty($required)) {
return false;
@@ -834,27 +803,26 @@ class Router extends Object {
return false;
}
}
-
$filled = array_intersect_key($url, array_combine($routeParams, array_keys($routeParams)));
- if (array_keys($filled) != $routeParams) {
+ if (array_diff(array_keys($filled), $routeParams) != array()) {
return false;
}
} else {
if (empty($required) && $defaults['plugin'] == $url['plugin'] && $defaults['controller'] == $url['controller'] && $defaults['action'] == $url['action']) {
- return array(Router::__mapRoute($route, am($url, compact('pass', 'named'))), $url);
+ return Router::__mapRoute($route, am($url, compact('pass', 'named', 'prefix')));
}
return false;
}
if (!empty($route[4])) {
foreach ($route[4] as $key => $reg) {
- if (isset($url[$key]) && !preg_match('/' . $reg . '/', $url[$key])) {
+ if (array_key_exists($key, $url) && !preg_match('/' . $reg . '/', $url[$key])) {
return false;
}
}
}
- return array(Router::__mapRoute($route, am($url, compact('pass', 'named'))), $url);
+ return Router::__mapRoute($route, am($filled, compact('pass', 'named', 'prefix')));
}
/**
* Merges URL parameters into a route string
@@ -866,8 +834,13 @@ class Router extends Object {
*/
function __mapRoute($route, $params = array()) {
$_this =& Router::getInstance();
+
+ if (isset($params['prefix']) && isset($params['action'])) {
+ $params['action'] = str_replace($params['prefix'] . '_', '', $params['action']);
+ unset($params['prefix']);
+ }
+
if (isset($params['pass']) && is_array($params['pass'])) {
- $_this->__mapped = $params['pass'];
$params['pass'] = implode('/', Set::filter($params['pass'], true));
} elseif (!isset($params['pass'])) {
$params['pass'] = '';
@@ -907,11 +880,6 @@ class Router extends Object {
unset($params[$key]);
}
$out = str_replace(':' . $key, $string, $out);
- $_this->__mapped[$key] = $string;
- }
-
- if (substr($out, -1) == '/') {
- $out = substr($out, 0, strlen($out) - 1);
}
return $out;
}
diff --git a/cake/libs/set.php b/cake/libs/set.php
index 43717c8f6..e38c41d92 100644
--- a/cake/libs/set.php
+++ b/cake/libs/set.php
@@ -115,8 +115,7 @@ class Set extends Object {
*/
function filter($var, $isArray = false) {
if (is_array($var) && (!empty($var) || $isArray)) {
- $set = new Set();
- return array_filter($var, array(&$set, 'filter'));
+ return array_filter($var, array('Set', 'filter'));
} else {
if ($var === 0 || $var === '0' || !empty($var)) {
return true;
@@ -531,7 +530,7 @@ class Set extends Object {
}
foreach ($val1 as $key => $val) {
- if (isset($val2[$key]) && $val2[$key] != $val) {
+ if (array_key_exists($key, $val2) && $val2[$key] != $val) {
$out[$key] = $val;
} elseif (!array_key_exists($key, $val2)) {
$out[$key] = $val;
@@ -540,7 +539,7 @@ class Set extends Object {
}
foreach ($val2 as $key => $val) {
- if (!isset($out[$key])) {
+ if (!array_key_exists($key, $out)) {
$out[$key] = $val;
}
}
diff --git a/cake/libs/view/helpers/paginator.php b/cake/libs/view/helpers/paginator.php
index 6aa35f69c..aff876533 100644
--- a/cake/libs/view/helpers/paginator.php
+++ b/cake/libs/view/helpers/paginator.php
@@ -267,7 +267,7 @@ class PaginatorHelper extends AppHelper {
$obj = isset($options['update']) ? 'Ajax' : 'Html';
$url = am(array('page' => $this->current($model)), $url);
- return $this->{$obj}->link($title, $url, $options);
+ return $this->{$obj}->link($title, Set::filter($url, true), $options);
}
/**
* Protected method for generating prev/next links
diff --git a/cake/tests/cases/libs/router.test.php b/cake/tests/cases/libs/router.test.php
index 6c97eb262..182b50217 100644
--- a/cake/tests/cases/libs/router.test.php
+++ b/cake/tests/cases/libs/router.test.php
@@ -36,6 +36,11 @@ uses('router', 'debugger');
*/
class RouterTest extends UnitTestCase {
+ function RouterTest() {
+ parent::UnitTestCase();
+ $this->startTime = getMicrotime();
+ }
+
function setUp() {
$this->router =& Router::getInstance();
}
@@ -130,9 +135,11 @@ class RouterTest extends UnitTestCase {
$this->assertEqual($result, $expected);
$this->router->reload();
+ $this->router->parse('/');
+
$this->router->connect('/:plugin/:id/*', array('controller' => 'posts', 'action' => 'view'), array('id' => $ID));
$result = $this->router->url(array('plugin' => 'cake_plugin', 'controller' => 'posts', 'action' => 'view', 'id' => '1'));
- $expected = '/cake_plugin/1/';
+ $expected = '/cake_plugin/1';
$this->assertEqual($result, $expected);
$result = $this->router->url(array('plugin' => 'cake_plugin', 'controller' => 'posts', 'action' => 'view', 'id' => '1', '0'));
@@ -140,12 +147,16 @@ class RouterTest extends UnitTestCase {
$this->assertEqual($result, $expected);
$this->router->reload();
+ $this->router->parse('/');
+
$this->router->connect('/:controller/:action/:id', array(), array('id' => $ID));
$result = $this->router->url(array('controller' => 'posts', 'action' => 'view', 'id' => '1'));
$expected = '/posts/view/1';
$this->assertEqual($result, $expected);
$this->router->reload();
+ $this->router->parse('/');
+
$this->router->connect('/:controller/:id', array('action' => 'view', 'id' => '1'));
$result = $this->router->url(array('controller' => 'posts', 'action' => 'view', 'id' => '1'));
$expected = '/posts/1';
@@ -170,14 +181,15 @@ class RouterTest extends UnitTestCase {
$this->assertEqual($result, $expected);
$this->router->connect('/view/*', array('controller' => 'posts', 'action' => 'view'));
+ $this->router->promote();
$result = $this->router->url(array('controller' => 'posts', 'action' => 'view', '1'));
$expected = '/view/1';
$this->assertEqual($result, $expected);
Configure::write('Routing.admin', 'admin');
$this->router->reload();
- $this->router->connect('/admin/subscriptions/:action/*', array('controller' => 'subscribe', 'admin' => true));
- Router::setRequestInfo(array(
+ $this->router->connect('/admin/subscriptions/:action/*', array('controller' => 'subscribe', 'admin' => true, 'prefix' => 'admin'));
+ $this->router->setRequestInfo(array(
array(
'pass' => array(), 'action' => 'admin_index', 'plugin' => null, 'controller' => 'subscribe',
'admin' => true, 'url' => array('url' => 'admin/subscriptions/index/page:2'), 'bare' => 0, 'webservices' => ''
@@ -195,7 +207,7 @@ class RouterTest extends UnitTestCase {
$this->assertEqual($result, $expected);
$this->router->reload();
- Router::setRequestInfo(array(
+ $this->router->setRequestInfo(array(
array(
'pass' => array(), 'action' => 'index', 'plugin' => null, 'controller' => 'real_controller_name',
'url' => array('url' => ''), 'bare' => 0, 'webservices' => ''
@@ -214,7 +226,7 @@ class RouterTest extends UnitTestCase {
$this->assertEqual($result, $expected);
$result = $this->router->url(array('action' => 'add'));
- $expected = '/short_controller_name/add/';
+ $expected = '/short_controller_name/add';
$this->assertEqual($result, $expected);
$this->router->reload();
@@ -263,11 +275,12 @@ class RouterTest extends UnitTestCase {
$this->assertEqual($result, $expected);
$result = $this->router->url(array('language' => 'eng', 'controller' => 'pages', 'action' => 'add'));
- $expected = '/eng/pages/add/';
+ $expected = '/eng/pages/add';
$this->assertEqual($result, $expected);
$this->router->reload();
- Router::setRequestInfo(array(
+ $this->router->parse('/');
+ $this->router->setRequestInfo(array(
array(
'pass' => array(), 'action' => 'index', 'plugin' => null, 'controller' => 'users',
'url' => array('url' => 'users'), 'bare' => 0, 'webservices' => ''
@@ -280,12 +293,50 @@ class RouterTest extends UnitTestCase {
));
$result = $this->router->url(array('action' => 'login'));
- $expected = '/users/login/';
- $this->assertEqual($result, $expected);
+ $expected = '/users/login';
+ $this->assertEqual($result, $expected);
+
+ $this->router->reload();
+ $this->router->parse('/');
+ $this->router->connect('/page/*', array('plugin' => null, 'controller' => 'pages', 'action' => 'view'));
+
+ $result = $this->router->url(array('plugin' => 'my_plugin', 'controller' => 'pages', 'action' => 'view', 'my-page'));
+ $expected = '/my_plugin/pages/view/my-page';
+ $this->assertEqual($result, $expected);
+
+ $this->router->reload();
+ $this->router->parse('/');
+ $this->router->connect('/forestillinger/:month/:year/*',
+ array('plugin' => 'shows', 'controller' => 'shows', 'action' => 'calendar'),
+ array('month' => '0[1-9]|1[012]', 'year' => '[12][0-9]{3}')
+ );
+
+ $result = $this->router->url(array('plugin' => 'shows', 'controller' => 'shows', 'action' => 'calendar', 'month' => 10, 'year' => 2007, 'min-forestilling'));
+ $expected = '/forestillinger/10/2007/min-forestilling';
+ $this->assertEqual($result, $expected);
+
+ $this->router->reload();
+ $this->router->parse('/');
+
+ $this->router->connect('/contact/:action', array('plugin' => 'contact', 'controller' => 'contact'));
+ $result = $this->router->url(array('plugin' => 'contact', 'controller' => 'contact', 'action' => 'me'));
+
+ $expected = '/contact/me';
+ $this->assertEqual($result, $expected);
+
+ Configure::write('Routing.admin', 'admin');
+ $this->router->reload();
+ $this->router->parse('/');
+
+ $result = $this->router->url(array('admin' => true, 'controller' => 'users', 'action' => 'login'));
+ $expected = '/admin/users/login';
+ $this->assertEqual($result, $expected);
+
}
function testUrlGenerationWithExtensions() {
$this->router->reload();
+ $this->router->parse('/');
$result = $this->router->url(array('plugin' => null, 'controller' => 'articles', 'action' => 'add', 'id' => null, 'ext' => 'json'));
$expected = '/articles/add.json';
$this->assertEqual($result, $expected);
@@ -307,11 +358,11 @@ class RouterTest extends UnitTestCase {
$this->router->setRequestInfo(array(
array(
'controller' => 'controller', 'action' => 'index', 'form' => array(),
- 'url' => array (), 'bare' => 0, 'webservices' => null, 'plugin' => 'test'
+ 'url' => array(), 'bare' => 0, 'webservices' => null, 'plugin' => 'test'
),
array (
'base' => '/base', 'here' => '/clients/sage/portal/donations', 'webroot' => '/base/',
- 'passedArgs' => array (), 'argSeparator' => ':', 'namedArgs' => array (), 'webservices' => null
+ 'passedArgs' => array(), 'argSeparator' => ':', 'namedArgs' => array (), 'webservices' => null
)
));
@@ -322,7 +373,6 @@ class RouterTest extends UnitTestCase {
function testUrlParsing() {
extract($this->router->getNamedExpressions());
- $this->router->routes = array();
$this->router->connect('/posts/:value/:somevalue/:othervalue/*', array('controller' => 'posts', 'action' => 'view'), array('value','somevalue', 'othervalue'));
$result = $this->router->parse('/posts/2007/08/01/title-of-post-here');
$expected = array('value' => '2007', 'somevalue' => '08', 'othervalue' => '01', 'controller' => 'posts', 'action' => 'view', 'plugin' =>'', 'pass' => array('0' => 'title-of-post-here'));
@@ -373,6 +423,17 @@ class RouterTest extends UnitTestCase {
$result = $this->router->parse('/eng/contact');
$expected = array('pass' => array(), 'language' => 'eng', 'plugin' => 'contact', 'controller' => 'contact', 'action' => 'index');
$this->assertEqual($result, $expected);
+
+ $this->router->reload();
+ $this->router->connect('/forestillinger/:month/:year/*',
+ array('plugin' => 'shows', 'controller' => 'shows', 'action' => 'calendar'),
+ array('month' => '0[1-9]|1[012]', 'year' => '[12][0-9]{3}')
+ );
+
+ $result = $this->router->parse('/forestillinger/10/2007/min-forestilling');
+ $expected = array('pass' => array('min-forestilling'), 'plugin' => 'shows', 'controller' => 'shows', 'action' => 'calendar', 'year' => 2007, 'month' => 10);
+ $this->assertEqual($result, $expected);
+
}
function testAdminRouting() {
@@ -450,7 +511,12 @@ class RouterTest extends UnitTestCase {
}
function testNamedArgsUrlGeneration() {
- Router::setRequestInfo(array(null, array('base' => '/', 'argSeparator' => ':')));
+ $this->router->setRequestInfo(array(null, array('base' => '/', 'argSeparator' => ':')));
+ $this->router->connectNamed(array(
+ 'published' => array('regex' => '[01]'),
+ 'deleted' => array('regex' => '[01]')
+ ));
+ $this->router->parse('/');
$result = $this->router->url(array('controller' => 'posts', 'action' => 'index', 'published' => 1, 'deleted' => 1));
$expected = '/posts/index/published:1/deleted:1';
@@ -463,17 +529,17 @@ class RouterTest extends UnitTestCase {
function testParamsUrlParsing() {
$this->router->routes = array();
- Router::connect('/', array('controller' => 'posts', 'action' => 'index'));
- Router::connect('/view/:user/*', array('controller' => 'posts', 'action' => 'view'), array('user'));
+ $this->router->connect('/', array('controller' => 'posts', 'action' => 'index'));
+ $this->router->connect('/view/:user/*', array('controller' => 'posts', 'action' => 'view'), array('user'));
$result = $this->router->parse('/view/gwoo/');
$expected = array('user' => 'gwoo', 'controller' => 'posts', 'action' => 'view', 'plugin' =>'', 'pass' => array());
$this->assertEqual($result, $expected);
}
function testPagesUrlParsing() {
- $this->router->routes = array();
- Router::connect('/', array('controller' => 'pages', 'action' => 'display', 'home'));
- Router::connect('/pages/*', array('controller' => 'pages', 'action' => 'display'));
+ $this->router->reload();
+ $this->router->connect('/', array('controller' => 'pages', 'action' => 'display', 'home'));
+ $this->router->connect('/pages/*', array('controller' => 'pages', 'action' => 'display'));
$result = $this->router->parse('/');
$expected = array('pass'=>array('home'), 'plugin' => null, 'controller' => 'pages', 'action' => 'display');
@@ -483,9 +549,9 @@ class RouterTest extends UnitTestCase {
$expected = array('pass' => array('home'), 'plugin' => null, 'controller' => 'pages', 'action' => 'display');
$this->assertEqual($result, $expected);
- $this->router->routes = array();
- Router::connect('/', array('controller' => 'posts', 'action' => 'index'));
- Router::connect('/pages/*', array('controller' => 'pages', 'action' => 'display'));
+ $this->router->reload();
+ $this->router->connect('/', array('controller' => 'posts', 'action' => 'index'));
+ $this->router->connect('/pages/*', array('controller' => 'pages', 'action' => 'display'));
$result = $this->router->parse('/pages/contact/');
$expected = array('pass'=>array('contact'), 'plugin'=> null, 'controller'=>'pages', 'action'=>'display');
@@ -519,6 +585,10 @@ class RouterTest extends UnitTestCase {
$expected = array('admin');
$this->assertEqual($result, $expected);
}
+
+ function testTheEnd() {
+ pr(getMicrotime() - $this->startTime);
+ }
}
?>
\ No newline at end of file
diff --git a/cake/tests/cases/libs/view/helpers/paginator.test.php b/cake/tests/cases/libs/view/helpers/paginator.test.php
index 30dd60bee..b8d83a65e 100644
--- a/cake/tests/cases/libs/view/helpers/paginator.test.php
+++ b/cake/tests/cases/libs/view/helpers/paginator.test.php
@@ -83,6 +83,7 @@ class PaginatorTest extends UnitTestCase {
function testSortLinks() {
Router::reload();
+ Router::parse('/');
Router::setRequestInfo(array(
array ('plugin' => null, 'controller' => 'accounts', 'action' => 'index', 'pass' => array(), 'form' => array(), 'url' => array('url' => 'accounts/', 'mod_rewrite' => 'true'), 'bare' => 0),
array ('plugin' => null, 'controller' => null, 'action' => null, 'base' => '/officespace', 'here' => '/officespace/accounts/', 'webroot' => '/officespace/', 'passedArgs' => array())