From 6049009cac01fa0c6e045e57f0b260cdb3cd1600 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jorge=20Gonz=C3=A1lez?= Date: Fri, 7 Mar 2014 19:29:27 +0000 Subject: [PATCH 01/14] improving docblock for Set::extract and Set::classicExtract return types --- lib/Cake/Utility/Set.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/Cake/Utility/Set.php b/lib/Cake/Utility/Set.php index 9c57dc570..8ace38b00 100644 --- a/lib/Cake/Utility/Set.php +++ b/lib/Cake/Utility/Set.php @@ -307,7 +307,7 @@ class Set { * @param string $path An absolute XPath 2.0 path * @param array $data An array of data to extract from * @param array $options Currently only supports 'flatten' which can be disabled for higher XPath-ness - * @return array An array of matched items + * @return array|mixed|null An array of matched items or the content of a single selected item or null in any of these cases: $path or $data are null, no items found. * @link http://book.cakephp.org/2.0/en/core-utility-libraries/set.html#Set::extract */ public static function extract($path, $data = null, $options = array()) { @@ -532,7 +532,7 @@ class Set { * * @param array $data Array from where to extract * @param string|array $path As an array, or as a dot-separated string. - * @return array|null Extracted data or null when $data or $path are empty. + * @return array|mixed|null An array of matched items or the content of a single selected item or null in any of these cases: $path or $data are null, no items found. * @link http://book.cakephp.org/2.0/en/core-utility-libraries/set.html#Set::classicExtract */ public static function classicExtract($data, $path = null) { From 4d510922594736422583191fc2ae0ad0c5096946 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jorge=20Gonz=C3=A1lez?= Date: Sat, 8 Mar 2014 10:26:03 +0000 Subject: [PATCH 02/14] fixing Set docblocks --- lib/Cake/Utility/Set.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/Cake/Utility/Set.php b/lib/Cake/Utility/Set.php index 8ace38b00..8c6bf3c17 100644 --- a/lib/Cake/Utility/Set.php +++ b/lib/Cake/Utility/Set.php @@ -307,7 +307,7 @@ class Set { * @param string $path An absolute XPath 2.0 path * @param array $data An array of data to extract from * @param array $options Currently only supports 'flatten' which can be disabled for higher XPath-ness - * @return array|mixed|null An array of matched items or the content of a single selected item or null in any of these cases: $path or $data are null, no items found. + * @return mixed An array of matched items or the content of a single selected item or null in any of these cases: $path or $data are null, no items found. * @link http://book.cakephp.org/2.0/en/core-utility-libraries/set.html#Set::extract */ public static function extract($path, $data = null, $options = array()) { @@ -532,7 +532,7 @@ class Set { * * @param array $data Array from where to extract * @param string|array $path As an array, or as a dot-separated string. - * @return array|mixed|null An array of matched items or the content of a single selected item or null in any of these cases: $path or $data are null, no items found. + * @return mixed An array of matched items or the content of a single selected item or null in any of these cases: $path or $data are null, no items found. * @link http://book.cakephp.org/2.0/en/core-utility-libraries/set.html#Set::classicExtract */ public static function classicExtract($data, $path = null) { From a77a0f76a16828c047c22a52f52f2ee12ab93f6b Mon Sep 17 00:00:00 2001 From: Kunal Panchal Date: Mon, 10 Mar 2014 14:05:03 +0530 Subject: [PATCH 03/14] Documentation correction TimeHelper::$niceFormat to CakeTime::$niceFormat --- lib/Cake/View/Helper/TimeHelper.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/Cake/View/Helper/TimeHelper.php b/lib/Cake/View/Helper/TimeHelper.php index dfbfe0778..fe81bccd8 100644 --- a/lib/Cake/View/Helper/TimeHelper.php +++ b/lib/Cake/View/Helper/TimeHelper.php @@ -184,7 +184,7 @@ class TimeHelper extends AppHelper { * * @param integer|string|DateTime $dateString UNIX timestamp, strtotime() valid string or DateTime object * @param string|DateTimeZone $timezone User's timezone string or DateTimeZone object - * @param string $format The format to use. If null, `TimeHelper::$niceFormat` is used + * @param string $format The format to use. If null, `CakeTime::$niceFormat` is used * @return string Formatted date string * @link http://book.cakephp.org/2.0/en/core-libraries/helpers/time.html#formatting */ From 00956110f52c243d0f9cade9c03d5162427831fc Mon Sep 17 00:00:00 2001 From: Mike Gibson Date: Mon, 10 Mar 2014 11:43:28 +0000 Subject: [PATCH 04/14] Sort route keys in reverse length order before replacing to prevent incorrect matching --- lib/Cake/Routing/Route/CakeRoute.php | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-) diff --git a/lib/Cake/Routing/Route/CakeRoute.php b/lib/Cake/Routing/Route/CakeRoute.php index cc9b0ff73..a91f509f1 100644 --- a/lib/Cake/Routing/Route/CakeRoute.php +++ b/lib/Cake/Routing/Route/CakeRoute.php @@ -518,7 +518,12 @@ class CakeRoute { $out = $this->template; $search = $replace = array(); - foreach ($this->keys as $key) { + $keys = $this->keys; + + // Sort the keys in reverse order by length to prevent mismatches + uasort($keys, array($this, '_sortKeys')); + + foreach ($keys as $key) { $string = null; if (isset($params[$key])) { $string = $params[$key]; @@ -537,4 +542,15 @@ class CakeRoute { return $out; } + /** + * Comparison method for sorting keys in reverse order by length. + * + * @param $a + * @param $b + * @return int + */ + protected function _sortKeys($a, $b) { + return strlen($a) > strlen($b) ? -1 : 1; + } + } From 12026583960409d6ff8447e60f293dcab07ca56c Mon Sep 17 00:00:00 2001 From: Mike Gibson Date: Mon, 10 Mar 2014 12:03:04 +0000 Subject: [PATCH 05/14] Added a test case --- .../Test/Case/Routing/Route/CakeRouteTest.php | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/lib/Cake/Test/Case/Routing/Route/CakeRouteTest.php b/lib/Cake/Test/Case/Routing/Route/CakeRouteTest.php index 1f0e283ca..16bed4cd6 100644 --- a/lib/Cake/Test/Case/Routing/Route/CakeRouteTest.php +++ b/lib/Cake/Test/Case/Routing/Route/CakeRouteTest.php @@ -855,6 +855,24 @@ class CakeRouteTest extends CakeTestCase { $this->assertEquals($expected, $result); } +/** + * Test matching of parameters where one parameter name starts with another parameter name + * + * @return void + */ + public function testMatchSimilarParameters() { + $route = new CakeRoute('/:thisParam/:thisParamIsLonger'); + + $url = array( + 'thisParam' => 'foo', + 'thisParamIsLonger' => 'bar' + ); + + $result = $route->match($url); + $expected = '/foo/bar'; + $this->assertEquals($expected, $result); + } + /** * test restructuring args with pass key * @@ -941,4 +959,5 @@ class CakeRouteTest extends CakeTestCase { $expected = array('section' => 'weblog', 'plugin' => 'blogs', 'controller' => 'posts', 'action' => 'index', 'pass' => array(), 'named' => array()); $this->assertEquals($expected, $result); } + } From 5e896aa94ae736350add2ac902dace0a8ad42997 Mon Sep 17 00:00:00 2001 From: Mike Gibson Date: Mon, 10 Mar 2014 13:03:04 +0000 Subject: [PATCH 06/14] Removed comparison method --- lib/Cake/Routing/Route/CakeRoute.php | 17 +++-------------- 1 file changed, 3 insertions(+), 14 deletions(-) diff --git a/lib/Cake/Routing/Route/CakeRoute.php b/lib/Cake/Routing/Route/CakeRoute.php index a91f509f1..48539b2d0 100644 --- a/lib/Cake/Routing/Route/CakeRoute.php +++ b/lib/Cake/Routing/Route/CakeRoute.php @@ -518,10 +518,10 @@ class CakeRoute { $out = $this->template; $search = $replace = array(); - $keys = $this->keys; - // Sort the keys in reverse order by length to prevent mismatches - uasort($keys, array($this, '_sortKeys')); + $lengths = array_map('strlen', $this->keys); + $keys = array_combine($lengths, $this->keys); + krsort($keys); foreach ($keys as $key) { $string = null; @@ -542,15 +542,4 @@ class CakeRoute { return $out; } - /** - * Comparison method for sorting keys in reverse order by length. - * - * @param $a - * @param $b - * @return int - */ - protected function _sortKeys($a, $b) { - return strlen($a) > strlen($b) ? -1 : 1; - } - } From 17652575f33a9b1494eb99de3b95a25bdf2e7e37 Mon Sep 17 00:00:00 2001 From: Mike Gibson Date: Mon, 10 Mar 2014 13:09:19 +0000 Subject: [PATCH 07/14] Fixed key sorting --- lib/Cake/Routing/Route/CakeRoute.php | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/lib/Cake/Routing/Route/CakeRoute.php b/lib/Cake/Routing/Route/CakeRoute.php index 48539b2d0..e12bff243 100644 --- a/lib/Cake/Routing/Route/CakeRoute.php +++ b/lib/Cake/Routing/Route/CakeRoute.php @@ -520,8 +520,9 @@ class CakeRoute { $search = $replace = array(); $lengths = array_map('strlen', $this->keys); - $keys = array_combine($lengths, $this->keys); - krsort($keys); + $flipped = array_combine($this->keys, $lengths); + arsort($flipped); + $keys = array_flip($flipped); foreach ($keys as $key) { $string = null; From 9be789d3714b5c48fac940729268c4eeae72cf40 Mon Sep 17 00:00:00 2001 From: Mike Gibson Date: Mon, 10 Mar 2014 13:29:43 +0000 Subject: [PATCH 08/14] Use array_keys instead of array_flip --- lib/Cake/Routing/Route/CakeRoute.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/Cake/Routing/Route/CakeRoute.php b/lib/Cake/Routing/Route/CakeRoute.php index e12bff243..93a7336b2 100644 --- a/lib/Cake/Routing/Route/CakeRoute.php +++ b/lib/Cake/Routing/Route/CakeRoute.php @@ -522,7 +522,7 @@ class CakeRoute { $lengths = array_map('strlen', $this->keys); $flipped = array_combine($this->keys, $lengths); arsort($flipped); - $keys = array_flip($flipped); + $keys = array_keys($flipped); foreach ($keys as $key) { $string = null; From d40452b43d50f11c078c432583c4d67be8bbb9f7 Mon Sep 17 00:00:00 2001 From: Mike Gibson Date: Mon, 10 Mar 2014 14:25:28 +0000 Subject: [PATCH 09/14] Test if keys are empty before sorting --- lib/Cake/Routing/Route/CakeRoute.php | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/lib/Cake/Routing/Route/CakeRoute.php b/lib/Cake/Routing/Route/CakeRoute.php index 93a7336b2..1f7cdaaf0 100644 --- a/lib/Cake/Routing/Route/CakeRoute.php +++ b/lib/Cake/Routing/Route/CakeRoute.php @@ -519,10 +519,14 @@ class CakeRoute { $search = $replace = array(); - $lengths = array_map('strlen', $this->keys); - $flipped = array_combine($this->keys, $lengths); - arsort($flipped); - $keys = array_keys($flipped); + if(empty($this->keys)) { + $keys = array(); + } else { + $lengths = array_map('strlen', $this->keys); + $flipped = array_combine($this->keys, $lengths); + arsort($flipped); + $keys = array_keys($flipped); + } foreach ($keys as $key) { $string = null; From 7205f5ec9c6256f4f52fc1d3d87dc6ccaa105af1 Mon Sep 17 00:00:00 2001 From: Mike Gibson Date: Mon, 10 Mar 2014 15:01:52 +0000 Subject: [PATCH 10/14] Made it slightly neater --- lib/Cake/Routing/Route/CakeRoute.php | 28 ++++++++++++++-------------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/lib/Cake/Routing/Route/CakeRoute.php b/lib/Cake/Routing/Route/CakeRoute.php index 1f7cdaaf0..2284c71aa 100644 --- a/lib/Cake/Routing/Route/CakeRoute.php +++ b/lib/Cake/Routing/Route/CakeRoute.php @@ -517,28 +517,28 @@ class CakeRoute { } $out = $this->template; - $search = $replace = array(); + if(!empty($this->keys)) { + + $search = $replace = array(); - if(empty($this->keys)) { - $keys = array(); - } else { $lengths = array_map('strlen', $this->keys); $flipped = array_combine($this->keys, $lengths); arsort($flipped); $keys = array_keys($flipped); - } - foreach ($keys as $key) { - $string = null; - if (isset($params[$key])) { - $string = $params[$key]; - } elseif (strpos($out, $key) != strlen($out) - strlen($key)) { - $key .= '/'; + foreach ($keys as $key) { + $string = null; + if (isset($params[$key])) { + $string = $params[$key]; + } elseif (strpos($out, $key) != strlen($out) - strlen($key)) { + $key .= '/'; + } + $search[] = ':' . $key; + $replace[] = $string; } - $search[] = ':' . $key; - $replace[] = $string; + $out = str_replace($search, $replace, $out); + } - $out = str_replace($search, $replace, $out); if (strpos($this->template, '*')) { $out = str_replace('*', $params['pass'], $out); From 60319832b505945b637ee1cc467ee7b6fa4a7c96 Mon Sep 17 00:00:00 2001 From: Mike Gibson Date: Mon, 10 Mar 2014 15:28:00 +0000 Subject: [PATCH 11/14] Fixed PHPCS error --- lib/Cake/Routing/Route/CakeRoute.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/Cake/Routing/Route/CakeRoute.php b/lib/Cake/Routing/Route/CakeRoute.php index 2284c71aa..c0327b162 100644 --- a/lib/Cake/Routing/Route/CakeRoute.php +++ b/lib/Cake/Routing/Route/CakeRoute.php @@ -517,7 +517,7 @@ class CakeRoute { } $out = $this->template; - if(!empty($this->keys)) { + if (!empty($this->keys)) { $search = $replace = array(); From 4927cf690177c6e9099319fb9eb79134254a6a54 Mon Sep 17 00:00:00 2001 From: Thomas Smith Date: Mon, 10 Mar 2014 10:16:32 -0700 Subject: [PATCH 12/14] #2994, unnecessary calls to Model::__isset(null) --- lib/Cake/Model/Model.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/Cake/Model/Model.php b/lib/Cake/Model/Model.php index 3cc81c304..c990c87eb 100644 --- a/lib/Cake/Model/Model.php +++ b/lib/Cake/Model/Model.php @@ -1408,7 +1408,7 @@ class Model extends Object implements CakeEventListener { list($model, $column) = explode('.', $column); } - if ($model != $this->alias && isset($this->{$model})) { + if (isset($model) && $model != $this->alias && isset($this->{$model})) { return $this->{$model}->getColumnType($column); } From 0de130711003234923fdd219695decdc838cc7c2 Mon Sep 17 00:00:00 2001 From: Mike Gibson Date: Mon, 10 Mar 2014 18:46:28 +0000 Subject: [PATCH 13/14] Changed if statement to force Travis rebuild --- lib/Cake/Routing/Route/CakeRoute.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/Cake/Routing/Route/CakeRoute.php b/lib/Cake/Routing/Route/CakeRoute.php index c0327b162..b40529c9c 100644 --- a/lib/Cake/Routing/Route/CakeRoute.php +++ b/lib/Cake/Routing/Route/CakeRoute.php @@ -517,7 +517,7 @@ class CakeRoute { } $out = $this->template; - if (!empty($this->keys)) { + if ($this->keys !== array()) { $search = $replace = array(); From c0ac61117e041b3d832454d38e5ddf66e9f33065 Mon Sep 17 00:00:00 2001 From: mark_story Date: Mon, 10 Mar 2014 21:42:26 -0400 Subject: [PATCH 14/14] Only sort the keys once per request instead of on each match. Sorting the keys property by value sorts keys with the same prefix for free. This does change the order of the keys, but I don't think that is actually a large issue as it is just a list. Refs #2991 --- lib/Cake/Routing/Route/CakeRoute.php | 16 ++++++---------- .../Test/Case/Routing/Route/CakeRouteTest.php | 12 ++++++------ 2 files changed, 12 insertions(+), 16 deletions(-) diff --git a/lib/Cake/Routing/Route/CakeRoute.php b/lib/Cake/Routing/Route/CakeRoute.php index b40529c9c..74595c400 100644 --- a/lib/Cake/Routing/Route/CakeRoute.php +++ b/lib/Cake/Routing/Route/CakeRoute.php @@ -173,6 +173,10 @@ class CakeRoute { foreach ($this->keys as $key) { unset($this->defaults[$key]); } + + $keys = $this->keys; + sort($keys); + $this->keys = array_reverse($keys); } /** @@ -420,7 +424,6 @@ class CakeRoute { $named = $pass = array(); foreach ($url as $key => $value) { - // keys that exist in the defaults and have different values is a match failure. $defaultExists = array_key_exists($key, $defaults); if ($defaultExists && $defaults[$key] != $value) { @@ -517,16 +520,10 @@ class CakeRoute { } $out = $this->template; - if ($this->keys !== array()) { - + if (!empty($this->keys)) { $search = $replace = array(); - $lengths = array_map('strlen', $this->keys); - $flipped = array_combine($this->keys, $lengths); - arsort($flipped); - $keys = array_keys($flipped); - - foreach ($keys as $key) { + foreach ($this->keys as $key) { $string = null; if (isset($params[$key])) { $string = $params[$key]; @@ -537,7 +534,6 @@ class CakeRoute { $replace[] = $string; } $out = str_replace($search, $replace, $out); - } if (strpos($this->template, '*')) { diff --git a/lib/Cake/Test/Case/Routing/Route/CakeRouteTest.php b/lib/Cake/Test/Case/Routing/Route/CakeRouteTest.php index 16bed4cd6..375cf4d59 100644 --- a/lib/Cake/Test/Case/Routing/Route/CakeRouteTest.php +++ b/lib/Cake/Test/Case/Routing/Route/CakeRouteTest.php @@ -118,7 +118,7 @@ class CakeRouteTest extends CakeTestCase { $this->assertRegExp($result, '/posts/view/518098'); $this->assertNotRegExp($result, '/posts/edit/name-of-post'); $this->assertNotRegExp($result, '/posts/edit/4/other:param'); - $this->assertEquals(array('controller', 'action', 'id'), $route->keys); + $this->assertEquals(array('id', 'controller', 'action'), $route->keys); $route = new CakeRoute( '/:lang/:controller/:action/:id', @@ -130,7 +130,7 @@ class CakeRouteTest extends CakeTestCase { $this->assertRegExp($result, '/cze/articles/view/1'); $this->assertNotRegExp($result, '/language/articles/view/2'); $this->assertNotRegExp($result, '/eng/articles/view/name-of-article'); - $this->assertEquals(array('lang', 'controller', 'action', 'id'), $route->keys); + $this->assertEquals(array('lang', 'id', 'controller', 'action'), $route->keys); foreach (array(':', '@', ';', '$', '-') as $delim) { $route = new CakeRoute('/posts/:id' . $delim . ':title'); @@ -141,7 +141,7 @@ class CakeRouteTest extends CakeTestCase { $this->assertNotRegExp($result, '/posts/11!nameofarticle'); $this->assertNotRegExp($result, '/posts/11'); - $this->assertEquals(array('id', 'title'), $route->keys); + $this->assertEquals(array('title', 'id'), $route->keys); } $route = new CakeRoute( @@ -155,7 +155,7 @@ class CakeRouteTest extends CakeTestCase { $this->assertNotRegExp($result, '/posts/hey_now:nameofarticle'); $this->assertNotRegExp($result, '/posts/:nameofarticle/2009'); $this->assertNotRegExp($result, '/posts/:nameofarticle/01'); - $this->assertEquals(array('id', 'title', 'year'), $route->keys); + $this->assertEquals(array('year', 'title', 'id'), $route->keys); $route = new CakeRoute( '/posts/:url_title-(uuid::id)', @@ -204,7 +204,7 @@ class CakeRouteTest extends CakeTestCase { $this->assertRegExp($result, '/some_extra/page/this_is_the_slug'); $this->assertRegExp($result, '/page/this_is_the_slug'); - $this->assertEquals(array('extra', 'slug'), $route->keys); + $this->assertEquals(array('slug', 'extra'), $route->keys); $this->assertEquals(array('extra' => '[a-z1-9_]*', 'slug' => '[a-z1-9_]+', 'action' => 'view'), $route->options); $expected = array( 'controller' => 'pages', @@ -864,8 +864,8 @@ class CakeRouteTest extends CakeTestCase { $route = new CakeRoute('/:thisParam/:thisParamIsLonger'); $url = array( + 'thisParamIsLonger' => 'bar', 'thisParam' => 'foo', - 'thisParamIsLonger' => 'bar' ); $result = $route->match($url);