#2001 - Make filename sanitizing more consistent.

- legal_file - added sanitize_filname() to sanitize photo/movie filenames.
- admin_watermarks - revised add() to use new function.
- item model - added _process_data_file_info() to validate the data file, get its metadata, and sanitize the item name.
- item model - revised save() for new items to use _process_data_file_info *before* the slug is checked.
- item model - revised save() for updated items to use _process_data_file_info.
- item model - revised save() for updated items to sanitize name if changed.
- uploader - removed call to smash_extensions (item model does this when it calls sanitize_filename).
- Legal_File_Helper_Test - added unit tests for sanitize_filename.
- Item_Model_Test - revised existing unit tests based on changes.
- Item_Model_Test - added new unit tests for names with legal but incorrect extensions.
- Averted take over by HAL with fix #2001...

1 files changed, 50 insertions, 45 deletions

diff --git a/modules/gallery/models/item.php b/modules/gallery/models/item.php
index 33b8a89d..43b9a292 100644
--- a/modules/gallery/models/item.php
+++ b/modules/gallery/models/item.php
@@ -365,6 +365,14 @@ 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;
+        }
+
         // 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));
@@ -377,31 +385,6 @@ class Item_Model_Core extends ORM_MPTT {
           }
         }
 
-        // Get the width, height and mime type from our data file for photos and movies.
-        if ($this->is_photo() || $this->is_movie()) {
-          try {
-            if ($this->is_photo()) {
-              list ($this->width, $this->height, $this->mime_type, $extension) =
-                photo::get_file_metadata($this->data_file);
-            } else if ($this->is_movie()) {
-              list ($this->width, $this->height, $this->mime_type, $extension) =
-                movie::get_file_metadata($this->data_file);
-            }
-
-            // Force an extension onto the name if necessary
-            $pi = pathinfo($this->data_file);
-            if (empty($pi["extension"])) {
-              $this->name = "{$this->name}.$extension";
-            }
-
-            // Data file valid - make sure the flag is reset to false.
-            $this->data_file_error = false;
-          } catch (Exception $e) {
-            // Data file invalid - set the flag so it's reported during item validation.
-            $this->data_file_error = true;
-          }
-        }
-
         $this->_check_and_fix_conflicts();
 
         parent::save();
@@ -439,31 +422,19 @@ class Item_Model_Core extends ORM_MPTT {
         // keep it around.
         $original = ORM::factory("item", $this->id);
 
-        // Preserve the extension of the data file. Many helpers, (e.g. ImageMagick), assume
+        // If we have a new data file, process its info.  This will get its metadata and
+        // preserve the extension of the data file. Many helpers, (e.g. ImageMagick), assume
         // the MIME type from the extension. So when we adopt the new data file, it's important
         // to adopt the new extension. That ensures that the item's extension is always
         // appropriate for its data. We don't try to preserve the name of the data file, though,
         // because the name is typically a temporary randomly-generated name.
         if (isset($this->data_file)) {
-          try {
-            $extension = pathinfo($this->data_file, PATHINFO_EXTENSION);
-            $new_name = pathinfo($this->name, PATHINFO_FILENAME) . ".$extension";
-            if (!empty($extension) && strcmp($this->name, $new_name)) {
-              $this->name = $new_name;
-            }
-            if ($this->is_photo()) {
-              list ($this->width, $this->height, $this->mime_type, $extension) =
-                photo::get_file_metadata($this->data_file);
-            } else if ($this->is_movie()) {
-              list ($this->width, $this->height, $this->mime_type, $extension) =
-                movie::get_file_metadata($this->data_file);
-            }
-            // Data file valid - make sure the flag is reset to false.
-            $this->data_file_error = false;
-          } catch (Exception $e) {
-            // Data file invalid - set the flag so it's reported during item validation.
-            $this->data_file_error = true;
-          }
+          $this->_process_data_file_info();
+        } else if (!$this->is_album() && array_key_exists("name", $this->changed)) {
+          // There's no new data file, but the name changed.  If it's a photo or movie,
+          // make sure the new name still agrees with the file type.
+          $this->name = legal_file::sanitize_filename($this->name,
+            pathinfo($original->name, PATHINFO_EXTENSION), $this->type);
         }
 
         // If an album's cover has changed (or been removed), delete any existing album cover,
@@ -625,6 +596,40 @@ class Item_Model_Core extends ORM_MPTT {
   }
 
   /**
+   * Process the data file info.  Get its metadata and extension.
+   * If valid, use it to sanitize the item name and update the
+   * width, height, and mime type.
+   */
+  private function _process_data_file_info() {
+    try {
+      if ($this->is_photo()) {
+        list ($this->width, $this->height, $this->mime_type, $extension) =
+          photo::get_file_metadata($this->data_file);
+      } else if ($this->is_movie()) {
+        list ($this->width, $this->height, $this->mime_type, $extension) =
+          movie::get_file_metadata($this->data_file);
+      } else {
+        // Albums don't have data files.
+        $this->data_file = null;
+        return;
+      }
+
+      // Sanitize the name based on the idenified extension, but only set $this->name if different
+      // to ensure it isn't unnecessarily marked as "changed"
+      $name = legal_file::sanitize_filename($this->name, $extension, $this->type);
+      if ($this->name != $name) {
+        $this->name = $name;
+      }
+
+      // Data file valid - make sure the flag is reset to false.
+      $this->data_file_error = false;
+    } catch (Exception $e) {
+      // Data file invalid - set the flag so it's reported during item validation.
+      $this->data_file_error = true;
+    }
+  }
+
+  /**
    * Return the Item_Model representing the cover for this album.
    * @return Item_Model or null if there's no cover
    */