From 020281d932c566476222e6c825ada3affff239a6 Mon Sep 17 00:00:00 2001 From: Andy Staudacher Date: Sat, 29 Aug 2009 10:45:47 -0700 Subject: Adding SafeString which is going to replace p::clean() and p::purify(). Refactoring of Xss_Security_Test. t() and t2() return a SafeString instance. TODO: - Update all code to use SafeString where appropriate. - Update golden fole of Xss_Security_Test - Stop reporting CLEAN vars in Xss_Security_Test --- modules/gallery/helpers/p.php | 16 +++------------- 1 file changed, 3 insertions(+), 13 deletions(-) (limited to 'modules/gallery/helpers') diff --git a/modules/gallery/helpers/p.php b/modules/gallery/helpers/p.php index 862c769b..e852c086 100644 --- a/modules/gallery/helpers/p.php +++ b/modules/gallery/helpers/p.php @@ -18,22 +18,12 @@ * Foundation, Inc., 51 Franklin Street - Fifth Floor, Boston, MA 02110-1301, USA. */ class p_Core { - private static $_purifier = null; static function clean($dirty_html) { - return html::specialchars($dirty_html); + return new SafeString($dirty_html); } + // Deprecated: Please use p::clean($var).purified_html() static function purify($dirty_html) { - if (empty(self::$_purifier)) { - require_once(dirname(__file__) . "/../lib/HTMLPurifier/HTMLPurifier.auto.php"); - $config = HTMLPurifier_Config::createDefault(); - foreach (Kohana::config('purifier') as $category => $key_value) { - foreach ($key_value as $key => $value) { - $config->set("$category.$key", $value); - } - } - self::$_purifier = new HTMLPurifier($config); - } - return self::$_purifier->purify($dirty_html); + return SafeString::of($dirty_html)->purified_html(); } } -- cgit v1.2.3 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 +++++++++++++++++++++++- modules/gallery/tests/Xss_Security_Test.php | 24 ++++++++++++++++++++---- 2 files changed, 43 insertions(+), 5 deletions(-) (limited to 'modules/gallery/helpers') 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(); + } + } diff --git a/modules/gallery/tests/Xss_Security_Test.php b/modules/gallery/tests/Xss_Security_Test.php index 1d52237c..e0e5bb86 100644 --- a/modules/gallery/tests/Xss_Security_Test.php +++ b/modules/gallery/tests/Xss_Security_Test.php @@ -126,7 +126,23 @@ class Xss_Security_Test extends Unit_Test_Case { $token_number++; $token = $tokens[$token_number]; } - } + } else if ($token[1] == "url") { + // url methods return a SafeString + 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); + + $method = $tokens[$token_number + 2][1]; + $frame->expr_append("::$method("); + + $token_number += 3; + $token = $tokens[$token_number]; + } + } } else if ($frame && $token[0] == T_OBJECT_OPERATOR) { $frame->expr_append($token[1]); @@ -155,8 +171,9 @@ class Xss_Security_Test extends Unit_Test_Case { } } - // Generate the report. /* + * Generate the report + * * States for uses of < ? = X ? >: * JS_XSS: * In
- p::purify($item->title))) ?> + SafeString::purify($item->title))) ?>
diff --git a/modules/organize/views/organize_album.html.php b/modules/organize/views/organize_album.html.php index ae2d5d51..4933ed32 100644 --- a/modules/organize/views/organize_album.html.php +++ b/modules/organize/views/organize_album.html.php @@ -7,7 +7,7 @@
gBranchText"> - title) ?> + title) ?>
"> diff --git a/modules/rss/views/feed.mrss.php b/modules/rss/views/feed.mrss.php index 447179a5..7298b7f4 100644 --- a/modules/rss/views/feed.mrss.php +++ b/modules/rss/views/feed.mrss.php @@ -6,9 +6,9 @@ xmlns:fh="http://purl.org/syndication/history/1.0"> gallery3 - <?= p::clean($feed->title) ?> + <?= SafeString::of($feed->title) ?> uri ?> - description) ?> + description) ?> en-us @@ -22,25 +22,25 @@ children as $child): ?> - <?= p::clean($child->title) ?> + <?= SafeString::of($child->title) ?> type}s/{$child->id}") ?> type}s/{$child->id}") ?> created); ?> description) ?> + description) ?>

type == "photo" || $child->type == "album"): ?>
type}s/{$child->id}") ?>">
- description) ?> + description) ?>

]]> diff --git a/modules/search/views/search.html.php b/modules/search/views/search.html.php index 6a222ef1..e5c7b4a6 100644 --- a/modules/search/views/search.html.php +++ b/modules/search/views/search.html.php @@ -8,10 +8,10 @@
  • - +
  • - " /> + for_html_attr() ?>" />
@@ -31,10 +31,10 @@ id") ?>"> thumb_img() ?>

- title) ?> + title) ?>

- description)) ?> + description)) ?>
@@ -44,7 +44,7 @@

- %term", array("term" => p::clean($q))) ?> + %term", array("term" => $q)) ?>

diff --git a/modules/server_add/controllers/admin_server_add.php b/modules/server_add/controllers/admin_server_add.php index 30109f42..fac2aa44 100644 --- a/modules/server_add/controllers/admin_server_add.php +++ b/modules/server_add/controllers/admin_server_add.php @@ -38,7 +38,7 @@ class Admin_Server_Add_Controller extends Admin_Controller { $path = $form->add_path->path->value; $paths[$path] = 1; module::set_var("server_add", "authorized_paths", serialize($paths)); - message::success(t("Added path %path", array("path" => p::clean($path)))); + message::success(t("Added path %path", array("path" => $path))); server_add::check_config($paths); url::redirect("admin/server_add"); } else { @@ -60,7 +60,7 @@ class Admin_Server_Add_Controller extends Admin_Controller { $paths = unserialize(module::get_var("server_add", "authorized_paths")); if (isset($paths[$path])) { unset($paths[$path]); - message::success(t("Removed path %path", array("path" => p::clean($path)))); + message::success(t("Removed path %path", array("path" => $path))); module::set_var("server_add", "authorized_paths", serialize($paths)); server_add::check_config($paths); } diff --git a/modules/server_add/views/server_add_tree.html.php b/modules/server_add/views/server_add_tree.html.php index 254a9da0..b68544ec 100644 --- a/modules/server_add/views/server_add_tree.html.php +++ b/modules/server_add/views/server_add_tree.html.php @@ -24,7 +24,7 @@ file="" > - + diff --git a/modules/server_add/views/server_add_tree_dialog.html.php b/modules/server_add/views/server_add_tree_dialog.html.php index 21952849..533cad04 100644 --- a/modules/server_add/views/server_add_tree_dialog.html.php +++ b/modules/server_add/views/server_add_tree_dialog.html.php @@ -5,17 +5,17 @@
-

p::purify($item->title))) ?>

+

SafeString::purify($item->title))) ?>

    parents() as $parent): ?>
  • - title) ?> + title) ?>
  • - title) ?> + title) ?>
diff --git a/modules/tag/controllers/admin_tags.php b/modules/tag/controllers/admin_tags.php index dcdc16b9..f1b4ca3a 100644 --- a/modules/tag/controllers/admin_tags.php +++ b/modules/tag/controllers/admin_tags.php @@ -53,8 +53,8 @@ class Admin_Tags_Controller extends Admin_Controller { $name = $tag->name; Database::instance()->delete("items_tags", array("tag_id" => "$tag->id")); $tag->delete(); - message::success(t("Deleted tag %tag_name", array("tag_name" => p::clean($name)))); - log::success("tags", t("Deleted tag %tag_name", array("tag_name" => p::clean($name)))); + message::success(t("Deleted tag %tag_name", array("tag_name" => $name))); + log::success("tags", t("Deleted tag %tag_name", array("tag_name" => $name))); print json_encode( array("result" => "success", @@ -98,7 +98,7 @@ class Admin_Tags_Controller extends Admin_Controller { $tag->save(); $message = t("Renamed tag %old_name to %new_name", - array("old_name" => p::clean($old_name), "new_name" => p::clean($tag->name))); + array("old_name" => $old_name, "new_name" => $tag->name)); message::success($message); log::success("tags", $message); @@ -106,7 +106,7 @@ class Admin_Tags_Controller extends Admin_Controller { array("result" => "success", "location" => url::site("admin/tags"), "tag_id" => $tag->id, - "new_tagname" => p::clean($tag->name))); + "new_tagname" => SafeString::of($tag->name))); } else { print json_encode( array("result" => "error", diff --git a/modules/tag/helpers/tag_rss.php b/modules/tag/helpers/tag_rss.php index f94508cf..7194586d 100644 --- a/modules/tag/helpers/tag_rss.php +++ b/modules/tag/helpers/tag_rss.php @@ -22,7 +22,7 @@ class tag_rss_Core { static function available_feeds($item, $tag) { if ($tag) { $feeds["tag/tag/{$tag->id}"] = - t("Tag feed for %tag_name", array("tag_name" => p::clean($tag->name))); + t("Tag feed for %tag_name", array("tag_name" => $tag->name)); return $feeds; } return array(); diff --git a/modules/tag/views/admin_tags.html.php b/modules/tag/views/admin_tags.html.php index 7d201da7..5bd23112 100644 --- a/modules/tag/views/admin_tags.html.php +++ b/modules/tag/views/admin_tags.html.php @@ -47,7 +47,7 @@
  • - name) ?> + name) ?> (count ?>) id") ?>" class="gDialogLink delete-link gButtonLink"> diff --git a/modules/tag/views/tag_cloud.html.php b/modules/tag/views/tag_cloud.html.php index eba615fc..b4c6ae34 100644 --- a/modules/tag/views/tag_cloud.html.php +++ b/modules/tag/views/tag_cloud.html.php @@ -3,7 +3,7 @@
  • count ?> photos are tagged with - id") ?>">name) ?> + id") ?>">name) ?>
    • diff --git a/modules/user/controllers/admin_users.php b/modules/user/controllers/admin_users.php index f87602b8..521f82fa 100644 --- a/modules/user/controllers/admin_users.php +++ b/modules/user/controllers/admin_users.php @@ -51,7 +51,7 @@ class Admin_Users_Controller extends Controller { $user->save(); module::event("user_add_form_admin_completed", $user, $form); - message::success(t("Created user %user_name", array("user_name" => p::clean($user->name)))); + message::success(t("Created user %user_name", array("user_name" => $user->name))); print json_encode(array("result" => "success")); } else { print json_encode(array("result" => "error", @@ -84,7 +84,7 @@ class Admin_Users_Controller extends Controller { "form" => $form->__toString())); } - $message = t("Deleted user %user_name", array("user_name" => p::clean($name))); + $message = t("Deleted user %user_name", array("user_name" => $name)); log::success("user", $message); message::success($message); print json_encode(array("result" => "success")); @@ -142,7 +142,7 @@ class Admin_Users_Controller extends Controller { $user->save(); module::event("user_edit_form_admin_completed", $user, $form); - message::success(t("Changed user %user_name", array("user_name" => p::clean($user->name)))); + message::success(t("Changed user %user_name", array("user_name" => $user->name))); print json_encode(array("result" => "success")); } else { print json_encode(array("result" => "error", @@ -204,7 +204,7 @@ class Admin_Users_Controller extends Controller { $group = group::create($new_name); $group->save(); message::success( - t("Created group %group_name", array("group_name" => p::clean($group->name)))); + t("Created group %group_name", array("group_name" => $group->name))); print json_encode(array("result" => "success")); } else { print json_encode(array("result" => "error", @@ -233,7 +233,7 @@ class Admin_Users_Controller extends Controller { "form" => $form->__toString())); } - $message = t("Deleted group %group_name", array("group_name" => p::clean($name))); + $message = t("Deleted group %group_name", array("group_name" => $name)); log::success("group", $message); message::success($message); print json_encode(array("result" => "success")); @@ -271,11 +271,11 @@ class Admin_Users_Controller extends Controller { $group->name = $form->edit_group->inputs["name"]->value; $group->save(); message::success( - t("Changed group %group_name", array("group_name" => p::clean($group->name)))); + t("Changed group %group_name", array("group_name" => $group->name))); print json_encode(array("result" => "success")); } else { message::error( - t("Failed to change group %group_name", array("group_name" => p::clean($group->name)))); + t("Failed to change group %group_name", array("group_name" => $group->name))); print json_encode(array("result" => "error", "form" => $form->__toString())); } diff --git a/modules/user/controllers/login.php b/modules/user/controllers/login.php index 4d901051..b81b17b2 100644 --- a/modules/user/controllers/login.php +++ b/modules/user/controllers/login.php @@ -63,7 +63,7 @@ class Login_Controller extends Controller { log::warning( "user", t("Failed login for %name", - array("name" => p::clean($form->login->inputs["name"]->value)))); + array("name" => $form->login->inputs["name"]->value))); $form->login->inputs["name"]->add_error("invalid_login", 1); $valid = false; } @@ -71,7 +71,7 @@ class Login_Controller extends Controller { if ($valid) { user::login($user); - log::info("user", t("User %name logged in", array("name" => p::clean($user->name)))); + log::info("user", t("User %name logged in", array("name" => $user->name))); } // Either way, regenerate the session id to avoid session trapping diff --git a/modules/user/controllers/logout.php b/modules/user/controllers/logout.php index 099b1952..4b141a1c 100644 --- a/modules/user/controllers/logout.php +++ b/modules/user/controllers/logout.php @@ -23,8 +23,8 @@ class Logout_Controller extends Controller { $user = user::active(); user::logout(); - log::info("user", t("User %name logged out", array("name" => p::clean($user->name))), - html::anchor("user/$user->id", p::clean($user->name))); + log::info("user", t("User %name logged out", array("name" => $user->name)), + html::anchor("user/$user->id", SafeString::of($user->name))); if ($continue_url = $this->input->get("continue")) { $item = url::get_item_from_uri($continue_url); if (access::can("view", $item)) { diff --git a/modules/user/controllers/password.php b/modules/user/controllers/password.php index 2af1b879..066efbba 100644 --- a/modules/user/controllers/password.php +++ b/modules/user/controllers/password.php @@ -74,7 +74,7 @@ class Password_Controller extends Controller { log::success( "user", - t("Password reset email sent for user %name", array("name" => p::clean($user->name)))); + t("Password reset email sent for user %name", array("name" => $user->name))); } else { // Don't include the username here until you're sure that it's XSS safe log::warning( diff --git a/modules/user/views/admin_users.html.php b/modules/user/views/admin_users.html.php index 542b8b8b..54c4847d 100644 --- a/modules/user/views/admin_users.html.php +++ b/modules/user/views/admin_users.html.php @@ -68,16 +68,16 @@ " title="" - alt="name) ?>" + alt="name) ?>" width="20" height="20" /> - name) ?> + name) ?> - full_name) ?> + full_name) ?> - email) ?> + email) ?> last_login == 0) ? "" : gallery::date($user->last_login) ?> diff --git a/modules/user/views/admin_users_group.html.php b/modules/user/views/admin_users_group.html.php index bfd79dba..f89a4392 100644 --- a/modules/user/views/admin_users_group.html.php +++ b/modules/user/views/admin_users_group.html.php @@ -1,9 +1,9 @@

    - name) ?> + name) ?> special): ?> id") ?>" - title=" p::clean($group->name))) ?>" + title=" $group->name)) ?>" class="gDialogLink gButtonLink ui-state-default ui-corner-all"> @@ -17,12 +17,12 @@

    diff --git a/modules/comment/views/admin_comments.html.php b/modules/comment/views/admin_comments.html.php index 8b0b4c29..801ce2b3 100644 --- a/modules/comment/views/admin_comments.html.php +++ b/modules/comment/views/admin_comments.html.php @@ -108,12 +108,12 @@ " class="gAvatar" - alt="author_name()) ?>" + alt="author_name()) ?>" width="40" height="40" /> -

    author_name()) ?>

    +

    author_name()) ?>

    @@ -122,7 +122,7 @@ has_thumb()): ?> <?= SafeString::purify($item->title) ?>thumb_width, $item->thumb_height, 75) ?> /> @@ -132,7 +132,7 @@

    created) ?>

    - text)) ?> + text)) ?>
      diff --git a/modules/comment/views/comment.html.php b/modules/comment/views/comment.html.php index 31bb7f4d..1d0786cb 100644 --- a/modules/comment/views/comment.html.php +++ b/modules/comment/views/comment.html.php @@ -4,15 +4,15 @@ " class="gAvatar" - alt="author_name()) ?>" + alt="author_name()) ?>" width="40" height="40" /> gallery::date_time($comment->created), - "author_name" => SafeString::of($comment->author_name()))) ?> + "author_name" => html::clean($comment->author_name()))) ?>

      - text)) ?> + text)) ?>
      diff --git a/modules/comment/views/comment.mrss.php b/modules/comment/views/comment.mrss.php index ae7762d9..c2a4b538 100644 --- a/modules/comment/views/comment.mrss.php +++ b/modules/comment/views/comment.mrss.php @@ -6,9 +6,9 @@ xmlns:fh="http://purl.org/syndication/history/1.0"> Gallery 3 - <?= SafeString::of($feed->title) ?> + <?= html::clean($feed->title) ?> uri ?> - description) ?> + description) ?> en-us @@ -22,14 +22,14 @@ children as $child): ?> - <?= SafeString::purify($child->title) ?> - item_uri) ?> - author) ?> + <?= html::purify($child->title) ?> + item_uri) ?> + author) ?> item_uri ?> pub_date ?> text)) ?>

      +

      text)) ?>

      diff --git a/modules/comment/views/comments.html.php b/modules/comment/views/comments.html.php index 9eac0502..1e45c946 100644 --- a/modules/comment/views/comments.html.php +++ b/modules/comment/views/comments.html.php @@ -18,16 +18,16 @@ " class="gAvatar" - alt="author_name()) ?>" + alt="author_name()) ?>" width="40" height="40" /> %name said', array("date" => date("Y-M-d H:i:s", $comment->created), - "name" => SafeString::of($comment->author_name()))); ?> + "name" => html::clean($comment->author_name()))); ?>

      - text)) ?> + text)) ?>
      diff --git a/modules/digibug/controllers/digibug.php b/modules/digibug/controllers/digibug.php index 509a8b70..0939704b 100644 --- a/modules/digibug/controllers/digibug.php +++ b/modules/digibug/controllers/digibug.php @@ -50,7 +50,7 @@ class Digibug_Controller extends Controller { "image_width_1" => $item->width, "thumb_height_1" => $item->thumb_height, "thumb_width_1" => $item->thumb_width, - "title_1" => SafeString::purify($item->title)); + "title_1" => html::purify($item->title)); print $v; } diff --git a/modules/exif/views/exif_dialog.html.php b/modules/exif/views/exif_dialog.html.php index a981ca09..11d1e212 100644 --- a/modules/exif/views/exif_dialog.html.php +++ b/modules/exif/views/exif_dialog.html.php @@ -14,14 +14,14 @@ - + - + diff --git a/modules/g2_import/helpers/g2_import.php b/modules/g2_import/helpers/g2_import.php index a01ca1db..7e5c6f75 100644 --- a/modules/g2_import/helpers/g2_import.php +++ b/modules/g2_import/helpers/g2_import.php @@ -590,7 +590,7 @@ class g2_import_Core { self::map($g2_comment->getId(), $comment->id); return t("Imported comment '%comment' for item with id: %id", array("id" => $comment->item_id, - "comment" => text::limit_words(nl2br(SafeString::purify($comment->text)), 50))); + "comment" => text::limit_words(nl2br(html::purify($comment->text)), 50))); } /** diff --git a/modules/gallery/controllers/admin_advanced_settings.php b/modules/gallery/controllers/admin_advanced_settings.php index d727b654..43c77340 100644 --- a/modules/gallery/controllers/admin_advanced_settings.php +++ b/modules/gallery/controllers/admin_advanced_settings.php @@ -46,7 +46,7 @@ class Admin_Advanced_Settings_Controller extends Admin_Controller { module::set_var($module_name, $var_name, Input::instance()->post("value")); message::success( t("Saved value for %var (%module_name)", - array("var" => SafeString::of($var_name), "module_name" => $module_name))); + array("var" => html::clean($var_name), "module_name" => $module_name))); print json_encode(array("result" => "success")); } diff --git a/modules/gallery/controllers/quick.php b/modules/gallery/controllers/quick.php index 8fddb563..20731f9c 100644 --- a/modules/gallery/controllers/quick.php +++ b/modules/gallery/controllers/quick.php @@ -75,7 +75,7 @@ class Quick_Controller extends Controller { access::required("view", $item->parent()); access::required("edit", $item->parent()); - $msg = t("Made %title this album's cover", array("title" => SafeString::purify($item->title))); + $msg = t("Made %title this album's cover", array("title" => html::purify($item->title))); item::make_album_cover($item); message::success($msg); @@ -91,10 +91,10 @@ class Quick_Controller extends Controller { if ($item->is_album()) { print t( "Delete the album %title? All photos and movies in the album will also be deleted.", - array("title" => SafeString::purify($item->title))); + array("title" => html::purify($item->title))); } else { print t("Are you sure you want to delete %title?", - array("title" => SafeString::purify($item->title))); + array("title" => html::purify($item->title))); } $form = item::get_delete_form($item); @@ -108,9 +108,9 @@ class Quick_Controller extends Controller { access::required("edit", $item); if ($item->is_album()) { - $msg = t("Deleted album %title", array("title" => SafeString::purify($item->title))); + $msg = t("Deleted album %title", array("title" => html::purify($item->title))); } else { - $msg = t("Deleted photo %title", array("title" => SafeString::purify($item->title))); + $msg = t("Deleted photo %title", array("title" => html::purify($item->title))); } $parent = $item->parent(); diff --git a/modules/gallery/helpers/MY_html.php b/modules/gallery/helpers/MY_html.php index eb388811..75114898 100644 --- a/modules/gallery/helpers/MY_html.php +++ b/modules/gallery/helpers/MY_html.php @@ -65,11 +65,11 @@ class html extends html_Core { * * Example:
          *   
 
      
-  
       SafeString::purify($album->title))) ?>
      
+  
       html::purify($album->title))) ?>
      
   
      
     
      
       
      
diff --git a/modules/organize/views/organize_tree.html.php b/modules/organize/views/organize_tree.html.php
index 387d5977..5b676889 100644
--- a/modules/organize/views/organize_tree.html.php
+++ b/modules/organize/views/organize_tree.html.php
@@ -5,7 +5,7 @@
   
   
   
-    title) ?>
+    title) ?>
   
   
        
     
@@ -17,7 +17,7 @@
       
       "
             ref="id ?>">
-        title) ?>
+        title) ?>
       
 
       id == $album->id): ?>
@@ -29,7 +29,7 @@
           
           
-            title) ?>
+            title) ?>
           
         
         
diff --git a/modules/rss/views/feed.mrss.php b/modules/rss/views/feed.mrss.php
index 7298b7f4..731703c7 100644
--- a/modules/rss/views/feed.mrss.php
+++ b/modules/rss/views/feed.mrss.php
@@ -6,9 +6,9 @@
    xmlns:fh="http://purl.org/syndication/history/1.0">
   
     gallery3
-    <?= SafeString::of($feed->title) ?>
+    <?= html::clean($feed->title) ?>
     uri ?>
-    description) ?>
+    description) ?>
     en-us
     
     
@@ -22,25 +22,25 @@
     
     children as $child): ?>
     
-      <?= SafeString::of($child->title) ?>
+      <?= html::clean($child->title) ?>
       type}s/{$child->id}") ?>
       type}s/{$child->id}") ?>
       created); ?>
       
         description) ?>
+          description) ?>
           
        
           type == "photo" || $child->type == "album"): ?>
             
        
           
             type}s/{$child->id}") ?>">
             
        
           
-            description) ?>
+            description) ?>
           
        
         ]]>
               
diff --git a/modules/rss/views/rss_block.html.php b/modules/rss/views/rss_block.html.php
index cd8db89d..737731b6 100644
--- a/modules/rss/views/rss_block.html.php
+++ b/modules/rss/views/rss_block.html.php
@@ -5,7 +5,7 @@
     
     
       
-      
+      
     
     
   
diff --git a/modules/search/views/search.html.php b/modules/search/views/search.html.php
index e5c7b4a6..7963948d 100644
--- a/modules/search/views/search.html.php
+++ b/modules/search/views/search.html.php
@@ -8,7 +8,7 @@
     
          
       
        • 
         
-        
+        
       
          • 
       
        • 
         for_html_attr() ?>" />
@@ -31,10 +31,10 @@
       id") ?>">
         thumb_img() ?>
         
          
-    title) ?>
+    title) ?>
         
          
         
          
-    description)) ?>
+    description)) ?>
         
          
                 
     
          • 
diff --git a/modules/server_add/views/admin_server_add.html.php b/modules/server_add/views/admin_server_add.html.php
index c4439bda..b48a19da 100644
--- a/modules/server_add/views/admin_server_add.html.php
+++ b/modules/server_add/views/admin_server_add.html.php
@@ -16,7 +16,7 @@
            class="gRemoveDir ui-icon ui-icon-trash">
           X
         
-        
+        
       
       
     
        
diff --git a/modules/server_add/views/server_add_tree.html.php b/modules/server_add/views/server_add_tree.html.php
index 2f65a590..dbae42c5 100644
--- a/modules/server_add/views/server_add_tree.html.php
+++ b/modules/server_add/views/server_add_tree.html.php
@@ -10,7 +10,7 @@
     
      • 
       
       
-        
+        
       
       
          
         
@@ -24,7 +24,7 @@
                 
                 file=" '\\"')) ?>"
                 >
-            
+            
           
         
         
diff --git a/modules/server_add/views/server_add_tree_dialog.html.php b/modules/server_add/views/server_add_tree_dialog.html.php
index 912e69b6..8eb6e4df 100644
--- a/modules/server_add/views/server_add_tree_dialog.html.php
+++ b/modules/server_add/views/server_add_tree_dialog.html.php
@@ -5,17 +5,17 @@
 
 
 
          
-  
           SafeString::purify($item->title))) ?>
          
+  
           html::purify($item->title))) ?>
          
 
   
          
   
            
     parents() as $parent): ?>
     
          • 
-      title) ?>
+      title) ?>
     
            • 
     
     
          • 
-      title) ?>
+      title) ?>
     
            • 
   
          
 
diff --git a/modules/tag/controllers/admin_tags.php b/modules/tag/controllers/admin_tags.php
index f1b4ca3a..8b8dde21 100644
--- a/modules/tag/controllers/admin_tags.php
+++ b/modules/tag/controllers/admin_tags.php
@@ -106,7 +106,7 @@ class Admin_Tags_Controller extends Admin_Controller {
         array("result" => "success",
               "location" => url::site("admin/tags"),
               "tag_id" => $tag->id,
-              "new_tagname" => SafeString::of($tag->name)));
+              "new_tagname" => html::clean($tag->name)));
     } else {
       print json_encode(
         array("result" => "error",
diff --git a/modules/tag/views/admin_tags.html.php b/modules/tag/views/admin_tags.html.php
index 30dd0728..3d805c5e 100644
--- a/modules/tag/views/admin_tags.html.php
+++ b/modules/tag/views/admin_tags.html.php
@@ -32,7 +32,7 @@
           name, 0, 1)) ?>
 
           
-            
+            
             
            
           
              $tags_per_column): /* new column */ ?>
@@ -42,12 +42,12 @@
             
 
             
          
-            
+            
             
diff --git a/modules/user/controllers/logout.php b/modules/user/controllers/logout.php
index 4b141a1c..fc3ced56 100644
--- a/modules/user/controllers/logout.php
+++ b/modules/user/controllers/logout.php
@@ -24,7 +24,7 @@ class Logout_Controller extends Controller {
     $user = user::active();
     user::logout();
     log::info("user", t("User %name logged out", array("name" => $user->name)),
-              html::anchor("user/$user->id", SafeString::of($user->name)));
+              html::anchor("user/$user->id", html::clean($user->name)));
     if ($continue_url = $this->input->get("continue")) {
       $item = url::get_item_from_uri($continue_url);
       if (access::can("view", $item)) {
diff --git a/modules/user/views/admin_users.html.php b/modules/user/views/admin_users.html.php
index 36c4f4fd..9455f9d9 100644
--- a/modules/user/views/admin_users.html.php
+++ b/modules/user/views/admin_users.html.php
@@ -68,16 +68,16 @@
         
           "
                title=""
-               alt="name) ?>"
+               alt="name) ?>"
                width="20"
                height="20" />
-          name) ?>
+          name) ?>
         
         
-          full_name) ?>
+          full_name) ?>
         
         
-          email) ?>
+          email) ?>
         
         
           last_login == 0) ? "" : gallery::date($user->last_login) ?>
diff --git a/modules/user/views/admin_users_group.html.php b/modules/user/views/admin_users_group.html.php
index f89a4392..8418ebc9 100644
--- a/modules/user/views/admin_users_group.html.php
+++ b/modules/user/views/admin_users_group.html.php
@@ -1,6 +1,6 @@
 
 
          
-  name) ?>
+  name) ?>
   special): ?>
   id") ?>"
     title=" $group->name)) ?>"
@@ -17,7 +17,7 @@
 
          
 
 
            
@@ -16,7 +16,7 @@
            width="thumb_width ?>"
            height="thumb_height ?>" />
     
-    
            title) ?>
            
+    
            title) ?>
            
     thumb_bottom($child) ?>
     
              
       thumb_info($child) ?>
diff --git a/themes/default/views/header.html.php b/themes/default/views/header.html.php
index 9e34401d..dcfa6fd8 100644
--- a/themes/default/views/header.html.php
+++ b/themes/default/views/header.html.php
@@ -19,10 +19,10 @@
   
   
            • 
     id}?show=$item->id") ?>">
-      title) ?>
+      title) ?>
     
   
              • 
   
-  
            • title) ?>
              • 
+  
            • title) ?>
              • 
 
            
 
diff --git a/themes/default/views/movie.html.php b/themes/default/views/movie.html.php
index 237743b7..910814dd 100644
--- a/themes/default/views/movie.html.php
+++ b/themes/default/views/movie.html.php
@@ -28,8 +28,8 @@
   movie_img(array("class" => "gMovie", "id" => "gMovieId-{$item->id}")) ?>
 
   
            
-    
            title) ?>
            
-       
            description)) ?>
            
+    
            title) ?>
            
+       
            description)) ?>
            
   
            
 
   photo_bottom() ?>
diff --git a/themes/default/views/photo.html.php b/themes/default/views/photo.html.php
index 5b5cb12b..c601c4cc 100644
--- a/themes/default/views/photo.html.php
+++ b/themes/default/views/photo.html.php
@@ -5,7 +5,7 @@
 
    *
      */ - static function clean_js($string) { + static function js_string($string) { return SafeString::of($string)->for_js(); } diff --git a/modules/gallery/libraries/SafeString.php b/modules/gallery/libraries/SafeString.php index 9614a213..0767a665 100644 --- a/modules/gallery/libraries/SafeString.php +++ b/modules/gallery/libraries/SafeString.php @@ -92,17 +92,17 @@ class SafeString_Core { } /** - * Safe for use in JavaScript. + * Safe for use as JavaScript string. * * Example:
          *   
    *
      * @return the string escaped for use in JavaScript. */ function for_js() { - return self::_escape_for_js($this->_raw_string); + return json_encode((string) $this->_raw_string); } /** @@ -152,14 +152,6 @@ class SafeString_Core { return html::specialchars($dirty_html); } - // Escapes special chars (quotes, backslash, etc.) with a backslash sequence. - private static function _escape_for_js($string) { - // From Smarty plugins/modifier.escape.php - // Might want to be stricter here. - return strtr($string, - array('\\'=>'\\\\',"'"=>"\\'",'"'=>'\\"',"\r"=>'\\r',"\n"=>'\\n',''<\/')); - } - // Purifies the string, removing any potentially malicious or unsafe HTML / JavaScript. private static function _purify_for_html($dirty_html) { if (empty(self::$_purifier)) { diff --git a/modules/gallery/tests/Html_Helper_Test.php b/modules/gallery/tests/Html_Helper_Test.php index a9903256..f5ce7fa4 100644 --- a/modules/gallery/tests/Html_Helper_Test.php +++ b/modules/gallery/tests/Html_Helper_Test.php @@ -40,9 +40,9 @@ class Html_Helper_Test extends Unit_Test_Case { $safe_string_2); } - public function clean_js_test() { - $string = html::clean_js("hello's

      world

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

      world<\\/p>", + public function js_string_test() { + $string = html::js_string("hello's

      world

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

      world<\\/p>"', $string); } diff --git a/modules/gallery/tests/SafeString_Test.php b/modules/gallery/tests/SafeString_Test.php index 0fc7f6f3..ede55240 100644 --- a/modules/gallery/tests/SafeString_Test.php +++ b/modules/gallery/tests/SafeString_Test.php @@ -49,7 +49,7 @@ class SafeString_Test extends Unit_Test_Case { public function for_js_test() { $safe_string = new SafeString('"Foo\'s bar"'); $js_string = $safe_string->for_js(); - $this->assert_equal('\\"Foo<\\/em>\\\'s bar\\"', + $this->assert_equal('"\\"Foo<\\/em>\'s bar\\""', $js_string); } @@ -96,21 +96,21 @@ class SafeString_Test extends Unit_Test_Case { public function of_fluid_api_test() { $escaped_string = SafeString::of("Foo's bar")->for_js(); - $this->assert_equal("Foo\\'s bar", $escaped_string); + $this->assert_equal('"Foo\'s bar"', $escaped_string); } public function safestring_of_safestring_preserves_safe_status_test() { $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()); + $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_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()); + $this->assert_equal('"hello\'s

      world<\\/p>"', $safe_string_2->for_js()); } public function safestring_of_safestring_safe_status_override_test() { diff --git a/modules/gallery/tests/Xss_Security_Test.php b/modules/gallery/tests/Xss_Security_Test.php index b385580d..3a22afc1 100644 --- a/modules/gallery/tests/Xss_Security_Test.php +++ b/modules/gallery/tests/Xss_Security_Test.php @@ -188,7 +188,7 @@ class Xss_Security_Test extends Unit_Test_Case { 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("clean", "purify", "clean_js", "clean_attribute")) && + array("clean", "purify", "js_string", "clean_attribute")) && self::_token_matches("(", $tokens, $token_number + 3)) { // Not checking for mark_safe(). We want such calls to be marked dirty (thus reviewed). @@ -198,7 +198,7 @@ class Xss_Security_Test extends Unit_Test_Case { $token_number += 3; $token = $tokens[$token_number]; - if ("clean_js" == $method) { + if ("js_string" == $method) { $frame->is_safe_js(true); } else { $frame->is_safe_html(true); -- cgit v1.2.3 From df38a890a64dd33eafe3aed51ce8fde732cf8b8b Mon Sep 17 00:00:00 2001 From: Andy Staudacher Date: Sun, 30 Aug 2009 18:07:13 -0700 Subject: Tabs to spaces cleanup --- modules/gallery/controllers/admin_languages.php | 32 +- modules/gallery/controllers/l10n_client.php | 8 +- modules/gallery/helpers/gallery.php | 6 +- modules/gallery/libraries/SafeString.php | 4 +- modules/gallery/tests/Html_Helper_Test.php | 10 +- modules/gallery/tests/SafeString_Test.php | 14 +- modules/gallery/tests/Xss_Security_Test.php | 380 ++++++++++++------------ modules/server_add/helpers/server_add_event.php | 2 +- 8 files changed, 228 insertions(+), 228 deletions(-) (limited to 'modules/gallery/helpers') diff --git a/modules/gallery/controllers/admin_languages.php b/modules/gallery/controllers/admin_languages.php index b1bc4cff..d85c47f9 100644 --- a/modules/gallery/controllers/admin_languages.php +++ b/modules/gallery/controllers/admin_languages.php @@ -21,10 +21,10 @@ class Admin_Languages_Controller extends Admin_Controller { public function index($share_translations_form=null) { $v = new Admin_View("admin.html"); $v->content = new View("admin_languages.html"); - $v->content->available_locales = locales::available(); + $v->content->available_locales = locales::available(); $v->content->installed_locales = locales::installed(); $v->content->default_locale = module::get_var("gallery", "default_locale"); - + if (empty($share_translations_form)) { $share_translations_form = $this->_share_translations_form(); } @@ -35,21 +35,21 @@ class Admin_Languages_Controller extends Admin_Controller { public function save() { access::verify_csrf(); - - locales::update_installed($this->input->post("installed_locales")); - - $installed_locales = array_keys(locales::installed()); + + locales::update_installed($this->input->post("installed_locales")); + + $installed_locales = array_keys(locales::installed()); $new_default_locale = $this->input->post("default_locale"); - if (!in_array($new_default_locale, $installed_locales)) { - if (!empty($installed_locales)) { - $new_default_locale = $installed_locales[0]; - } else { - $new_default_locale = "en_US"; - } - } - module::set_var("gallery", "default_locale", $new_default_locale); - - print json_encode(array("result" => "success")); + if (!in_array($new_default_locale, $installed_locales)) { + if (!empty($installed_locales)) { + $new_default_locale = $installed_locales[0]; + } else { + $new_default_locale = "en_US"; + } + } + module::set_var("gallery", "default_locale", $new_default_locale); + + print json_encode(array("result" => "success")); } public function share() { diff --git a/modules/gallery/controllers/l10n_client.php b/modules/gallery/controllers/l10n_client.php index 0775791e..16d39024 100644 --- a/modules/gallery/controllers/l10n_client.php +++ b/modules/gallery/controllers/l10n_client.php @@ -90,13 +90,13 @@ class L10n_Client_Controller extends Controller { } $session = Session::instance(); - $l10n_mode = $session->get("l10n_mode", false); + $l10n_mode = $session->get("l10n_mode", false); $session->set("l10n_mode", !$l10n_mode); $redirect_url = "admin/languages"; - if (!$l10n_mode) { - $redirect_url .= "#l10n-client"; - } + if (!$l10n_mode) { + $redirect_url .= "#l10n-client"; + } url::redirect($redirect_url); } diff --git a/modules/gallery/helpers/gallery.php b/modules/gallery/helpers/gallery.php index 122227fc..035ed1da 100644 --- a/modules/gallery/helpers/gallery.php +++ b/modules/gallery/helpers/gallery.php @@ -92,7 +92,7 @@ class gallery_Core { $can_add = $item && access::can("add", $item); if ($can_add) { - $menu->append($add_menu = Menu::factory("submenu") + $menu->append($add_menu = Menu::factory("submenu") ->id("add_menu") ->label(t("Add"))); $add_menu->append(Menu::factory("dialog") @@ -100,11 +100,11 @@ class gallery_Core { ->label(t("Add photos")) ->url(url::site("simple_uploader/app/$item->id"))); if ($item->is_album()) { - $add_menu->append(Menu::factory("dialog") + $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") diff --git a/modules/gallery/libraries/SafeString.php b/modules/gallery/libraries/SafeString.php index 0767a665..cc542e01 100644 --- a/modules/gallery/libraries/SafeString.php +++ b/modules/gallery/libraries/SafeString.php @@ -120,8 +120,8 @@ class SafeString_Core { function for_html_attr() { $string = (string) $this->for_html(); return strtr($string, - array("'"=>"'", - '"'=>'"')); + array("'"=>"'", + '"'=>'"')); } /** diff --git a/modules/gallery/tests/Html_Helper_Test.php b/modules/gallery/tests/Html_Helper_Test.php index f5ce7fa4..3623705e 100644 --- a/modules/gallery/tests/Html_Helper_Test.php +++ b/modules/gallery/tests/Html_Helper_Test.php @@ -21,14 +21,14 @@ class Html_Helper_Test extends Unit_Test_Case { public function clean_test() { $safe_string = html::clean("hello

      world

      "); $this->assert_equal("hello <p >world</p>", - $safe_string); + $safe_string); $this->assert_true($safe_string instanceof SafeString); } public function purify_test() { $safe_string = html::purify("hello

      world

      "); $this->assert_equal("hello

      world

      ", - $safe_string); + $safe_string); $this->assert_true($safe_string instanceof SafeString); } @@ -37,19 +37,19 @@ class Html_Helper_Test extends Unit_Test_Case { $this->assert_true($safe_string instanceof SafeString); $safe_string_2 = html::clean($safe_string); $this->assert_equal("hello

      world

      ", - $safe_string_2); + $safe_string_2); } public function js_string_test() { $string = html::js_string("hello's

      world

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

      world<\\/p>"', - $string); + $string); } public function clean_attribute_test() { $safe_string = SafeString::of_safe_html("hello's

      world

      "); $safe_string = html::clean_attribute($safe_string); $this->assert_equal("hello's

      world

      ", - $safe_string); + $safe_string); } } \ No newline at end of file diff --git a/modules/gallery/tests/SafeString_Test.php b/modules/gallery/tests/SafeString_Test.php index ede55240..0895b7dd 100644 --- a/modules/gallery/tests/SafeString_Test.php +++ b/modules/gallery/tests/SafeString_Test.php @@ -21,19 +21,19 @@ class SafeString_Test extends Unit_Test_Case { public function toString_escapes_for_html_test() { $safe_string = new SafeString("hello

      world

      "); $this->assert_equal("hello <p>world</p>", - $safe_string); + $safe_string); } public function toString_for_safe_string_test() { $safe_string = SafeString::of_safe_html("hello

      world

      "); $this->assert_equal("hello

      world

      ", - $safe_string); + $safe_string); } public function for_html_test() { $safe_string = new SafeString("hello

      world

      "); $this->assert_equal("hello <p>world</p>", - $safe_string->for_html()); + $safe_string->for_html()); } public function safestring_of_safestring_test() { @@ -50,27 +50,27 @@ class SafeString_Test extends Unit_Test_Case { $safe_string = new SafeString('"Foo\'s bar"'); $js_string = $safe_string->for_js(); $this->assert_equal('"\\"Foo<\\/em>\'s bar\\""', - $js_string); + $js_string); } public function for_html_attr_test() { $safe_string = new SafeString('"Foo\'s bar"'); $attr_string = $safe_string->for_html_attr(); $this->assert_equal('"<em>Foo</em>'s bar"', - $attr_string); + $attr_string); } public function for_html_attr_with_safe_html_test() { $safe_string = SafeString::of_safe_html('"Foo\'s bar"'); $attr_string = $safe_string->for_html_attr(); $this->assert_equal('"Foo's bar"', - $attr_string); + $attr_string); } public function string_safestring_equality_test() { $safe_string = new SafeString("hello

      world

      "); $this->assert_equal("hello

      world

      ", - $safe_string->unescaped()); + $safe_string->unescaped()); $escaped_string = "hello <p>world</p>"; $this->assert_equal($escaped_string, $safe_string); diff --git a/modules/gallery/tests/Xss_Security_Test.php b/modules/gallery/tests/Xss_Security_Test.php index 3a22afc1..6c141c52 100644 --- a/modules/gallery/tests/Xss_Security_Test.php +++ b/modules/gallery/tests/Xss_Security_Test.php @@ -24,9 +24,9 @@ class Xss_Security_Test extends Unit_Test_Case { // List of all tokens without whitespace, simplifying parsing. $tokens = array(); foreach (token_get_all(file_get_contents($view)) as $token) { - if (!is_array($token) || ($token[0] != T_WHITESPACE)) { - $tokens[] = $token; - } + if (!is_array($token) || ($token[0] != T_WHITESPACE)) { + $tokens[] = $token; + } } $frame = null; @@ -34,199 +34,199 @@ class Xss_Security_Test extends Unit_Test_Case { $in_script_block = false; for ($token_number = 0; $token_number < count($tokens); $token_number++) { - $token = $tokens[$token_number]; - - // Are we in a block? - if (is_array($token) && $token[0] == T_INLINE_HTML) { - $inline_html = $token[1]; - // T_INLINE_HTML blocks can be split. Need to handle the case - // where one token has "expr_append($inline_html); - } - - // Note: This approach won't catch }i', $inline_html, $matches, PREG_OFFSET_CAPTURE)) { - $last_match = array_pop($matches[0]); - if (is_array($last_match)) { - $closing_script_pos = $last_match[1]; - } else { - $closing_script_pos = $last_match; - } - } - if (preg_match('{]*>}i', $inline_html, $matches, PREG_OFFSET_CAPTURE)) { - $last_match = array_pop($matches[0]); - if (is_array($last_match)) { - $opening_script_pos = $last_match[1]; - } else { - $opening_script_pos = $last_match; - } - } - if ($opening_script_pos != $closing_script_pos) { - $in_script_block = $opening_script_pos > $closing_script_pos; - } - } - - // Look and report each instance of < ? = ... ? > - if (!is_array($token)) { - // A single char token, e.g: ; ( ) - if ($frame) { - $frame->expr_append($token); - } - } else if ($token[0] == T_OPEN_TAG_WITH_ECHO) { - // No need for a stack here - assume < ? = cannot be nested. - $frame = self::_create_frame($token, $in_script_block); + $token = $tokens[$token_number]; + + // Are we in a block? + if (is_array($token) && $token[0] == T_INLINE_HTML) { + $inline_html = $token[1]; + // T_INLINE_HTML blocks can be split. Need to handle the case + // where one token has "expr_append($inline_html); + } + + // Note: This approach won't catch }i', $inline_html, $matches, PREG_OFFSET_CAPTURE)) { + $last_match = array_pop($matches[0]); + if (is_array($last_match)) { + $closing_script_pos = $last_match[1]; + } else { + $closing_script_pos = $last_match; + } + } + if (preg_match('{]*>}i', $inline_html, $matches, PREG_OFFSET_CAPTURE)) { + $last_match = array_pop($matches[0]); + if (is_array($last_match)) { + $opening_script_pos = $last_match[1]; + } else { + $opening_script_pos = $last_match; + } + } + if ($opening_script_pos != $closing_script_pos) { + $in_script_block = $opening_script_pos > $closing_script_pos; + } + } + + // Look and report each instance of < ? = ... ? > + if (!is_array($token)) { + // A single char token, e.g: ; ( ) + if ($frame) { + $frame->expr_append($token); + } + } else if ($token[0] == T_OPEN_TAG_WITH_ECHO) { + // No need for a stack here - assume < ? = cannot be nested. + $frame = self::_create_frame($token, $in_script_block); } else if ($frame && $token[0] == T_CLOSE_TAG) { - // Store the < ? = ... ? > block that just ended here. - $found[$view][] = $frame; - $frame = null; + // Store the < ? = ... ? > block that just ended here. + $found[$view][] = $frame; + $frame = null; } else if ($frame && $token[0] == T_VARIABLE) { - $frame->expr_append($token[1]); + $frame->expr_append($token[1]); if ($token[1] == '$theme') { - if (self::_token_matches(array(T_OBJECT_OPERATOR, "->"), $tokens, $token_number + 1) && - self::_token_matches(array(T_STRING), $tokens, $token_number + 2) && - in_array($tokens[$token_number + 2][1], - array("thumb_proportion", "site_menu", "album_menu", "tag_menu", "photo_menu", + if (self::_token_matches(array(T_OBJECT_OPERATOR, "->"), $tokens, $token_number + 1) && + self::_token_matches(array(T_STRING), $tokens, $token_number + 2) && + in_array($tokens[$token_number + 2][1], + array("thumb_proportion", "site_menu", "album_menu", "tag_menu", "photo_menu", "context_menu", "pager", "site_status", "messages", "album_blocks", "album_bottom", "album_top", "body_attributes", "credits", "dynamic_bottom", "dynamic_top", "footer", "head", "header_bottom", "header_top", "page_bottom", "page_top", "photo_blocks", "photo_bottom", "photo_top", "resize_bottom", "resize_top", "sidebar_blocks", "sidebar_bottom", "sidebar_top", "thumb_bottom", "thumb_info", "thumb_top")) && - self::_token_matches("(", $tokens, $token_number + 3)) { + self::_token_matches("(", $tokens, $token_number + 3)) { - $method = $tokens[$token_number + 2][1]; - $frame->expr_append("->$method("); + $method = $tokens[$token_number + 2][1]; + $frame->expr_append("->$method("); - $token_number += 3; - $token = $tokens[$token_number]; + $token_number += 3; + $token = $tokens[$token_number]; $frame->is_safe_html(true); - } else if (self::_token_matches(array(T_OBJECT_OPERATOR, "->"), $tokens, $token_number + 1) && - self::_token_matches(array(T_STRING), $tokens, $token_number + 2) && - in_array($tokens[$token_number + 2][1], - array("css", "script", "url")) && - self::_token_matches("(", $tokens, $token_number + 3) && - // Only allow constant strings here - self::_token_matches(array(T_CONSTANT_ENCAPSED_STRING), $tokens, $token_number + 4)) { + } else if (self::_token_matches(array(T_OBJECT_OPERATOR, "->"), $tokens, $token_number + 1) && + self::_token_matches(array(T_STRING), $tokens, $token_number + 2) && + in_array($tokens[$token_number + 2][1], + array("css", "script", "url")) && + self::_token_matches("(", $tokens, $token_number + 3) && + // Only allow constant strings here + self::_token_matches(array(T_CONSTANT_ENCAPSED_STRING), $tokens, $token_number + 4)) { - $method = $tokens[$token_number + 2][1]; - $frame->expr_append("->$method("); + $method = $tokens[$token_number + 2][1]; + $frame->expr_append("->$method("); - $token_number += 4; - $token = $tokens[$token_number]; + $token_number += 4; + $token = $tokens[$token_number]; $frame->is_safe_html(true); - } + } } - } else if ($frame && $token[0] == T_STRING) { - $frame->expr_append($token[1]); - // t() and t2() are special in that they're guaranteed to return a SafeString(). - if (in_array($token[1], array("t", "t2"))) { - if (self::_token_matches("(", $tokens, $token_number + 1)) { - $frame->is_safe_html(true); - $frame->expr_append("("); - - $token_number++; - $token = $tokens[$token_number]; - } - } 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), $tokens, $token_number + 2) && - in_array($tokens[$token_number + 2][1], array("of", "purify")) && - self::_token_matches("(", $tokens, $token_number + 3)) { + } else if ($frame && $token[0] == T_STRING) { + $frame->expr_append($token[1]); + // t() and t2() are special in that they're guaranteed to return a SafeString(). + if (in_array($token[1], array("t", "t2"))) { + if (self::_token_matches("(", $tokens, $token_number + 1)) { + $frame->is_safe_html(true); + $frame->expr_append("("); + + $token_number++; + $token = $tokens[$token_number]; + } + } 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), $tokens, $token_number + 2) && + in_array($tokens[$token_number + 2][1], array("of", "purify")) && + self::_token_matches("(", $tokens, $token_number + 3)) { // Not checking for of_safe_html(). We want such calls to be marked dirty (thus reviewed). - $frame->is_safe_html(true); - - $method = $tokens[$token_number + 2][1]; - $frame->expr_append("::$method("); - - $token_number += 3; - $token = $tokens[$token_number]; - } - } else if ($token[1] == "json_encode") { - if (self::_token_matches("(", $tokens, $token_number + 1)) { - $frame->is_safe_js(true); - $frame->expr_append("("); - - $token_number++; - $token = $tokens[$token_number]; - } - } else if ($token[1] == "url") { - // 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_safe_html(true); - - $method = $tokens[$token_number + 2][1]; - $frame->expr_append("::$method("); - - $token_number += 3; - $token = $tokens[$token_number]; - } - } else if ($token[1] == "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("clean", "purify", "js_string", "clean_attribute")) && - self::_token_matches("(", $tokens, $token_number + 3)) { + $frame->is_safe_html(true); + + $method = $tokens[$token_number + 2][1]; + $frame->expr_append("::$method("); + + $token_number += 3; + $token = $tokens[$token_number]; + } + } else if ($token[1] == "json_encode") { + if (self::_token_matches("(", $tokens, $token_number + 1)) { + $frame->is_safe_js(true); + $frame->expr_append("("); + + $token_number++; + $token = $tokens[$token_number]; + } + } else if ($token[1] == "url") { + // 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_safe_html(true); + + $method = $tokens[$token_number + 2][1]; + $frame->expr_append("::$method("); + + $token_number += 3; + $token = $tokens[$token_number]; + } + } else if ($token[1] == "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("clean", "purify", "js_string", "clean_attribute")) && + self::_token_matches("(", $tokens, $token_number + 3)) { // Not checking for mark_safe(). We want such calls to be marked dirty (thus reviewed). - $method = $tokens[$token_number + 2][1]; - $frame->expr_append("::$method("); + $method = $tokens[$token_number + 2][1]; + $frame->expr_append("::$method("); - $token_number += 3; - $token = $tokens[$token_number]; + $token_number += 3; + $token = $tokens[$token_number]; if ("js_string" == $method) { $frame->is_safe_js(true); } else { $frame->is_safe_html(true); } - } - } - } else if ($frame && $token[0] == T_OBJECT_OPERATOR) { - $frame->expr_append($token[1]); - - if (self::_token_matches(array(T_STRING), $tokens, $token_number + 1) && - in_array($tokens[$token_number + 1][1], - array("for_js", "for_html", "purified_html", "for_html_attr")) && - self::_token_matches("(", $tokens, $token_number + 2)) { - $method = $tokens[$token_number + 1][1]; - $frame->expr_append("$method("); - - $token_number += 2; - $token = $tokens[$token_number]; - - if ("for_js" == $method) { - $frame->is_safe_js(true); - } else { - $frame->is_safe_html(true); - } - } + } + } + } else if ($frame && $token[0] == T_OBJECT_OPERATOR) { + $frame->expr_append($token[1]); + + if (self::_token_matches(array(T_STRING), $tokens, $token_number + 1) && + in_array($tokens[$token_number + 1][1], + array("for_js", "for_html", "purified_html", "for_html_attr")) && + self::_token_matches("(", $tokens, $token_number + 2)) { + $method = $tokens[$token_number + 1][1]; + $frame->expr_append("$method("); + + $token_number += 2; + $token = $tokens[$token_number]; + + if ("for_js" == $method) { + $frame->is_safe_js(true); + } else { + $frame->is_safe_html(true); + } + } } else if ($frame) { - $frame->expr_append($token[1]); - } + $frame->expr_append($token[1]); + } } } @@ -252,26 +252,26 @@ class Xss_Security_Test extends Unit_Test_Case { ksort($found); foreach ($found as $view => $frames) { foreach ($frames as $frame) { - $state = "DIRTY"; - if ($frame->in_script_block()) { - $state = "DIRTY_JS"; - if ($frame->is_safe_js()) { - $state = "CLEAN"; - } - } else { - if ($frame->is_safe_html()) { - $state = "CLEAN"; - } - } - - if ("CLEAN" == $state) { - // Don't print CLEAN instances - No need to update the golden - // file when adding / moving clean instances. - continue; - } - - fprintf($fd, "%-60s %-3s %-8s %s\n", - $view, $frame->line(), $state, $frame->expr()); + $state = "DIRTY"; + if ($frame->in_script_block()) { + $state = "DIRTY_JS"; + if ($frame->is_safe_js()) { + $state = "CLEAN"; + } + } else { + if ($frame->is_safe_html()) { + $state = "CLEAN"; + } + } + + if ("CLEAN" == $state) { + // Don't print CLEAN instances - No need to update the golden + // file when adding / moving clean instances. + continue; + } + + fprintf($fd, "%-60s %-3s %-8s %s\n", + $view, $frame->line(), $state, $frame->expr()); } } fclose($fd); @@ -280,7 +280,7 @@ class Xss_Security_Test extends Unit_Test_Case { $canonical = MODPATH . "gallery/tests/xss_data.txt"; exec("diff $canonical $new", $output, $return_value); $this->assert_false( - $return_value, "XSS golden file mismatch. Output:\n" . implode("\n", $output) ); + $return_value, "XSS golden file mismatch. Output:\n" . implode("\n", $output) ); } private static function _create_frame($token, $in_script_block) { @@ -296,9 +296,9 @@ class Xss_Security_Test extends Unit_Test_Case { if (is_array($expected_token)) { for ($i = 0; $i < count($expected_token); $i++) { - if ($expected_token[$i] != $token[$i]) { - return false; - } + if ($expected_token[$i] != $token[$i]) { + return false; + } } return true; } else { diff --git a/modules/server_add/helpers/server_add_event.php b/modules/server_add/helpers/server_add_event.php index 6b21ec2e..b2d55153 100644 --- a/modules/server_add/helpers/server_add_event.php +++ b/modules/server_add/helpers/server_add_event.php @@ -35,7 +35,7 @@ class server_add_event_Core { // turn that into a dropdown if there are two different ways to add things. Do that in a // portable way for now. If we find ourselves duplicating this pattern, we should make an // API method for this. - $add_menu = $menu->get("add_menu"); + $add_menu = $menu->get("add_menu"); $add_menu->append(Menu::factory("dialog") ->id("server_add") ->label(t("Server add")) -- cgit v1.2.3