Merge pull request #103 from shadlaws/fix_1965

#1965 - Improve sanity checks and copy/convert/process logic for rotate and resize

2 files changed, 167 insertions, 6 deletions

diff --git a/modules/gallery/helpers/gallery_graphics.php b/modules/gallery/helpers/gallery_graphics.php
index 1f0728c0..e81d5468 100644
--- a/modules/gallery/helpers/gallery_graphics.php
+++ b/modules/gallery/helpers/gallery_graphics.php
@@ -31,6 +31,15 @@ class gallery_graphics_Core {
 
     module::event("graphics_rotate", $input_file, $output_file, $options, $item);
 
+    if (@filesize($input_file) == 0) {
+      throw new Exception("@todo EMPTY_INPUT_FILE");
+    }
+
+    if (!isset($options["degrees"])) {
+      $options["degrees"] = 0;
+    }
+
+    // Rotate the image.  This also implicitly converts its format if needed.
     Image::factory($input_file)
       ->quality(module::get_var("gallery", "image_quality"))
       ->rotate($options["degrees"])
@@ -57,11 +66,26 @@ class gallery_graphics_Core {
       throw new Exception("@todo EMPTY_INPUT_FILE");
     }
 
-    $dims = getimagesize($input_file);
-    if (max($dims[0], $dims[1]) <= min($options["width"], $options["height"])) {
-      // Image would get upscaled; do nothing
-      copy($input_file, $output_file);
+    list ($input_width, $input_height, $input_mime, $input_extension) =
+      photo::get_file_metadata($input_file);
+    if ($input_width && $input_height &&
+        (empty($options["width"]) || empty($options["height"]) || empty($options["master"]) ||
+        (max($input_width, $input_height) <= min($options["width"], $options["height"])))) {
+      // Photo dimensions well-defined, but options not well-defined or would upscale the image.
+      // Do not resize.  Check mimes to see if we can copy the file or if we need to convert it.
+      // (checking mimes avoids needlessly converting jpg to jpeg, etc.)
+      $output_mime = legal_file::get_photo_types_by_extension(pathinfo($output_file, PATHINFO_EXTENSION));
+      if ($input_mime && $output_mime && ($input_mime == $output_mime)) {
+        // Mimes well-defined and identical - copy input to output
+        copy($input_file, $output_file);
+      } else {
+        // Mimes not well-defined or not the same - convert input to output
+        $image = Image::factory($input_file)
+          ->quality(module::get_var("gallery", "image_quality"))
+          ->save($output_file);
+      }
     } else {
+      // Resize the image.  This also implicitly converts its format if needed.
       $image = Image::factory($input_file)
         ->resize($options["width"], $options["height"], $options["master"])
         ->quality(module::get_var("gallery", "image_quality"));
@@ -96,8 +120,8 @@ class gallery_graphics_Core {
 
       module::event("graphics_composite", $input_file, $output_file, $options, $item);
 
-      list ($width, $height) = getimagesize($input_file);
-      list ($w_width, $w_height) = getimagesize($options["file"]);
+      list ($width, $height) = photo::get_file_metadata($input_file);
+      list ($w_width, $w_height) = photo::get_file_metadata($options["file"]);
 
       $pad = isset($options["padding"]) ? $options["padding"] : 10;
       $top = $pad;
diff --git a/modules/gallery/tests/Gallery_Graphics_Helper_Test.php b/modules/gallery/tests/Gallery_Graphics_Helper_Test.php
new file mode 100644
index 00000000..20096b23
--- /dev/null
+++ b/modules/gallery/tests/Gallery_Graphics_Helper_Test.php
@@ -0,0 +1,137 @@
+<?php defined("SYSPATH") or die("No direct script access.");
+/**
+ * Gallery - a web based photo album viewer and editor
+ * Copyright (C) 2000-2013 Bharat Mediratta
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or (at
+ * your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street - Fifth Floor, Boston, MA  02110-1301, USA.
+ */
+class Gallery_Graphics_Helper_Test extends Gallery_Unit_Test_Case {
+  public function rotate_jpg_test() {
+    // Input is a 1024x768 jpg, output is rotated 90 degrees
+    $input_file = MODPATH . "gallery/tests/test.jpg";
+    $output_file = TMPPATH . test::random_name() . ".jpg";
+    $options = array("degrees" => 90);
+    gallery_graphics::rotate($input_file, $output_file, $options, null);
+
+    // Output is rotated to 768x1024 jpg
+    $this->assert_equal(array(768, 1024, "image/jpeg", "jpg"), photo::get_file_metadata($output_file));
+  }
+
+  public function rotate_jpg_without_options_test() {
+    // Input is a 1024x768 jpg, output options undefined
+    $input_file = MODPATH . "gallery/tests/test.jpg";
+    $output_file = TMPPATH . test::random_name() . ".jpg";
+    gallery_graphics::rotate($input_file, $output_file, null, null);
+
+    // Output is not rotated, still a 1024x768 jpg
+    $this->assert_equal(array(1024, 768, "image/jpeg", "jpg"), photo::get_file_metadata($output_file));
+  }
+
+  public function rotate_bad_jpg_test() {
+    // Input is a garbled jpg, output is jpg autofit to 300x300
+    $input_file = TMPPATH . test::random_name() . ".jpg";
+    $output_file = TMPPATH . test::random_name() . ".jpg";
+    $options = array("degrees" => 90);
+    file_put_contents($input_file, test::lorem_ipsum(200));
+
+    // Should get passed to Image library and throw an exception
+    try {
+      gallery_graphics::rotate($input_file, $output_file, $options, null);
+      $this->assert_true(false, "Shouldn't get here");
+    } catch (Exception $e) {
+      // pass
+    }
+  }
+
+  public function resize_jpg_test() {
+    // Input is a 1024x768 jpg, output is jpg autofit to 300x300
+    $input_file = MODPATH . "gallery/tests/test.jpg";
+    $output_file = TMPPATH . test::random_name() . ".jpg";
+    $options = array("width" => 300, "height" => 300, "master" => Image::AUTO);
+    gallery_graphics::resize($input_file, $output_file, $options, null);
+
+    // Output is resized to 300x225 jpg
+    $this->assert_equal(array(300, 225, "image/jpeg", "jpg"), photo::get_file_metadata($output_file));
+  }
+
+  public function resize_jpg_to_png_test() {
+    // Input is a 1024x768 jpg, output is png autofit to 300x300
+    $input_file = MODPATH . "gallery/tests/test.jpg";
+    $output_file = TMPPATH . test::random_name() . ".png";
+    $options = array("width" => 300, "height" => 300, "master" => Image::AUTO);
+    gallery_graphics::resize($input_file, $output_file, $options, null);
+
+    // Output is resized to 300x225 png
+    $this->assert_equal(array(300, 225, "image/png", "png"), photo::get_file_metadata($output_file));
+  }
+
+  public function resize_jpg_with_no_upscale_test() {
+    // Input is a 1024x768 jpg, output is jpg autofit to 1200x1200 - should not upscale
+    $input_file = MODPATH . "gallery/tests/test.jpg";
+    $output_file = TMPPATH . test::random_name() . ".jpg";
+    $options = array("width" => 1200, "height" => 1200, "master" => Image::AUTO);
+    gallery_graphics::resize($input_file, $output_file, $options, null);
+
+    // Output is copied directly from input
+    $this->assert_equal(file_get_contents($input_file), file_get_contents($output_file));
+  }
+
+  public function resize_jpg_to_png_with_no_upscale_test() {
+    // Input is a 1024x768 jpg, output is png autofit to 1200x1200 - should not upscale
+    $input_file = MODPATH . "gallery/tests/test.jpg";
+    $output_file = TMPPATH . test::random_name() . ".png";
+    $options = array("width" => 1200, "height" => 1200, "master" => Image::AUTO);
+    gallery_graphics::resize($input_file, $output_file, $options, null);
+
+    // Output is converted from input without resize
+    $this->assert_equal(array(1024, 768, "image/png", "png"), photo::get_file_metadata($output_file));
+  }
+
+  public function resize_jpg_without_options_test() {
+    // Input is a 1024x768 jpg, output is jpg without options - should not attempt resize
+    $input_file = MODPATH . "gallery/tests/test.jpg";
+    $output_file = TMPPATH . test::random_name() . ".jpg";
+    gallery_graphics::resize($input_file, $output_file, null, null);
+
+    // Output is copied directly from input
+    $this->assert_equal(file_get_contents($input_file), file_get_contents($output_file));
+  }
+
+  public function resize_jpg_to_png_without_options_test() {
+    // Input is a 1024x768 jpg, output is png without options - should not attempt resize
+    $input_file = MODPATH . "gallery/tests/test.jpg";
+    $output_file = TMPPATH . test::random_name() . ".png";
+    gallery_graphics::resize($input_file, $output_file, null, null);
+
+    // Output is converted from input without resize
+    $this->assert_equal(array(1024, 768, "image/png", "png"), photo::get_file_metadata($output_file));
+  }
+
+  public function resize_bad_jpg_test() {
+    // Input is a garbled jpg, output is jpg autofit to 300x300
+    $input_file = TMPPATH . test::random_name() . ".jpg";
+    $output_file = TMPPATH . test::random_name() . ".jpg";
+    $options = array("width" => 300, "height" => 300, "master" => Image::AUTO);
+    file_put_contents($input_file, test::lorem_ipsum(200));
+
+    // Should get passed to Image library and throw an exception
+    try {
+      gallery_graphics::resize($input_file, $output_file, $options, null);
+      $this->assert_true(false, "Shouldn't get here");
+    } catch (Exception $e) {
+      // pass
+    }
+  }
+}
\ No newline at end of file