From 79740a2c77ad5c9b048e094cc164fd0129aba16a Mon Sep 17 00:00:00 2001
From: Bharat Mediratta <bharat@menalto.com>
Date: Tue, 14 Dec 2010 21:18:40 -0800
Subject: Move photo/movie file extension validation into the model.  Fixes
 #1524.

---
 modules/gallery/tests/Item_Model_Test.php | 23 +++++++++++++++++++++++
 1 file changed, 23 insertions(+)

(limited to 'modules/gallery/tests')

diff --git a/modules/gallery/tests/Item_Model_Test.php b/modules/gallery/tests/Item_Model_Test.php
index 264a2128..1e6d54d0 100644
--- a/modules/gallery/tests/Item_Model_Test.php
+++ b/modules/gallery/tests/Item_Model_Test.php
@@ -431,4 +431,27 @@ class Item_Model_Test extends Gallery_Unit_Test_Case {
       preg_match("|http://./var/thumbs/name_\d+/\.album\.jpg\?m=\d+|", $album->thumb_url()),
       $album->thumb_url() . " is malformed");
   }
+
+  public function legal_extension_test() {
+    foreach (array("test.gif", "test.GIF", "test.Gif", "test.jpeg", "test.JPG") as $name) {
+      $photo = test::random_photo_unsaved(item::root());
+      $photo->name = $name;
+      $photo->save();
+    }
+  }
+
+  public function illegal_extension_test() {
+    foreach (array("test.php", "test.PHP", "test.php5", "test.php4", "test.pl") as $name) {
+      try {
+        $photo = test::random_photo_unsaved(item::root());
+        $photo->name = $name;
+        $photo->save();
+      } catch (ORM_Validation_Exception $e) {
+        $this->assert_equal(array("name" => "illegal_data_file_extension"),
+                            $e->validation->errors());
+        continue;
+      }
+      $this->assert_true(false, "Shouldn't get here");
+    }
+  }
 }
-- 
cgit v1.2.3


From 18470a60012b8ab48b8c2a6a94673b69500de4a7 Mon Sep 17 00:00:00 2001
From: Bharat Mediratta <bharat@menalto.com>
Date: Wed, 15 Dec 2010 12:07:02 -0800
Subject: Clean up xss golden file as a follow-on to
 945e79b17182f7c96eb4b566bd41d26b5f8c4288 for #1478.

---
 modules/gallery/tests/xss_data.txt | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

(limited to 'modules/gallery/tests')

diff --git a/modules/gallery/tests/xss_data.txt b/modules/gallery/tests/xss_data.txt
index 0345df96..bd72509c 100644
--- a/modules/gallery/tests/xss_data.txt
+++ b/modules/gallery/tests/xss_data.txt
@@ -43,8 +43,8 @@ modules/digibug/views/digibug_form.html.php                  6   DIRTY    form::
 modules/exif/views/exif_dialog.html.php                      14  DIRTY    $details[$i]["caption"]
 modules/exif/views/exif_dialog.html.php                      21  DIRTY    $details[$i]["caption"]
 modules/g2_import/views/admin_g2_import.html.php             9   DIRTY    $form
-modules/gallery/views/admin_advanced_settings.html.php       20  DIRTY_ATTR text::alternate("g-odd","g-even")
-modules/gallery/views/admin_advanced_settings.html.php       21  DIRTY    $var->module_name
+modules/gallery/views/admin_advanced_settings.html.php       21  DIRTY_ATTR text::alternate("g-odd","g-even")
+modules/gallery/views/admin_advanced_settings.html.php       22  DIRTY    $var->module_name
 modules/gallery/views/admin_block_log_entries.html.php       4   DIRTY_ATTR log::severity_class($entry->severity)
 modules/gallery/views/admin_block_log_entries.html.php       8   DIRTY_JS user_profile::url($entry->user->id)
 modules/gallery/views/admin_block_log_entries.html.php       10  DIRTY    gallery::date_time($entry->timestamp)
-- 
cgit v1.2.3


From ff4c930338a80a7a5eb220f539aac1f06c748bc4 Mon Sep 17 00:00:00 2001
From: Bharat Mediratta <bharat@menalto.com>
Date: Wed, 15 Dec 2010 12:08:56 -0800
Subject: Clean up golden file as a follow-on to
 3c523bcac6e902973c5891c02842254eaecd33a4 for #1499.

---
 modules/gallery/tests/xss_data.txt | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

(limited to 'modules/gallery/tests')

diff --git a/modules/gallery/tests/xss_data.txt b/modules/gallery/tests/xss_data.txt
index bd72509c..7c5e803d 100644
--- a/modules/gallery/tests/xss_data.txt
+++ b/modules/gallery/tests/xss_data.txt
@@ -260,8 +260,8 @@ modules/gallery/views/upgrader.html.php                      123 DIRTY_ATTR $don
 modules/gallery/views/user_languages_block.html.php          2   DIRTY    form::dropdown("g-select-session-locale",$installed_locales,$selected)
 modules/gallery/views/user_profile.html.php                  34  DIRTY_ATTR $user->avatar_url(40,$theme->url(,true))
 modules/gallery/views/user_profile.html.php                  43  DIRTY    $info->view
-modules/image_block/views/image_block_block.html.php         3   DIRTY_JS $item->url()
-modules/image_block/views/image_block_block.html.php         4   DIRTY    $item->thumb_img(array("class"=>"g-thumbnail"))
+modules/image_block/views/image_block_block.html.php         4   DIRTY_JS $item->url()
+modules/image_block/views/image_block_block.html.php         5   DIRTY    $item->thumb_img(array("class"=>"g-thumbnail"))
 modules/info/views/info_block.html.php                       22  DIRTY    gallery::date_time($item->captured)
 modules/info/views/info_block.html.php                       29  DIRTY_JS $item->owner->url
 modules/notification/views/comment_published.html.php        28  DIRTY_JS $comment->item()->abs_url()
-- 
cgit v1.2.3


From 6fb0eb4e97e3bbfa50029f0b4249c10bd22abe09 Mon Sep 17 00:00:00 2001
From: Bharat Mediratta <bharat@menalto.com>
Date: Wed, 15 Dec 2010 12:24:31 -0800
Subject: Follow on to d2be26e407aeb620082bcad2d5a45272868b38a1 to update the
 unit tests to reflect the proper type filtering for #1503.

---
 modules/gallery/tests/Items_Rest_Helper_Test.php | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

(limited to 'modules/gallery/tests')

diff --git a/modules/gallery/tests/Items_Rest_Helper_Test.php b/modules/gallery/tests/Items_Rest_Helper_Test.php
index 8e53110a..49e77876 100644
--- a/modules/gallery/tests/Items_Rest_Helper_Test.php
+++ b/modules/gallery/tests/Items_Rest_Helper_Test.php
@@ -65,21 +65,21 @@ class Items_Rest_Helper_Test extends Gallery_Unit_Test_Case {
     $request = new stdClass();
     $request->params = new stdClass();
     $request->params->urls = json_encode(array(
-      rest::url("item", $photo1),
-      rest::url("item", $album2)));
+      rest::url("item", $photo2),
+      rest::url("item", $album1)));
     $request->params->type = "album";
     $this->assert_equal_array(
       array(
-         array("url" => rest::url("item", $album2),
-               "entity" => $album2->as_restful_array(),
+         array("url" => rest::url("item", $album1),
+               "entity" => $album1->as_restful_array(),
                "relationships" => array(
                  "comments" => array(
-                   "url" => rest::url("item_comments", $album2)),
+                   "url" => rest::url("item_comments", $album1)),
                  "tags" => array(
-                   "url" => rest::url("item_tags", $album2),
+                   "url" => rest::url("item_tags", $album1),
                    "members" => array())),
                "members" => array(
-                 rest::url("item", $photo2)))),
+                 rest::url("item", $album2)))),
       items_rest::get($request));
   }
 
-- 
cgit v1.2.3


From 45c63f4d118bfc99924edb8685442035349af6db Mon Sep 17 00:00:00 2001
From: Bharat Mediratta <bharat@menalto.com>
Date: Wed, 15 Dec 2010 12:48:56 -0800
Subject: Use mt_rand() instead of rand() since it provides better portability.
 Fixes #1527.

---
 installer/installer.php                           |  4 +--
 modules/digibug/controllers/digibug.php           |  2 +-
 modules/digibug/tests/Digibug_Controller_Test.php |  2 +-
 modules/gallery/controllers/upgrader.php          |  2 +-
 modules/gallery/helpers/access.php                |  2 +-
 modules/gallery/helpers/block_manager.php         |  2 +-
 modules/gallery/helpers/gallery_installer.php     |  4 +--
 modules/gallery/models/item.php                   |  4 +--
 modules/gallery/tests/Albums_Controller_Test.php  |  2 +-
 modules/gallery/tests/Cache_Test.php              | 30 +++++++++++------------
 modules/gallery/tests/Item_Helper_Test.php        |  2 +-
 modules/gallery/tests/Item_Model_Test.php         |  4 +--
 modules/gallery_unit_test/helpers/test.php        | 12 ++++-----
 modules/rest/helpers/rest_event.php               |  6 ++---
 14 files changed, 39 insertions(+), 39 deletions(-)

(limited to 'modules/gallery/tests')

diff --git a/installer/installer.php b/installer/installer.php
index 9a957b43..7d358e70 100644
--- a/installer/installer.php
+++ b/installer/installer.php
@@ -138,7 +138,7 @@ class installer {
       $char += ($char > 90) ? 13 : ($char > 57) ? 7 : 0;
       $salt .= chr($char);
     }
-    $password = substr(md5(time() * rand()), 0, 6);
+    $password = substr(md5(time() . mt_rand()), 0, 6);
     // Escape backslash in preparation for our UPDATE statement.
     $hashed_password = str_replace("\\", "\\\\", $salt . md5($salt . $password));
     $sql = self::prepend_prefix($config["prefix"],
@@ -152,7 +152,7 @@ class installer {
   }
 
   static function create_admin_session($config) {
-    $session_id = md5(time() * rand());
+    $session_id = md5(time() . mt_rand());
     $user_agent = $_SERVER["HTTP_USER_AGENT"];
     $user_agent_len = strlen($user_agent);
     $now = time();
diff --git a/modules/digibug/controllers/digibug.php b/modules/digibug/controllers/digibug.php
index 88d1ace0..a9e49de7 100644
--- a/modules/digibug/controllers/digibug.php
+++ b/modules/digibug/controllers/digibug.php
@@ -28,7 +28,7 @@ class Digibug_Controller extends Controller {
       $thumb_url = $item->thumb_url(true);
     } else {
       $proxy = ORM::factory("digibug_proxy");
-      $proxy->uuid =  md5(rand());
+      $proxy->uuid = md5(mt_rand());
       $proxy->item_id = $item->id;
       $proxy->save();
       $full_url = url::abs_site("digibug/print_proxy/full/$proxy->uuid");
diff --git a/modules/digibug/tests/Digibug_Controller_Test.php b/modules/digibug/tests/Digibug_Controller_Test.php
index 19a3f9da..6f9e20df 100644
--- a/modules/digibug/tests/Digibug_Controller_Test.php
+++ b/modules/digibug/tests/Digibug_Controller_Test.php
@@ -36,7 +36,7 @@ class Digibug_Controller_Test extends Gallery_Unit_Test_Case {
     access::deny(identity::registered_users(), "view_full", $album);
 
     $proxy = ORM::factory("digibug_proxy");
-    $proxy->uuid = md5(rand());
+    $proxy->uuid = md5(mt_rand());
     $proxy->item_id = $photo->id;
     return $proxy->save();
   }
diff --git a/modules/gallery/controllers/upgrader.php b/modules/gallery/controllers/upgrader.php
index b2646874..50f6b8f0 100644
--- a/modules/gallery/controllers/upgrader.php
+++ b/modules/gallery/controllers/upgrader.php
@@ -23,7 +23,7 @@ class Upgrader_Controller extends Controller {
 
     // Make sure we have an upgrade token
     if (!($upgrade_token = $session->get("upgrade_token", null))) {
-      $session->set("upgrade_token", $upgrade_token = md5(rand()));
+      $session->set("upgrade_token", $upgrade_token = md5(time() . mt_rand()));
     }
 
     // If the upgrade token exists, then bless this session
diff --git a/modules/gallery/helpers/access.php b/modules/gallery/helpers/access.php
index 0b0dcbc1..6a948999 100644
--- a/modules/gallery/helpers/access.php
+++ b/modules/gallery/helpers/access.php
@@ -426,7 +426,7 @@ class access_Core {
     $session = Session::instance();
     $csrf = $session->get("csrf");
     if (empty($csrf)) {
-      $csrf = md5(rand());
+      $csrf = md5(time() . mt_rand());
       $session->set("csrf", $csrf);
     }
     return $csrf;
diff --git a/modules/gallery/helpers/block_manager.php b/modules/gallery/helpers/block_manager.php
index 2237b702..e7247edc 100644
--- a/modules/gallery/helpers/block_manager.php
+++ b/modules/gallery/helpers/block_manager.php
@@ -28,7 +28,7 @@ class block_manager_Core {
 
   static function add($location, $module_name, $block_id) {
     $blocks = block_manager::get_active($location);
-    $blocks[rand()] = array($module_name, $block_id);
+    $blocks[mt_rand()] = array($module_name, $block_id);
 
     block_manager::set_active($location, $blocks);
   }
diff --git a/modules/gallery/helpers/gallery_installer.php b/modules/gallery/helpers/gallery_installer.php
index 3d82bc69..9c42caad 100644
--- a/modules/gallery/helpers/gallery_installer.php
+++ b/modules/gallery/helpers/gallery_installer.php
@@ -459,7 +459,7 @@ class gallery_installer {
         $blocks = block_manager::get_active($location);
         $new_blocks = array();
         foreach ($blocks as $block) {
-          $new_blocks[rand()] = $block;
+          $new_blocks[mt_rand()] = $block;
         }
         block_manager::set_active($location, $new_blocks);
       }
@@ -507,7 +507,7 @@ class gallery_installer {
                ->execute() as $row) {
         $new_slug = item::convert_filename_to_slug($row->slug);
         if (empty($new_slug)) {
-          $new_slug = rand();
+          $new_slug = mt_rand();
         }
         db::build()
           ->update("items")
diff --git a/modules/gallery/models/item.php b/modules/gallery/models/item.php
index a4d24b8f..b6713fc3 100644
--- a/modules/gallery/models/item.php
+++ b/modules/gallery/models/item.php
@@ -390,7 +390,7 @@ class Item_Model_Core extends ORM_MPTT {
           if (file_exists($this->resize_path()) ||
               file_exists($this->thumb_path())) {
             $pi = pathinfo($this->name);
-            $this->name = $pi["filename"] . "-" . rand() . "." . $pi["extension"];
+            $this->name = $pi["filename"] . "-" . mt_rand() . "." . $pi["extension"];
             parent::save();
           }
 
@@ -512,7 +512,7 @@ class Item_Model_Core extends ORM_MPTT {
            ->or_where("slug", "=", $this->slug)
            ->close()
            ->find()->id) {
-      $rand = rand();
+      $rand = mt_rand();
       if ($base_ext) {
         $this->name = "$base_name-$rand.$base_ext";
       } else {
diff --git a/modules/gallery/tests/Albums_Controller_Test.php b/modules/gallery/tests/Albums_Controller_Test.php
index 6c64394d..86c74890 100644
--- a/modules/gallery/tests/Albums_Controller_Test.php
+++ b/modules/gallery/tests/Albums_Controller_Test.php
@@ -31,7 +31,7 @@ class Albums_Controller_Test extends Gallery_Unit_Test_Case {
     $album = test::random_album();
 
     // Randomize to avoid conflicts.
-    $new_name = "new_name_" . rand();
+    $new_name = "new_name_" . mt_rand();
 
     $_POST["name"] = $new_name;
     $_POST["title"] = "new title";
diff --git a/modules/gallery/tests/Cache_Test.php b/modules/gallery/tests/Cache_Test.php
index e8d8b6f4..6cee2862 100644
--- a/modules/gallery/tests/Cache_Test.php
+++ b/modules/gallery/tests/Cache_Test.php
@@ -27,7 +27,7 @@ class Cache_Test extends Gallery_Unit_Test_Case {
   public function cache_exists_test() {
     $this->assert_false($this->_driver->exists("test_key"), "test_key should not be defined");
 
-    $id = md5(rand());
+    $id = md5(mt_rand());
     db::build()
       ->insert("caches")
       ->columns("key", "tags", "expiration", "cache")
@@ -38,7 +38,7 @@ class Cache_Test extends Gallery_Unit_Test_Case {
   }
 
   public function cache_get_test() {
-    $id = md5(rand());
+    $id = md5(mt_rand());
 
     db::build()
       ->insert("caches")
@@ -54,7 +54,7 @@ class Cache_Test extends Gallery_Unit_Test_Case {
   }
 
   public function cache_set_test() {
-    $id = md5(rand());
+    $id = md5(mt_rand());
     $original_data = array("field1" => "value1", "field2" => "value2");
     $this->_driver->set(array($id => $original_data), array("tag1", "tag2"), 84600);
 
@@ -63,15 +63,15 @@ class Cache_Test extends Gallery_Unit_Test_Case {
   }
 
   public function cache_get_tag_test() {
-    $id1 = md5(rand());
+    $id1 = md5(mt_rand());
     $value1 = array("field1" => "value1", "field2" => "value2");
     $this->_driver->set(array($id1 => $value1), array("tag1", "tag2"), 84600);
 
-    $id2 = md5(rand());
+    $id2 = md5(mt_rand());
     $value2 = array("field3" => "value3", "field4" => "value4");
     $this->_driver->set(array($id2 => $value2), array("tag2", "tag3"), 84600);
 
-    $id3 = md5(rand());
+    $id3 = md5(mt_rand());
     $value3 = array("field5" => "value5", "field6" => "value6");
     $this->_driver->set(array($id3 => $value3), array("tag3", "tag4"), 84600);
 
@@ -86,15 +86,15 @@ class Cache_Test extends Gallery_Unit_Test_Case {
   }
 
   public function cache_delete_id_test() {
-    $id1 = md5(rand());
+    $id1 = md5(mt_rand());
     $value1 = array("field1" => "value1", "field2" => "value2");
     $this->_driver->set(array($id1 => $value1), array("tag1", "tag2"), 84600);
 
-    $id2 = md5(rand());
+    $id2 = md5(mt_rand());
     $value2 = array("field3" => "value3", "field4" => "value4");
     $this->_driver->set(array($id2 => $value2), array("tag2", "tag3"), 846000);
 
-    $id3 = md5(rand());
+    $id3 = md5(mt_rand());
     $value3 = array("field5" => "value5", "field6" => "value6");
     $this->_driver->set(array($id3 => $value3), array("tag3", "tag4"), 84600);
 
@@ -106,15 +106,15 @@ class Cache_Test extends Gallery_Unit_Test_Case {
   }
 
   public function cache_delete_tag_test() {
-    $id1 = md5(rand());
+    $id1 = md5(mt_rand());
     $value1 = array("field1" => "value1", "field2" => "value2");
     $this->_driver->set(array($id1 => $value1), array("tag1", "tag2"), 84600);
 
-    $id2 = md5(rand());
+    $id2 = md5(mt_rand());
     $value2 = array("field3" => "value3", "field4" => "value4");
     $this->_driver->set(array($id2 => $value2), array("tag2", "tag3"), 846000);
 
-    $id3 = md5(rand());
+    $id3 = md5(mt_rand());
     $value3 = array("field5" => "value5", "field6" => "value6");
     $this->_driver->set(array($id3 => $value3), array("tag3", "tag4"), 84600);
 
@@ -126,15 +126,15 @@ class Cache_Test extends Gallery_Unit_Test_Case {
   }
 
   public function cache_delete_all_test() {
-    $id1 = md5(rand());
+    $id1 = md5(mt_rand());
     $value1 = array("field1" => "value1", "field2" => "value2");
     $this->_driver->set(array($id1 => $value1), array("tag1", "tag2"), 84600);
 
-    $id2 = md5(rand());
+    $id2 = md5(mt_rand());
     $value2 = array("field3" => "value3", "field4" => "value4");
     $this->_driver->set(array($id2 => $value2), array("tag2", "tag3"), 846000);
 
-    $id3 = md5(rand());
+    $id3 = md5(mt_rand());
     $value3 = array("field5" => "value5", "field6" => "value6");
     $this->_driver->set(array($id3 => $value3), array("tag3", "tag4"), 84600);
 
diff --git a/modules/gallery/tests/Item_Helper_Test.php b/modules/gallery/tests/Item_Helper_Test.php
index eb2458cb..c93cc239 100644
--- a/modules/gallery/tests/Item_Helper_Test.php
+++ b/modules/gallery/tests/Item_Helper_Test.php
@@ -92,7 +92,7 @@ class Item_Helper_Test extends Gallery_Unit_Test_Case {
   }
 
   public function move_conflicts_result_in_a_rename_test() {
-    $rand = rand();
+    $rand = mt_rand();
     $photo1 = test::random_photo_unsaved(item::root());
     $photo1->name = "{$rand}.jpg";
     $photo1->slug = (string)$rand;
diff --git a/modules/gallery/tests/Item_Model_Test.php b/modules/gallery/tests/Item_Model_Test.php
index 1e6d54d0..0d6d10af 100644
--- a/modules/gallery/tests/Item_Model_Test.php
+++ b/modules/gallery/tests/Item_Model_Test.php
@@ -278,10 +278,10 @@ class Item_Model_Test extends Gallery_Unit_Test_Case {
 
   public function basic_validation_test() {
     $item = ORM::factory("item");
-    $item->album_cover_item_id = rand();  // invalid
+    $item->album_cover_item_id = mt_rand();  // invalid
     $item->description = str_repeat("x", 70000);  // invalid
     $item->name = null;
-    $item->parent_id = rand();
+    $item->parent_id = mt_rand();
     $item->slug = null;
     $item->sort_column = "bogus";
     $item->sort_order = "bogus";
diff --git a/modules/gallery_unit_test/helpers/test.php b/modules/gallery_unit_test/helpers/test.php
index 1be82a74..d5149492 100644
--- a/modules/gallery_unit_test/helpers/test.php
+++ b/modules/gallery_unit_test/helpers/test.php
@@ -19,7 +19,7 @@
  */
 class test_Core {
   static function random_album_unsaved($parent=null) {
-    $rand = rand();
+    $rand = mt_rand();
 
     $album = ORM::factory("item");
     $album->type = "album";
@@ -34,7 +34,7 @@ class test_Core {
   }
 
   static function random_photo_unsaved($parent=null) {
-    $rand = rand();
+    $rand = mt_rand();
     $photo = ORM::factory("item");
     $photo->type = "photo";
     $photo->parent_id = $parent ? $parent->id : 1;
@@ -49,16 +49,16 @@ class test_Core {
   }
 
   static function random_user($password="password") {
-    $rand = "name_" . rand();
+    $rand = "name_" . mt_rand();
     return identity::create_user($rand, $rand, $password, "$rand@rand.com");
   }
 
   static function random_group() {
-    return identity::create_group((string)rand());
+    return identity::create_group((string)mt_rand());
   }
 
   static function random_name($item=null) {
-    $rand = "name_" . rand();
+    $rand = "name_" . mt_rand();
     if ($item && $item->is_photo()) {
       $rand .= ".jpg";
     }
@@ -77,7 +77,7 @@ class test_Core {
 
   static function random_tag() {
     $tag = ORM::factory("tag");
-    $tag->name = (string)rand();
+    $tag->name = (string)mt_rand();
 
     // Reload so that ORM coerces all fields into strings.
     return $tag->save()->reload();
diff --git a/modules/rest/helpers/rest_event.php b/modules/rest/helpers/rest_event.php
index d8c69e94..4d7a4a1b 100644
--- a/modules/rest/helpers/rest_event.php
+++ b/modules/rest/helpers/rest_event.php
@@ -43,7 +43,7 @@ class rest_event {
   static function user_add_form_admin_completed($user, $form) {
     $key = ORM::factory("user_access_key");
     $key->user_id = $user->id;
-    $key->access_key = md5($user->name . rand());
+    $key->access_key = md5($user->name . time() . mt_rand());
     $key->save();
   }
 
@@ -64,7 +64,7 @@ class rest_event {
 
     if (!$key->loaded()) {
       $key->user_id = $user->id;
-      $key->access_key = md5($user->name . rand());
+      $key->access_key = md5($user->name . time() . mt_rand());
       $key->save();
     }
 
@@ -93,7 +93,7 @@ class rest_event {
 
     if (!$key->loaded()) {
       $key->user_id = $data->user->id;
-      $key->access_key = md5($data->user->name . rand());
+      $key->access_key = md5($data->user->name . time() . mt_rand());
       $key->save();
     }
     $view->rest_key = $key->access_key;
-- 
cgit v1.2.3


From cd48b89f3166e7fa732b5cb06d33fba018af9127 Mon Sep 17 00:00:00 2001
From: Bharat Mediratta <bharat@menalto.com>
Date: Wed, 15 Dec 2010 14:57:00 -0800
Subject: Consolidate all the random code into a random helper that offers:  
 random::hash()   random::string()   random::percent()   random::int()

So that we don't have lots of different ways to get random values all
over the code.  Follow-on to #1527.
---
 modules/digibug/controllers/digibug.php           |  2 +-
 modules/digibug/tests/Digibug_Controller_Test.php |  2 +-
 modules/gallery/controllers/upgrader.php          |  2 +-
 modules/gallery/helpers/access.php                |  2 +-
 modules/gallery/helpers/block_manager.php         |  2 +-
 modules/gallery/helpers/gallery_installer.php     |  4 +-
 modules/gallery/helpers/item.php                  |  2 +-
 modules/gallery/helpers/random.php                | 50 +++++++++++++++++++++++
 modules/gallery/models/item.php                   |  6 +--
 modules/gallery/tests/Albums_Controller_Test.php  |  2 +-
 modules/gallery/tests/Cache_Test.php              | 30 +++++++-------
 modules/gallery/tests/Item_Helper_Test.php        |  2 +-
 modules/gallery/tests/Item_Model_Test.php         | 14 +++----
 modules/gallery_unit_test/helpers/test.php        | 12 +++---
 modules/rest/helpers/rest.php                     |  2 +-
 modules/rest/helpers/rest_event.php               |  6 +--
 modules/user/controllers/password.php             |  2 +-
 17 files changed, 96 insertions(+), 46 deletions(-)
 create mode 100644 modules/gallery/helpers/random.php

(limited to 'modules/gallery/tests')

diff --git a/modules/digibug/controllers/digibug.php b/modules/digibug/controllers/digibug.php
index a9e49de7..bc0c7c5e 100644
--- a/modules/digibug/controllers/digibug.php
+++ b/modules/digibug/controllers/digibug.php
@@ -28,7 +28,7 @@ class Digibug_Controller extends Controller {
       $thumb_url = $item->thumb_url(true);
     } else {
       $proxy = ORM::factory("digibug_proxy");
-      $proxy->uuid = md5(mt_rand());
+      $proxy->uuid = random::hash();
       $proxy->item_id = $item->id;
       $proxy->save();
       $full_url = url::abs_site("digibug/print_proxy/full/$proxy->uuid");
diff --git a/modules/digibug/tests/Digibug_Controller_Test.php b/modules/digibug/tests/Digibug_Controller_Test.php
index 6f9e20df..d331b0ae 100644
--- a/modules/digibug/tests/Digibug_Controller_Test.php
+++ b/modules/digibug/tests/Digibug_Controller_Test.php
@@ -36,7 +36,7 @@ class Digibug_Controller_Test extends Gallery_Unit_Test_Case {
     access::deny(identity::registered_users(), "view_full", $album);
 
     $proxy = ORM::factory("digibug_proxy");
-    $proxy->uuid = md5(mt_rand());
+    $proxy->uuid = random::hash();
     $proxy->item_id = $photo->id;
     return $proxy->save();
   }
diff --git a/modules/gallery/controllers/upgrader.php b/modules/gallery/controllers/upgrader.php
index 50f6b8f0..66c71648 100644
--- a/modules/gallery/controllers/upgrader.php
+++ b/modules/gallery/controllers/upgrader.php
@@ -23,7 +23,7 @@ class Upgrader_Controller extends Controller {
 
     // Make sure we have an upgrade token
     if (!($upgrade_token = $session->get("upgrade_token", null))) {
-      $session->set("upgrade_token", $upgrade_token = md5(time() . mt_rand()));
+      $session->set("upgrade_token", $upgrade_token = random::hash());
     }
 
     // If the upgrade token exists, then bless this session
diff --git a/modules/gallery/helpers/access.php b/modules/gallery/helpers/access.php
index 6a948999..a7ac3f9f 100644
--- a/modules/gallery/helpers/access.php
+++ b/modules/gallery/helpers/access.php
@@ -426,7 +426,7 @@ class access_Core {
     $session = Session::instance();
     $csrf = $session->get("csrf");
     if (empty($csrf)) {
-      $csrf = md5(time() . mt_rand());
+      $csrf = random::hash();
       $session->set("csrf", $csrf);
     }
     return $csrf;
diff --git a/modules/gallery/helpers/block_manager.php b/modules/gallery/helpers/block_manager.php
index e7247edc..4bd649c2 100644
--- a/modules/gallery/helpers/block_manager.php
+++ b/modules/gallery/helpers/block_manager.php
@@ -28,7 +28,7 @@ class block_manager_Core {
 
   static function add($location, $module_name, $block_id) {
     $blocks = block_manager::get_active($location);
-    $blocks[mt_rand()] = array($module_name, $block_id);
+    $blocks[random::int()] = array($module_name, $block_id);
 
     block_manager::set_active($location, $blocks);
   }
diff --git a/modules/gallery/helpers/gallery_installer.php b/modules/gallery/helpers/gallery_installer.php
index 9c42caad..a6b8e6a2 100644
--- a/modules/gallery/helpers/gallery_installer.php
+++ b/modules/gallery/helpers/gallery_installer.php
@@ -459,7 +459,7 @@ class gallery_installer {
         $blocks = block_manager::get_active($location);
         $new_blocks = array();
         foreach ($blocks as $block) {
-          $new_blocks[mt_rand()] = $block;
+          $new_blocks[random::int()] = $block;
         }
         block_manager::set_active($location, $new_blocks);
       }
@@ -507,7 +507,7 @@ class gallery_installer {
                ->execute() as $row) {
         $new_slug = item::convert_filename_to_slug($row->slug);
         if (empty($new_slug)) {
-          $new_slug = mt_rand();
+          $new_slug = random::int();
         }
         db::build()
           ->update("items")
diff --git a/modules/gallery/helpers/item.php b/modules/gallery/helpers/item.php
index 052b1c8e..664da812 100644
--- a/modules/gallery/helpers/item.php
+++ b/modules/gallery/helpers/item.php
@@ -232,7 +232,7 @@ class item_Core {
     // distributed so this is going to be more efficient with larger data sets.
     return ORM::factory("item")
       ->viewable()
-      ->where("rand_key", "<", ((float)mt_rand()) / (float)mt_getrandmax())
+      ->where("rand_key", "<", random::percent())
       ->order_by("rand_key", "DESC");
   }
 }
\ No newline at end of file
diff --git a/modules/gallery/helpers/random.php b/modules/gallery/helpers/random.php
new file mode 100644
index 00000000..a26762bd
--- /dev/null
+++ b/modules/gallery/helpers/random.php
@@ -0,0 +1,50 @@
+<?php defined("SYSPATH") or die("No direct script access.");
+/**
+ * Gallery - a web based photo album viewer and editor
+ * Copyright (C) 2000-2010 Bharat Mediratta
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or (at
+ * your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street - Fifth Floor, Boston, MA  02110-1301, USA.
+ */
+class random_Core {
+  /**
+   * Return a random 32 bit hash value.
+   * @param string extra entropy data
+   */
+  static function hash($entropy="") {
+    return md5($entropy . uniqid(mt_rand(), true));
+  }
+
+  /**
+   * Return a random hexadecimal string of the given length.
+   * @param int the desired length of the string
+   */
+  static function string($length) {
+    return substr(random::hash(), 0, $length);
+  }
+
+  /**
+   * Return a random floating point number between 0 and 1
+   */
+  static function percent() {
+    return ((float)mt_rand()) / (float)mt_getrandmax();
+  }
+
+  /**
+   * Return a random number between 0 and mt_getrandmax()
+   */
+  static function int() {
+    return mt_rand();
+  }
+}
\ No newline at end of file
diff --git a/modules/gallery/models/item.php b/modules/gallery/models/item.php
index b6713fc3..ad577066 100644
--- a/modules/gallery/models/item.php
+++ b/modules/gallery/models/item.php
@@ -28,7 +28,7 @@ class Item_Model_Core extends ORM_MPTT {
     if (!$this->loaded()) {
       // Set reasonable defaults
       $this->created = time();
-      $this->rand_key = ((float)mt_rand()) / (float)mt_getrandmax();
+      $this->rand_key = random::percent();
       $this->thumb_dirty = 1;
       $this->resize_dirty = 1;
       $this->sort_column = "created";
@@ -390,7 +390,7 @@ class Item_Model_Core extends ORM_MPTT {
           if (file_exists($this->resize_path()) ||
               file_exists($this->thumb_path())) {
             $pi = pathinfo($this->name);
-            $this->name = $pi["filename"] . "-" . mt_rand() . "." . $pi["extension"];
+            $this->name = $pi["filename"] . "-" . random::int() . "." . $pi["extension"];
             parent::save();
           }
 
@@ -512,7 +512,7 @@ class Item_Model_Core extends ORM_MPTT {
            ->or_where("slug", "=", $this->slug)
            ->close()
            ->find()->id) {
-      $rand = mt_rand();
+      $rand = random::int();
       if ($base_ext) {
         $this->name = "$base_name-$rand.$base_ext";
       } else {
diff --git a/modules/gallery/tests/Albums_Controller_Test.php b/modules/gallery/tests/Albums_Controller_Test.php
index 86c74890..35a3bdbb 100644
--- a/modules/gallery/tests/Albums_Controller_Test.php
+++ b/modules/gallery/tests/Albums_Controller_Test.php
@@ -31,7 +31,7 @@ class Albums_Controller_Test extends Gallery_Unit_Test_Case {
     $album = test::random_album();
 
     // Randomize to avoid conflicts.
-    $new_name = "new_name_" . mt_rand();
+    $new_name = "new_name_" . random::string(6);
 
     $_POST["name"] = $new_name;
     $_POST["title"] = "new title";
diff --git a/modules/gallery/tests/Cache_Test.php b/modules/gallery/tests/Cache_Test.php
index 6cee2862..b95ef0a2 100644
--- a/modules/gallery/tests/Cache_Test.php
+++ b/modules/gallery/tests/Cache_Test.php
@@ -27,7 +27,7 @@ class Cache_Test extends Gallery_Unit_Test_Case {
   public function cache_exists_test() {
     $this->assert_false($this->_driver->exists("test_key"), "test_key should not be defined");
 
-    $id = md5(mt_rand());
+    $id = random::hash();
     db::build()
       ->insert("caches")
       ->columns("key", "tags", "expiration", "cache")
@@ -38,7 +38,7 @@ class Cache_Test extends Gallery_Unit_Test_Case {
   }
 
   public function cache_get_test() {
-    $id = md5(mt_rand());
+    $id = random::hash();
 
     db::build()
       ->insert("caches")
@@ -54,7 +54,7 @@ class Cache_Test extends Gallery_Unit_Test_Case {
   }
 
   public function cache_set_test() {
-    $id = md5(mt_rand());
+    $id = random::hash();
     $original_data = array("field1" => "value1", "field2" => "value2");
     $this->_driver->set(array($id => $original_data), array("tag1", "tag2"), 84600);
 
@@ -63,15 +63,15 @@ class Cache_Test extends Gallery_Unit_Test_Case {
   }
 
   public function cache_get_tag_test() {
-    $id1 = md5(mt_rand());
+    $id1 = random::hash();
     $value1 = array("field1" => "value1", "field2" => "value2");
     $this->_driver->set(array($id1 => $value1), array("tag1", "tag2"), 84600);
 
-    $id2 = md5(mt_rand());
+    $id2 = random::hash();
     $value2 = array("field3" => "value3", "field4" => "value4");
     $this->_driver->set(array($id2 => $value2), array("tag2", "tag3"), 84600);
 
-    $id3 = md5(mt_rand());
+    $id3 = random::hash();
     $value3 = array("field5" => "value5", "field6" => "value6");
     $this->_driver->set(array($id3 => $value3), array("tag3", "tag4"), 84600);
 
@@ -86,15 +86,15 @@ class Cache_Test extends Gallery_Unit_Test_Case {
   }
 
   public function cache_delete_id_test() {
-    $id1 = md5(mt_rand());
+    $id1 = random::hash();
     $value1 = array("field1" => "value1", "field2" => "value2");
     $this->_driver->set(array($id1 => $value1), array("tag1", "tag2"), 84600);
 
-    $id2 = md5(mt_rand());
+    $id2 = random::hash();
     $value2 = array("field3" => "value3", "field4" => "value4");
     $this->_driver->set(array($id2 => $value2), array("tag2", "tag3"), 846000);
 
-    $id3 = md5(mt_rand());
+    $id3 = random::hash();
     $value3 = array("field5" => "value5", "field6" => "value6");
     $this->_driver->set(array($id3 => $value3), array("tag3", "tag4"), 84600);
 
@@ -106,15 +106,15 @@ class Cache_Test extends Gallery_Unit_Test_Case {
   }
 
   public function cache_delete_tag_test() {
-    $id1 = md5(mt_rand());
+    $id1 = random::hash();
     $value1 = array("field1" => "value1", "field2" => "value2");
     $this->_driver->set(array($id1 => $value1), array("tag1", "tag2"), 84600);
 
-    $id2 = md5(mt_rand());
+    $id2 = random::hash();
     $value2 = array("field3" => "value3", "field4" => "value4");
     $this->_driver->set(array($id2 => $value2), array("tag2", "tag3"), 846000);
 
-    $id3 = md5(mt_rand());
+    $id3 = random::hash();
     $value3 = array("field5" => "value5", "field6" => "value6");
     $this->_driver->set(array($id3 => $value3), array("tag3", "tag4"), 84600);
 
@@ -126,15 +126,15 @@ class Cache_Test extends Gallery_Unit_Test_Case {
   }
 
   public function cache_delete_all_test() {
-    $id1 = md5(mt_rand());
+    $id1 = random::hash();
     $value1 = array("field1" => "value1", "field2" => "value2");
     $this->_driver->set(array($id1 => $value1), array("tag1", "tag2"), 84600);
 
-    $id2 = md5(mt_rand());
+    $id2 = random::hash();
     $value2 = array("field3" => "value3", "field4" => "value4");
     $this->_driver->set(array($id2 => $value2), array("tag2", "tag3"), 846000);
 
-    $id3 = md5(mt_rand());
+    $id3 = random::hash();
     $value3 = array("field5" => "value5", "field6" => "value6");
     $this->_driver->set(array($id3 => $value3), array("tag3", "tag4"), 84600);
 
diff --git a/modules/gallery/tests/Item_Helper_Test.php b/modules/gallery/tests/Item_Helper_Test.php
index c93cc239..26db5a63 100644
--- a/modules/gallery/tests/Item_Helper_Test.php
+++ b/modules/gallery/tests/Item_Helper_Test.php
@@ -92,7 +92,7 @@ class Item_Helper_Test extends Gallery_Unit_Test_Case {
   }
 
   public function move_conflicts_result_in_a_rename_test() {
-    $rand = mt_rand();
+    $rand = random::int();
     $photo1 = test::random_photo_unsaved(item::root());
     $photo1->name = "{$rand}.jpg";
     $photo1->slug = (string)$rand;
diff --git a/modules/gallery/tests/Item_Model_Test.php b/modules/gallery/tests/Item_Model_Test.php
index 0d6d10af..4987d2f9 100644
--- a/modules/gallery/tests/Item_Model_Test.php
+++ b/modules/gallery/tests/Item_Model_Test.php
@@ -278,10 +278,10 @@ class Item_Model_Test extends Gallery_Unit_Test_Case {
 
   public function basic_validation_test() {
     $item = ORM::factory("item");
-    $item->album_cover_item_id = mt_rand();  // invalid
+    $item->album_cover_item_id = random::int();  // invalid
     $item->description = str_repeat("x", 70000);  // invalid
     $item->name = null;
-    $item->parent_id = mt_rand();
+    $item->parent_id = random::int();
     $item->slug = null;
     $item->sort_column = "bogus";
     $item->sort_order = "bogus";
@@ -411,24 +411,24 @@ class Item_Model_Test extends Gallery_Unit_Test_Case {
   public function urls_test() {
     $photo = test::random_photo();
     $this->assert_true(
-      preg_match("|http://./var/resizes/name_\d+\.jpg\?m=\d+|", $photo->resize_url()),
+      preg_match("|http://./var/resizes/name_\w+\.jpg\?m=\d+|", $photo->resize_url()),
       $photo->resize_url() . " is malformed");
     $this->assert_true(
-      preg_match("|http://./var/thumbs/name_\d+\.jpg\?m=\d+|", $photo->thumb_url()),
+      preg_match("|http://./var/thumbs/name_\w+\.jpg\?m=\d+|", $photo->thumb_url()),
       $photo->thumb_url() . " is malformed");
     $this->assert_true(
-      preg_match("|http://./var/albums/name_\d+\.jpg\?m=\d+|", $photo->file_url()),
+      preg_match("|http://./var/albums/name_\w+\.jpg\?m=\d+|", $photo->file_url()),
       $photo->file_url() . " is malformed");
 
     // Albums have special thumbnails.  Empty album has cachebuster of 0 since it has no thumbnail
     $album = test::random_album();
     $this->assert_true(
-      preg_match("|http://./var/thumbs/name_\d+/\.album\.jpg\?m=0|", $album->thumb_url()),
+      preg_match("|http://./var/thumbs/name_\w+/\.album\.jpg\?m=0|", $album->thumb_url()),
       $album->thumb_url() . " is malformed");
 
     $photo = test::random_photo($album);
     $this->assert_true(
-      preg_match("|http://./var/thumbs/name_\d+/\.album\.jpg\?m=\d+|", $album->thumb_url()),
+      preg_match("|http://./var/thumbs/name_\w+/\.album\.jpg\?m=\d+|", $album->thumb_url()),
       $album->thumb_url() . " is malformed");
   }
 
diff --git a/modules/gallery_unit_test/helpers/test.php b/modules/gallery_unit_test/helpers/test.php
index d5149492..65c7f6b4 100644
--- a/modules/gallery_unit_test/helpers/test.php
+++ b/modules/gallery_unit_test/helpers/test.php
@@ -19,7 +19,7 @@
  */
 class test_Core {
   static function random_album_unsaved($parent=null) {
-    $rand = mt_rand();
+    $rand = random::string(6);
 
     $album = ORM::factory("item");
     $album->type = "album";
@@ -34,7 +34,7 @@ class test_Core {
   }
 
   static function random_photo_unsaved($parent=null) {
-    $rand = mt_rand();
+    $rand = random::string(6);
     $photo = ORM::factory("item");
     $photo->type = "photo";
     $photo->parent_id = $parent ? $parent->id : 1;
@@ -49,16 +49,16 @@ class test_Core {
   }
 
   static function random_user($password="password") {
-    $rand = "name_" . mt_rand();
+    $rand = "name_" . random::string(6);
     return identity::create_user($rand, $rand, $password, "$rand@rand.com");
   }
 
   static function random_group() {
-    return identity::create_group((string)mt_rand());
+    return identity::create_group(random::string(6));
   }
 
   static function random_name($item=null) {
-    $rand = "name_" . mt_rand();
+    $rand = "name_" . random::string(6);
     if ($item && $item->is_photo()) {
       $rand .= ".jpg";
     }
@@ -77,7 +77,7 @@ class test_Core {
 
   static function random_tag() {
     $tag = ORM::factory("tag");
-    $tag->name = (string)mt_rand();
+    $tag->name = random::string(6);
 
     // Reload so that ORM coerces all fields into strings.
     return $tag->save()->reload();
diff --git a/modules/rest/helpers/rest.php b/modules/rest/helpers/rest.php
index 58943700..9406e209 100644
--- a/modules/rest/helpers/rest.php
+++ b/modules/rest/helpers/rest.php
@@ -104,7 +104,7 @@ class rest_Core {
 
     if (!$key->loaded()) {
       $key->user_id = identity::active_user()->id;
-      $key->access_key = md5(md5(uniqid(mt_rand(), true) . access::private_key()));
+      $key->access_key = md5(random::hash() . access::private_key());
       $key->save();
     }
 
diff --git a/modules/rest/helpers/rest_event.php b/modules/rest/helpers/rest_event.php
index 4d7a4a1b..9e241bd0 100644
--- a/modules/rest/helpers/rest_event.php
+++ b/modules/rest/helpers/rest_event.php
@@ -43,7 +43,7 @@ class rest_event {
   static function user_add_form_admin_completed($user, $form) {
     $key = ORM::factory("user_access_key");
     $key->user_id = $user->id;
-    $key->access_key = md5($user->name . time() . mt_rand());
+    $key->access_key = random::hash($user->name);
     $key->save();
   }
 
@@ -64,7 +64,7 @@ class rest_event {
 
     if (!$key->loaded()) {
       $key->user_id = $user->id;
-      $key->access_key = md5($user->name . time() . mt_rand());
+      $key->access_key = random::hash($user->name);
       $key->save();
     }
 
@@ -93,7 +93,7 @@ class rest_event {
 
     if (!$key->loaded()) {
       $key->user_id = $data->user->id;
-      $key->access_key = md5($data->user->name . time() . mt_rand());
+      $key->access_key = random::hash($data->user->name);
       $key->save();
     }
     $view->rest_key = $key->access_key;
diff --git a/modules/user/controllers/password.php b/modules/user/controllers/password.php
index 2e5eac5f..567e56dc 100644
--- a/modules/user/controllers/password.php
+++ b/modules/user/controllers/password.php
@@ -51,7 +51,7 @@ class Password_Controller extends Controller {
     $user_name = $form->reset->inputs["name"]->value;
     $user = user::lookup_by_name($user_name);
     if ($user && !empty($user->email)) {
-      $user->hash = md5(uniqid(mt_rand(), true));
+      $user->hash = random::hash();
       $user->save();
       $message = new View("reset_password.html");
       $message->confirm_url = url::abs_site("password/do_reset?key=$user->hash");
-- 
cgit v1.2.3


From 48640005a4edac955d9087f62fed1ab5f756b686 Mon Sep 17 00:00:00 2001
From: Kriss Andsten <kriss@sverok.se>
Date: Tue, 21 Dec 2010 09:03:46 +0800
Subject: Packaging + tests of Bharat's find_by_path routine.

---
 modules/gallery/helpers/item.php           | 25 +++++++++++++++-
 modules/gallery/tests/Item_Helper_Test.php | 48 ++++++++++++++++++++++++++++++
 2 files changed, 72 insertions(+), 1 deletion(-)

(limited to 'modules/gallery/tests')

diff --git a/modules/gallery/helpers/item.php b/modules/gallery/helpers/item.php
index 664da812..dbad59b9 100644
--- a/modules/gallery/helpers/item.php
+++ b/modules/gallery/helpers/item.php
@@ -208,7 +208,30 @@ class item_Core {
 
     return $model;
   }
-
+  
+  static function find_by_path($path) {
+    $path = trim($path, '/');
+    
+    // The root path name is NULL, not '', hence this workaround.
+    if ($path == '') {
+      return ORM::factory("item", 1);
+    }
+    
+    $paths = explode("/", $path);
+    $count = count($paths);
+    foreach (ORM::factory("item")
+             ->where('name', '=', $paths[$count - 1])
+             ->where('level', '=', $count + 1)
+             ->find_all() as $item) {
+      if (urldecode($item->relative_path()) == $path) {
+        return $item;
+      }
+    }
+    
+    return false;
+  }
+  
+  
   /**
    * Return the root Item_Model
    * @return Item_Model
diff --git a/modules/gallery/tests/Item_Helper_Test.php b/modules/gallery/tests/Item_Helper_Test.php
index 26db5a63..1fced654 100644
--- a/modules/gallery/tests/Item_Helper_Test.php
+++ b/modules/gallery/tests/Item_Helper_Test.php
@@ -125,4 +125,52 @@ class Item_Helper_Test extends Gallery_Unit_Test_Case {
     $this->assert_same($photo2->id, $album->album_cover_item_id);
     $this->assert_same($photo2->id, $parent->album_cover_item_id);
   }
+
+  public function find_by_path_does_the_right_thing_test() {
+    $level1 = test::random_album();
+    $level2 = test::random_album($level1);    
+    $level3 = test::random_photo($level2);
+    $level3->name = 'same.jpg';
+    $level3->save();
+    
+    $level2b = test::random_album($level1);
+    $level3b = test::random_photo($level2b);
+    $level3b->name = 'same.jpg';
+    $level3b->save();
+    
+    // Item in album
+    $this->assert_same(
+      item::find_by_path('/' . $level1->name . '/' . $level2->name . '/' . $level3->name)->id,
+      $level3->id);
+    
+    // Album, ends with a slash
+    $this->assert_same(
+      item::find_by_path($level1->name . '/' . $level2->name . '/')->id,
+      $level2->id);
+    
+    // Album, ends without a slash
+    $this->assert_same(
+      item::find_by_path('/' . $level1->name . '/' . $level2->name)->id,
+      $level2->id);
+    
+    // Return root if '' is passed
+    $this->assert_same(
+      item::find_by_path('')->id,
+      "1");
+
+    // Verify that we don't get confused by the part names
+    $this->assert_same(
+      item::find_by_path($level1->name . '/' . $level2->name . '/' . $level3->name)->id,
+      $level3->id);
+
+    $this->assert_same(
+      item::find_by_path($level1->name . '/' . $level2b->name . '/' . $level3b->name)->id,
+      $level3b->id);
+    
+    // Verify that we don't get false positives
+    $this->assert_same(
+      item::find_by_path('foo/bar/baz'),
+      false);
+
+  }
 }
-- 
cgit v1.2.3


From addd384bbdca6a9f066403c1d2919f3e863e072e Mon Sep 17 00:00:00 2001
From: Kriss Andsten <kriss@sverok.se>
Date: Wed, 22 Dec 2010 07:55:26 +0800
Subject: Minor changes to satisfy the G3 code standards.

---
 modules/gallery/helpers/item.php           | 15 ++++++++++-----
 modules/gallery/tests/Item_Helper_Test.php | 18 +++++++++---------
 2 files changed, 19 insertions(+), 14 deletions(-)

(limited to 'modules/gallery/tests')

diff --git a/modules/gallery/helpers/item.php b/modules/gallery/helpers/item.php
index dbad59b9..f38d9888 100644
--- a/modules/gallery/helpers/item.php
+++ b/modules/gallery/helpers/item.php
@@ -208,20 +208,25 @@ class item_Core {
 
     return $model;
   }
-  
+
+  /**
+   * Return an item by path.
+   * @param string $path
+   * @return object item
+   */
   static function find_by_path($path) {
-    $path = trim($path, '/');
+    $path = trim($path, "/");
     
     // The root path name is NULL, not '', hence this workaround.
     if ($path == '') {
-      return ORM::factory("item", 1);
+      return ORM::factory("item", item::root());
     }
     
     $paths = explode("/", $path);
     $count = count($paths);
     foreach (ORM::factory("item")
-             ->where('name', '=', $paths[$count - 1])
-             ->where('level', '=', $count + 1)
+             ->where("name", "=", $paths[$count - 1])
+             ->where("level", "=", $count + 1)
              ->find_all() as $item) {
       if (urldecode($item->relative_path()) == $path) {
         return $item;
diff --git a/modules/gallery/tests/Item_Helper_Test.php b/modules/gallery/tests/Item_Helper_Test.php
index 1fced654..4bc64ff0 100644
--- a/modules/gallery/tests/Item_Helper_Test.php
+++ b/modules/gallery/tests/Item_Helper_Test.php
@@ -130,46 +130,46 @@ class Item_Helper_Test extends Gallery_Unit_Test_Case {
     $level1 = test::random_album();
     $level2 = test::random_album($level1);    
     $level3 = test::random_photo($level2);
-    $level3->name = 'same.jpg';
+    $level3->name = "same.jpg";
     $level3->save();
     
     $level2b = test::random_album($level1);
     $level3b = test::random_photo($level2b);
-    $level3b->name = 'same.jpg';
+    $level3b->name = "same.jpg";
     $level3b->save();
     
     // Item in album
     $this->assert_same(
-      item::find_by_path('/' . $level1->name . '/' . $level2->name . '/' . $level3->name)->id,
+      item::find_by_path("/" . $level1->name . "/" . $level2->name . "/" . $level3->name)->id,
       $level3->id);
     
     // Album, ends with a slash
     $this->assert_same(
-      item::find_by_path($level1->name . '/' . $level2->name . '/')->id,
+      item::find_by_path($level1->name . "/" . $level2->name . "/")->id,
       $level2->id);
     
     // Album, ends without a slash
     $this->assert_same(
-      item::find_by_path('/' . $level1->name . '/' . $level2->name)->id,
+      item::find_by_path("/" . $level1->name . "/" . $level2->name)->id,
       $level2->id);
     
     // Return root if '' is passed
     $this->assert_same(
-      item::find_by_path('')->id,
+      item::find_by_path("")->id,
       "1");
 
     // Verify that we don't get confused by the part names
     $this->assert_same(
-      item::find_by_path($level1->name . '/' . $level2->name . '/' . $level3->name)->id,
+      item::find_by_path($level1->name . "/" . $level2->name . "/" . $level3->name)->id,
       $level3->id);
 
     $this->assert_same(
-      item::find_by_path($level1->name . '/' . $level2b->name . '/' . $level3b->name)->id,
+      item::find_by_path($level1->name . "/" . $level2b->name . "/" . $level3b->name)->id,
       $level3b->id);
     
     // Verify that we don't get false positives
     $this->assert_same(
-      item::find_by_path('foo/bar/baz'),
+      item::find_by_path("foo/bar/baz"),
       false);
 
   }
-- 
cgit v1.2.3


From c7096cc9ebe4b737c06629b975317aaa9806297f Mon Sep 17 00:00:00 2001
From: Bharat Mediratta <bharat@menalto.com>
Date: Tue, 21 Dec 2010 16:32:02 -0800
Subject: Fix assertion order to be (expected,actual) Change strinct
 concatenation to be variable interpolation to tighten it up.

---
 modules/gallery/tests/Item_Helper_Test.php | 47 ++++++++++++++----------------
 1 file changed, 22 insertions(+), 25 deletions(-)

(limited to 'modules/gallery/tests')

diff --git a/modules/gallery/tests/Item_Helper_Test.php b/modules/gallery/tests/Item_Helper_Test.php
index 4bc64ff0..d60380f0 100644
--- a/modules/gallery/tests/Item_Helper_Test.php
+++ b/modules/gallery/tests/Item_Helper_Test.php
@@ -126,51 +126,48 @@ class Item_Helper_Test extends Gallery_Unit_Test_Case {
     $this->assert_same($photo2->id, $parent->album_cover_item_id);
   }
 
-  public function find_by_path_does_the_right_thing_test() {
+  public function find_by_path_test() {
     $level1 = test::random_album();
-    $level2 = test::random_album($level1);    
+    $level2 = test::random_album($level1);
     $level3 = test::random_photo($level2);
     $level3->name = "same.jpg";
     $level3->save();
-    
+
     $level2b = test::random_album($level1);
     $level3b = test::random_photo($level2b);
     $level3b->name = "same.jpg";
     $level3b->save();
-    
+
     // Item in album
     $this->assert_same(
-      item::find_by_path("/" . $level1->name . "/" . $level2->name . "/" . $level3->name)->id,
-      $level3->id);
-    
+      $level3->id,
+      item::find_by_path("/{$level1->name}/{$level2->name}/{$level3->name}")->id);
+
     // Album, ends with a slash
     $this->assert_same(
-      item::find_by_path($level1->name . "/" . $level2->name . "/")->id,
-      $level2->id);
-    
+      $level2->id,
+      item::find_by_path("{$level1->name}/{$level2->name}/")->id);
+
     // Album, ends without a slash
     $this->assert_same(
-      item::find_by_path("/" . $level1->name . "/" . $level2->name)->id,
-      $level2->id);
-    
-    // Return root if '' is passed
-    $this->assert_same(
-      item::find_by_path("")->id,
-      "1");
+      $level2->id,
+      item::find_by_path("/{$level1->name}/{$level2->name}")->id);
+
+    // Return root if "" is passed
+    $this->assert_same(item::root()->id, item::find_by_path("")->id);
 
     // Verify that we don't get confused by the part names
     $this->assert_same(
-      item::find_by_path($level1->name . "/" . $level2->name . "/" . $level3->name)->id,
-      $level3->id);
+      $level3->id,
+      item::find_by_path("{$level1->name}/{$level2->name}/{$level3->name}")->id);
 
     $this->assert_same(
-      item::find_by_path($level1->name . "/" . $level2b->name . "/" . $level3b->name)->id,
-      $level3b->id);
-    
+      $level3b->id,
+      item::find_by_path("{$level1->name}/{$level2b->name}/{$level3b->name}")->id);
+
     // Verify that we don't get false positives
     $this->assert_same(
-      item::find_by_path("foo/bar/baz"),
-      false);
-
+      false,
+      item::find_by_path("foo/bar/baz"));
   }
 }
-- 
cgit v1.2.3


From f493130e59f26d41f090c5ca40e95b416b9b154b Mon Sep 17 00:00:00 2001
From: Bharat Mediratta <bharat@menalto.com>
Date: Tue, 21 Dec 2010 16:55:01 -0800
Subject: Tighten up item::find_by_path slightly.  Augment the tests to cover
 special characters in the file name ("+" is an edge case differentiator
 between rawurlencode and urlencode).

---
 modules/gallery/helpers/item.php           | 25 ++++++++++++-------------
 modules/gallery/tests/Item_Helper_Test.php |  2 +-
 2 files changed, 13 insertions(+), 14 deletions(-)

(limited to 'modules/gallery/tests')

diff --git a/modules/gallery/helpers/item.php b/modules/gallery/helpers/item.php
index f38d9888..3596a2bf 100644
--- a/modules/gallery/helpers/item.php
+++ b/modules/gallery/helpers/item.php
@@ -210,33 +210,32 @@ class item_Core {
   }
 
   /**
-   * Return an item by path.
+   * Find an item by its path.  If there's no match, return an empty Item_Model.
    * @param string $path
-   * @return object item
+   * @return object Item_Model
    */
   static function find_by_path($path) {
     $path = trim($path, "/");
-    
-    // The root path name is NULL, not '', hence this workaround.
-    if ($path == '') {
-      return ORM::factory("item", item::root());
+
+    // The root path name is NULL not "", hence this workaround.
+    if ($path == "") {
+      return item::root();
     }
-    
+
     $paths = explode("/", $path);
-    $count = count($paths);
     foreach (ORM::factory("item")
-             ->where("name", "=", $paths[$count - 1])
-             ->where("level", "=", $count + 1)
+             ->where("name", "=", end($paths))
+             ->where("level", "=", count($paths) + 1)
              ->find_all() as $item) {
       if (urldecode($item->relative_path()) == $path) {
         return $item;
       }
     }
-    
+
     return false;
   }
-  
-  
+
+
   /**
    * Return the root Item_Model
    * @return Item_Model
diff --git a/modules/gallery/tests/Item_Helper_Test.php b/modules/gallery/tests/Item_Helper_Test.php
index d60380f0..4124e453 100644
--- a/modules/gallery/tests/Item_Helper_Test.php
+++ b/modules/gallery/tests/Item_Helper_Test.php
@@ -135,7 +135,7 @@ class Item_Helper_Test extends Gallery_Unit_Test_Case {
 
     $level2b = test::random_album($level1);
     $level3b = test::random_photo($level2b);
-    $level3b->name = "same.jpg";
+    $level3b->name = "has spaces+plusses.jpg";
     $level3b->save();
 
     // Item in album
-- 
cgit v1.2.3


From 2a08cbf76da0f9984c0e182e6c448b516d8d7db3 Mon Sep 17 00:00:00 2001
From: Bharat Mediratta <bharat@menalto.com>
Date: Tue, 21 Dec 2010 16:58:54 -0800
Subject: Return an empty Item_Model when item::find_by_path fails

---
 modules/gallery/helpers/item.php           | 2 +-
 modules/gallery/tests/Item_Helper_Test.php | 5 ++---
 2 files changed, 3 insertions(+), 4 deletions(-)

(limited to 'modules/gallery/tests')

diff --git a/modules/gallery/helpers/item.php b/modules/gallery/helpers/item.php
index 3596a2bf..08a04ad0 100644
--- a/modules/gallery/helpers/item.php
+++ b/modules/gallery/helpers/item.php
@@ -232,7 +232,7 @@ class item_Core {
       }
     }
 
-    return false;
+    return new Item_Model();
   }
 
 
diff --git a/modules/gallery/tests/Item_Helper_Test.php b/modules/gallery/tests/Item_Helper_Test.php
index 4124e453..0aa7504e 100644
--- a/modules/gallery/tests/Item_Helper_Test.php
+++ b/modules/gallery/tests/Item_Helper_Test.php
@@ -166,8 +166,7 @@ class Item_Helper_Test extends Gallery_Unit_Test_Case {
       item::find_by_path("{$level1->name}/{$level2b->name}/{$level3b->name}")->id);
 
     // Verify that we don't get false positives
-    $this->assert_same(
-      false,
-      item::find_by_path("foo/bar/baz"));
+    $this->assert_false(
+      item::find_by_path("foo/bar/baz")->loaded());
   }
 }
-- 
cgit v1.2.3


From d9299f3b3f4b1a52f5b68399cfcaa96d5b367899 Mon Sep 17 00:00:00 2001
From: Bharat Mediratta <bharat@menalto.com>
Date: Tue, 21 Dec 2010 19:33:47 -0800
Subject: Change item::find_by_path() to check the relative_path_cache first,
 and only fall back the name/level comparison if there's no cached entry. 
 Update tests accordingly.

---
 modules/gallery/helpers/item.php           | 16 ++++++++++++++++
 modules/gallery/tests/Item_Helper_Test.php | 20 ++++++++++++++++----
 2 files changed, 32 insertions(+), 4 deletions(-)

(limited to 'modules/gallery/tests')

diff --git a/modules/gallery/helpers/item.php b/modules/gallery/helpers/item.php
index 08a04ad0..bac189f4 100644
--- a/modules/gallery/helpers/item.php
+++ b/modules/gallery/helpers/item.php
@@ -211,6 +211,7 @@ class item_Core {
 
   /**
    * Find an item by its path.  If there's no match, return an empty Item_Model.
+   * NOTE: the caller is responsible for performing security checks on the resulting item.
    * @param string $path
    * @return object Item_Model
    */
@@ -222,6 +223,21 @@ class item_Core {
       return item::root();
     }
 
+    // Check to see if there's an item in the database with a matching relative_path_cache value.
+    // Since that field is urlencoded, we must urlencoded the components of the path.
+    foreach (explode("/", $path) as $part) {
+      $encoded_array[] = rawurlencode($part);
+    }
+    $encoded_path = join("/", $encoded_array);
+    $item = ORM::factory("item")
+      ->where("relative_path_cache", "=", $encoded_path)
+      ->find();
+    if ($item->loaded()) {
+      return $item;
+    }
+
+    // Since the relative_path_cache field is a cache, it can be unavailable.  If we don't find
+    // anything, fall back to checking the path the hard way.
     $paths = explode("/", $path);
     foreach (ORM::factory("item")
              ->where("name", "=", end($paths))
diff --git a/modules/gallery/tests/Item_Helper_Test.php b/modules/gallery/tests/Item_Helper_Test.php
index 0aa7504e..13ecec2b 100644
--- a/modules/gallery/tests/Item_Helper_Test.php
+++ b/modules/gallery/tests/Item_Helper_Test.php
@@ -129,14 +129,21 @@ class Item_Helper_Test extends Gallery_Unit_Test_Case {
   public function find_by_path_test() {
     $level1 = test::random_album();
     $level2 = test::random_album($level1);
-    $level3 = test::random_photo($level2);
+    $level3 = test::random_photo_unsaved($level2);
     $level3->name = "same.jpg";
-    $level3->save();
+    $level3->save()->reload();
 
     $level2b = test::random_album($level1);
-    $level3b = test::random_photo($level2b);
+    $level3b = test::random_photo_unsaved($level2b);
     $level3b->name = "has spaces+plusses.jpg";
-    $level3b->save();
+    $level3b->save()->reload();
+
+    // Make sure that some of the calls below use the fallback code.
+    db::build()
+      ->update("items")
+      ->set(array("relative_url_cache" => null, "relative_path_cache" => null))
+      ->where("id", "IN", array($level3->id, $level3b->id))
+      ->execute();
 
     // Item in album
     $this->assert_same(
@@ -168,5 +175,10 @@ class Item_Helper_Test extends Gallery_Unit_Test_Case {
     // Verify that we don't get false positives
     $this->assert_false(
       item::find_by_path("foo/bar/baz")->loaded());
+
+    // Verify that the fallback code works
+    $this->assert_same(
+      $level3b->id,
+      item::find_by_path("{$level1->name}/{$level2b->name}/{$level3b->name}")->id);
   }
 }
-- 
cgit v1.2.3


From 98fd1e9957ff0d65d1bbb0eaa2df6c1e59487b25 Mon Sep 17 00:00:00 2001
From: Bharat Mediratta <bharat@menalto.com>
Date: Tue, 21 Dec 2010 20:47:07 -0800
Subject: Implement item::find_by_relative_url with tests.

---
 modules/gallery/helpers/item.php           | 26 +++++++++++
 modules/gallery/tests/Item_Helper_Test.php | 70 +++++++++++++++++++++++++-----
 2 files changed, 86 insertions(+), 10 deletions(-)

(limited to 'modules/gallery/tests')

diff --git a/modules/gallery/helpers/item.php b/modules/gallery/helpers/item.php
index bac189f4..29dd8603 100644
--- a/modules/gallery/helpers/item.php
+++ b/modules/gallery/helpers/item.php
@@ -252,6 +252,32 @@ class item_Core {
   }
 
 
+  /**
+   * Locate an item using the URL.  We assume that the url is in the form /a/b/c where each
+   * component matches up with an item slug.  If there's no match, return an empty Item_Model
+   * NOTE: the caller is responsible for performing security checks on the resulting item.
+   * @param string $url the relative url fragment
+   * @return Item_Model
+   */
+  static function find_by_relative_url($relative_url) {
+    // 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.  The fallback code will fix caches
+    // as it goes, so it'll never be run frequently.
+    $item = ORM::factory("item")->where("relative_url_cache", "=", $relative_url)->find();
+    if (!$item->loaded()) {
+      $segments = explode("/", $relative_url);
+      foreach (ORM::factory("item")
+               ->where("slug", "=", end($segments))
+               ->where("level", "=", count($segments) + 1)
+               ->find_all() as $match) {
+        if ($match->relative_url() == $relative_url) {
+          $item = $match;
+        }
+      }
+    }
+    return $item;
+  }
+
   /**
    * Return the root Item_Model
    * @return Item_Model
diff --git a/modules/gallery/tests/Item_Helper_Test.php b/modules/gallery/tests/Item_Helper_Test.php
index 13ecec2b..42acfb18 100644
--- a/modules/gallery/tests/Item_Helper_Test.php
+++ b/modules/gallery/tests/Item_Helper_Test.php
@@ -128,23 +128,19 @@ class Item_Helper_Test extends Gallery_Unit_Test_Case {
 
   public function find_by_path_test() {
     $level1 = test::random_album();
-    $level2 = test::random_album($level1);
+    $level2 = test::random_album_unsaved($level1);
+    $level2->name = "plus + space";
+    $level2->save()->reload();
+
     $level3 = test::random_photo_unsaved($level2);
     $level3->name = "same.jpg";
     $level3->save()->reload();
 
     $level2b = test::random_album($level1);
     $level3b = test::random_photo_unsaved($level2b);
-    $level3b->name = "has spaces+plusses.jpg";
+    $level3b->name = "same.jpg";
     $level3b->save()->reload();
 
-    // Make sure that some of the calls below use the fallback code.
-    db::build()
-      ->update("items")
-      ->set(array("relative_url_cache" => null, "relative_path_cache" => null))
-      ->where("id", "IN", array($level3->id, $level3b->id))
-      ->execute();
-
     // Item in album
     $this->assert_same(
       $level3->id,
@@ -163,7 +159,12 @@ class Item_Helper_Test extends Gallery_Unit_Test_Case {
     // Return root if "" is passed
     $this->assert_same(item::root()->id, item::find_by_path("")->id);
 
-    // Verify that we don't get confused by the part names
+    // Verify that we don't get confused by the part names, using the fallback code.
+    db::build()
+      ->update("items")
+      ->set(array("relative_path_cache" => null))
+      ->where("id", "IN", array($level3->id, $level3b->id))
+      ->execute();
     $this->assert_same(
       $level3->id,
       item::find_by_path("{$level1->name}/{$level2->name}/{$level3->name}")->id);
@@ -181,4 +182,53 @@ class Item_Helper_Test extends Gallery_Unit_Test_Case {
       $level3b->id,
       item::find_by_path("{$level1->name}/{$level2b->name}/{$level3b->name}")->id);
   }
+
+  public function find_by_relative_url_test() {
+    $level1 = test::random_album();
+    $level2 = test::random_album($level1);
+    $level3 = test::random_photo_unsaved($level2);
+    $level3->slug = "same";
+    $level3->save()->reload();
+
+    $level2b = test::random_album($level1);
+    $level3b = test::random_photo_unsaved($level2b);
+    $level3b->slug = "same";
+    $level3b->save()->reload();
+
+    // Item in album
+    $this->assert_same(
+      $level3->id,
+      item::find_by_relative_url("{$level1->slug}/{$level2->slug}/{$level3->slug}")->id);
+
+    // Album, ends without a slash
+    $this->assert_same(
+      $level2->id,
+      item::find_by_relative_url("{$level1->slug}/{$level2->slug}")->id);
+
+    // Return root if "" is passed
+    $this->assert_same(item::root()->id, item::find_by_relative_url("")->id);
+
+    // Verify that we don't get confused by the part slugs, using the fallback code.
+    db::build()
+      ->update("items")
+      ->set(array("relative_url_cache" => null))
+      ->where("id", "IN", array($level3->id, $level3b->id))
+      ->execute();
+    $this->assert_same(
+      $level3->id,
+      item::find_by_relative_url("{$level1->slug}/{$level2->slug}/{$level3->slug}")->id);
+
+    $this->assert_same(
+      $level3b->id,
+      item::find_by_relative_url("{$level1->slug}/{$level2b->slug}/{$level3b->slug}")->id);
+
+    // Verify that we don't get false positives
+    $this->assert_false(
+      item::find_by_relative_url("foo/bar/baz")->loaded());
+
+    // Verify that the fallback code works
+    $this->assert_same(
+      $level3b->id,
+      item::find_by_relative_url("{$level1->slug}/{$level2b->slug}/{$level3b->slug}")->id);
+  }
 }
-- 
cgit v1.2.3