From 1d633457c4482ab96bf936e9951ded2d5ebc8c74 Mon Sep 17 00:00:00 2001 From: Andy Staudacher Date: Sat, 29 Aug 2009 11:31:00 -0700 Subject: Have url::site() and other methods return a SafeString, just as t() and t2(). Benefits: - url::site() is often used in views and we can ensure in the url class that returned strings are indeed safe for use in HTML. Makes the list of vars of unknown safety status shorter. - url::site() is often used as message parameter to t() and t2(). The parameter would be HTML-escaped if it wasn't marked as safe HTML already. Makes the usage simpler / shorter. --- modules/gallery/helpers/MY_url.php | 24 +++++++++++++++++++++++- 1 file changed, 23 insertions(+), 1 deletion(-) (limited to 'modules/gallery/helpers/MY_url.php') diff --git a/modules/gallery/helpers/MY_url.php b/modules/gallery/helpers/MY_url.php index c4967c52..b4b7f352 100644 --- a/modules/gallery/helpers/MY_url.php +++ b/modules/gallery/helpers/MY_url.php @@ -30,7 +30,8 @@ class url extends url_Core { if ($parts[0] == "albums" || $parts[0] == "photos") { $uri = model_cache::get("item", $parts[1])->relative_path(); } - return parent::site($uri . $query, $protocol); + $url = parent::site($uri . $query, $protocol); + return SafeString::of($url)->mark_html_safe(); } static function parse_url() { @@ -99,4 +100,25 @@ class url extends url_Core { static function abs_current($qs=false) { return self::abs_site(url::current($qs)); } + + public static function base($index=false, $protocol=false) { + $url = parent::base($index, $protocol); + return SafeString::of($url)->mark_html_safe(); + } + + public static function current($qs=false) { + $url = parent::current($qs); + return SafeString::of($url)->mark_html_safe(); + } + + public static function file($file, $index=false) { + $url = parent::file($file, $index); + return SafeString::of($url)->mark_html_safe(); + } + + public static function merge(array $arguments) { + $url = parent::merge($arguments); + return SafeString::of($url)->mark_html_safe(); + } + } -- cgit v1.2.3 From a10063ff68cf5988297dcad889384ab2080c3850 Mon Sep 17 00:00:00 2001 From: Andy Staudacher Date: Sat, 29 Aug 2009 12:34:09 -0700 Subject: Add more factory methods for convenience: SafeString::purify() and SafeString::of_safe_html(). Removing SafeString::mark_html_safe() since it's no longer needed. --- modules/gallery/helpers/MY_url.php | 10 +++++----- modules/gallery/libraries/I18n.php | 2 +- modules/gallery/libraries/SafeString.php | 27 +++++++++++++++++++++------ modules/gallery/tests/SafeString_Test.php | 19 +++++++++++-------- modules/gallery/tests/Xss_Security_Test.php | 7 +++++-- 5 files changed, 43 insertions(+), 22 deletions(-) (limited to 'modules/gallery/helpers/MY_url.php') diff --git a/modules/gallery/helpers/MY_url.php b/modules/gallery/helpers/MY_url.php index b4b7f352..6092a9d8 100644 --- a/modules/gallery/helpers/MY_url.php +++ b/modules/gallery/helpers/MY_url.php @@ -31,7 +31,7 @@ class url extends url_Core { $uri = model_cache::get("item", $parts[1])->relative_path(); } $url = parent::site($uri . $query, $protocol); - return SafeString::of($url)->mark_html_safe(); + return SafeString::of_safe_html($url); } static function parse_url() { @@ -103,22 +103,22 @@ class url extends url_Core { public static function base($index=false, $protocol=false) { $url = parent::base($index, $protocol); - return SafeString::of($url)->mark_html_safe(); + return SafeString::of_safe_html($url); } public static function current($qs=false) { $url = parent::current($qs); - return SafeString::of($url)->mark_html_safe(); + return SafeString::of_safe_html($url); } public static function file($file, $index=false) { $url = parent::file($file, $index); - return SafeString::of($url)->mark_html_safe(); + return SafeString::of_safe_html($url); } public static function merge(array $arguments) { $url = parent::merge($arguments); - return SafeString::of($url)->mark_html_safe(); + return SafeString::of_safe_html($url); } } diff --git a/modules/gallery/libraries/I18n.php b/modules/gallery/libraries/I18n.php index 8dc42e04..a53d5ae9 100644 --- a/modules/gallery/libraries/I18n.php +++ b/modules/gallery/libraries/I18n.php @@ -116,7 +116,7 @@ class I18n_Core { $entry = $this->interpolate($locale, $entry, $values); - return SafeString::of($entry)->mark_html_safe(); + return SafeString::of_safe_html($entry); } private function lookup($locale, $message) { diff --git a/modules/gallery/libraries/SafeString.php b/modules/gallery/libraries/SafeString.php index 709ab5f6..9a269ed4 100644 --- a/modules/gallery/libraries/SafeString.php +++ b/modules/gallery/libraries/SafeString.php @@ -24,6 +24,7 @@ class SafeString_Core { private $_raw_string; protected $_is_safe_html = false; + protected $_is_purified_html = false; private static $_purifier = null; @@ -44,11 +45,25 @@ class SafeString_Core { } /** - * Marks this string as safe to be used in HTML without any escaping. + * Factory method returning a new SafeString instance after HTML purifying + * the given string. */ - function mark_html_safe() { - $this->_is_safe_html = true; - return $this; + static function purify($string) { + if ($string instanceof SafeString) { + $string = $string->unescaped(); + } + $safe_string = self::of_safe_html(self::_purify_for_html($string)); + $safe_string->_is_purified_html = true; + return $safe_string; + } + + /** + * Factory method returning a new SafeString instance which won't HTML escape. + */ + static function of_safe_html($string) { + $safe_string = new SafeString($string); + $safe_string->_is_safe_html = true; + return $safe_string; } /** @@ -117,10 +132,10 @@ class SafeString_Core { * @return the string escaped for use in HTML. */ function purified_html() { - if ($this->_is_safe_html) { + if ($this->_is_purified_html) { return $this; } else { - return SafeString::of(self::_purify_for_html($this->_raw_string), true); + return self::purify($this); } } diff --git a/modules/gallery/tests/SafeString_Test.php b/modules/gallery/tests/SafeString_Test.php index 73d82c34..0fc7f6f3 100644 --- a/modules/gallery/tests/SafeString_Test.php +++ b/modules/gallery/tests/SafeString_Test.php @@ -25,8 +25,7 @@ class SafeString_Test extends Unit_Test_Case { } public function toString_for_safe_string_test() { - $safe_string = new SafeString("hello

world

"); - $safe_string->mark_html_safe(); + $safe_string = SafeString::of_safe_html("hello

world

"); $this->assert_equal("hello

world

", $safe_string); } @@ -62,7 +61,7 @@ class SafeString_Test extends Unit_Test_Case { } public function for_html_attr_with_safe_html_test() { - $safe_string = SafeString::of('"Foo\'s bar"')->mark_html_safe(); + $safe_string = SafeString::of_safe_html('"Foo\'s bar"'); $attr_string = $safe_string->for_html_attr(); $this->assert_equal('"Foo's bar"', $attr_string); @@ -86,25 +85,29 @@ class SafeString_Test extends Unit_Test_Case { } public function of_safe_html_test() { - $safe_string = SafeString::of("hello

world

")->mark_html_safe(); + $safe_string = SafeString::of_safe_html("hello

world

"); $this->assert_equal("hello

world

", $safe_string->for_html()); } + public function purify_test() { + $safe_string = SafeString::purify("hello

world

"); + $this->assert_equal("hello

world

", $safe_string); + } + public function of_fluid_api_test() { $escaped_string = SafeString::of("Foo's bar")->for_js(); $this->assert_equal("Foo\\'s bar", $escaped_string); } public function safestring_of_safestring_preserves_safe_status_test() { - $safe_string = SafeString::of("hello's

world

")->mark_html_safe(); + $safe_string = SafeString::of_safe_html("hello's

world

"); $safe_string_2 = new SafeString($safe_string); $this->assert_equal("hello's

world

", $safe_string_2); $this->assert_equal("hello\\'s

world<\\/p>", $safe_string_2->for_js()); } public function safestring_of_safestring_preserves_html_safe_status_test() { - $safe_string = SafeString::of("hello's

world

") - ->mark_html_safe(); + $safe_string = SafeString::of_safe_html("hello's

world

"); $safe_string_2 = new SafeString($safe_string); $this->assert_equal("hello's

world

", $safe_string_2); $this->assert_equal("hello\\'s

world<\\/p>", $safe_string_2->for_js()); @@ -112,7 +115,7 @@ class SafeString_Test extends Unit_Test_Case { public function safestring_of_safestring_safe_status_override_test() { $safe_string = new SafeString("hello

world

"); - $safe_string_2 = SafeString::of($safe_string)->mark_html_safe(); + $safe_string_2 = SafeString::of_safe_html($safe_string); $this->assert_equal("hello

world

", $safe_string_2); } } diff --git a/modules/gallery/tests/Xss_Security_Test.php b/modules/gallery/tests/Xss_Security_Test.php index e0e5bb86..fd596c69 100644 --- a/modules/gallery/tests/Xss_Security_Test.php +++ b/modules/gallery/tests/Xss_Security_Test.php @@ -110,10 +110,13 @@ class Xss_Security_Test extends Unit_Test_Case { } else if ($token[1] == "SafeString") { // Looking for SafeString::of(... if (self::_token_matches(array(T_DOUBLE_COLON, "::"), $tokens, $token_number + 1) && - self::_token_matches(array(T_STRING, "of"), $tokens, $token_number + 2) && + self::_token_matches(array(T_STRING), $tokens, $token_number + 2) && + in_array($tokens[$token_number + 2][1], array("of", "of_safe_html", "purify")) && self::_token_matches("(", $tokens, $token_number + 3)) { $frame->is_safestring(true); - $frame->expr_append("::of("); + + $method = $tokens[$token_number + 2][1]; + $frame->expr_append("::$method("); $token_number += 3; $token = $tokens[$token_number]; -- cgit v1.2.3 From b4b638be44375c93f5222c7b48ed547845d6d7e5 Mon Sep 17 00:00:00 2001 From: Andy Staudacher Date: Sat, 29 Aug 2009 16:28:30 -0700 Subject: Undo url helper changes - url methods no longer return a SafeString. Adding SafeString::of_safe_html() calls where urls are passed as parameters to t() and t2(). --- modules/akismet/helpers/akismet.php | 2 +- modules/digibug/views/admin_digibug.html.php | 2 +- modules/exif/helpers/exif.php | 2 +- modules/g2_import/views/admin_g2_import.html.php | 10 ++++----- modules/gallery/helpers/MY_url.php | 24 +--------------------- modules/gallery/helpers/graphics.php | 2 +- modules/gallery/tests/Xss_Security_Test.php | 15 +++++++++++--- modules/gallery/views/admin_block_welcome.html.php | 10 ++++----- modules/gallery/views/upgrader.html.php | 2 +- modules/recaptcha/helpers/recaptcha.php | 2 +- modules/search/helpers/search.php | 2 +- modules/server_add/helpers/server_add.php | 2 +- modules/user/views/reset_password.html.php | 4 +++- system/helpers/request.php | 2 +- 14 files changed, 35 insertions(+), 46 deletions(-) (limited to 'modules/gallery/helpers/MY_url.php') diff --git a/modules/akismet/helpers/akismet.php b/modules/akismet/helpers/akismet.php index db45a6ab..abca78d2 100644 --- a/modules/akismet/helpers/akismet.php +++ b/modules/akismet/helpers/akismet.php @@ -94,7 +94,7 @@ class akismet_Core { if (empty($api_key)) { site_status::warning( t("Akismet is not quite ready! Please provide an API Key", - array("url" => url::site("admin/akismet"))), + array("url" => SafeString::of_safe_html(url::site("admin/akismet")))), "akismet_config"); } else { site_status::clear("akismet_config"); diff --git a/modules/digibug/views/admin_digibug.html.php b/modules/digibug/views/admin_digibug.html.php index 7e4436ff..5f27a3fd 100644 --- a/modules/digibug/views/admin_digibug.html.php +++ b/modules/digibug/views/admin_digibug.html.php @@ -16,7 +16,7 @@

register with Digibug and enter your Digibug id in the Advanced Settings page you can make money off of your photos!", array("signup_url" => "http://www.digibug.com/signup.php", - "advanced_settings_url" => url::site("admin/advanced_settings"))) ?> + "advanced_settings_url" => SafeString::of_safe_html(url::site("admin/advanced_settings")))) ?>

diff --git a/modules/exif/helpers/exif.php b/modules/exif/helpers/exif.php index 20ecd0cb..d4e60338 100644 --- a/modules/exif/helpers/exif.php +++ b/modules/exif/helpers/exif.php @@ -164,7 +164,7 @@ class exif_Core { if ($remaining) { site_status::warning( t('Your Exif index needs to be updated. Fix this now', - array("url" => url::site("admin/maintenance/start/exif_task::update_index?csrf=__CSRF__"))), + array("url" => SafeString::of_safe_html(url::site("admin/maintenance/start/exif_task::update_index?csrf=__CSRF__")))), "exif_index_out_of_date"); } } diff --git a/modules/g2_import/views/admin_g2_import.html.php b/modules/g2_import/views/admin_g2_import.html.php index da2bb5d1..f53510f6 100644 --- a/modules/g2_import/views/admin_g2_import.html.php +++ b/modules/g2_import/views/admin_g2_import.html.php @@ -37,9 +37,9 @@
  • Using the same value will speed up your import.", - array("g2_pixels" => $g2_sizes["thumb"]["size"], - "g3_pixels" => $thumb_size, - "url" => url::site("admin/theme_options"))) ?> + array("g2_pixels" => $g2_sizes["thumb"]["size"], + "g3_pixels" => $thumb_size, + "url" => SafeString::of_safe_html(url::site("admin/theme_options")))) ?>
    • @@ -47,8 +47,8 @@
  • Using the same value will speed up your import.", array("g2_pixels" => $g2_sizes["resize"]["size"], - "g3_pixels" => $resize_size, - "url" => url::site("admin/theme_options"))) ?> + "g3_pixels" => $resize_size, + "url" => SafeString::of_safe_html(url::site("admin/theme_options")))) ?>
    • diff --git a/modules/gallery/helpers/MY_url.php b/modules/gallery/helpers/MY_url.php index 6092a9d8..c4967c52 100644 --- a/modules/gallery/helpers/MY_url.php +++ b/modules/gallery/helpers/MY_url.php @@ -30,8 +30,7 @@ class url extends url_Core { if ($parts[0] == "albums" || $parts[0] == "photos") { $uri = model_cache::get("item", $parts[1])->relative_path(); } - $url = parent::site($uri . $query, $protocol); - return SafeString::of_safe_html($url); + return parent::site($uri . $query, $protocol); } static function parse_url() { @@ -100,25 +99,4 @@ class url extends url_Core { static function abs_current($qs=false) { return self::abs_site(url::current($qs)); } - - public static function base($index=false, $protocol=false) { - $url = parent::base($index, $protocol); - return SafeString::of_safe_html($url); - } - - public static function current($qs=false) { - $url = parent::current($qs); - return SafeString::of_safe_html($url); - } - - public static function file($file, $index=false) { - $url = parent::file($file, $index); - return SafeString::of_safe_html($url); - } - - public static function merge(array $arguments) { - $url = parent::merge($arguments); - return SafeString::of_safe_html($url); - } - } diff --git a/modules/gallery/helpers/graphics.php b/modules/gallery/helpers/graphics.php index 7dc46eeb..fbb85bec 100644 --- a/modules/gallery/helpers/graphics.php +++ b/modules/gallery/helpers/graphics.php @@ -442,7 +442,7 @@ class graphics_Core { if (!module::get_var("gallery", "graphics_toolkit")) { site_status::warning( t("Graphics toolkit missing! Please choose a toolkit", - array("url" => url::site("admin/graphics"))), + array("url" => SafeString::of_safe_html(url::site("admin/graphics")))), "missing_graphics_toolkit"); } } diff --git a/modules/gallery/tests/Xss_Security_Test.php b/modules/gallery/tests/Xss_Security_Test.php index 690dc760..a2d3d59b 100644 --- a/modules/gallery/tests/Xss_Security_Test.php +++ b/modules/gallery/tests/Xss_Security_Test.php @@ -130,14 +130,14 @@ class Xss_Security_Test extends Unit_Test_Case { $token = $tokens[$token_number]; } } else if ($token[1] == "url") { - // url methods return a SafeString + // url methods return safe HTML if (self::_token_matches(array(T_DOUBLE_COLON, "::"), $tokens, $token_number + 1) && self::_token_matches(array(T_STRING), $tokens, $token_number + 2) && in_array($tokens[$token_number + 2][1], array("site", "current", "base", "file", "abs_site", "abs_current", "abs_file", "merge")) && self::_token_matches("(", $tokens, $token_number + 3)) { - $frame->is_safestring(true); + $frame->is_safe_html(true); $method = $tokens[$token_number + 2][1]; $frame->expr_append("::$method("); @@ -203,7 +203,8 @@ class Xss_Security_Test extends Unit_Test_Case { $state = "CLEAN"; } } else { - if ($frame->is_safestring() || $frame->purified_html_called() || $frame->for_html_called()) { + if ($frame->is_safe_html() || $frame->is_safestring() || + $frame->purified_html_called() || $frame->for_html_called()) { $state = "CLEAN"; } } @@ -259,6 +260,7 @@ class Xss_Security_Test_Frame { private $_for_html_called = false; private $_purified_html_called = false; private $_json_encode_called = false; + private $_is_safe_html = false; private $_line; function __construct($line_number, $in_script_block) { @@ -288,6 +290,13 @@ class Xss_Security_Test_Frame { return $this->_is_safestring; } + function is_safe_html($new_val=NULL) { + if ($new_val !== NULL) { + $this->_is_safe_html = (bool) $new_val; + } + return $this->_is_safe_html; + } + function json_encode_called($new_val=NULL) { if ($new_val !== NULL) { $this->_json_encode_called = (bool) $new_val; diff --git a/modules/gallery/views/admin_block_welcome.html.php b/modules/gallery/views/admin_block_welcome.html.php index 38d2bd56..c6ccdbf3 100644 --- a/modules/gallery/views/admin_block_welcome.html.php +++ b/modules/gallery/views/admin_block_welcome.html.php @@ -5,16 +5,16 @@
    • graphics and language settings.", - array("graphics_url" => url::site("admin/graphics"), - "language_url" => url::site("admin/languages"))) ?> + array("graphics_url" => SafeString::of_safe_html(url::site("admin/graphics")), + "language_url" => SafeString::of_safe_html(url::site("admin/languages")))) ?>
    • choose a theme, or customize the way it looks.", - array("theme_url" => url::site("admin/themes"), - "theme_options_url" => url::site("admin/theme_options"))) ?> + array("theme_url" => SafeString::of_safe_html(url::site("admin/themes")), + "theme_options_url" => SafeString::of_safe_html(url::site("admin/theme_options")))) ?>
    • install modules to add cool features!", - array("modules_url" => url::site("admin/modules"))) ?> + array("modules_url" => SafeString::of_safe_html(url::site("admin/modules")))) ?>
    diff --git a/modules/gallery/views/upgrader.html.php b/modules/gallery/views/upgrader.html.php index 37578855..ccc86da8 100644 --- a/modules/gallery/views/upgrader.html.php +++ b/modules/gallery/views/upgrader.html.php @@ -18,7 +18,7 @@

    Gallery is up to date.", - array("url" => url::site("albums/1"))) ?> + array("url" => SafeString::of_safe_html(url::site("albums/1")))) ?>

    diff --git a/modules/recaptcha/helpers/recaptcha.php b/modules/recaptcha/helpers/recaptcha.php index 501dd972..35d9febd 100644 --- a/modules/recaptcha/helpers/recaptcha.php +++ b/modules/recaptcha/helpers/recaptcha.php @@ -43,7 +43,7 @@ class recaptcha_Core { if (empty($public_key) || empty($private_key)) { site_status::warning( t("reCAPTCHA is not quite ready! Please configure the reCAPTCHA Keys", - array("url" => url::site("admin/recaptcha"))), + array("url" => SafeString::of_safe_html(url::site("admin/recaptcha")))), "recaptcha_config"); } else { site_status::clear("recaptcha_config"); diff --git a/modules/search/helpers/search.php b/modules/search/helpers/search.php index 355c4493..4be04039 100644 --- a/modules/search/helpers/search.php +++ b/modules/search/helpers/search.php @@ -58,7 +58,7 @@ class search_Core { if ($remaining) { site_status::warning( t('Your search index needs to be updated. Fix this now', - array("url" => url::site("admin/maintenance/start/search_task::update_index?csrf=__CSRF__"))), + array("url" => SafeString::of_safe_html(url::site("admin/maintenance/start/search_task::update_index?csrf=__CSRF__")))), "search_index_out_of_date"); } } diff --git a/modules/server_add/helpers/server_add.php b/modules/server_add/helpers/server_add.php index 74f51ad9..57afac12 100644 --- a/modules/server_add/helpers/server_add.php +++ b/modules/server_add/helpers/server_add.php @@ -25,7 +25,7 @@ class server_add_Core { if (empty($paths)) { site_status::warning( t("Server Add needs configuration. Configure it now!", - array("url" => url::site("admin/server_add"))), + array("url" => SafeString::of_safe_html(url::site("admin/server_add")))), "server_add_configuration"); } else { site_status::clear("server_add_configuration"); diff --git a/modules/user/views/reset_password.html.php b/modules/user/views/reset_password.html.php index 3dc7aebf..6fa92d54 100644 --- a/modules/user/views/reset_password.html.php +++ b/modules/user/views/reset_password.html.php @@ -9,7 +9,9 @@ $user->full_name ? $user->full_name : $user->name)) ?>

    - %site_url. If you made this request, you can confirm it by clicking this link. If you didn't request this password reset, it's ok to ignore this mail.", array("site_url" => url::base(false, "http"), "confirm_url" => $confirm_url)) ?> + %site_url. If you made this request, you can confirm it by clicking this link. If you didn't request this password reset, it's ok to ignore this mail.", + array("site_url" => SafeString::of_safe_html(url::base(false, "http")), + "confirm_url" => $confirm_url)) ?>

    diff --git a/system/helpers/request.php b/system/helpers/request.php index 15b8edfa..4203d0e5 100644 --- a/system/helpers/request.php +++ b/system/helpers/request.php @@ -30,7 +30,7 @@ class request_Core { // Set referrer $ref = $_SERVER['HTTP_REFERER']; - if (strpos($ref, (string) url::base(FALSE)) === 0) + if (strpos($ref, url::base(FALSE)) === 0) { // Remove the base URL from the referrer $ref = substr($ref, strlen(url::base(FALSE))); -- cgit v1.2.3 From a73b5e822684ae0138f3baa733b0013122b0368c Mon Sep 17 00:00:00 2001 From: Bharat Mediratta Date: Mon, 7 Sep 2009 20:59:42 -0700 Subject: Switch to using Item_Model::relative_url() for the url path. --- modules/gallery/helpers/MY_url.php | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) (limited to 'modules/gallery/helpers/MY_url.php') diff --git a/modules/gallery/helpers/MY_url.php b/modules/gallery/helpers/MY_url.php index c4967c52..6e3657fd 100644 --- a/modules/gallery/helpers/MY_url.php +++ b/modules/gallery/helpers/MY_url.php @@ -28,7 +28,7 @@ class url extends url_Core { $parts = explode("/", $uri, 3); if ($parts[0] == "albums" || $parts[0] == "photos") { - $uri = model_cache::get("item", $parts[1])->relative_path(); + $uri = model_cache::get("item", $parts[1])->relative_url(); } return parent::site($uri . $query, $protocol); } @@ -55,17 +55,20 @@ class url extends url_Core { } /** - * Return the item that the uri is referencing + * Locate an item using the URI. We assume that the uri is in the form /a/b/c where each + * component matches up with an item slug. + * @param string $uri the uri fragment + * @return Item_Model */ static function get_item_from_uri($uri) { $current_uri = html_entity_decode($uri, ENT_QUOTES); - $item = ORM::factory("item")->where("relative_path_cache", $current_uri)->find(); + // In most cases, we'll have an exact match in the relative_url_cache item field. + // but failing that, walk down the tree until we find it. + $item = ORM::factory("item")->where("relative_url_cache", $current_uri)->find(); if (!$item->loaded) { - // It's possible that the relative path cache for the item we're looking for is out of date, - // so find it the hard way. $count = count(Router::$segments); foreach (ORM::factory("item") - ->where("name", html_entity_decode(Router::$segments[$count - 1], ENT_QUOTES)) + ->where("slug", html_entity_decode(Router::$segments[$count - 1], ENT_QUOTES)) ->where("level", $count + 1) ->find_all() as $match) { if ($match->relative_path() == $current_uri) { -- cgit v1.2.3 From 7889ae1085963eaf40afbb72c044da7630a5af63 Mon Sep 17 00:00:00 2001 From: Bharat Mediratta Date: Mon, 7 Sep 2009 21:33:00 -0700 Subject: Fix a bug where we were not properly decoding the path in the fallback code in get_item_from_uri() by using relative_url() instead of relative_path(). --- modules/gallery/helpers/MY_url.php | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) (limited to 'modules/gallery/helpers/MY_url.php') diff --git a/modules/gallery/helpers/MY_url.php b/modules/gallery/helpers/MY_url.php index 6e3657fd..c25f2293 100644 --- a/modules/gallery/helpers/MY_url.php +++ b/modules/gallery/helpers/MY_url.php @@ -63,7 +63,8 @@ class url extends url_Core { static function get_item_from_uri($uri) { $current_uri = html_entity_decode($uri, ENT_QUOTES); // In most cases, we'll have an exact match in the relative_url_cache item field. - // but failing that, walk down the tree until we find it. + // but failing that, walk down the tree until we find it. The fallback code will fix caches + // as it goes, so it'll never be run frequently. $item = ORM::factory("item")->where("relative_url_cache", $current_uri)->find(); if (!$item->loaded) { $count = count(Router::$segments); @@ -71,7 +72,7 @@ class url extends url_Core { ->where("slug", html_entity_decode(Router::$segments[$count - 1], ENT_QUOTES)) ->where("level", $count + 1) ->find_all() as $match) { - if ($match->relative_path() == $current_uri) { + if ($match->relative_url() == $current_uri) { $item = $match; } } -- cgit v1.2.3 From 714a82d1e01996df451ebffb321181441ed065df Mon Sep 17 00:00:00 2001 From: Bharat Mediratta Date: Mon, 7 Sep 2009 21:49:19 -0700 Subject: Automagically generate pretty urls for movies, too. --- modules/gallery/helpers/MY_url.php | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) (limited to 'modules/gallery/helpers/MY_url.php') diff --git a/modules/gallery/helpers/MY_url.php b/modules/gallery/helpers/MY_url.php index c25f2293..129efb91 100644 --- a/modules/gallery/helpers/MY_url.php +++ b/modules/gallery/helpers/MY_url.php @@ -27,7 +27,9 @@ class url extends url_Core { } $parts = explode("/", $uri, 3); - if ($parts[0] == "albums" || $parts[0] == "photos") { + // @todo if we're only doing this for Item_Model, why not just put this + // all into Item_Model::url()? It'd make url::site() faster. + if ($parts[0] == "albums" || $parts[0] == "photos" || $parts[0] == "movies") { $uri = model_cache::get("item", $parts[1])->relative_url(); } return parent::site($uri . $query, $protocol); -- cgit v1.2.3 From 60848480887594e61339000259e5322340518071 Mon Sep 17 00:00:00 2001 From: Bharat Mediratta Date: Tue, 8 Sep 2009 10:20:06 -0700 Subject: Improve comment. --- modules/gallery/helpers/MY_url.php | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) (limited to 'modules/gallery/helpers/MY_url.php') diff --git a/modules/gallery/helpers/MY_url.php b/modules/gallery/helpers/MY_url.php index 129efb91..1ca9a58f 100644 --- a/modules/gallery/helpers/MY_url.php +++ b/modules/gallery/helpers/MY_url.php @@ -26,9 +26,11 @@ class url extends url_Core { $query = ""; } - $parts = explode("/", $uri, 3); // @todo if we're only doing this for Item_Model, why not just put this - // all into Item_Model::url()? It'd make url::site() faster. + // all into Item_Model::url()? It'd make url::site() faster. Downside is that + // anywhere we refer to an item by id, eg url::site("albums/123") would have + // to load the item and do $item->url(); + $parts = explode("/", $uri, 3); if ($parts[0] == "albums" || $parts[0] == "photos" || $parts[0] == "movies") { $uri = model_cache::get("item", $parts[1])->relative_url(); } -- cgit v1.2.3 From 2aad580f53dbc06bb170c710467b47a5a532c6c8 Mon Sep 17 00:00:00 2001 From: Bharat Mediratta Date: Tue, 8 Sep 2009 13:44:52 -0700 Subject: Move specialized (pretty) url generation back into Item_Model so that we're not relying on overriding url::site() to do tricks around item urls. This means that you won't get item urls by doing url::site("albums/37"), for example, but it also means that we won't get pretty urls where we don't expect them (like in the action of a
    element). Incidentally, this will help us move over to using the slug format because if you've got a bad character in a url, the edit forms will now work on it since they'll be id based. --- modules/gallery/controllers/admin_themes.php | 4 +- modules/gallery/controllers/after_install.php | 2 +- modules/gallery/controllers/albums.php | 16 ++++---- modules/gallery/controllers/move.php | 2 +- modules/gallery/controllers/movies.php | 2 +- modules/gallery/controllers/photos.php | 2 +- modules/gallery/controllers/quick.php | 2 +- modules/gallery/helpers/MY_url.php | 19 ---------- modules/gallery/helpers/gallery.php | 44 +++++++++++----------- modules/gallery/helpers/gallery_rss.php | 4 +- modules/gallery/helpers/item.php | 8 ++++ modules/gallery/models/item.php | 21 +++++++++-- .../views/admin_block_photo_stream.html.php | 2 +- modules/gallery/views/upgrader.html.php | 2 +- .../notification/views/comment_published.html.php | 4 +- modules/notification/views/item_deleted.html.php | 4 +- modules/search/views/search.html.php | 2 +- modules/user/controllers/login.php | 2 +- modules/user/controllers/logout.php | 2 +- modules/user/controllers/password.php | 2 +- themes/admin_default/views/admin.html.php | 6 +-- themes/default/views/page.html.php | 4 +- 22 files changed, 80 insertions(+), 76 deletions(-) (limited to 'modules/gallery/helpers/MY_url.php') diff --git a/modules/gallery/controllers/admin_themes.php b/modules/gallery/controllers/admin_themes.php index da001c55..24f91aba 100644 --- a/modules/gallery/controllers/admin_themes.php +++ b/modules/gallery/controllers/admin_themes.php @@ -38,7 +38,7 @@ class Admin_Themes_Controller extends Admin_Controller { $theme_info = new ArrayObject(parse_ini_file($file), ArrayObject::ARRAY_AS_PROPS); $theme_info->description = t($theme_info->description); $theme_info->name = t($theme_info->name); - + $themes[$theme_name] = $theme_info; } return $themes; @@ -54,7 +54,7 @@ class Admin_Themes_Controller extends Admin_Controller { if ($type == "admin") { $view->url = url::site("admin?theme=$theme_name"); } else { - $view->url = url::site("albums/1?theme=$theme_name"); + $view->url = item::root()->url("theme=$theme_name"); } print $view; } diff --git a/modules/gallery/controllers/after_install.php b/modules/gallery/controllers/after_install.php index f066afe4..b640092f 100644 --- a/modules/gallery/controllers/after_install.php +++ b/modules/gallery/controllers/after_install.php @@ -20,7 +20,7 @@ class After_Install_Controller extends Controller { public function index() { if (!user::active()->admin) { - url::redirect("albums/1"); + url::redirect(item::root()->url()); } $v = new View("after_install.html"); diff --git a/modules/gallery/controllers/albums.php b/modules/gallery/controllers/albums.php index b7a9f339..abcabfa6 100644 --- a/modules/gallery/controllers/albums.php +++ b/modules/gallery/controllers/albums.php @@ -42,9 +42,9 @@ class Albums_Controller extends Items_Controller { $index = $album->get_position($show); $page = ceil($index / $page_size); if ($page == 1) { - url::redirect("albums/$album->id"); + url::redirect($album->url()); } else { - url::redirect("albums/$album->id?page=$page"); + url::redirect($album->url("page=$page")); } } @@ -55,9 +55,9 @@ class Albums_Controller extends Items_Controller { // Make sure that the page references a valid offset if ($page < 1) { - url::redirect("albums/$album->id"); + url::redirect($album->url()); } else if ($page > $max_pages) { - url::redirect("albums/$album->id?page=$max_pages"); + url::redirect($album->url("page=$max_pages")); } $template = new Theme_View("page.html", "album"); @@ -116,8 +116,8 @@ class Albums_Controller extends Items_Controller { print json_encode( array("result" => "success", - "location" => url::site("albums/$new_album->id"), - "resource" => url::site("albums/$new_album->id"))); + "location" => $new_album->url(), + "resource" => $new_album->url())); } else { print json_encode( array( @@ -149,8 +149,8 @@ class Albums_Controller extends Items_Controller { print json_encode( array("result" => "success", - "resource" => url::site("photos/$photo->id"), - "location" => url::site("photos/$photo->id"))); + "resource" => $photo->url(), + "location" => $photo->url())); } else { print json_encode( array("result" => "error", diff --git a/modules/gallery/controllers/move.php b/modules/gallery/controllers/move.php index 93ef05a6..87b73436 100644 --- a/modules/gallery/controllers/move.php +++ b/modules/gallery/controllers/move.php @@ -43,7 +43,7 @@ class Move_Controller extends Controller { print json_encode( array("result" => "success", - "location" => url::site("albums/{$target->id}"))); + "location" => $target->url())); } public function show_sub_tree($source_id, $target_id) { diff --git a/modules/gallery/controllers/movies.php b/modules/gallery/controllers/movies.php index c549dbf8..1c266cc8 100644 --- a/modules/gallery/controllers/movies.php +++ b/modules/gallery/controllers/movies.php @@ -105,7 +105,7 @@ class Movies_Controller extends Items_Controller { $photo->save(); module::event("item_edit_form_completed", $photo, $form); - log::success("content", "Updated photo", "id\">view"); + log::success("content", "Updated photo", "url()}\">view"); message::success( t("Saved photo %photo_title", array("photo_title" => $photo->title))); diff --git a/modules/gallery/controllers/photos.php b/modules/gallery/controllers/photos.php index 959097b2..79ad674a 100644 --- a/modules/gallery/controllers/photos.php +++ b/modules/gallery/controllers/photos.php @@ -97,7 +97,7 @@ class Photos_Controller extends Items_Controller { $photo->save(); module::event("item_edit_form_completed", $photo, $form); - log::success("content", "Updated photo", "id\">view"); + log::success("content", "Updated photo", "url()}\">view"); message::success( t("Saved photo %photo_title", array("photo_title" => html::purify($photo->title)))); diff --git a/modules/gallery/controllers/quick.php b/modules/gallery/controllers/quick.php index 20731f9c..2ac54754 100644 --- a/modules/gallery/controllers/quick.php +++ b/modules/gallery/controllers/quick.php @@ -121,7 +121,7 @@ class Quick_Controller extends Controller { print json_encode(array("result" => "success", "reload" => 1)); } else { print json_encode(array("result" => "success", - "location" => url::site("albums/$parent->id"))); + "location" => $parent->url())); } } diff --git a/modules/gallery/helpers/MY_url.php b/modules/gallery/helpers/MY_url.php index 1ca9a58f..368c947e 100644 --- a/modules/gallery/helpers/MY_url.php +++ b/modules/gallery/helpers/MY_url.php @@ -18,25 +18,6 @@ * Foundation, Inc., 51 Franklin Street - Fifth Floor, Boston, MA 02110-1301, USA. */ class url extends url_Core { - static function site($uri, $protocol=false) { - if (($pos = strpos($uri, "?")) !== false) { - list ($uri, $query) = explode("?", $uri, 2); - $query = "?$query"; - } else { - $query = ""; - } - - // @todo if we're only doing this for Item_Model, why not just put this - // all into Item_Model::url()? It'd make url::site() faster. Downside is that - // anywhere we refer to an item by id, eg url::site("albums/123") would have - // to load the item and do $item->url(); - $parts = explode("/", $uri, 3); - if ($parts[0] == "albums" || $parts[0] == "photos" || $parts[0] == "movies") { - $uri = model_cache::get("item", $parts[1])->relative_url(); - } - return parent::site($uri . $query, $protocol); - } - static function parse_url() { if (Router::$controller) { return; diff --git a/modules/gallery/helpers/gallery.php b/modules/gallery/helpers/gallery.php index f72ef982..813134eb 100644 --- a/modules/gallery/helpers/gallery.php +++ b/modules/gallery/helpers/gallery.php @@ -82,9 +82,9 @@ class gallery_Core { static function site_menu($menu, $theme) { if ($theme->page_type != "login") { $menu->append(Menu::factory("link") - ->id("home") - ->label(t("Home")) - ->url(url::site("albums/1"))); + ->id("home") + ->label(t("Home")) + ->url(item::root()->url())); $item = $theme->item(); @@ -92,39 +92,39 @@ class gallery_Core { $can_add = $item && access::can("add", $item); if ($can_add) { - $menu->append($add_menu = Menu::factory("submenu") - ->id("add_menu") - ->label(t("Add"))); + $menu->append($add_menu = Menu::factory("submenu") + ->id("add_menu") + ->label(t("Add"))); $add_menu->append(Menu::factory("dialog") - ->id("add_photos_item") - ->label(t("Add photos")) - ->url(url::site("simple_uploader/app/$item->id"))); + ->id("add_photos_item") + ->label(t("Add photos")) + ->url(url::site("simple_uploader/app/$item->id"))); if ($item->is_album()) { - $add_menu->append(Menu::factory("dialog") - ->id("add_album_item") - ->label(t("Add an album")) - ->url(url::site("form/add/albums/$item->id?type=album"))); - } + $add_menu->append(Menu::factory("dialog") + ->id("add_album_item") + ->label(t("Add an album")) + ->url(url::site("form/add/albums/$item->id?type=album"))); + } } $menu->append($options_menu = Menu::factory("submenu") - ->id("options_menu") - ->label(t("Photo options"))); + ->id("options_menu") + ->label(t("Photo options"))); if ($item && ($can_edit || $can_add)) { if ($can_edit) { $options_menu->append(Menu::factory("dialog") - ->id("edit_item") - ->label($item->is_album() ? t("Edit album") : t("Edit photo")) - ->url(url::site("form/edit/{$item->type}s/$item->id"))); + ->id("edit_item") + ->label($item->is_album() ? t("Edit album") : t("Edit photo")) + ->url(url::site("form/edit/{$item->type}s/$item->id"))); } if ($item->is_album()) { $options_menu->label(t("Album options")); if ($can_edit) { $options_menu->append(Menu::factory("dialog") - ->id("edit_permissions") - ->label(t("Edit permissions")) - ->url(url::site("permissions/browse/$item->id"))); + ->id("edit_permissions") + ->label(t("Edit permissions")) + ->url(url::site("permissions/browse/$item->id"))); } } } diff --git a/modules/gallery/helpers/gallery_rss.php b/modules/gallery/helpers/gallery_rss.php index dee6ae40..f30df092 100644 --- a/modules/gallery/helpers/gallery_rss.php +++ b/modules/gallery/helpers/gallery_rss.php @@ -40,7 +40,7 @@ class gallery_rss_Core { $feed->max_pages = ceil($all_children->find_all()->count() / $limit); $feed->title = t("Recent Updates"); - $feed->link = url::abs_site("albums/1"); + $feed->link = item::root()->abs_url(); $feed->description = t("Recent Updates"); return $feed; @@ -54,7 +54,7 @@ class gallery_rss_Core { $feed->max_pages = ceil( $item->viewable()->descendants_count(array("type" => "photo")) / $limit); $feed->title = html::purify($item->title); - $feed->link = url::abs_site("albums/{$item->id}"); + $feed->link = $item->abs_url(); $feed->description = nl2br(html::purify($item->description)); return $feed; diff --git a/modules/gallery/helpers/item.php b/modules/gallery/helpers/item.php index 8da88b6e..d907a177 100644 --- a/modules/gallery/helpers/item.php +++ b/modules/gallery/helpers/item.php @@ -184,4 +184,12 @@ class item_Core { return $model; } + + /** + * Return the root Item_Model + * @return Item_Model + */ + static function root() { + return model_cache::get("item", 1); + } } \ No newline at end of file diff --git a/modules/gallery/models/item.php b/modules/gallery/models/item.php index 0ec5d048..6e9debea 100644 --- a/modules/gallery/models/item.php +++ b/modules/gallery/models/item.php @@ -168,9 +168,24 @@ class Item_Model extends ORM_MPTT { * * @param string $query the query string (eg "show=3") */ - public function url($query=array(), $full_uri=false) { - $url = ($full_uri ? url::abs_site("{$this->type}s/$this->id") - : url::site("{$this->type}s/$this->id")); + public function url($query=null) { + $relative_url = $this->relative_url(); + $url = url::site($relative_url); + if ($query) { + $url .= "?$query"; + } + return $url; + } + + /** + * album: url::abs_site("albums/2") + * photo: url::abs_site("photos/3") + * + * @param string $query the query string (eg "show=3") + */ + public function abs_url($query=null) { + $relative_url = $this->relative_url(); + $url = url::abs_site($relative_url); if ($query) { $url .= "?$query"; } diff --git a/modules/gallery/views/admin_block_photo_stream.html.php b/modules/gallery/views/admin_block_photo_stream.html.php index 1b9d8ff5..4968c39b 100644 --- a/modules/gallery/views/admin_block_photo_stream.html.php +++ b/modules/gallery/views/admin_block_photo_stream.html.php @@ -2,7 +2,7 @@