#1943 - Make legal_file::change_extension more robust.

Previously would fail with dots in the directory but no extension.  Added unit tests to verify the new change works.

2 files changed, 21 insertions, 6 deletions

diff --git a/modules/gallery/helpers/legal_file.php b/modules/gallery/helpers/legal_file.php
index e6f4cb54..e24aca36 100644
--- a/modules/gallery/helpers/legal_file.php
+++ b/modules/gallery/helpers/legal_file.php
@@ -137,15 +137,12 @@ class legal_file_Core {
   }
 
   /**
-   * Convert the extension of a filename.  If the original filename has no
+   * Change the extension of a filename.  If the original filename has no
    * extension, add the new one to the end.
    */
   static function change_extension($filename, $new_ext) {
-    if (strpos($filename, ".") === false) {
-      return "{$filename}.{$new_ext}";
-    } else {
-      return preg_replace("/\.[^\.]*?$/", ".{$new_ext}", $filename);
-    }
+    $filename_no_ext = preg_replace("/\.[^\.\/]*?$/", "", $filename);
+    return "{$filename_no_ext}.{$new_ext}";
   }
 
   /**
diff --git a/modules/gallery/tests/Legal_File_Helper_Test.php b/modules/gallery/tests/Legal_File_Helper_Test.php
index d80bcafe..d2813633 100644
--- a/modules/gallery/tests/Legal_File_Helper_Test.php
+++ b/modules/gallery/tests/Legal_File_Helper_Test.php
@@ -36,6 +36,24 @@ class Legal_File_Helper_Test extends Gallery_Unit_Test_Case {
       legal_file::change_extension("/website/foo.com/VID_20120513_105421.mp4", "jpg"));
   }
 
+  public function change_extension_path_containing_dots_and_no_extension_test() {
+    $this->assert_equal(
+      "/website/foo.com/VID_20120513_105421.jpg",
+      legal_file::change_extension("/website/foo.com/VID_20120513_105421", "jpg"));
+  }
+
+  public function change_extension_path_containing_dots_and_dot_extension_test() {
+    $this->assert_equal(
+      "/website/foo.com/VID_20120513_105421.jpg",
+      legal_file::change_extension("/website/foo.com/VID_20120513_105421.", "jpg"));
+  }
+
+  public function change_extension_path_containing_dots_and_non_standard_chars_test() {
+    $this->assert_equal(
+      "/j'écris@un#nom/bizarre(mais quand.même/ça_passe.jpg",
+      legal_file::change_extension("/j'écris@un#nom/bizarre(mais quand.même/ça_passe.\$ÇÀ@€#_", "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"));