5 files changed, 168 insertions, 65 deletions

diff --git a/modules/gallery/tests/Item_Model_Test.php b/modules/gallery/tests/Item_Model_Test.php
index 41361b32..fcb5c2ad 100644
--- a/modules/gallery/tests/Item_Model_Test.php
+++ b/modules/gallery/tests/Item_Model_Test.php
@@ -126,14 +126,9 @@ class Item_Model_Test extends Gallery_Unit_Test_Case {
 
   public function item_rename_wont_accept_slash_test() {
     $item = test::random_photo();
-    try {
-      $item->name = test::random_name() . "/";
-      $item->save();
-    } catch (ORM_Validation_Exception $e) {
-      $this->assert_equal(array("name" => "no_slashes"), $e->validation->errors());
-      return;
-    }
-    $this->assert_true(false, "Shouldn't get here");
+    $item->name = "/no_slashes/allowed/";
+    $item->save();
+    $this->assert_equal("no_slashes_allowed.jpg", $item->name);
   }
 
   public function move_album_test() {
@@ -328,30 +323,17 @@ class Item_Model_Test extends Gallery_Unit_Test_Case {
   }
 
   public function photo_files_must_have_an_extension_test() {
-    try {
-      $photo = test::random_photo_unsaved();
-      $photo->mime_type = "image/jpeg";
-      $photo->name = "no_extension";
-      $photo->save();
-    } catch (ORM_Validation_Exception $e) {
-      $this->assert_same(array("name" => "illegal_data_file_extension"), $e->validation->errors());
-      return;  // pass
-    }
-    $this->assert_true(false, "Shouldn't get here");
+    $photo = test::random_photo_unsaved();
+    $photo->name = "no_extension_photo";
+    $photo->save();
+    $this->assert_equal("no_extension_photo.jpg", $photo->name);
   }
 
   public function movie_files_must_have_an_extension_test() {
-    try {
-      $movie = test::random_movie_unsaved();
-      $movie->type = "movie";
-      $movie->mime_type = "video/x-flv";
-      $movie->name = "no_extension";
-      $movie->save();
-    } catch (ORM_Validation_Exception $e) {
-      $this->assert_same(array("name" => "illegal_data_file_extension"), $e->validation->errors());
-      return;  // pass
-    }
-    $this->assert_true(false, "Shouldn't get here");
+    $movie = test::random_movie_unsaved();
+    $movie->name = "no_extension_movie";
+    $movie->save();
+    $this->assert_equal("no_extension_movie.flv", $movie->name);
   }
 
   public function cant_delete_root_album_test() {
@@ -445,8 +427,21 @@ class Item_Model_Test extends Gallery_Unit_Test_Case {
       $photo->set_data_file(MODPATH . "gallery/tests/Item_Model_Test.php");
       $photo->save();
     } catch (ORM_Validation_Exception $e) {
-      $this->assert_same(array("mime_type" => "invalid", "name" => "illegal_data_file_extension"),
-                         $e->validation->errors());
+      $this->assert_same(array("name" => "invalid_data_file"), $e->validation->errors());
+      return;  // pass
+    }
+    $this->assert_true(false, "Shouldn't get here");
+  }
+
+  public function unsafe_data_file_replacement_with_valid_extension_test() {
+    $temp_file = TMPPATH . "masquerading_php.jpg";
+    copy(MODPATH . "gallery/tests/Item_Model_Test.php", $temp_file);
+    try {
+      $photo = test::random_photo();
+      $photo->set_data_file($temp_file);
+      $photo->save();
+    } catch (ORM_Validation_Exception $e) {
+      $this->assert_same(array("name" => "invalid_data_file"), $e->validation->errors());
       return;  // pass
     }
     $this->assert_true(false, "Shouldn't get here");
@@ -464,55 +459,72 @@ class Item_Model_Test extends Gallery_Unit_Test_Case {
       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_\w+/\.album\.jpg\?m=0|", $album->thumb_url()),
+      preg_match("|http://./var/thumbs/name_\w+/\.album\.jpg\?m=\d+|", $album->thumb_url()),
       $album->thumb_url() . " is malformed");
 
     $photo = test::random_photo($album);
     $this->assert_true(
       preg_match("|http://./var/thumbs/name_\w+/\.album\.jpg\?m=\d+|", $album->thumb_url()),
       $album->thumb_url() . " is malformed");
+
+    // If the file does not exist, we should return a cache buster of m=0.
+    unlink($album->thumb_path());
+    $this->assert_true(
+      preg_match("|http://./var/thumbs/name_\w+/\.album\.jpg\?m=0|", $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) {
+  public function legal_extension_that_does_match_gets_used_test() {
+    foreach (array("jpg", "JPG", "Jpg", "jpeg") as $extension) {
       $photo = test::random_photo_unsaved(item::root());
-      $photo->name = $name;
+      $photo->name = test::random_name() . ".{$extension}";
       $photo->save();
+      // Should get renamed with the correct jpg extension of the data file.
+      $this->assert_equal($extension, pathinfo($photo->name, PATHINFO_EXTENSION));
     }
   }
 
   public function illegal_extension_test() {
     foreach (array("test.php", "test.PHP", "test.php5", "test.php4",
                    "test.pl", "test.php.png") 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");
+      $photo = test::random_photo_unsaved(item::root());
+      $photo->name = $name;
+      $photo->save();
+      // Should get renamed with the correct jpg extension of the data file.
+      $this->assert_equal("jpg", pathinfo($photo->name, PATHINFO_EXTENSION));
     }
   }
 
   public function cant_rename_to_illegal_extension_test() {
     foreach (array("test.php.test", "test.php", "test.PHP",
                    "test.php5", "test.php4", "test.pl") as $name) {
-      try {
-        $photo = test::random_photo(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");
+      $photo = test::random_photo(item::root());
+      $photo->name = $name;
+      $photo->save();
+      // Should get renamed with the correct jpg extension of the data file.
+      $this->assert_equal("jpg", pathinfo($photo->name, PATHINFO_EXTENSION));
+    }
+  }
+
+  public function legal_extension_that_doesnt_match_gets_fixed_test() {
+    foreach (array("test.png", "test.mp4", "test.GIF") as $name) {
+      $photo = test::random_photo_unsaved(item::root());
+      $photo->name = $name;
+      $photo->save();
+      // Should get renamed with the correct jpg extension of the data file.
+      $this->assert_equal("jpg", pathinfo($photo->name, PATHINFO_EXTENSION));
+    }
+  }
+
+  public function rename_to_legal_extension_that_doesnt_match_gets_fixed_test() {
+    foreach (array("test.png", "test.mp4", "test.GIF") as $name) {
+      $photo = test::random_photo(item::root());
+      $photo->name = $name;
+      $photo->save();
+      // Should get renamed with the correct jpg extension of the data file.
+      $this->assert_equal("jpg", pathinfo($photo->name, PATHINFO_EXTENSION));
     }
   }
 
diff --git a/modules/gallery/tests/Legal_File_Helper_Test.php b/modules/gallery/tests/Legal_File_Helper_Test.php
index 84a29a52..7ed5214b 100644
--- a/modules/gallery/tests/Legal_File_Helper_Test.php
+++ b/modules/gallery/tests/Legal_File_Helper_Test.php
@@ -136,10 +136,62 @@ class Legal_File_Helper_Test extends Gallery_Unit_Test_Case {
   public function smash_extensions_test() {
     $this->assert_equal("foo_bar.jpg", legal_file::smash_extensions("foo.bar.jpg"));
     $this->assert_equal("foo_bar_baz.jpg", legal_file::smash_extensions("foo.bar.baz.jpg"));
-    $this->assert_equal("foo_bar_baz.jpg", legal_file::smash_extensions("foo.bar.baz.jpg"));
     $this->assert_equal("foo_bar_baz.jpg", legal_file::smash_extensions("...foo...bar..baz...jpg"));
     $this->assert_equal("/path/to/foo_bar.jpg", legal_file::smash_extensions("/path/to/foo.bar.jpg"));
     $this->assert_equal("/path/to.to/foo_bar.jpg", legal_file::smash_extensions("/path/to.to/foo.bar.jpg"));
     $this->assert_equal("foo_bar-12345678.jpg", legal_file::smash_extensions("foo.bar-12345678.jpg"));
   }
+
+  public function smash_extensions_pass_thru_names_without_extensions_test() {
+    $this->assert_equal("foo", legal_file::smash_extensions("foo"));
+    $this->assert_equal("foo.", legal_file::smash_extensions("foo."));
+    $this->assert_equal(".foo", legal_file::smash_extensions(".foo"));
+    $this->assert_equal(".", legal_file::smash_extensions("."));
+    $this->assert_equal("", legal_file::smash_extensions(""));
+    $this->assert_equal(null, legal_file::smash_extensions(null));
+  }
+
+  public function sanitize_filename_with_no_rename_test() {
+    $this->assert_equal("foo.jpeg", legal_file::sanitize_filename("foo.jpeg", "jpg", "photo"));
+    $this->assert_equal("foo.jpg", legal_file::sanitize_filename("foo.jpg", "jpeg", "photo"));
+    $this->assert_equal("foo.MP4", legal_file::sanitize_filename("foo.MP4", "mp4", "movie"));
+    $this->assert_equal("foo.mp4", legal_file::sanitize_filename("foo.mp4", "MP4", "movie"));
+  }
+
+  public function sanitize_filename_with_corrected_extension_test() {
+    $this->assert_equal("foo.jpg", legal_file::sanitize_filename("foo.png", "jpg", "photo"));
+    $this->assert_equal("foo.MP4", legal_file::sanitize_filename("foo.jpg", "MP4", "movie"));
+    $this->assert_equal("foo.jpg", legal_file::sanitize_filename("foo.php", "jpg", "photo"));
+  }
+
+  public function sanitize_filename_with_non_standard_chars_and_dots_test() {
+    $this->assert_equal("foo.jpg", legal_file::sanitize_filename("foo", "jpg", "photo"));
+    $this->assert_equal("foo.mp4", legal_file::sanitize_filename("foo.", "mp4", "movie"));
+    $this->assert_equal("foo.jpeg", legal_file::sanitize_filename(".foo.jpeg", "jpg", "photo"));
+    $this->assert_equal("foo_2013_02_10.jpeg",
+      legal_file::sanitize_filename("foo.2013/02/10.jpeg", "jpg", "photo"));
+    $this->assert_equal("foo_bar_baz.jpg",
+      legal_file::sanitize_filename("...foo...bar..baz...png", "jpg", "photo"));
+    $this->assert_equal("j'écris@un#nom_bizarre(mais quand_même_ça_passe.jpg",
+      legal_file::sanitize_filename("/j'écris@un#nom/bizarre(mais quand.même/ça_passe.\$ÇÀ@€#_", "jpg", "photo"));
+  }
+
+  public function sanitize_filename_with_no_base_name_test() {
+    $this->assert_equal("photo.jpg", legal_file::sanitize_filename(".png", "jpg", "photo"));
+    $this->assert_equal("movie.mp4", legal_file::sanitize_filename("__..__", "mp4", "movie"));
+    $this->assert_equal("photo.jpg", legal_file::sanitize_filename(".", "jpg", "photo"));
+    $this->assert_equal("movie.mp4", legal_file::sanitize_filename(null, "mp4", "movie"));
+  }
+
+  public function sanitize_filename_with_invalid_arguments_test() {
+    foreach (array("flv" => "photo", "jpg" => "movie", "php" => "photo",
+                   null => "movie", "jpg" => "album", "jpg" => null) as $extension => $type) {
+      try {
+        legal_file::sanitize_filename("foo.jpg", $extension, $type);
+        $this->assert_true(false, "Shouldn't get here");
+      } catch (Exception $e) {
+        // pass
+      }
+    }
+  }
 }
\ No newline at end of file
diff --git a/modules/gallery/tests/Movie_Helper_Test.php b/modules/gallery/tests/Movie_Helper_Test.php
index 0c262620..03fa2da9 100644
--- a/modules/gallery/tests/Movie_Helper_Test.php
+++ b/modules/gallery/tests/Movie_Helper_Test.php
@@ -64,18 +64,42 @@ class Movie_Helper_Test extends Gallery_Unit_Test_Case {
 
   public function get_file_metadata_with_no_extension_test() {
     copy(MODPATH . "gallery/tests/test.flv", TMPPATH . "test_flv_with_no_extension");
-    $this->assert_equal(array(360, 288, null, null, 6.00),
-                        movie::get_file_metadata(TMPPATH . "test_flv_with_no_extension"));
+    // Since mime type and extension are based solely on the filename, this is considered invalid.
+    try {
+      $metadata = movie::get_file_metadata(TMPPATH . "test_flv_with_no_extension");
+      $this->assert_true(false, "Shouldn't get here");
+    } catch (Exception $e) {
+      // pass
+    }
   }
 
   public function get_file_metadata_with_illegal_extension_test() {
-    $this->assert_equal(array(0, 0, null, null, 0),
-                        movie::get_file_metadata(MODPATH . "gallery/tests/Movie_Helper_Test.php"));
+    try {
+      $metadata = movie::get_file_metadata(MODPATH . "gallery/tests/Movie_Helper_Test.php");
+      $this->assert_true(false, "Shouldn't get here");
+    } catch (Exception $e) {
+      // pass
+    }
   }
 
   public function get_file_metadata_with_illegal_extension_but_valid_file_contents_test() {
     copy(MODPATH . "gallery/tests/test.flv", TMPPATH . "test_flv_with_php_extension.php");
-    $this->assert_equal(array(360, 288, null, null, 6.00),
-                        movie::get_file_metadata(TMPPATH . "test_flv_with_php_extension.php"));
+    // Since mime type and extension are based solely on the filename, this is considered invalid.
+    try {
+      $metadata = movie::get_file_metadata(TMPPATH . "test_flv_with_php_extension.php");
+      $this->assert_true(false, "Shouldn't get here");
+    } catch (Exception $e) {
+      // pass
+    }
+  }
+
+  public function get_file_metadata_with_valid_extension_but_illegal_file_contents_test() {
+    copy(MODPATH . "gallery/tests/Photo_Helper_Test.php", TMPPATH . "test_php_with_flv_extension.flv");
+    // Since mime type and extension are based solely on the filename, this is considered valid.
+    // Of course, FFmpeg cannot extract width, height, or duration from the file.  Note that this
+    // isn't a really a security problem, since the filename doesn't have a php extension and
+    // therefore will never be executed.
+    $this->assert_equal(array(0, 0, "video/x-flv", "flv", 0),
+                        movie::get_file_metadata(TMPPATH . "test_php_with_flv_extension.flv"));
   }
 }
diff --git a/modules/gallery/tests/Photo_Helper_Test.php b/modules/gallery/tests/Photo_Helper_Test.php
index 5207a6db..79b5ccfd 100644
--- a/modules/gallery/tests/Photo_Helper_Test.php
+++ b/modules/gallery/tests/Photo_Helper_Test.php
@@ -40,8 +40,12 @@ class Photo_Helper_Test extends Gallery_Unit_Test_Case {
   }
 
   public function get_file_metadata_with_illegal_extension_test() {
-    $this->assert_equal(array(0, 0, null, null),
-                        photo::get_file_metadata(MODPATH . "gallery/tests/Photo_Helper_Test.php"));
+    try {
+      $metadata = photo::get_file_metadata(MODPATH . "gallery/tests/Photo_Helper_Test.php");
+      $this->assert_true(false, "Shouldn't get here");
+    } catch (Exception $e) {
+      // pass
+    }
   }
 
   public function get_file_metadata_with_illegal_extension_but_valid_file_contents_test() {
@@ -53,4 +57,14 @@ class Photo_Helper_Test extends Gallery_Unit_Test_Case {
     $this->assert_equal(array(1024, 768, "image/jpeg", "jpg"),
                         photo::get_file_metadata(TMPPATH . "test_jpg_with_php_extension.php"));
   }
+
+  public function get_file_metadata_with_valid_extension_but_illegal_file_contents_test() {
+    copy(MODPATH . "gallery/tests/Photo_Helper_Test.php", TMPPATH . "test_php_with_jpg_extension.jpg");
+    try {
+      $metadata = photo::get_file_metadata(TMPPATH . "test_php_with_jpg_extension.jpg");
+      $this->assert_true(false, "Shouldn't get here");
+    } catch (Exception $e) {
+      // pass
+    }
+  }
 }
diff --git a/modules/gallery/tests/xss_data.txt b/modules/gallery/tests/xss_data.txt
index 51347f86..67a8b948 100644
--- a/modules/gallery/tests/xss_data.txt
+++ b/modules/gallery/tests/xss_data.txt
@@ -111,6 +111,7 @@ modules/gallery/views/admin_modules_confirm.html.php         11  DIRTY_ATTR $css
 modules/gallery/views/admin_modules_confirm.html.php         11  DIRTY    $message
 modules/gallery/views/admin_modules_confirm.html.php         16  DIRTY    access::csrf_form_field()
 modules/gallery/views/admin_modules_confirm.html.php         18  DIRTY    form::hidden($module,1)
+modules/gallery/views/admin_movies.html.php                  43  DIRTY    $form
 modules/gallery/views/admin_sidebar.html.php                 50  DIRTY    $available
 modules/gallery/views/admin_sidebar.html.php                 58  DIRTY    $active
 modules/gallery/views/admin_sidebar_blocks.html.php          4   DIRTY_ATTR $ref