#2059 - Add album name sanitizing similar to photo/movie filename sanitizing.

- added legal_file::sanitize_dirname(), analogous to sanitize_filename.
- revised item model to use new function when adding or updating an album.
- added some legal_file unit tests.
- revised some item model unit tests.

4 files changed, 153 insertions, 27 deletions

diff --git a/modules/gallery/helpers/legal_file.php b/modules/gallery/helpers/legal_file.php
index f8547011..9f02fe70 100644
--- a/modules/gallery/helpers/legal_file.php
+++ b/modules/gallery/helpers/legal_file.php
@@ -298,7 +298,7 @@ class legal_file_Core {
     $filename = str_replace("/", "_", $filename);
     $filename = str_replace("\\", "_", $filename);
 
-    // Remove extra dots from the filename.  This will also remove extraneous underscores.
+    // Remove extra dots from the filename.  Also removes extraneous and leading/trailing underscores.
     $filename = legal_file::smash_extensions($filename);
 
     // It's possible that the filename has no base (e.g. ".jpg") - if so, give it a generic one.
@@ -308,4 +308,31 @@ class legal_file_Core {
 
     return $filename;
   }
+
+  /**
+   * Sanitize a directory name for an album.  This returns a completely legal and valid
+   * directory name.
+   *
+   * @param  string $dirname (with no parent directory)
+   * @return string sanitized dirname
+   */
+  static function sanitize_dirname($dirname) {
+    // It should be a dirname without a parent directory - remove all slashes (and backslashes).
+    $dirname = str_replace("/", "_", $dirname);
+    $dirname = str_replace("\\", "_", $dirname);
+
+    // Remove extraneous and leading/trailing underscores.
+    $dirname = preg_replace("/[_]+/", "_", $dirname);
+    $dirname = trim($dirname, "_");
+
+    // Remove any trailing dots.
+    $dirname = rtrim($dirname, ".");
+
+    // It's possible that the dirname is now empty - if so, give it a generic one.
+    if (empty($dirname)) {
+      $dirname = "album";
+    }
+
+    return $dirname;
+  }
 }
diff --git a/modules/gallery/models/item.php b/modules/gallery/models/item.php
index b708c503..1d4f35da 100644
--- a/modules/gallery/models/item.php
+++ b/modules/gallery/models/item.php
@@ -365,14 +365,20 @@ class Item_Model_Core extends ORM_MPTT {
           $this->weight = item::get_max_weight();
         }
 
-        // Process the data file info.
-        if (isset($this->data_file)) {
-          $this->_process_data_file_info();
-        } else if (!$this->is_album()) {
-          // Unless it's an album, new items must have a data file.
-          $this->data_file_error = true;
+        if ($this->is_album()) {
+          // Sanitize the album name.
+          $this->name = legal_file::sanitize_dirname($this->name);
+        } else {
+          // Process the data file info.  This also sanitizes the item name.
+          if (isset($this->data_file)) {
+            $this->_process_data_file_info();
+          } else {
+            // New photos and movies must have a data file.
+            $this->data_file_error = true;
+          }
         }
 
+
         // Make an url friendly slug from the name, if necessary
         if (empty($this->slug)) {
           $this->slug = item::convert_filename_to_slug(pathinfo($this->name, PATHINFO_FILENAME));
@@ -437,6 +443,11 @@ class Item_Model_Core extends ORM_MPTT {
             pathinfo($original->name, PATHINFO_EXTENSION), $this->type);
         }
 
+        // If an album's name changed, sanitize it.
+        if ($this->is_album() && array_key_exists("name", $this->changed)) {
+          $this->name = legal_file::sanitize_dirname($this->name);
+        }
+
         // If an album's cover has changed (or been removed), delete any existing album cover,
         // reset the thumb metadata, and mark the thumb as dirty.
         if (array_key_exists("album_cover_item_id", $this->changed) && $this->is_album()) {
diff --git a/modules/gallery/tests/Item_Model_Test.php b/modules/gallery/tests/Item_Model_Test.php
index e3a4a6b7..b6849413 100644
--- a/modules/gallery/tests/Item_Model_Test.php
+++ b/modules/gallery/tests/Item_Model_Test.php
@@ -125,53 +125,123 @@ class Item_Model_Test extends Gallery_Unit_Test_Case {
   }
 
   public function photo_rename_wont_accept_slash_test() {
-    $item = test::random_photo();
+    $item = test::random_photo_unsaved();
+    $item->name = "/no_slashes/allowed/";
+    // Should fail on validate.
+    try {
+      $item->validate();
+      $this->assert_true(false, "Shouldn't get here");
+    } catch (ORM_Validation_Exception $e) {
+      $errors = $e->validation->errors();
+      $this->assert_same("no_slashes", $errors["name"]);
+    }
+    // Should be corrected on save.
+    $item->save();
+    $this->assert_equal("no_slashes_allowed.jpg", $item->name);
+    // Should be corrected on update.
     $item->name = "/no_slashes/allowed/";
     $item->save();
     $this->assert_equal("no_slashes_allowed.jpg", $item->name);
   }
 
   public function photo_rename_wont_accept_backslash_test() {
-    $item = test::random_photo();
+    $item = test::random_photo_unsaved();
+    $item->name = "\\no_backslashes\\allowed\\";
+    // Should fail on validate.
+    try {
+      $item->validate();
+      $this->assert_true(false, "Shouldn't get here");
+    } catch (ORM_Validation_Exception $e) {
+      $errors = $e->validation->errors();
+      $this->assert_same("no_backslashes", $errors["name"]);
+    }
+    // Should be corrected on save.
+    $item->save();
+    $this->assert_equal("no_backslashes_allowed.jpg", $item->name);
+    // Should be corrected on update.
     $item->name = "\\no_backslashes\\allowed\\";
     $item->save();
     $this->assert_equal("no_backslashes_allowed.jpg", $item->name);
   }
 
+  public function photo_rename_wont_accept_trailing_period_test() {
+    $item = test::random_photo_unsaved();
+    $item->name = "no_trailing_period_allowed.";
+    // Should fail on validate.
+    try {
+      $item->validate();
+      $this->assert_true(false, "Shouldn't get here");
+    } catch (ORM_Validation_Exception $e) {
+      $errors = $e->validation->errors();
+      $this->assert_same("no_trailing_period", $errors["name"]);
+    }
+    // Should be corrected on save.
+    $item->save();
+    $this->assert_equal("no_trailing_period_allowed.jpg", $item->name);
+    // Should be corrected on update.
+    $item->name = "no_trailing_period_allowed.";
+    $item->save();
+    $this->assert_equal("no_trailing_period_allowed.jpg", $item->name);
+  }
+
   public function album_rename_wont_accept_slash_test() {
+    $item = test::random_album_unsaved();
+    $item->name = "/no_album_slashes/allowed/";
+    // Should fail on validate.
     try {
-      $item = test::random_album();
-      $item->name = "/no_album_slashes/allowed/";
-      $item->save();
+      $item->validate();
+      $this->assert_true(false, "Shouldn't get here");
     } catch (ORM_Validation_Exception $e) {
-      $this->assert_same(array("name" => "no_slashes"), $e->validation->errors());
-      return;  // pass
+      $errors = $e->validation->errors();
+      $this->assert_same("no_slashes", $errors["name"]);
     }
-    $this->assert_true(false, "Shouldn't get here");
+    // Should be corrected on save.
+    $item->save();
+    $this->assert_equal("no_album_slashes_allowed", $item->name);
+    // Should be corrected on update.
+    $item->name = "/no_album_slashes/allowed/";
+    $item->save();
+    $this->assert_equal("no_album_slashes_allowed", $item->name);
   }
 
   public function album_rename_wont_accept_backslash_test() {
+    $item = test::random_album_unsaved();
+    $item->name = "\\no_album_backslashes\\allowed\\";
+    // Should fail on validate.
     try {
-      $item = test::random_album();
-      $item->name = "\\no_album_backslashes\\allowed\\";
-      $item->save();
+      $item->validate();
+      $this->assert_true(false, "Shouldn't get here");
     } catch (ORM_Validation_Exception $e) {
-      $this->assert_same(array("name" => "no_backslashes"), $e->validation->errors());
-      return;  // pass
+      $errors = $e->validation->errors();
+      $this->assert_same("no_backslashes", $errors["name"]);
     }
-    $this->assert_true(false, "Shouldn't get here");
+    // Should be corrected on save.
+    $item->save();
+    $this->assert_equal("no_album_backslashes_allowed", $item->name);
+    // Should be corrected on update.
+    $item->name = "\\no_album_backslashes\\allowed\\";
+    $item->save();
+    $this->assert_equal("no_album_backslashes_allowed", $item->name);
   }
 
   public function album_rename_wont_accept_trailing_period_test() {
+    $item = test::random_album_unsaved();
+    $item->name = ".no_trailing_period.allowed.";
+    // Should fail on validate.
     try {
-      $item = test::random_album();
-      $item->name = ".no_trailing_period.allowed.";
-      $item->save();
+      $item->validate();
+      $this->assert_true(false, "Shouldn't get here");
     } catch (ORM_Validation_Exception $e) {
-      $this->assert_same(array("name" => "no_trailing_period"), $e->validation->errors());
-      return;  // pass
+      $errors = $e->validation->errors();
+      $this->assert_same("no_trailing_period", $errors["name"]);
     }
-    $this->assert_true(false, "Shouldn't get here");
+    // Should be corrected on save.
+    $item->save();
+    $this->assert_equal(".no_trailing_period.allowed", $item->name);
+    // Should be corrected on update.
+    $item->name = ".no_trailing_period.allowed.";
+    $item->save();
+    $this->assert_equal(".no_trailing_period.allowed", $item->name);
   }
 
   public function move_album_test() {
diff --git a/modules/gallery/tests/Legal_File_Helper_Test.php b/modules/gallery/tests/Legal_File_Helper_Test.php
index 3f520131..aab41c41 100644
--- a/modules/gallery/tests/Legal_File_Helper_Test.php
+++ b/modules/gallery/tests/Legal_File_Helper_Test.php
@@ -194,4 +194,22 @@ class Legal_File_Helper_Test extends Gallery_Unit_Test_Case {
       }
     }
   }
+
+  public function sanitize_dirname_with_no_rename_test() {
+    $this->assert_equal("foo", legal_file::sanitize_dirname("foo"));
+    $this->assert_equal("foo.bar", legal_file::sanitize_dirname("foo.bar"));
+    $this->assert_equal(".foo.bar...baz", legal_file::sanitize_dirname(".foo.bar...baz"));
+    $this->assert_equal("foo bar  spaces", legal_file::sanitize_dirname("foo bar  spaces"));
+    $this->assert_equal("j'écris@un#nom_bizarre(mais quand_même_ça_passe \$ÇÀ@€",
+      legal_file::sanitize_dirname("j'écris@un#nom_bizarre(mais quand_même_ça_passe \$ÇÀ@€"));
+  }
+
+  public function sanitize_filename_with_corrections_test() {
+    $this->assert_equal("foo_bar", legal_file::sanitize_dirname("/foo/bar/"));
+    $this->assert_equal("foo_bar", legal_file::sanitize_dirname("\\foo\\bar\\"));
+    $this->assert_equal(".foo..bar", legal_file::sanitize_dirname(".foo..bar."));
+    $this->assert_equal("foo_bar", legal_file::sanitize_dirname("_foo__bar_"));
+    $this->assert_equal("album", legal_file::sanitize_dirname("_"));
+    $this->assert_equal("album", legal_file::sanitize_dirname(null));
+  }
 }
\ No newline at end of file