From 483d7124ddf0c8794f3999fbd68979a2435593bb Mon Sep 17 00:00:00 2001 From: Rachman Chavik Date: Mon, 28 May 2012 10:19:44 +0700 Subject: [PATCH 1/5] debug() may output path that is incorrectly truncated This happens when debug is called in core source files that resides in a different directory to the app. --- lib/Cake/Test/Case/BasicsTest.php | 23 ++++++++++++----------- lib/Cake/basics.php | 2 +- 2 files changed, 13 insertions(+), 12 deletions(-) diff --git a/lib/Cake/Test/Case/BasicsTest.php b/lib/Cake/Test/Case/BasicsTest.php index 253122c3a59..1bca6cb6809 100644 --- a/lib/Cake/Test/Case/BasicsTest.php +++ b/lib/Cake/Test/Case/BasicsTest.php @@ -691,7 +691,8 @@ public function testDebug() { 'this-is-a-test' ########################### EXPECTED; - $expected = sprintf($expectedText, substr(__FILE__, strlen(ROOT)), __LINE__ - 8); + $expected = sprintf($expectedText, str_replace(CAKE_CORE_INCLUDE_PATH, '', __FILE__), __LINE__ - 8); + $this->assertEquals($expected, $result); ob_start(); @@ -705,7 +706,7 @@ public function testDebug() { EXPECTED; - $expected = sprintf($expectedHtml, substr(__FILE__, strlen(ROOT)), __LINE__ - 10); + $expected = sprintf($expectedHtml, str_replace(CAKE_CORE_INCLUDE_PATH, '', __FILE__), __LINE__ - 10); $this->assertEquals($expected, $result); ob_start(); @@ -719,7 +720,7 @@ public function testDebug() { EXPECTED; - $expected = sprintf($expected, substr(__FILE__, strlen(ROOT)), __LINE__ - 10); + $expected = sprintf($expected, str_replace(CAKE_CORE_INCLUDE_PATH, '', __FILE__), __LINE__ - 10); $this->assertEquals($expected, $result); ob_start(); @@ -733,7 +734,7 @@ public function testDebug() { EXPECTED; - $expected = sprintf($expected, substr(__FILE__, strlen(ROOT)), __LINE__ - 10); + $expected = sprintf($expected, str_replace(CAKE_CORE_INCLUDE_PATH, '', __FILE__), __LINE__ - 10); $this->assertEquals($expected, $result); ob_start(); @@ -754,9 +755,9 @@ public function testDebug() { ########################### EXPECTED; if (php_sapi_name() == 'cli') { - $expected = sprintf($expectedText, substr(__FILE__, strlen(ROOT)), __LINE__ - 17); + $expected = sprintf($expectedText, str_replace(CAKE_CORE_INCLUDE_PATH, '', __FILE__), __LINE__ - 17); } else { - $expected = sprintf($expectedHtml, substr(__FILE__, strlen(ROOT)), __LINE__ - 19); + $expected = sprintf($expectedHtml, str_replace(CAKE_CORE_INCLUDE_PATH, '', __FILE__), __LINE__ - 19); } $this->assertEquals($expected, $result); @@ -778,9 +779,9 @@ public function testDebug() { ########################### EXPECTED; if (php_sapi_name() == 'cli') { - $expected = sprintf($expectedText, substr(__FILE__, strlen(ROOT)), __LINE__ - 17); + $expected = sprintf($expectedText, str_replace(CAKE_CORE_INCLUDE_PATH, '', __FILE__), __LINE__ - 17); } else { - $expected = sprintf($expectedHtml, substr(__FILE__, strlen(ROOT)), __LINE__ - 19); + $expected = sprintf($expectedHtml, str_replace(CAKE_CORE_INCLUDE_PATH, '', __FILE__), __LINE__ - 19); } $this->assertEquals($expected, $result); @@ -793,7 +794,7 @@ public function testDebug() { '
this-is-a-test
' ########################### EXPECTED; - $expected = sprintf($expected, substr(__FILE__, strlen(ROOT)), __LINE__ - 8); + $expected = sprintf($expected, str_replace(CAKE_CORE_INCLUDE_PATH, '', __FILE__), __LINE__ - 8); $this->assertEquals($expected, $result); ob_start(); @@ -805,7 +806,7 @@ public function testDebug() { '
this-is-a-test
' ########################### EXPECTED; - $expected = sprintf($expected, substr(__FILE__, strlen(ROOT)), __LINE__ - 8); + $expected = sprintf($expected, str_replace(CAKE_CORE_INCLUDE_PATH, '', __FILE__), __LINE__ - 8); $this->assertEquals($expected, $result); ob_start(); @@ -817,7 +818,7 @@ public function testDebug() { '
this-is-a-test
' ########################### EXPECTED; - $expected = sprintf($expected, substr(__FILE__, strlen(ROOT)), __LINE__ - 8); + $expected = sprintf($expected, str_replace(CAKE_CORE_INCLUDE_PATH, '', __FILE__), __LINE__ - 8); $this->assertEquals($expected, $result); } diff --git a/lib/Cake/basics.php b/lib/Cake/basics.php index 694e1ab163a..e2bc6e6852a 100644 --- a/lib/Cake/basics.php +++ b/lib/Cake/basics.php @@ -77,7 +77,7 @@ function debug($var = false, $showHtml = null, $showFrom = true) { $lineInfo = ''; if ($showFrom) { $trace = Debugger::trace(array('start' => 1, 'depth' => 2, 'format' => 'array')); - $file = substr($trace[0]['file'], strlen(ROOT)); + $file = str_replace(array(CAKE_CORE_INCLUDE_PATH, ROOT), '', $trace[0]['file']); $line = $trace[0]['line']; } $html = << Date: Fri, 1 Jun 2012 11:42:55 +0700 Subject: [PATCH 2/5] Helpers in custom CakeErrorController are lost Since many exceptions do not have its own 'template' file, customized APP/Controller/CakeErrorController with its own list of helpers could be ignored. This happens becase ExceptionRenderer is forced to to use _outputMessageSafe when a template is missing. This causes Controller::$helpers to be reset with default values. --- lib/Cake/Error/ExceptionRenderer.php | 6 +++ .../Test/Case/Error/ExceptionRendererTest.php | 43 +++++++++++++++++++ .../Controller/TestAppsErrorController.php | 14 ++++++ .../Error/TestAppsExceptionRenderer.php | 21 +++++++++ .../test_app/View/Helper/BananaHelper.php | 5 +++ .../Test/test_app/View/Layouts/banana.ctp | 5 +++ 6 files changed, 94 insertions(+) create mode 100644 lib/Cake/Test/test_app/Controller/TestAppsErrorController.php create mode 100644 lib/Cake/Test/test_app/Error/TestAppsExceptionRenderer.php create mode 100644 lib/Cake/Test/test_app/View/Layouts/banana.ctp diff --git a/lib/Cake/Error/ExceptionRenderer.php b/lib/Cake/Error/ExceptionRenderer.php index c855199f014..e991ac92410 100644 --- a/lib/Cake/Error/ExceptionRenderer.php +++ b/lib/Cake/Error/ExceptionRenderer.php @@ -263,6 +263,12 @@ protected function _outputMessage($template) { $this->controller->render($template); $this->controller->afterFilter(); $this->controller->response->send(); + } catch (MissingViewException $e) { + try { + $this->_outputMessage('error500'); + } catch (Exception $e) { + $this->_outputMessageSafe('error500'); + } } catch (Exception $e) { $this->_outputMessageSafe('error500'); } diff --git a/lib/Cake/Test/Case/Error/ExceptionRendererTest.php b/lib/Cake/Test/Case/Error/ExceptionRendererTest.php index 41c10113adc..e1fb8bd8bec 100644 --- a/lib/Cake/Test/Case/Error/ExceptionRendererTest.php +++ b/lib/Cake/Test/Case/Error/ExceptionRendererTest.php @@ -277,6 +277,49 @@ public function testErrorMethodCoercion() { $this->assertEquals($exception, $ExceptionRenderer->error); } +/** + * test that helpers in custom CakeErrorController are not lost + */ + public function testCakeErrorHelpersNotLost() { + $testApp = CAKE . 'Test' . DS . 'test_app' . DS; + App::build(array( + 'Controller' => array( + $testApp . 'Controller' . DS + ), + 'View/Helper' => array( + $testApp . 'View' . DS . 'Helper' . DS + ), + 'View/Layouts' => array( + $testApp . 'View' . DS . 'Layouts' . DS + ), + 'Error' => array( + $testApp . 'Error' . DS + ), + ), App::RESET); + Configure::write('Error', array( + 'handler' => 'TestAppsErrorHandler::handleError', + 'level' => E_ALL & ~E_DEPRECATED, + 'trace' => true + )); + + Configure::write('Exception', array( + 'handler' => 'TestAppsErrorHandler::handleException', + 'renderer' => 'TestAppsExceptionRenderer', + 'log' => true + )); + + App::uses('TestAppsErrorController', 'Controller'); + App::uses('TestAppsExceptionRenderer', 'Error'); + + $exception = new SocketException('socket exception'); + $renderer = new TestAppsExceptionRenderer($exception); + + ob_start(); + $renderer->render(); + $result = ob_get_clean(); + $this->assertContains('peeled', $result); + } + /** * test that unknown exception types with valid status codes are treated correctly. * diff --git a/lib/Cake/Test/test_app/Controller/TestAppsErrorController.php b/lib/Cake/Test/test_app/Controller/TestAppsErrorController.php new file mode 100644 index 00000000000..9ca1d7e3ba5 --- /dev/null +++ b/lib/Cake/Test/test_app/Controller/TestAppsErrorController.php @@ -0,0 +1,14 @@ + Configure::read('App.encoding'))); + try { + $controller = new TestAppsErrorController($request, $response); + $controller->layout = 'banana'; + } catch (Exception $e) { + $controller = new Controller($request, $response); + $controller->viewPath = 'Errors'; + } + return $controller; + } + +} diff --git a/lib/Cake/Test/test_app/View/Helper/BananaHelper.php b/lib/Cake/Test/test_app/View/Helper/BananaHelper.php index 1ceac01c97b..57759ac6a62 100644 --- a/lib/Cake/Test/test_app/View/Helper/BananaHelper.php +++ b/lib/Cake/Test/test_app/View/Helper/BananaHelper.php @@ -19,4 +19,9 @@ App::uses('Helper', 'View'); class BananaHelper extends Helper { + + public function peel() { + return 'peeled'; + } + } diff --git a/lib/Cake/Test/test_app/View/Layouts/banana.ctp b/lib/Cake/Test/test_app/View/Layouts/banana.ctp new file mode 100644 index 00000000000..b44e3aabb40 --- /dev/null +++ b/lib/Cake/Test/test_app/View/Layouts/banana.ctp @@ -0,0 +1,5 @@ + +Banana->peel(); +?> + \ No newline at end of file From 18b335a605356e11c1fec8341782e0b298125e2d Mon Sep 17 00:00:00 2001 From: Jelle Henkens Date: Thu, 31 May 2012 18:38:40 +0100 Subject: [PATCH 3/5] Replacing crc32 with md5 for less collisions in method caching --- lib/Cake/Model/Datasource/DboSource.php | 11 +++++------ .../Test/Case/Model/Datasource/DboSourceTest.php | 15 +++++++++++++++ 2 files changed, 20 insertions(+), 6 deletions(-) diff --git a/lib/Cake/Model/Datasource/DboSource.php b/lib/Cake/Model/Datasource/DboSource.php index c5a621d092b..fe64e06628b 100644 --- a/lib/Cake/Model/Datasource/DboSource.php +++ b/lib/Cake/Model/Datasource/DboSource.php @@ -53,9 +53,8 @@ class DboSource extends DataSource { /** * Caches result from query parsing operations. Cached results for both DboSource::name() and - * DboSource::conditions() will be stored here. Method caching uses `crc32()` which is - * fast but can collisions more easily than other hashing algorithms. If you have problems - * with collisions, set DboSource::$cacheMethods to false. + * DboSource::conditions() will be stored here. Method caching uses `md5()`. If you have + * problems with collisions, set DboSource::$cacheMethods to false. * * @var array */ @@ -767,7 +766,7 @@ public function cacheMethod($method, $key, $value = null) { * Strips fields out of SQL functions before quoting. * * Results of this method are stored in a memory cache. This improves performance, but - * because the method uses a simple hashing algorithm it can infrequently have collisions. + * because the method uses a hashing algorithm it can have collisions. * Setting DboSource::$cacheMethods to false will disable the memory cache. * * @param mixed $data Either a string with a column to quote. An array of columns to quote or an @@ -787,7 +786,7 @@ public function name($data) { } return $data; } - $cacheKey = crc32($this->startQuote . $data . $this->endQuote); + $cacheKey = md5($this->startQuote . $data . $this->endQuote); if ($return = $this->cacheMethod(__FUNCTION__, $cacheKey)) { return $return; } @@ -2273,7 +2272,7 @@ public function fields(Model $model, $alias = null, $fields = array(), $quote = * is given it will be integer cast as condition. Null will return 1 = 1. * * Results of this method are stored in a memory cache. This improves performance, but - * because the method uses a simple hashing algorithm it can infrequently have collisions. + * because the method uses a hashing algorithm it can have collisions. * Setting DboSource::$cacheMethods to false will disable the memory cache. * * @param mixed $conditions Array or string of conditions, or any value. diff --git a/lib/Cake/Test/Case/Model/Datasource/DboSourceTest.php b/lib/Cake/Test/Case/Model/Datasource/DboSourceTest.php index 41fccf31d81..00bcdb96fe4 100644 --- a/lib/Cake/Test/Case/Model/Datasource/DboSourceTest.php +++ b/lib/Cake/Test/Case/Model/Datasource/DboSourceTest.php @@ -599,6 +599,21 @@ public function testCacheMethod() { $this->assertNull($result); } + +/** + * Test that rare collisions do not happen with method caching + * + * @return void + */ + public function testNameMethodCacheCollisions() { + $this->testDb->cacheMethods = true; + $this->testDb->flushMethodCache(); + $this->testDb->name('Model.fieldlbqndkezcoapfgirmjsh'); + $result = $this->testDb->name('Model.fieldkhdfjmelarbqnzsogcpi'); + $expected = '`Model`.`fieldkhdfjmelarbqnzsogcpi`'; + $this->assertEquals($expected, $result); + } + /** * testLog method * From e5eb7b490e9d0853d61f8ba437bca0756088987d Mon Sep 17 00:00:00 2001 From: Jelle Henkens Date: Thu, 31 May 2012 20:00:06 +0100 Subject: [PATCH 4/5] Preventing cache collisions by adding the the datasource key --- lib/Cake/Model/Datasource/DboSource.php | 3 +- .../Case/Model/Datasource/DboSourceTest.php | 55 +++++++++++++++++++ 2 files changed, 57 insertions(+), 1 deletion(-) diff --git a/lib/Cake/Model/Datasource/DboSource.php b/lib/Cake/Model/Datasource/DboSource.php index fe64e06628b..04e78e92471 100644 --- a/lib/Cake/Model/Datasource/DboSource.php +++ b/lib/Cake/Model/Datasource/DboSource.php @@ -2172,7 +2172,8 @@ public function fields(Model $model, $alias = null, $fields = array(), $quote = $model->alias, $virtualFields, $fields, - $quote + $quote, + ConnectionManager::getSourceName($this) ); $cacheKey = md5(serialize($cacheKey)); if ($return = $this->cacheMethod(__FUNCTION__, $cacheKey)) { diff --git a/lib/Cake/Test/Case/Model/Datasource/DboSourceTest.php b/lib/Cake/Test/Case/Model/Datasource/DboSourceTest.php index 00bcdb96fe4..1c9fa489b74 100644 --- a/lib/Cake/Test/Case/Model/Datasource/DboSourceTest.php +++ b/lib/Cake/Test/Case/Model/Datasource/DboSourceTest.php @@ -53,6 +53,30 @@ public function setConnection($conn) { } +class DboSecondTestSource extends DboSource { + + public $startQuote = '_'; + + public $endQuote = '_'; + + public function connect($config = array()) { + $this->connected = true; + } + + public function mergeAssociation(&$data, &$merge, $association, $type, $selfJoin = false) { + return parent::_mergeAssociation($data, $merge, $association, $type, $selfJoin); + } + + public function setConfig($config = array()) { + $this->config = $config; + } + + public function setConnection($conn) { + $this->_connection = $conn; + } + +} + /** * DboSourceTest class * @@ -790,6 +814,37 @@ public function testFieldsUsingMethodCache() { $this->assertTrue(empty(DboTestSource::$methodCache['fields']), 'Cache not empty'); } + +/** + * test that fields() method cache detects datasource changes + * + * @return void + */ + public function testFieldsCacheKeyWithDatasourceChange() { + ConnectionManager::create('firstschema', array( + 'datasource' => 'DboTestSource' + )); + ConnectionManager::create('secondschema', array( + 'datasource' => 'DboSecondTestSource' + )); + Cache::delete('method_cache', '_cake_core_'); + DboTestSource::$methodCache = array(); + $Article = ClassRegistry::init('Article'); + + $Article->setDataSource('firstschema'); + $ds = $Article->getDataSource(); + $ds->cacheMethods = true; + $first = $ds->fields($Article, null, array('title', 'body', 'published')); + + $Article->setDataSource('secondschema'); + $ds = $Article->getDataSource(); + $ds->cacheMethods = true; + $second = $ds->fields($Article, null, array('title', 'body', 'published')); + + $this->assertNotEquals($first, $second); + $this->assertEquals(2, count(DboTestSource::$methodCache['fields'])); + } + /** * Test that group works without a model * From 048dc8d254bbb96180545dad6040521fde12b3cf Mon Sep 17 00:00:00 2001 From: Ceeram Date: Sat, 2 Jun 2012 01:39:53 +0200 Subject: [PATCH 5/5] casting schema to array, fixes error for array_keys when return is 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 2eecea43663..a2bf2a5882f 100644 --- a/lib/Cake/Model/Model.php +++ b/lib/Cake/Model/Model.php @@ -1166,7 +1166,7 @@ public function set($one, $two = null) { */ protected function _setAliasData($data) { $models = array_keys($this->getAssociated()); - $schema = array_keys($this->schema()); + $schema = array_keys((array)$this->schema()); foreach ($data as $field => $value) { if (in_array($field, $schema) || !in_array($field, $models)) { $data[$this->alias][$field] = $value;