Merge pull request #209 from shadlaws/fix_2057

#2057 - Revise item name and slug validation - backslashes, refactor, error messages.

1 files changed, 29 insertions, 15 deletions

diff --git a/modules/gallery/models/item.php b/modules/gallery/models/item.php
index 1e16d307..b708c503 100644
--- a/modules/gallery/models/item.php
+++ b/modules/gallery/models/item.php
@@ -889,12 +889,17 @@ class Item_Model_Core extends ORM_MPTT {
   }
 
   /**
-   * Validate that the desired slug does not conflict.
+   * Validate the item slug.  It can return the following error messages:
+   * - not_url_safe: has illegal characters
+   * - conflict: has conflicting slug
+   * - reserved (items in root only): has same slug as a controller
    */
   public function valid_slug(Validation $v, $field) {
     if (preg_match("/[^A-Za-z0-9-_]/", $this->slug)) {
       $v->add_error("slug", "not_url_safe");
-    } else if (db::build()
+    }
+
+    if (db::build()
         ->from("items")
         ->where("parent_id", "=", $this->parent_id)
         ->where("id", "<>", $this->id)
@@ -902,11 +907,20 @@ class Item_Model_Core extends ORM_MPTT {
         ->count_records()) {
       $v->add_error("slug", "conflict");
     }
+
+    if ($this->parent_id == 1 && Kohana::auto_load("{$this->slug}_Controller")) {
+      $v->add_error("slug", "reserved");
+      return;
+    }
   }
 
   /**
-   * Validate the item name.  It can't conflict with other names, can't contain slashes or
-   * trailing periods.
+   * Validate the item name.  It can return the following error messages:
+   * - no_slashes: contains slashes
+   * - no_backslashes: contains backslashes
+   * - no_trailing_period: has a trailing period
+   * - illegal_data_file_extension (non-albums only): has double, no, or illegal extension
+   * - conflict: has conflicting name
    */
   public function valid_name(Validation $v, $field) {
     if (strpos($this->name, "/") !== false) {
@@ -914,18 +928,23 @@ class Item_Model_Core extends ORM_MPTT {
       return;
     }
 
-    if (rtrim($this->name, ".") !== $this->name) {
-      $v->add_error("name", "no_trailing_period");
+    if (strpos($this->name, "\\") !== false) {
+      $v->add_error("name", "no_backslashes");
       return;
     }
 
-    // Do not accept files with double extensions, they can cause problems on some
-    // versions of Apache.
-    if (!$this->is_album() && substr_count($this->name, ".") > 1) {
-      $v->add_error("name", "illegal_data_file_extension");
+    if (rtrim($this->name, ".") !== $this->name) {
+      $v->add_error("name", "no_trailing_period");
+      return;
     }
 
     if ($this->is_movie() || $this->is_photo()) {
+      if (substr_count($this->name, ".") > 1) {
+        // Do not accept files with double extensions, as they can
+        // cause problems on some versions of Apache.
+        $v->add_error("name", "illegal_data_file_extension");
+      }
+
       $ext = pathinfo($this->name, PATHINFO_EXTENSION);
 
       if (!$this->loaded() && !$ext) {
@@ -967,11 +986,6 @@ class Item_Model_Core extends ORM_MPTT {
         return;
       }
     }
-
-    if ($this->parent_id == 1 && Kohana::auto_load("{$this->slug}_Controller")) {
-      $v->add_error("slug", "reserved");
-      return;
-    }
   }
 
   /**