From 0dc184e99f0ca607774a68257432a9a981f4d5b7 Mon Sep 17 00:00:00 2001 From: Bharat Mediratta Date: Mon, 18 Jan 2010 11:10:37 -0800 Subject: Overload url::current() and url::merge() to make the current url XSS safe. Add tests to make sure that it doesn't relapse with future Kohana changes. Fixes ticket #983. Ref: http://gallery.menalto.com/node/93738 --- modules/gallery/helpers/MY_url.php | 14 ++++++++++ modules/gallery/tests/Url_Security_Test.php | 43 +++++++++++++++++++++++++++++ modules/rss/controllers/rss.php | 8 ++---- 3 files changed, 60 insertions(+), 5 deletions(-) create mode 100644 modules/gallery/tests/Url_Security_Test.php diff --git a/modules/gallery/helpers/MY_url.php b/modules/gallery/helpers/MY_url.php index 74284951..8a7909b6 100644 --- a/modules/gallery/helpers/MY_url.php +++ b/modules/gallery/helpers/MY_url.php @@ -89,4 +89,18 @@ class url extends url_Core { static function abs_current($qs=false) { return self::abs_site(url::current($qs)); } + + /** + * Just like url::merge except that it escapes any XSS in the path. + */ + static function merge($params) { + return htmlspecialchars(parent::merge($params)); + } + + /** + * Just like url::current except that it escapes any XSS in the path. + */ + static function current($qs=false, $suffix=false) { + return htmlspecialchars(parent::current($qs, $suffix)); + } } diff --git a/modules/gallery/tests/Url_Security_Test.php b/modules/gallery/tests/Url_Security_Test.php new file mode 100644 index 00000000..de25880f --- /dev/null +++ b/modules/gallery/tests/Url_Security_Test.php @@ -0,0 +1,43 @@ +save = array(Router::$current_uri, Router::$complete_uri, $_GET); + } + + public function teardown() { + list(Router::$current_uri, Router::$complete_uri, $_GET) = $this->save; + } + + public function xss_in_current_url_test() { + Router::$current_uri = "foo//bar"; + Router::$complete_uri = "foo//bar?foo=bar"; + $this->assert_same("foo/<xss>/bar", url::current()); + $this->assert_same("foo/<xss>/bar?foo=bar", url::current(true)); + } + + public function xss_in_merged_url_test() { + Router::$current_uri = "foo//bar"; + Router::$complete_uri = "foo//bar?foo=bar"; + $_GET = array("foo" => "bar"); + $this->assert_same("foo/<xss>/bar?foo=bar", url::merge(array())); + $this->assert_same("foo/<xss>/bar?foo=bar&a=b", url::merge(array("a" => "b"))); + } +} \ No newline at end of file diff --git a/modules/rss/controllers/rss.php b/modules/rss/controllers/rss.php index 41c781d9..3066ba16 100644 --- a/modules/rss/controllers/rss.php +++ b/modules/rss/controllers/rss.php @@ -52,14 +52,12 @@ class Rss_Controller extends Controller { $view->feed = $feed; $view->pub_date = date("D, d M Y H:i:s T"); - $feed->uri = url::abs_site(str_replace("&", "&", url::merge($_GET))); + $feed->uri = url::abs_site(url::merge($_GET)); if ($page > 1) { - $feed->previous_page_uri = - url::abs_site(str_replace("&", "&", url::merge(array("page" => $page - 1)))); + $feed->previous_page_uri = url::abs_site(url::merge(array("page" => $page - 1))); } if ($page < $feed->max_pages) { - $feed->next_page_uri = - url::abs_site(str_replace("&", "&", url::merge(array("page" => $page + 1)))); + $feed->next_page_uri = url::abs_site(url::merge(array("page" => $page + 1))); } header("Content-Type: application/rss+xml"); -- cgit v1.2.3 From 4eafe97b4858cb8d1b3367574f7e7ef473127d7c Mon Sep 17 00:00:00 2001 From: Bharat Mediratta Date: Mon, 18 Jan 2010 11:57:23 -0800 Subject: Reload $item after removing view permissions. --- modules/gallery/tests/Access_Helper_Test.php | 1 + 1 file changed, 1 insertion(+) diff --git a/modules/gallery/tests/Access_Helper_Test.php b/modules/gallery/tests/Access_Helper_Test.php index 084bfb47..b2244766 100644 --- a/modules/gallery/tests/Access_Helper_Test.php +++ b/modules/gallery/tests/Access_Helper_Test.php @@ -74,6 +74,7 @@ class Access_Helper_Test extends Unit_Test_Case { access::deny(identity::everybody(), "view", $item); access::deny(identity::registered_users(), "view", $item); + $item->reload(); $user = identity::create_user("access_test", "Access Test", ""); foreach ($user->groups() as $group) { -- cgit v1.2.3 From 64357b3a2edf28c3c342931b13f6594153847148 Mon Sep 17 00:00:00 2001 From: Bharat Mediratta Date: Mon, 18 Jan 2010 12:04:54 -0800 Subject: Updated Kohana to r4742 --- system/libraries/Database_Builder.php | 1006 ++++++++++++++++++++------------- system/libraries/ORM.php | 73 +-- 2 files changed, 624 insertions(+), 455 deletions(-) diff --git a/system/libraries/Database_Builder.php b/system/libraries/Database_Builder.php index 4e6951e7..62b2a163 100644 --- a/system/libraries/Database_Builder.php +++ b/system/libraries/Database_Builder.php @@ -1,6 +1,16 @@ select() + * ->where('name', '=', 'Kohana') + * ->from('frameworks') + * ->execute(); * * @package Kohana * @author Kohana Team @@ -30,6 +40,7 @@ class Database_Builder_Core { protected $values = array(); protected $type; protected $distinct = FALSE; + protected $reset = TRUE; // TTL for caching (using Cache library) protected $ttl = FALSE; @@ -40,317 +51,333 @@ class Database_Builder_Core { } /** - * Resets all query components + * Compiles the builder object into a SQL query. Useful for debugging + * + * ##### Example + * + * echo $builder->select()->from('products'); + * // Output: SELECT * FROM `products` + * + * @return string Compiled query */ - public function reset() - { - $this->select = array(); - $this->from = array(); - $this->join = array(); - $this->where = array(); - $this->group_by = array(); - $this->having = array(); - $this->order_by = array(); - $this->limit = NULL; - $this->offset = NULL; - $this->set = array(); - $this->values = array(); - } - public function __toString() { return $this->compile(); } /** - * Compiles the builder object into a SQL query + * Creates a `SELECT` query with support for column aliases, database functions, + * subqueries or a [Database_Expression] + * + * ##### Examples + * + * // Simple select + * echo $builder->select()->from('products'); + * + * // Select with database function + * echo $builder->select(array('records_found' => 'COUNT("*")'))->from('products'); * - * @return string Compiled query + * // Select with sub query + * echo $builder->select(array('field', 'test' => db::select('test')->from('table')))->from('products'); + * + * @chainable + * @param string|array column name or array(alias => column) + * @return Database_Builder */ - protected function compile() + public function select($columns = NULL) { - if ( ! is_object($this->db)) - { - // Use default database for compiling to string if none is given - $this->db = Database::instance($this->db); - } - - if ($this->type === Database::SELECT) - { - // SELECT columns FROM table - $sql = $this->distinct ? 'SELECT DISTINCT ' : 'SELECT '; - $sql .= $this->compile_select(); - - if ( ! empty($this->from)) - { - $sql .= "\nFROM ".$this->compile_from(); - } - } - elseif ($this->type === Database::UPDATE) - { - $sql = 'UPDATE '.$this->compile_from()."\n".'SET '.$this->compile_set(); - } - elseif ($this->type === Database::INSERT) - { - $sql = 'INSERT INTO '.$this->compile_from()."\n".$this->compile_columns()."\nVALUES ".$this->compile_values(); - } - elseif ($this->type === Database::DELETE) - { - $sql = 'DELETE FROM '.$this->compile_from(); - } + $this->type = Database::SELECT; - if ( ! empty($this->join)) + if ($columns === NULL) { - $sql .= $this->compile_join(); + $columns = array('*'); } - - if ( ! empty($this->where)) + elseif ( ! is_array($columns)) { - $sql .= "\n".'WHERE '.$this->compile_conditions($this->where); + $columns = func_get_args(); } - if ( ! empty($this->having)) - { - $sql .= "\n".'HAVING '.$this->compile_conditions($this->having); - } + $this->select = array_merge($this->select, $columns); - if ( ! empty($this->group_by)) - { - $sql .= "\n".'GROUP BY '.$this->compile_group_by(); - } + return $this; + } - if ( ! empty($this->order_by)) - { - $sql .= "\nORDER BY ".$this->compile_order_by(); - } + /** + * Creates a `DISTINCT SELECT` query. For more information see see [Database_Builder::select]. + * + * @chainable + * @param string|array column name or array(alias => column) + * @return Database_Builder + */ + public function select_distinct($columns = NULL) + { + $this->select($columns); + $this->distinct = TRUE; + return $this; + } - if (is_int($this->limit)) + /** + * Add tables to the FROM portion of the builder + * + * ##### Example + * + * $builder->select()->from('products') + * ->from(array('other' => 'other_table')); + * // Output: SELECT * FROM `products`, `other_table` AS `other` + * + * @chainable + * @param string|array table name or array(alias => table) + * @return Database_Builder + */ + public function from($tables) + { + if ( ! is_array($tables)) { - $sql .= "\nLIMIT ".$this->limit; + $tables = func_get_args(); } - if (is_int($this->offset)) - { - $sql .= "\nOFFSET ".$this->offset; - } + $this->from = array_merge($this->from, $tables); - return $sql; + return $this; } /** - * Compiles the SELECT portion of the query + * Add conditions to the `WHERE` clause. Alias for [Database_Builder::and_where]. * - * @return string + * @chainable + * @param mixed Column name or array of columns => vals + * @param string Operation to perform + * @param mixed Value + * @return Database_Builder */ - protected function compile_select() + public function where($columns, $op = '=', $value = NULL) { - $vals = array(); - - foreach ($this->select as $alias => $name) - { - if ($name instanceof Database_Builder) - { - // Using a subquery - $name->db = $this->db; - $vals[] = '('.(string) $name.') AS '.$this->db->quote_column($alias); - } - elseif (is_string($alias)) - { - $vals[] = $this->db->quote_column($name, $alias); - } - else - { - $vals[] = $this->db->quote_column($name); - } - } - - return implode(', ', $vals); + return $this->and_where($columns, $op, $value); } /** - * Compiles the FROM portion of the query + * Add conditions to the `WHERE` clause separating multiple conditions with `AND`. + * This function supports all `WHERE` operators including `LIKE` and `IN`. It can + * also be used with a [Database_Expression] or subquery. * - * @return string + * ##### Examples + * + * // Basic where condition + * $builder->where('field', '=', 'value'); + * + * // Multiple conditions with an array (you can also chain where() function calls) + * $builder->where(array(array('field', '=', 'value'), array(...))); + * + * // With a database expression + * $builder->where('field', '=', db::expr('field + 1')); + * // or a function + * $builder->where('field', '=', db::expr('UNIX_TIMESTAMP()')); + * + * // With a subquery + * $builder->where('field', 'IN', db::select('id')->from('table')); + * + * [!!] You must manually escape all data you pass into a database expression! + * + * @chainable + * @param mixed Column name or array of triplets + * @param string Operation to perform + * @param mixed Value + * @return Database_Builder */ - protected function compile_from() + public function and_where($columns, $op = '=', $value = NULL) { - $vals = array(); - - foreach ($this->from as $alias => $name) + if (is_array($columns)) { - if (is_string($alias)) - { - // Using AS format so escape both - $vals[] = $this->db->quote_table($name, $alias); - } - else + foreach ($columns as $column) { - // Just using the table name itself - $vals[] = $this->db->quote_table($name); + $this->where[] = array('AND' => $column); } } - - return implode(', ', $vals); + else + { + $this->where[] = array('AND' => array($columns, $op, $value)); + } + return $this; } /** - * Compiles the JOIN portion of the query + * Add conditions to the `WHERE` clause separating multiple conditions with `OR`. + * For more information about building a `WHERE` clause see [Database_Builder::and_where] * - * @return string + * @chainable + * @param mixed Column name or array of triplets + * @param string Operation to perform + * @param mixed Value + * @return Database_Builder */ - protected function compile_join() + public function or_where($columns, $op = '=', $value = NULL) { - $sql = ''; - foreach ($this->join as $join) + if (is_array($columns)) { - list($table, $keys, $type) = $join; - - if ($type !== NULL) - { - // Join type - $sql .= ' '.$type; - } - - $sql .= ' JOIN '.$this->db->quote_table($table); - - $condition = ''; - if ($keys instanceof Database_Expression) - { - $condition = (string) $keys; - } - elseif (is_array($keys)) - { - // ON condition is an array of matches - foreach ($keys as $key => $value) - { - if ( ! empty($condition)) - { - $condition .= ' AND '; - } - - $condition .= $this->db->quote_column($key).' = '.$this->db->quote_column($value); - } - } - - if ( ! empty($condition)) + foreach ($columns as $column) { - // Add ON condition - $sql .= ' ON ('.$condition.')'; + $this->where[] = array('OR' => $column); } } - - return $sql; + else + { + $this->where[] = array('OR' => array($columns, $op, $value)); + } + return $this; } /** - * Compiles the GROUP BY portion of the query + * Join tables to the builder * - * @return string + * ##### Example + * + * // Basic join + * db::select()->from('products') + * ->join('reviews', 'reviews.product_id', 'products.id'); + * + * // Advanced joins + * echo db::select()->from('products') + * ->join('reviews', 'field', db::expr('advanced condition here'), 'RIGHT'); + * + * @chainable + * @param mixed Table name + * @param mixed Key, or an array of key => value pair, for join condition (can be a Database_Expression) + * @param mixed Value if $keys is not an array or Database_Expression + * @param string Join type (LEFT, RIGHT, INNER, etc.) + * @return Database_Builder */ - protected function compile_group_by() + public function join($table, $keys, $value = NULL, $type = NULL) { - $vals = array(); + if (is_string($keys)) + { + $keys = array($keys => $value); + } - foreach ($this->group_by as $column) + if ($type !== NULL) { - // Escape the column - $vals[] = $this->db->quote_column($column); + $type = strtoupper($type); } - return implode(', ', $vals); + $this->join[] = array($table, $keys, $type); + + return $this; } /** - * Compiles the ORDER BY portion of the query + * This function is an alias for [Database_Builder::join] + * with the join type set to `LEFT`. * - * @return string + * @chainable + * @param mixed Table name + * @param mixed Key, or an array of key => value pair, for join condition (can be a Database_Expression) + * @param mixed Value if $keys is not an array or Database_Expression + * @return Database_Builder */ - public function compile_order_by() + public function left_join($table, $keys, $value = NULL) { - $ordering = array(); - - foreach ($this->order_by as $column => $order_by) - { - list($column, $direction) = each($order_by); - - $column = $this->db->quote_column($column); - - if ($direction !== NULL) - { - $direction = ' '.$direction; - } - - $ordering[] = $column.$direction; - } - - return implode(', ', $ordering); + return $this->join($table, $keys, $value, 'LEFT'); } /** - * Compiles the SET portion of the query for UPDATE + * This function is an alias for [Database_Builder::join] + * with the join type set to `RIGHT`. * - * @return string + * @chainable + * @param mixed Table name + * @param mixed Key, or an array of key => value pair, for join condition (can be a Database_Expression) + * @param mixed Value if $keys is not an array or Database_Expression + * @return Database_Builder */ - public function compile_set() + public function right_join($table, $keys, $value = NULL) { - $vals = array(); - - foreach ($this->set as $key => $value) - { - // Using an UPDATE so Key = Val - $vals[] = $this->db->quote_column($key).' = '.$this->db->quote($value); - } - - return implode(', ', $vals); + return $this->join($table, $keys, $value, 'RIGHT'); } /** - * Join tables to the builder + * This function is an alias for [Database_Builder::join] + * with the join type set to `INNER`. * + * @chainable * @param mixed Table name * @param mixed Key, or an array of key => value pair, for join condition (can be a Database_Expression) * @param mixed Value if $keys is not an array or Database_Expression - * @param string Join type (LEFT, RIGHT, INNER, etc.) * @return Database_Builder */ - public function join($table, $keys, $value = NULL, $type = NULL) + public function inner_join($table, $keys, $value = NULL) { - if (is_string($keys)) - { - $keys = array($keys => $value); - } - - if ($type !== NULL) - { - $type = strtoupper($type); - } - - $this->join[] = array($table, $keys, $type); + return $this->join($table, $keys, $value, 'INNER'); + } - return $this; + /** + * This function is an alias for [Database_Builder::join] + * with the join type set to `OUTER`. + * + * @chainable + * @param mixed Table name + * @param mixed Key, or an array of key => value pair, for join condition (can be a Database_Expression) + * @param mixed Value if $keys is not an array or Database_Expression + * @return Database_Builder + */ + public function outer_join($table, $keys, $value = NULL) + { + return $this->join($table, $keys, $value, 'OUTER'); } /** - * Add tables to the FROM portion of the builder + * This function is an alias for [Database_Builder::join] + * with the join type set to `FULL`. * - * @param string|array table name or array(alias => table) - * @return Database_Builder + * @chainable + * @param mixed Table name + * @param mixed Key, or an array of key => value pair, for join condition (can be a Database_Expression) + * @param mixed Value if $keys is not an array or Database_Expression + * @return Database_Builder */ - public function from($tables) + public function full_join($table, $keys, $value = NULL) { - if ( ! is_array($tables)) - { - $tables = func_get_args(); - } + return $this->join($table, $keys, $value, 'FULL'); + } - $this->from = array_merge($this->from, $tables); + /** + * This function is an alias for [Database_Builder::join] + * with the join type set to `LEFT INNER`. + * + * @chainable + * @param mixed Table name + * @param mixed Key, or an array of key => value pair, for join condition (can be a Database_Expression) + * @param mixed Value if $keys is not an array or Database_Expression + * @return Database_Builder + */ + public function left_inner_join($table, $keys, $value = NULL) + { + return $this->join($table, $keys, $value, 'LEFT INNER'); + } - return $this; + /** + * This function is an alias for [Database_Builder::join] + * with the join type set to `RIGHT INNER`. + * + * @chainable + * @param mixed Table name + * @param mixed Key, or an array of key => value pair, for join condition (can be a Database_Expression) + * @param mixed Value if $keys is not an array or Database_Expression + * @return Database_Builder + */ + public function right_inner_join($table, $keys, $value = NULL) + { + return $this->join($table, $keys, $value, 'RIGHT INNER'); } /** * Add fields to the GROUP BY portion * + * ##### Example + * + * db::select()->from('products') + * ->group_by(array('name', 'cat_id')); + * // Output: SELECT * FROM `products` GROUP BY `name`, `cat_id` + * + * @chainable * @param mixed Field names or an array of fields * @return Database_Builder */ @@ -369,6 +396,7 @@ class Database_Builder_Core { /** * Add conditions to the HAVING clause (AND) * + * @chainable * @param mixed Column name or array of columns => vals * @param string Operation to perform * @param mixed Value @@ -382,6 +410,7 @@ class Database_Builder_Core { /** * Add conditions to the HAVING clause (AND) * + * @chainable * @param mixed Column name or array of triplets * @param string Operation to perform * @param mixed Value @@ -406,6 +435,7 @@ class Database_Builder_Core { /** * Add conditions to the HAVING clause (OR) * + * @chainable * @param mixed Column name or array of triplets * @param string Operation to perform * @param mixed Value @@ -430,6 +460,7 @@ class Database_Builder_Core { /** * Add fields to the ORDER BY portion * + * @chainable * @param mixed Field names or an array of fields (field => direction) * @param string Direction or NULL for ascending * @return Database_Builder @@ -461,6 +492,7 @@ class Database_Builder_Core { /** * Limit rows returned * + * @chainable * @param int Number of rows * @return Database_Builder */ @@ -474,6 +506,7 @@ class Database_Builder_Core { /** * Offset into result set * + * @chainable * @param int Offset * @return Database_Builder */ @@ -484,46 +517,25 @@ class Database_Builder_Core { return $this; } - public function left_join($table, $keys, $value = NULL) - { - return $this->join($table, $keys, $value, 'LEFT'); - } - - public function right_join($table, $keys, $value = NULL) - { - return $this->join($table, $keys, $value, 'RIGHT'); - } - - public function inner_join($table, $keys, $value = NULL) - { - return $this->join($table, $keys, $value, 'INNER'); - } - - public function outer_join($table, $keys, $value = NULL) - { - return $this->join($table, $keys, $value, 'OUTER'); - } - - public function full_join($table, $keys, $value = NULL) - { - return $this->join($table, $keys, $value, 'FULL'); - } - - public function left_inner_join($table, $keys, $value = NULL) - { - return $this->join($table, $keys, $value, 'LEFT INNER'); - } - - public function right_inner_join($table, $keys, $value = NULL) - { - return $this->join($table, $keys, $value, 'RIGHT INNER'); - } - + /** + * Alias for [Database_Builder::and_open] + * + * @chainable + * @param string Clause (WHERE OR HAVING) + * @return Database_Builder + */ public function open($clause = 'WHERE') { return $this->and_open($clause); } + /** + * Open new **ANDs** parenthesis set + * + * @chainable + * @param string Clause (WHERE OR HAVING) + * @return Database_Builder + */ public function and_open($clause = 'WHERE') { if ($clause === 'WHERE') @@ -538,6 +550,13 @@ class Database_Builder_Core { return $this; } + /** + * Open new **OR** parenthesis set + * + * @chainable + * @param string Clause (WHERE OR HAVING) + * @return Database_Builder + */ public function or_open($clause = 'WHERE') { if ($clause === 'WHERE') @@ -552,6 +571,13 @@ class Database_Builder_Core { return $this; } + /** + * Close close parenthesis set + * + * @chainable + * @param string Clause (WHERE OR HAVING) + * @return Database_Builder + */ public function close($clause = 'WHERE') { if ($clause === 'WHERE') @@ -567,63 +593,102 @@ class Database_Builder_Core { } /** - * Add conditions to the WHERE clause (AND) + * Set values for UPDATE * + * @chainable * @param mixed Column name or array of columns => vals - * @param string Operation to perform - * @param mixed Value + * @param mixed Value (can be a Database_Expression) * @return Database_Builder */ - public function where($columns, $op = '=', $value = NULL) + public function set($keys, $value = NULL) { - return $this->and_where($columns, $op, $value); + if (is_string($keys)) + { + $keys = array($keys => $value); + } + + $this->set = array_merge($keys, $this->set); + + return $this; } /** - * Add conditions to the WHERE clause (AND) + * Columns used for INSERT queries * - * @param mixed Column name or array of triplets - * @param string Operation to perform - * @param mixed Value + * @chainable + * @param array Columns * @return Database_Builder */ - public function and_where($columns, $op = '=', $value = NULL) + public function columns($columns) { - if (is_array($columns)) - { - foreach ($columns as $column) - { - $this->where[] = array('AND' => $column); - } - } - else + if ( ! is_array($columns)) { - $this->where[] = array('AND' => array($columns, $op, $value)); + $columns = func_get_args(); } + + $this->columns = $columns; + return $this; } /** - * Add conditions to the WHERE clause (OR) + * Values used for INSERT queries * - * @param mixed Column name or array of triplets - * @param string Operation to perform - * @param mixed Value + * @chainable + * @param array Values * @return Database_Builder */ - public function or_where($columns, $op = '=', $value = NULL) + public function values($values) { - if (is_array($columns)) - { - foreach ($columns as $column) - { - $this->where[] = array('OR' => $column); - } - } - else + if ( ! is_array($values)) { - $this->where[] = array('OR' => array($columns, $op, $value)); + $values = func_get_args(); } + + $this->values[] = $values; + + return $this; + } + + /** + * Set caching for the query + * + * @chainable + * @param mixed Time-to-live (FALSE to disable, NULL for Cache default, seconds otherwise) + * @return Database_Builder + */ + public function cache($ttl = NULL) + { + $this->ttl = $ttl; + + return $this; + } + + /** + * Resets the database builder after execution. By default after you `execute()` a query + * the database builder will reset to its default state. You can use `reset(FALSE)` + * to stop this from happening. This is useful for pagination when you might want to + * apply a limit to the previous query. + * + * ##### Example + * + * $db = new Database_Builder; + * $all_results = $db->select() + * ->where('id', '=', 3) + * ->from('products') + * ->reset(FALSE) + * ->execute(); + * + * // Run the query again with a limit of 10 + * $ten_results = $db->limit(10) + * ->execute(); + * @chainable + * @param bool reset builder + * @return Database_Builder + */ + public function reset($reset = TRUE) + { + $this->reset = (bool) $reset; return $this; } @@ -734,70 +799,15 @@ class Database_Builder_Core { } /** - * Set values for UPDATE + * Compiles the columns portion of the query for INSERT * - * @param mixed Column name or array of columns => vals - * @param mixed Value (can be a Database_Expression) - * @return Database_Builder + * @return string */ - public function set($keys, $value = NULL) - { - if (is_string($keys)) - { - $keys = array($keys => $value); - } - - $this->set = array_merge($keys, $this->set); - - return $this; - } - - /** - * Columns used for INSERT queries - * - * @param array Columns - * @return Database_Builder - */ - public function columns($columns) - { - if ( ! is_array($columns)) - { - $columns = func_get_args(); - } - - $this->columns = $columns; - - return $this; - } - - /** - * Compiles the columns portion of the query for INSERT - * - * @return string - */ - protected function compile_columns() + protected function compile_columns() { return '('.implode(', ', array_map(array($this->db, 'quote_column'), $this->columns)).')'; } - /** - * Values used for INSERT queries - * - * @param array Values - * @return Database_Builder - */ - public function values($values) - { - if ( ! is_array($values)) - { - $values = func_get_args(); - } - - $this->values[] = $values; - - return $this; - } - /** * Compiles the VALUES portion of the query for INSERT * @@ -815,46 +825,10 @@ class Database_Builder_Core { return implode(', ', $values); } - /** - * Create a SELECT query and specify selected columns - * - * @param string|array column name or array(alias => column) - * @return Database_Builder - */ - public function select($columns = NULL) - { - $this->type = Database::SELECT; - - if ($columns === NULL) - { - $columns = array('*'); - } - elseif ( ! is_array($columns)) - { - $columns = func_get_args(); - } - - $this->select = array_merge($this->select, $columns); - - return $this; - } - - /** - * Create a SELECT query and specify selected columns - * - * @param string|array column name or array(alias => column) - * @return Database_Builder - */ - public function select_distinct($columns = NULL) - { - $this->select($columns); - $this->distinct = TRUE; - return $this; - } - /** * Create an UPDATE query * + * @chainable * @param string Table name * @param array Array of Keys => Values * @param array WHERE conditions @@ -885,6 +859,7 @@ class Database_Builder_Core { /** * Create an INSERT query. Use 'columns' and 'values' methods for multi-row inserts * + * @chainable * @param string Table name * @param array Array of Keys => Values * @return Database_Builder @@ -910,6 +885,7 @@ class Database_Builder_Core { /** * Create a DELETE query * + * @chainable * @param string Table name * @param array WHERE conditions * @return Database_Builder @@ -980,8 +956,11 @@ class Database_Builder_Core { $query = $this->compile(); - // Reset the query after executing - $this->reset(); + if ($this->reset) + { + // Reset the query after executing + $this->_reset(); + } if ($this->ttl !== FALSE AND $this->type === Database::SELECT) { @@ -996,16 +975,267 @@ class Database_Builder_Core { } /** - * Set caching for the query + * Compiles the builder object into a SQL query * - * @param mixed Time-to-live (FALSE to disable, NULL for Cache default, seconds otherwise) - * @return Database_Builder + * @return string Compiled query */ - public function cache($ttl = NULL) + protected function compile() { - $this->ttl = $ttl; + if ( ! is_object($this->db)) + { + // Use default database for compiling to string if none is given + $this->db = Database::instance($this->db); + } - return $this; + if ($this->type === Database::SELECT) + { + // SELECT columns FROM table + $sql = $this->distinct ? 'SELECT DISTINCT ' : 'SELECT '; + $sql .= $this->compile_select(); + + if ( ! empty($this->from)) + { + $sql .= "\nFROM ".$this->compile_from(); + } + } + elseif ($this->type === Database::UPDATE) + { + $sql = 'UPDATE '.$this->compile_from()."\n".'SET '.$this->compile_set(); + } + elseif ($this->type === Database::INSERT) + { + $sql = 'INSERT INTO '.$this->compile_from()."\n".$this->compile_columns()."\nVALUES ".$this->compile_values(); + } + elseif ($this->type === Database::DELETE) + { + $sql = 'DELETE FROM '.$this->compile_from(); + } + + if ( ! empty($this->join)) + { + $sql .= $this->compile_join(); + } + + if ( ! empty($this->where)) + { + $sql .= "\n".'WHERE '.$this->compile_conditions($this->where); + } + + if ( ! empty($this->having)) + { + $sql .= "\n".'HAVING '.$this->compile_conditions($this->having); + } + + if ( ! empty($this->group_by)) + { + $sql .= "\n".'GROUP BY '.$this->compile_group_by(); + } + + if ( ! empty($this->order_by)) + { + $sql .= "\nORDER BY ".$this->compile_order_by(); + } + + if (is_int($this->limit)) + { + $sql .= "\nLIMIT ".$this->limit; + } + + if (is_int($this->offset)) + { + $sql .= "\nOFFSET ".$this->offset; + } + + return $sql; + } + + /** + * Compiles the SELECT portion of the query + * + * @return string + */ + protected function compile_select() + { + $vals = array(); + + foreach ($this->select as $alias => $name) + { + if ($name instanceof Database_Builder) + { + // Using a subquery + $name->db = $this->db; + $vals[] = '('.(string) $name.') AS '.$this->db->quote_column($alias); + } + elseif (is_string($alias)) + { + $vals[] = $this->db->quote_column($name, $alias); + } + else + { + $vals[] = $this->db->quote_column($name); + } + } + + return implode(', ', $vals); + } + + /** + * Compiles the FROM portion of the query + * + * @return string + */ + protected function compile_from() + { + $vals = array(); + + foreach ($this->from as $alias => $name) + { + if (is_string($alias)) + { + // Using AS format so escape both + $vals[] = $this->db->quote_table($name, $alias); + } + else + { + // Just using the table name itself + $vals[] = $this->db->quote_table($name); + } + } + + return implode(', ', $vals); + } + + /** + * Compiles the JOIN portion of the query + * + * @return string + */ + protected function compile_join() + { + $sql = ''; + foreach ($this->join as $join) + { + list($table, $keys, $type) = $join; + + if ($type !== NULL) + { + // Join type + $sql .= ' '.$type; + } + + $sql .= ' JOIN '.$this->db->quote_table($table); + + $condition = ''; + if ($keys instanceof Database_Expression) + { + $condition = (string) $keys; + } + elseif (is_array($keys)) + { + // ON condition is an array of matches + foreach ($keys as $key => $value) + { + if ( ! empty($condition)) + { + $condition .= ' AND '; + } + + $condition .= $this->db->quote_column($key).' = '.$this->db->quote_column($value); + } + } + + if ( ! empty($condition)) + { + // Add ON condition + $sql .= ' ON ('.$condition.')'; + } + } + + return $sql; + } + + /** + * Compiles the GROUP BY portion of the query + * + * @return string + */ + protected function compile_group_by() + { + $vals = array(); + + foreach ($this->group_by as $column) + { + // Escape the column + $vals[] = $this->db->quote_column($column); + } + + return implode(', ', $vals); + } + + /** + * Compiles the ORDER BY portion of the query + * + * @return string + */ + protected function compile_order_by() + { + $ordering = array(); + + foreach ($this->order_by as $column => $order_by) + { + list($column, $direction) = each($order_by); + + $column = $this->db->quote_column($column); + + if ($direction !== NULL) + { + $direction = ' '.$direction; + } + + $ordering[] = $column.$direction; + } + + return implode(', ', $ordering); + } + + /** + * Compiles the SET portion of the query for UPDATE + * + * @return string + */ + protected function compile_set() + { + $vals = array(); + + foreach ($this->set as $key => $value) + { + // Using an UPDATE so Key = Val + $vals[] = $this->db->quote_column($key).' = '.$this->db->quote($value); + } + + return implode(', ', $vals); + } + + /** + * Resets all query components + */ + protected function _reset() + { + $this->select = array(); + $this->from = array(); + $this->join = array(); + $this->where = array(); + $this->group_by = array(); + $this->having = array(); + $this->order_by = array(); + $this->limit = NULL; + $this->offset = NULL; + $this->set = array(); + $this->values = array(); + $this->type = NULL; + $this->distinct = FALSE; + $this->reset = TRUE; + $this->ttl = FALSE; } } // End Database_Builder diff --git a/system/libraries/ORM.php b/system/libraries/ORM.php index 60439106..4dd2eaf0 100644 --- a/system/libraries/ORM.php +++ b/system/libraries/ORM.php @@ -443,11 +443,11 @@ class ORM_Core { // Data has changed $this->changed[$column] = $column; - // Object is no longer saved - $this->_saved = FALSE; + // Object is no longer saved or valid + $this->_saved = $this->_valid = FALSE; } - $this->object[$column] = $this->load_type($column, $value); + $this->object[$column] = $value; } elseif (in_array($column, $this->has_and_belongs_to_many) AND is_array($value)) { @@ -904,6 +904,9 @@ class ORM_Core { if ($this->saved() === TRUE) { + // Always force revalidation after saving + $this->_valid = FALSE; + // Clear the per-request database cache $this->db->clear_cache(NULL, Database::PER_REQUEST); } @@ -1374,12 +1377,6 @@ class ORM_Core { { if ( ! $ignore_changed OR ! isset($this->changed[$column])) { - if (isset($this->table_columns[$column])) - { - // The type of the value can be determined, convert the value - $value = $this->load_type($column, $value); - } - $this->object[$column] = $value; } } @@ -1403,64 +1400,6 @@ class ORM_Core { return $this; } - - /** - * Loads a value according to the types defined by the column metadata. - * - * @param string column name - * @param mixed value to load - * @return mixed - */ - protected function load_type($column, $value) - { - $type = gettype($value); - if ($type == 'object' OR $type == 'array' OR ! isset($this->table_columns[$column])) - return $value; - - // Load column data - $column = $this->table_columns[$column]; - - if ($value === NULL AND ! empty($column['nullable'])) - return $value; - - if ( ! empty($column['binary']) AND ! empty($column['exact']) AND (int) $column['length'] === 1) - { - // Use boolean for BINARY(1) fields - $column['type'] = 'boolean'; - } - - switch ($column['type']) - { - case 'int': - if ($value === '' AND ! empty($column['nullable'])) - { - // Forms will only submit strings, so empty integer values must be null - $value = NULL; - } - elseif ((float) $value > PHP_INT_MAX) - { - // This number cannot be represented by a PHP integer, so we convert it to a string - $value = (string) $value; - } - else - { - $value = (int) $value; - } - break; - case 'float': - $value = (float) $value; - break; - case 'boolean': - $value = (bool) $value; - break; - case 'string': - $value = (string) $value; - break; - } - - return $value; - } - /** * Loads a database result, either as a new object for this model, or as * an iterator for multiple rows. -- cgit v1.2.3 From 41a392611c0e602d2e14859e5c0d5bf9e61d0073 Mon Sep 17 00:00:00 2001 From: Bharat Mediratta Date: Mon, 18 Jan 2010 12:08:05 -0800 Subject: Change DENY and ALLOW to "0" and "1" to match the fact that ORM no longer typecasts values as of http://dev.kohanaphp.com/issues/2459 --- modules/gallery/helpers/access.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/modules/gallery/helpers/access.php b/modules/gallery/helpers/access.php index 8ce7e436..e0a0e979 100644 --- a/modules/gallery/helpers/access.php +++ b/modules/gallery/helpers/access.php @@ -66,8 +66,8 @@ * the Access_Intent_Model */ class access_Core { - const DENY = false; - const ALLOW = true; + const DENY = "0"; + const ALLOW = "1"; const INHERIT = null; // access_intent const UNKNOWN = null; // cache (access_cache, items) -- cgit v1.2.3 From 284788d964688385f77b18bc063a841d0dbcdcd8 Mon Sep 17 00:00:00 2001 From: Bharat Mediratta Date: Mon, 18 Jan 2010 12:08:39 -0800 Subject: Switch from stdClass to arrays which works around issues caused in http://dev.kohanaphp.com/issues/2459 -- I don't exactly know why, but the solutions are equivalent so I'm not going to dig too far. --- modules/gallery/helpers/model_cache.php | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/modules/gallery/helpers/model_cache.php b/modules/gallery/helpers/model_cache.php index 302e42d9..88756407 100644 --- a/modules/gallery/helpers/model_cache.php +++ b/modules/gallery/helpers/model_cache.php @@ -18,27 +18,25 @@ * Foundation, Inc., 51 Franklin Street - Fifth Floor, Boston, MA 02110-1301, USA. */ class model_cache_Core { - private static $cache; + private static $cache = array(); static function get($model_name, $id, $field_name="id") { - if (TEST_MODE || empty(self::$cache->$model_name->$field_name->$id)) { + if (TEST_MODE || empty(self::$cache[$model_name][$field_name][$id])) { $model = ORM::factory($model_name)->where($field_name, "=", $id)->find(); if (!$model->loaded()) { throw new Exception("@todo MISSING_MODEL $model_name:$id"); } - self::$cache->$model_name->$field_name->$id = $model; + self::$cache[$model_name][$field_name][$id] = $model; } - return self::$cache->$model_name->$field_name->$id; + return self::$cache[$model_name][$field_name][$id]; } static function clear() { - self::$cache = new stdClass(); + self::$cache = array(); } static function set($model) { - self::$cache->{$model->object_name} - ->{$model->primary_key} - ->{$model->{$model->primary_key}} = $model; + self::$cache[$model->object_name][$model->primary_key][$model->{$model->primary_key}] = $model; } } -- cgit v1.2.3 From 51427d540464ffa5b4663a9c3b794cefc637925a Mon Sep 17 00:00:00 2001 From: Bharat Mediratta Date: Mon, 18 Jan 2010 12:21:57 -0800 Subject: Verified --- modules/gallery/tests/xss_data.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/modules/gallery/tests/xss_data.txt b/modules/gallery/tests/xss_data.txt index a264286c..1530c73e 100644 --- a/modules/gallery/tests/xss_data.txt +++ b/modules/gallery/tests/xss_data.txt @@ -137,7 +137,7 @@ modules/gallery/views/l10n_client.html.php 26 DIRTY $strin modules/gallery/views/l10n_client.html.php 32 DIRTY $l10n_search_form modules/gallery/views/l10n_client.html.php 41 DIRTY access::csrf_form_field() modules/gallery/views/l10n_client.html.php 42 DIRTY form::hidden("l10n-message-key") -modules/gallery/views/l10n_client.html.php 43 DIRTY form::textarea("l10n-edit-translation","",' rows="5" class="translationField"') +modules/gallery/views/l10n_client.html.php 43 DIRTY form::textarea("l10n-edit-translation","",' id="l10n-edit-translation" rows="5" class="translationField"') modules/gallery/views/l10n_client.html.php 46 DIRTY form::textarea("l10n-edit-plural-translation-zero","",' rows="2"') modules/gallery/views/l10n_client.html.php 50 DIRTY form::textarea("l10n-edit-plural-translation-one","",' rows="2"') modules/gallery/views/l10n_client.html.php 54 DIRTY form::textarea("l10n-edit-plural-translation-two","",' rows="2"') -- cgit v1.2.3 From cac4692510d6b5da0d0a63f2ec54783df6ca86e4 Mon Sep 17 00:00:00 2001 From: Bharat Mediratta Date: Mon, 18 Jan 2010 12:35:26 -0800 Subject: Don't use rand() as the name. Now that ORM::load_types() is gone, it won't get coerced to a string, and then we wind up comparing: 12345 != 12345-12321 In the old approach, they'd both be strings so they'd be inequal. But in the new approach the first value is an integer (sinced it came from rand()) so the second value is typecast to an integer which drops everything after the - sign so they appear equal. --- modules/gallery/tests/Album_Helper_Test.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/modules/gallery/tests/Album_Helper_Test.php b/modules/gallery/tests/Album_Helper_Test.php index 1284b8cc..ef0905da 100644 --- a/modules/gallery/tests/Album_Helper_Test.php +++ b/modules/gallery/tests/Album_Helper_Test.php @@ -38,7 +38,7 @@ class Album_Helper_Test extends Unit_Test_Case { } public function create_conflicting_album_test() { - $rand = rand(); + $rand = "name_" . rand(); $root = ORM::factory("item", 1); $album1 = album::create($root, $rand, $rand, $rand); $album2 = album::create($root, $rand, $rand, $rand); -- cgit v1.2.3 From cf236a228a8ea3316506f6d855bcade92676674c Mon Sep 17 00:00:00 2001 From: Bharat Mediratta Date: Mon, 18 Jan 2010 12:37:20 -0800 Subject: Don't assert_same() now that typecasting is gone from ORM. --- modules/gallery/tests/Item_Model_Test.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/modules/gallery/tests/Item_Model_Test.php b/modules/gallery/tests/Item_Model_Test.php index bf5fca1a..d03a03f4 100644 --- a/modules/gallery/tests/Item_Model_Test.php +++ b/modules/gallery/tests/Item_Model_Test.php @@ -52,7 +52,7 @@ class Item_Model_Test extends Unit_Test_Case { public function updating_view_count_only_doesnt_change_updated_date_test() { $item = self::_create_random_item(); $item->reload(); - $this->assert_same(0, $item->view_count); + $this->assert_equal(0, $item->view_count); // Force the updated date to something well known db::build() -- cgit v1.2.3 From a9f07986f64faf5062c2d86aa34710892d8ac8b3 Mon Sep 17 00:00:00 2001 From: Bharat Mediratta Date: Mon, 18 Jan 2010 12:39:42 -0800 Subject: The root parent id is 0, not null (this deviation exposed by the new lack of typecasting in ORM). --- modules/gallery/tests/Gallery_Installer_Test.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/modules/gallery/tests/Gallery_Installer_Test.php b/modules/gallery/tests/Gallery_Installer_Test.php index 43399fb4..74a07b1a 100644 --- a/modules/gallery/tests/Gallery_Installer_Test.php +++ b/modules/gallery/tests/Gallery_Installer_Test.php @@ -41,7 +41,7 @@ class Gallery_Installer_Test extends Unit_Test_Case { $this->assert_equal("Gallery", $root->title); $this->assert_equal(1, $root->left_ptr); $this->assert_equal($max_right_ptr, $root->right_ptr); - $this->assert_equal(null, $root->parent_id); + $this->assert_equal(0, $root->parent_id); $this->assert_equal(1, $root->level); } } -- cgit v1.2.3 From 06cabecd76781d7075cbacb5cf433042b4296dc5 Mon Sep 17 00:00:00 2001 From: Bharat Mediratta Date: Mon, 18 Jan 2010 12:49:58 -0800 Subject: Coerce some integers to strings now that ORM isn't typecasting anymore. --- modules/gallery/tests/Gallery_Rest_Helper_Test.php | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/modules/gallery/tests/Gallery_Rest_Helper_Test.php b/modules/gallery/tests/Gallery_Rest_Helper_Test.php index cd0aabae..f8cf6190 100644 --- a/modules/gallery/tests/Gallery_Rest_Helper_Test.php +++ b/modules/gallery/tests/Gallery_Rest_Helper_Test.php @@ -94,8 +94,8 @@ class Gallery_Rest_Helper_Test extends Unit_Test_Case { "path" => $photo->relative_url(), "thumb_url" => $photo->thumb_url(), "thumb_dimensions" => array( - "width" => $photo->thumb_width, - "height" => $photo->thumb_height), + "width" => (string)$photo->thumb_width, + "height" => (string)$photo->thumb_height), "has_thumb" => true, "title" => $photo->title))))), gallery_rest::get($request)); @@ -115,14 +115,14 @@ class Gallery_Rest_Helper_Test extends Unit_Test_Case { "parent_path" => $child->relative_url(), "title" => $photo->title, "thumb_url" => $photo->thumb_url(), - "thumb_size" => array("height" => $photo->thumb_height, - "width" => $photo->thumb_width), + "thumb_size" => array("height" => (string)$photo->thumb_height, + "width" => (string)$photo->thumb_width), "resize_url" => $photo->resize_url(), "resize_size" => array("height" => $photo->resize_height, "width" => $photo->resize_width), "url" => $photo->file_url(), - "size" => array("height" => $photo->height, - "width" => $photo->width), + "size" => array("height" => (string)$photo->height, + "width" => (string)$photo->width), "description" => $photo->description, "slug" => $photo->slug))), gallery_rest::get($request)); -- cgit v1.2.3 From 9384f987bb96d0d39787ff9d3d16a70c01cd76e0 Mon Sep 17 00:00:00 2001 From: Bharat Mediratta Date: Mon, 18 Jan 2010 12:52:52 -0800 Subject: Coerce some integers to strings now that ORM isn't typecasting anymore. --- modules/tag/tests/Tag_Rest_Helper_Test.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/modules/tag/tests/Tag_Rest_Helper_Test.php b/modules/tag/tests/Tag_Rest_Helper_Test.php index 4e8dd527..514538d4 100644 --- a/modules/tag/tests/Tag_Rest_Helper_Test.php +++ b/modules/tag/tests/Tag_Rest_Helper_Test.php @@ -85,8 +85,8 @@ class Tag_Rest_Helper_Test extends Unit_Test_Case { $this->assert_equal( json_encode(array("status" => "OK", - "tags" => array(array("name" => "albums", "count" => 2), - array("name" => "photos", "count" => 2)))), + "tags" => array(array("name" => "albums", "count" => "2"), + array("name" => "photos", "count" => "2")))), tag_rest::get($request)); } -- cgit v1.2.3