Properly deal with invalid images. This fixes ticket #611 which shows

a BMP masquerading as a .jpg causing us to be unable to rebuild
resizes and thumbnails.  Now if that happens, we discard the file, log
it and move on.

4 files changed, 26 insertions, 16 deletions

diff --git a/modules/gallery/controllers/simple_uploader.php b/modules/gallery/controllers/simple_uploader.php
index e7c0bd6f..156d18ac 100644
--- a/modules/gallery/controllers/simple_uploader.php
+++ b/modules/gallery/controllers/simple_uploader.php
@@ -71,7 +71,7 @@ class Simple_Uploader_Controller extends Controller {
           unlink($temp_filename);
         }
         header("HTTP/1.1 500 Internal Server Error");
-        print "ERROR:" . $e->getMessage();
+        print "ERROR: " . $e->getMessage();
         return;
       }
       unlink($temp_filename);
diff --git a/modules/gallery/helpers/photo.php b/modules/gallery/helpers/photo.php
index 5cf37de1..96a66d29 100644
--- a/modules/gallery/helpers/photo.php
+++ b/modules/gallery/helpers/photo.php
@@ -109,8 +109,12 @@ class photo_Core {
     // there's only one save() happening here.
     module::event("item_created", $photo);
 
-    // Build our thumbnail/resizes
-    graphics::generate($photo);
+    // Build our thumbnail/resizes.  If we fail to build thumbnail/resize we assume that the image
+    // is bad in some way and discard it.
+    if (!graphics::generate($photo)) {
+      $photo->delete();
+      throw new Exception("@todo BAD_IMAGE_FILE");
+    }
 
     // If the parent has no cover item, make this it.
     if (access::can("edit", $parent) && $parent->album_cover_item_id == null)  {
diff --git a/modules/gallery/views/simple_uploader.html.php b/modules/gallery/views/simple_uploader.html.php
index 38ac518c..29a0dfe8 100644
--- a/modules/gallery/views/simple_uploader.html.php
+++ b/modules/gallery/views/simple_uploader.html.php
@@ -218,7 +218,7 @@
     var fp = new File_Progress(file);
     switch (error_code) {
     case SWFUpload.UPLOAD_ERROR.HTTP_ERROR:
-      fp.set_status("error", "<?= t("Upload error: ") ?>" + message);
+      fp.set_status("error", "<?= t("Upload error: bad image file") ?>");
       break;
     case SWFUpload.UPLOAD_ERROR.UPLOAD_FAILED:
       fp.set_status("error", "<?= t("Upload failed") ?>");
diff --git a/modules/server_add/controllers/server_add.php b/modules/server_add/controllers/server_add.php
index bfb96506..26b3bd08 100644
--- a/modules/server_add/controllers/server_add.php
+++ b/modules/server_add/controllers/server_add.php
@@ -220,19 +220,25 @@ class Server_Add_Controller extends Admin_Controller {
           $album = album::create($parent, $name, $title, null, $owner_id);
           $entry->item_id = $album->id;
         } else {
-          $extension = strtolower(pathinfo($name, PATHINFO_EXTENSION));
-          if (in_array($extension, array("gif", "png", "jpg", "jpeg"))) {
-            $photo = photo::create($parent, $entry->file, $name, $title, null, $owner_id);
-            $entry->item_id = $photo->id;
-          } else if (in_array($extension, array("flv", "mp4"))) {
-            $movie = movie::create($parent, $entry->file, $name, $title, null, $owner_id);
-            $entry->item_id = $movie->id;
-          } else {
-            // This should never happen, because we don't add stuff to the list that we can't
-            // process.  But just in, case.. set this to a non-null value so that we skip this
-            // entry.
+          try {
+            $extension = strtolower(pathinfo($name, PATHINFO_EXTENSION));
+            if (in_array($extension, array("gif", "png", "jpg", "jpeg"))) {
+              $photo = photo::create($parent, $entry->file, $name, $title, null, $owner_id);
+              $entry->item_id = $photo->id;
+            } else if (in_array($extension, array("flv", "mp4"))) {
+              $movie = movie::create($parent, $entry->file, $name, $title, null, $owner_id);
+              $entry->item_id = $movie->id;
+            } else {
+              // This should never happen, because we don't add stuff to the list that we can't
+              // process.  But just in, case.. set this to a non-null value so that we skip this
+              // entry.
+              $entry->item_id = 0;
+              $task->log("Skipping unknown file type: $entry->file");
+            }
+          } catch (Exception $e) {
+            // This can happen if a photo file is invalid, like a BMP masquerading as a .jpg
             $entry->item_id = 0;
-            $task->log("Skipping unknown file type: $entry->file");
+            $task->log("Skipping invalid file: $entry->file");
           }
         }