Smash multiple extensions down into a single one when accepting file

uploads.  Fixes #1872.

6 files changed, 42 insertions, 2 deletions

diff --git a/modules/gallery/controllers/uploader.php b/modules/gallery/controllers/uploader.php
index 906373b6..4ea55ff6 100644
--- a/modules/gallery/controllers/uploader.php
+++ b/modules/gallery/controllers/uploader.php
@@ -63,6 +63,10 @@ class Uploader_Controller extends Controller {
         $item->parent_id = $album->id;
         $item->set_data_file($temp_filename);
 
+        // Remove double extensions from the filename - they'll be disallowed in the model but if
+        // we don't do it here then it'll result in a failed upload.
+        $item->name = legal_file::smash_extensions($item->name);
+
         $path_info = @pathinfo($temp_filename);
         if (array_key_exists("extension", $path_info) &&
             in_array(strtolower($path_info["extension"]), array("flv", "mp4", "m4v"))) {
diff --git a/modules/gallery/helpers/legal_file.php b/modules/gallery/helpers/legal_file.php
index 075de9cd..bd48d7b7 100644
--- a/modules/gallery/helpers/legal_file.php
+++ b/modules/gallery/helpers/legal_file.php
@@ -92,4 +92,20 @@ class legal_file_Core {
       return preg_replace("/\.[^\.]*?$/", ".{$new_ext}", $filename);
     }
   }
+
+  /**
+   * Reduce the given file to having a single extension.
+   */
+  static function smash_extensions($filename) {
+    $parts = pathinfo($filename);
+    $result = "";
+    if ($parts["dirname"] != ".") {
+      $result .= $parts["dirname"] . "/";
+    }
+    $parts["filename"] = str_replace(".", "_", $parts["filename"]);
+    $parts["filename"] = preg_replace("/[_]+/", "_", $parts["filename"]);
+    $parts["filename"] = trim($parts["filename"], "_");
+    $result .= "{$parts['filename']}.{$parts['extension']}";
+    return $result;
+  }
 }
diff --git a/modules/gallery/models/item.php b/modules/gallery/models/item.php
index 992af0cc..903dadad 100644
--- a/modules/gallery/models/item.php
+++ b/modules/gallery/models/item.php
@@ -797,11 +797,19 @@ class Item_Model_Core extends ORM_MPTT {
     if (strpos($this->name, "/") !== false) {
       $v->add_error("name", "no_slashes");
       return;
-    } else if (rtrim($this->name, ".") !== $this->name) {
+    }
+
+    if (rtrim($this->name, ".") !== $this->name) {
       $v->add_error("name", "no_trailing_period");
       return;
     }
 
+    // Do not accept files with double extensions, they can cause problems on some
+    // versions of Apache.
+    if (substr_count($this->name, ".") > 1) {
+      $v->add_error("name", "illegal_data_file_extension");
+    }
+
     if ($this->is_movie() || $this->is_photo()) {
       $ext = pathinfo($this->name, PATHINFO_EXTENSION);
 
diff --git a/modules/gallery/tests/Item_Model_Test.php b/modules/gallery/tests/Item_Model_Test.php
index 6d40230f..876fc137 100644
--- a/modules/gallery/tests/Item_Model_Test.php
+++ b/modules/gallery/tests/Item_Model_Test.php
@@ -490,7 +490,8 @@ class Item_Model_Test extends Gallery_Unit_Test_Case {
   }
 
   public function illegal_extension_test() {
-    foreach (array("test.php", "test.PHP", "test.php5", "test.php4", "test.pl") as $name) {
+    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;
diff --git a/modules/gallery/tests/Legal_File_Helper_Test.php b/modules/gallery/tests/Legal_File_Helper_Test.php
index 6f94c9cd..d80bcafe 100644
--- a/modules/gallery/tests/Legal_File_Helper_Test.php
+++ b/modules/gallery/tests/Legal_File_Helper_Test.php
@@ -35,4 +35,14 @@ class Legal_File_Helper_Test extends Gallery_Unit_Test_Case {
       "/website/foo.com/VID_20120513_105421.jpg",
       legal_file::change_extension("/website/foo.com/VID_20120513_105421.mp4", "jpg"));
   }
+
+  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"));
+  }
 }
\ No newline at end of file
diff --git a/modules/watermark/controllers/admin_watermarks.php b/modules/watermark/controllers/admin_watermarks.php
index 92a44a86..a80f82a9 100644
--- a/modules/watermark/controllers/admin_watermarks.php
+++ b/modules/watermark/controllers/admin_watermarks.php
@@ -98,6 +98,7 @@ class Admin_Watermarks_Controller extends Admin_Controller {
       $pathinfo = pathinfo($file);
       // Forge prefixes files with "uploadfile-xxxxxxx" for uniqueness
       $name = preg_replace("/uploadfile-[^-]+-(.*)/", '$1', $pathinfo["basename"]);
+      $name = legal_file::smash_extensions($name);
 
       if (!($image_info = getimagesize($file)) ||
           !in_array($image_info[2], array(IMAGETYPE_GIF, IMAGETYPE_JPEG, IMAGETYPE_PNG))) {