[shotwell] Don't crash on imported photo: Bug #734986

[Jim Nelson <jnelson src gnome org>]

commit 1491418f8cddb2cecc7001a1e9f7109ee8281a8d
Author: Jim Nelson <jim yorba org>
Date:   Tue Aug 19 14:34:00 2014 -0700

    Don't crash on imported photo: Bug #734986
    
    The user's photo was identified as JFIF but is an incomplete file.
    The JFIF sniffer reported it could not identify the file, which is
    somewhat incorrect.  The RAW sniffer then tried to identify the file
    and crashed inside of libraw.
    
    Now when a sniffer can appropriately detect the file is corrupt it
    will be reported as such.  All GdkSniffers will do this if they (a)
    read the entire file and (b) no image was prepared.  Because they all
    use pre-condition checking to sanity check that the file is their
    format (via magic header bytes), this works.

 src/Photo.vala                   |   17 +++++++++++++----
 src/photos/BmpSupport.vala       |   11 +++++++----
 src/photos/GdkSupport.vala       |    7 +++++--
 src/photos/JfifSupport.vala      |    7 +++++--
 src/photos/PhotoFileFormat.vala  |    4 ++--
 src/photos/PhotoFileSniffer.vala |   20 +++++++++++++++++---
 src/photos/PngSupport.vala       |   11 +++++++----
 src/photos/RawSupport.vala       |    5 ++++-
 src/photos/TiffSupport.vala      |    7 +++++--
 9 files changed, 65 insertions(+), 24 deletions(-)
---
diff --git a/src/Photo.vala b/src/Photo.vala
index ab449dc..ac324fe 100644
--- a/src/Photo.vala
+++ b/src/Photo.vala
@@ -619,6 +619,12 @@ public abstract class Photo : PhotoSource, Dateable {
         interrogator.interrogate();
         
         DetectedPhotoInformation? detected = interrogator.get_detected_photo_information();
+        if (detected == null || interrogator.get_is_photo_corrupted()) {
+            // TODO: Probably should remove from database, but simply exiting for now (prior code
+            // didn't even do this check)
+            return;
+        }
+        
         bpr.dim = detected.image_dim;
         bpr.filesize = info.get_size();
         bpr.timestamp = timestamp.tv_sec;
@@ -1149,9 +1155,12 @@ public abstract class Photo : PhotoSource, Dateable {
             return ImportResult.DECODE_ERROR;
         }
         
+        if (interrogator.get_is_photo_corrupted())
+            return ImportResult.NOT_AN_IMAGE;
+        
         // if not detected photo information, unsupported
         DetectedPhotoInformation? detected = interrogator.get_detected_photo_information();
-        if (detected == null)
+        if (detected == null || detected.file_format == PhotoFileFormat.UNKNOWN)
             return ImportResult.UNSUPPORTED_FORMAT;
         
         // copy over supplied MD5s if provided
@@ -1261,7 +1270,7 @@ public abstract class Photo : PhotoSource, Dateable {
         try {
             interrogator.interrogate();
             DetectedPhotoInformation? detected = interrogator.get_detected_photo_information();
-            if (detected != null)
+            if (detected != null && !interrogator.get_is_photo_corrupted() && detected.file_format != 
PhotoFileFormat.UNKNOWN)
                 params.row.master.file_format = detected.file_format;
         } catch (Error err) {
             debug("Unable to interrogate photo file %s: %s", file.get_path(), err.message);
@@ -1288,7 +1297,7 @@ public abstract class Photo : PhotoSource, Dateable {
         PhotoFileInterrogator interrogator = new PhotoFileInterrogator(file, options);
         interrogator.interrogate();
         detected = interrogator.get_detected_photo_information();
-        if (detected == null) {
+        if (detected == null || interrogator.get_is_photo_corrupted()) {
             critical("Photo update: %s no longer a recognized image", to_string());
             
             return null;
@@ -2232,7 +2241,7 @@ public abstract class Photo : PhotoSource, Dateable {
         }
         
         DetectedPhotoInformation? detected = interrogator.get_detected_photo_information();
-        if (detected == null) {
+        if (detected == null || interrogator.get_is_photo_corrupted()) {
             critical("file_exif_updated: %s no longer an image", to_string());
             
             return;
diff --git a/src/photos/BmpSupport.vala b/src/photos/BmpSupport.vala
index 546bed2..dbeb64c 100644
--- a/src/photos/BmpSupport.vala
+++ b/src/photos/BmpSupport.vala
@@ -71,14 +71,17 @@ public class BmpSniffer : GdkSniffer {
         return true;
     }
 
-    public override DetectedPhotoInformation? sniff() throws Error {
+    public override DetectedPhotoInformation? sniff(out bool is_corrupted) throws Error {
+        // Rely on GdkSniffer to detect corruption
+        is_corrupted = false;
+        
         if (!is_bmp_file(file))
             return null;
-
-        DetectedPhotoInformation? detected = base.sniff();
+        
+        DetectedPhotoInformation? detected = base.sniff(out is_corrupted);
         if (detected == null)
             return null;
-
+        
         return (detected.file_format == PhotoFileFormat.BMP) ? detected : null;
     }
 }
diff --git a/src/photos/GdkSupport.vala b/src/photos/GdkSupport.vala
index 4ca0893..ed2ff63 100644
--- a/src/photos/GdkSupport.vala
+++ b/src/photos/GdkSupport.vala
@@ -34,7 +34,7 @@ public abstract class GdkSniffer : PhotoFileSniffer {
         base (file, options);
     }
     
-    public override DetectedPhotoInformation? sniff() throws Error {
+    public override DetectedPhotoInformation? sniff(out bool is_corrupted) throws Error {
         detected = new DetectedPhotoInformation();
         
         Gdk.PixbufLoader pixbuf_loader = new Gdk.PixbufLoader();
@@ -53,7 +53,7 @@ public abstract class GdkSniffer : PhotoFileSniffer {
             // no metadata detected
             detected.metadata = null;
         }
-
+        
         if (calc_md5 && detected.metadata != null) {
             uint8[]? flattened_sans_thumbnail = detected.metadata.flatten_exif(false);
             if (flattened_sans_thumbnail != null && flattened_sans_thumbnail.length > 0)
@@ -102,6 +102,9 @@ public abstract class GdkSniffer : PhotoFileSniffer {
         if (calc_md5)
             detected.md5 = md5_checksum.get_string();
         
+        // if size and area are not ready, treat as corrupted file (entire file was read)
+        is_corrupted = !size_ready || !area_prepared;
+        
         return detected;
     }
     
diff --git a/src/photos/JfifSupport.vala b/src/photos/JfifSupport.vala
index 12ac80a..d721441 100644
--- a/src/photos/JfifSupport.vala
+++ b/src/photos/JfifSupport.vala
@@ -102,11 +102,14 @@ public class JfifSniffer : GdkSniffer {
         base (file, options);
     }
     
-    public override DetectedPhotoInformation? sniff() throws Error {
+    public override DetectedPhotoInformation? sniff(out bool is_corrupted) throws Error {
+        // Rely on GdkSniffer to detect corruption
+        is_corrupted = false;
+        
         if (!Jpeg.is_jpeg(file))
             return null;
         
-        DetectedPhotoInformation? detected = base.sniff();
+        DetectedPhotoInformation? detected = base.sniff(out is_corrupted);
         if (detected == null)
             return null;
         
diff --git a/src/photos/PhotoFileFormat.vala b/src/photos/PhotoFileFormat.vala
index 926254d..2ab2f00 100644
--- a/src/photos/PhotoFileFormat.vala
+++ b/src/photos/PhotoFileFormat.vala
@@ -202,9 +202,9 @@ public enum PhotoFileFormat {
             
             case "tiff":
                 return PhotoFileFormat.TIFF;
-
+            
             case "bmp":
-                return PhotoFileFormat.BMP;                
+                return PhotoFileFormat.BMP;
             
             default:
                 return PhotoFileFormat.UNKNOWN;
diff --git a/src/photos/PhotoFileSniffer.vala b/src/photos/PhotoFileSniffer.vala
index 8bd6711..3f65ac2 100644
--- a/src/photos/PhotoFileSniffer.vala
+++ b/src/photos/PhotoFileSniffer.vala
@@ -46,7 +46,7 @@ public abstract class PhotoFileSniffer {
         calc_md5 = (options & Options.NO_MD5) == 0;
     }
     
-    public abstract DetectedPhotoInformation? sniff() throws Error;
+    public abstract DetectedPhotoInformation? sniff(out bool is_corrupted) throws Error;
 }
 
 //
@@ -62,6 +62,7 @@ public class PhotoFileInterrogator {
     private File file;
     private PhotoFileSniffer.Options options;
     private DetectedPhotoInformation? detected = null;
+    private bool is_photo_corrupted = false;
     
     public PhotoFileInterrogator(File file,
         PhotoFileSniffer.Options options = PhotoFileSniffer.Options.GET_ALL) {
@@ -75,14 +76,27 @@ public class PhotoFileInterrogator {
         return detected;
     }
     
+    // Call after interrogate().
+    public bool get_is_photo_corrupted() {
+        return is_photo_corrupted;
+    }
+    
     public void interrogate() throws Error {
         foreach (PhotoFileFormat file_format in PhotoFileFormat.get_supported()) {
             PhotoFileSniffer sniffer = file_format.create_sniffer(file, options);
-            detected = sniffer.sniff();
-            if (detected != null) {
+            
+            bool is_corrupted;
+            detected = sniffer.sniff(out is_corrupted);
+            if (detected != null && !is_corrupted) {
                 assert(detected.file_format == file_format);
                 
                 break;
+            } else if (is_corrupted) {
+                message("Sniffing halted for %s: potentially corrupted image file", file.get_path());
+                is_photo_corrupted = true;
+                detected = null;
+                
+                break;
             }
         }
     }
diff --git a/src/photos/PngSupport.vala b/src/photos/PngSupport.vala
index ffc7faa..2cde6a2 100644
--- a/src/photos/PngSupport.vala
+++ b/src/photos/PngSupport.vala
@@ -69,14 +69,17 @@ public class PngSniffer : GdkSniffer {
         return true;
     }
 
-    public override DetectedPhotoInformation? sniff() throws Error {
+    public override DetectedPhotoInformation? sniff(out bool is_corrupted) throws Error {
+        // Rely on GdkSniffer to detect corruption
+        is_corrupted = false;
+        
         if (!is_png_file(file))
             return null;
-
-        DetectedPhotoInformation? detected = base.sniff();
+        
+        DetectedPhotoInformation? detected = base.sniff(out is_corrupted);
         if (detected == null)
             return null;
-
+        
         return (detected.file_format == PhotoFileFormat.PNG) ? detected : null;
     }
 }
diff --git a/src/photos/RawSupport.vala b/src/photos/RawSupport.vala
index bad9572..98bc982 100644
--- a/src/photos/RawSupport.vala
+++ b/src/photos/RawSupport.vala
@@ -163,7 +163,10 @@ public class RawSniffer : PhotoFileSniffer {
         base (file, options);
     }
     
-    public override DetectedPhotoInformation? sniff() throws Error {
+    public override DetectedPhotoInformation? sniff(out bool is_corrupted) throws Error {
+        // this sniffer doesn't detect corrupted files
+        is_corrupted = false;
+        
         DetectedPhotoInformation detected = new DetectedPhotoInformation();
         
         GRaw.Processor processor = new GRaw.Processor();
diff --git a/src/photos/TiffSupport.vala b/src/photos/TiffSupport.vala
index decc052..ee8b087 100644
--- a/src/photos/TiffSupport.vala
+++ b/src/photos/TiffSupport.vala
@@ -104,11 +104,14 @@ private class TiffSniffer : GdkSniffer {
         base (file, options);
     }
     
-    public override DetectedPhotoInformation? sniff() throws Error {
+    public override DetectedPhotoInformation? sniff(out bool is_corrupted) throws Error {
+        // Rely on GdkSniffer to detect corruption
+        is_corrupted = false;
+        
         if (!is_tiff(file))
             return null;
         
-        DetectedPhotoInformation? detected = base.sniff();
+        DetectedPhotoInformation? detected = base.sniff(out is_corrupted);
         if (detected == null)
             return null;