#1994 - Make get_file_metadata throw an exception if photo or movie is unidentifiable/illegal.

- photo & movie helpers: modified to throw exceptions when file is known to be unidentifiable/illegal.
- item model: revised to work with exceptions and be more explicit when the data file is invalid.
- item model: removed duplicate get_file_metadata call for updated items.
- admin_watermarks controller: revised to work with exceptions (really cleans up logic here).
- graphics helper: revised to handle invalid placeholders (a nearly-impossible corner case, but still...).
- photo & movie helper tests: revised to work with exceptions, added new tests for illegal files with valid extensions.
- item model tests: revised to work with exceptions, added new tests for illegal files with valid extensions.

4 files changed, 32 insertions, 3 deletions

diff --git a/modules/gallery/helpers/gallery_graphics.php b/modules/gallery/helpers/gallery_graphics.php
index b78bd9a7..eb76353f 100644
--- a/modules/gallery/helpers/gallery_graphics.php
+++ b/modules/gallery/helpers/gallery_graphics.php
@@ -172,6 +172,11 @@ class gallery_graphics_Core {
 
       module::event("graphics_composite_completed", $input_file, $output_file, $options, $item);
     } catch (ErrorException $e) {
+      // Unlike rotate and resize, composite catches its exceptions here.  This is because
+      // composite is typically called for watermarks.  If during thumb/resize generation
+      // the watermark fails, we'd still like the image resized, just without its watermark.
+      // If the exception isn't caught here, graphics::generate will replace it with a
+      // placeholder.
       Kohana_Log::add("error", $e->getMessage());
     }
   }
diff --git a/modules/gallery/helpers/graphics.php b/modules/gallery/helpers/graphics.php
index 4df57fba..e34af018 100644
--- a/modules/gallery/helpers/graphics.php
+++ b/modules/gallery/helpers/graphics.php
@@ -224,7 +224,16 @@ class graphics_Core {
         graphics::_replace_image_with_placeholder($item, "resize");
       }
       graphics::_replace_image_with_placeholder($item, "thumb");
-      graphics::_update_item_dimensions($item);
+      try {
+        graphics::_update_item_dimensions($item);
+      } catch (Exception $e) {
+        // Looks like get_file_metadata couldn't identify our placeholders.  We should never get
+        // here, but in the odd case we do, we need to do something.  Let's put in hardcoded values.
+        if ($item->is_photo()) {
+          list ($item->resize_width, $item->resize_height) = array(200, 200);
+        }
+        list ($item->thumb_width, $item->thumb_height) = array(200, 200);
+      }
       $item->save();
       throw $e;
     }
diff --git a/modules/gallery/helpers/movie.php b/modules/gallery/helpers/movie.php
index 6844771b..d4b907a2 100644
--- a/modules/gallery/helpers/movie.php
+++ b/modules/gallery/helpers/movie.php
@@ -192,8 +192,16 @@ class movie_Core {
       $metadata->extension = strtolower($extension);
     }
 
-    // Run movie_get_file_metadata events which can modify the class, then return results.
+    // Run movie_get_file_metadata events which can modify the class.
     module::event("movie_get_file_metadata", $file_path, $metadata);
+
+    // If the post-events results are invalid, throw an exception.  Note that, unlike photos, having
+    // zero width and height isn't considered invalid (as is the case when FFmpeg isn't installed).
+    if (!$metadata->mime_type || !$metadata->extension ||
+        ($metadata->mime_type != legal_file::get_movie_types_by_extension($metadata->extension))) {
+      throw new Exception("@todo ILLEGAL_OR_UNINDENTIFIABLE_FILE");
+    }
+
     return array($metadata->width, $metadata->height, $metadata->mime_type,
                  $metadata->extension, $metadata->duration);
   }
diff --git a/modules/gallery/helpers/photo.php b/modules/gallery/helpers/photo.php
index 51e51507..2d32f0d3 100644
--- a/modules/gallery/helpers/photo.php
+++ b/modules/gallery/helpers/photo.php
@@ -133,8 +133,15 @@ class photo_Core {
       $metadata->height = 0;
     }
 
-    // Run photo_get_file_metadata events which can modify the class, then return results.
+    // Run photo_get_file_metadata events which can modify the class.
     module::event("photo_get_file_metadata", $file_path, $metadata);
+
+    // If the post-events results are invalid, throw an exception.
+    if (!$metadata->width || !$metadata->height || !$metadata->mime_type || !$metadata->extension ||
+        ($metadata->mime_type != legal_file::get_photo_types_by_extension($metadata->extension))) {
+      throw new Exception("@todo ILLEGAL_OR_UNINDENTIFIABLE_FILE");
+    }
+
     return array($metadata->width, $metadata->height, $metadata->mime_type, $metadata->extension);
   }
 }