Show attachments lacking a Content-Disposition: Bug #713830

Attachments without Content-Disposition are now generated and shown
in the client.  This requires a database upgrade as well as rescanning
all messages to generate the previously missing attachments.

In addition, this upgrade now stores the attachments' Content-ID in
the database.  This makes it much easier for the client to associate
a particular MIME section in the RFC822 message with an attachment in
the database and on disk.

sql/version-022.sql

@@ -0,0 +1,10 @@
+--
+-- Database upgrade to repopulate attachments.  Bug #713830 revealed that
+-- non-text and non-image files with no Content-Disposition were being dropped.
+-- Also add Content-ID to database so attachments in RCF822 messages can be paired
+-- to extracted attachments on filesystem.
+--
+
+ALTER TABLE MessageAttachmentTable ADD COLUMN content_id TEXT DEFAULT NULL;
+ALTER TABLE MessageAttachmentTable ADD COLUMN description TEXT DEFAULT NULL;
+

src/client/conversation-viewer/conversation-viewer.vala

@@ -151,6 +151,7 @@ public class ConversationViewer : Gtk.Box {
     private Geary.State.Machine fsm;
     private DisplayMode display_mode = DisplayMode.NONE;
     private uint select_conversation_timeout_id = 0;
+    private Gee.HashSet<string> inlined_content_ids = new Gee.HashSet<string>();
     
     public ConversationViewer() {
         Object(orientation: Gtk.Orientation.VERTICAL, spacing: 0);
@@ -223,6 +224,7 @@ public class ConversationViewer : Gtk.Box {
         }
         email_to_element.clear();
         messages.clear();
+        inlined_content_ids.clear();
         
         current_account_information = account_information;
     }
@@ -1728,6 +1730,10 @@ public class ConversationViewer : Gtk.Box {
                     img.set_attribute("class", DATA_IMAGE_CLASS);
                     if (!Geary.String.is_empty(filename))
                         img.set_attribute("alt", filename);
+                    
+                    // stash here so inlined image isn't listed as attachment (esp. if it has no
+                    // Content-Disposition)
+                    inlined_content_ids.add(mime_id);
                 } else if (!src.has_prefix("data:")) {
                     remote_images = true;
                 }
@@ -1854,7 +1860,11 @@ public class ConversationViewer : Gtk.Box {
         header_text += create_header_row(Geary.HTML.escape_markup(title), value, important);
     }
     
-    private static bool should_show_attachment(Geary.Attachment attachment) {
+    private bool should_show_attachment(Geary.Attachment attachment) {
+        // if displayed inline, don't include in attachment list
+        if (attachment.content_id in inlined_content_ids)
+            return false;
+        
         switch (attachment.content_disposition.disposition_type) {
             case Geary.Mime.DispositionType.ATTACHMENT:
                 return true;
@@ -1867,7 +1877,7 @@ public class ConversationViewer : Gtk.Box {
         }
     }
     
-    private static int displayed_attachments(Geary.Email email) {
+    private int displayed_attachments(Geary.Email email) {
         int ret = 0;
         foreach (Geary.Attachment attachment in email.attachments) {
             if (should_show_attachment(attachment)) {

src/engine/api/geary-attachment.vala

@@ -49,14 +49,31 @@ public abstract class Geary.Attachment : BaseObject {
      */
     public Mime.ContentDisposition content_disposition { get; private set; }
     
+    /**
+     * The Content-ID of the attachment.
+     *
+     * See [[https://tools.ietf.org/html/rfc2111]]
+     */
+    public string? content_id { get; private set; }
+    
+    /**
+     * The Content-Description of the attachment.
+     *
+     * See [[https://tools.ietf.org/html/rfc2045#section-8]]
+     */
+    public string? content_description { get; private set; }
+    
     protected Attachment(string id, File file, bool has_supplied_filename, Mime.ContentType content_type,
-        int64 filesize, Mime.ContentDisposition content_disposition) {
+        int64 filesize, Mime.ContentDisposition content_disposition, string? content_id,
+        string? content_description) {
         this.id = id;
         this.file = file;
         this.has_supplied_filename = has_supplied_filename;
         this.content_type = content_type;
         this.filesize = filesize;
         this.content_disposition = content_disposition;
+        this.content_id = content_id;
+        this.content_description = content_description;
     }
 }
 

src/engine/imap-db/imap-db-attachment.vala

@@ -10,9 +10,11 @@ private class Geary.ImapDB.Attachment : Geary.Attachment {
     private const string ATTACHMENTS_DIR = "attachments";
     
     protected Attachment(File data_dir, string? filename, Mime.ContentType content_type, int64 filesize,
-        int64 message_id, int64 attachment_id, Mime.ContentDisposition content_disposition) {
+        int64 message_id, int64 attachment_id, Mime.ContentDisposition content_disposition,
+        string? content_id, string? content_description) {
         base (generate_id(attachment_id),generate_file(data_dir, message_id, attachment_id, filename),
-            !String.is_empty(filename), content_type, filesize, content_disposition);
+            !String.is_empty(filename), content_type, filesize, content_disposition, content_id,
+            content_description);
     }
     
     private static string generate_id(int64 attachment_id) {

src/engine/imap-db/imap-db-database.vala

@@ -105,6 +105,10 @@ private class Geary.ImapDB.Database : Geary.Db.VersionedDatabase {
             case 19:
                 post_upgrade_validate_contacts();
             break;
+            
+            case 22:
+                post_rebuild_attachments();
+            break;
         }
     }
     
@@ -402,6 +406,71 @@ private class Geary.ImapDB.Database : Geary.Db.VersionedDatabase {
         }
     }
     
+    // Version 22
+    private void post_rebuild_attachments() {
+        try {
+            exec_transaction(Db.TransactionType.RW, (cx) => {
+                Db.Statement stmt = cx.prepare("""
+                    SELECT id, header, body
+                    FROM MessageTable
+                    WHERE (fields & ?) = ?
+                    """);
+                stmt.bind_int(0, Geary.Email.REQUIRED_FOR_MESSAGE);
+                stmt.bind_int(1, Geary.Email.REQUIRED_FOR_MESSAGE);
+                
+                Db.Result results = stmt.exec();
+                if (results.finished)
+                    return Db.TransactionOutcome.ROLLBACK;
+                
+                do {
+                    int64 message_id = results.rowid_at(0);
+                    Geary.Memory.Buffer header = results.string_buffer_at(1);
+                    Geary.Memory.Buffer body = results.string_buffer_at(2);
+                    
+                    Geary.RFC822.Message message;
+                    try {
+                        message = new Geary.RFC822.Message.from_parts(
+                            new RFC822.Header(header), new RFC822.Text(body));
+                    } catch (Error err) {
+                        debug("Error decoding message: %s", err.message);
+                        
+                        continue;
+                    }
+                    
+                    // build a list of attachments in the message itself
+                    Gee.List<GMime.Part> msg_attachments = message.get_attachments();
+                    
+                    // delete all attachments for this message
+                    try {
+                        Geary.ImapDB.Folder.do_delete_attachments(cx, message_id);
+                    } catch (Error err) {
+                        debug("Error deleting existing attachments: %s", err.message);
+                        
+                        continue;
+                    }
+                    
+                    // rebuild all
+                    try {
+                        Geary.ImapDB.Folder.do_save_attachments_db(cx, message_id, msg_attachments,
+                            this, null);
+                    } catch (Error err) {
+                        debug("Error saving attachments: %s", err.message);
+                        
+                        // fallthrough
+                    }
+                } while (results.next());
+                
+                // rebuild search table due to potentially new attachments
+                cx.exec("DELETE FROM MessageSearchTable");
+                
+                return Db.TransactionOutcome.COMMIT;
+            });
+        } catch (Error e) {
+            debug("Error populating old inline attachments during upgrade to database schema 13: %s",
+                e.message);
+        }
+    }
+    
     private void on_prepare_database_connection(Db.Connection cx) throws Error {
         cx.set_busy_timeout_msec(Db.Connection.RECOMMENDED_BUSY_TIMEOUT_MSEC);
         cx.set_foreign_keys(true);

src/engine/imap-db/imap-db-folder.vala

@@ -1874,7 +1874,7 @@ private class Geary.ImapDB.Folder : BaseObject, Geary.ReferenceSemantics {
     private static Gee.List<Geary.Attachment>? do_list_attachments(Db.Connection cx, int64 message_id,
         Cancellable? cancellable) throws Error {
         Db.Statement stmt = cx.prepare("""
-            SELECT id, filename, mime_type, filesize, disposition
+            SELECT id, filename, mime_type, filesize, disposition, content_id, description
             FROM MessageAttachmentTable
             WHERE message_id = ?
             ORDER BY id
@@ -1891,7 +1891,8 @@ private class Geary.ImapDB.Folder : BaseObject, Geary.ReferenceSemantics {
                 Mime.DispositionType.from_int(results.int_at(4)));
             list.add(new ImapDB.Attachment(cx.database.db_file.get_parent(), results.string_at(1),
                 Mime.ContentType.deserialize(results.nonnull_string_at(2)), results.int64_at(3),
-                message_id, results.rowid_at(0), disposition));
+                message_id, results.rowid_at(0), disposition, results.string_at(5),
+                results.string_at(6)));
         } while (results.next(cancellable));
         
         return list;
@@ -1909,12 +1910,17 @@ private class Geary.ImapDB.Folder : BaseObject, Geary.ReferenceSemantics {
             return;
         
         foreach (GMime.Part attachment in attachments) {
-            string mime_type = attachment.get_content_type().to_string();
+            GMime.ContentType? content_type = attachment.get_content_type();
-            string disposition = attachment.get_disposition();
+            string mime_type = (content_type != null)
+                ? content_type.to_string()
+                : Mime.ContentType.DEFAULT_CONTENT_TYPE;
+            string? disposition = attachment.get_disposition();
+            string? content_id = attachment.get_content_id();
+            string? description = attachment.get_content_description();
             string filename = RFC822.Utils.get_clean_attachment_filename(attachment);
             
             // Convert the attachment content into a usable ByteArray.
-            GMime.DataWrapper attachment_data = attachment.get_content_object();
+            GMime.DataWrapper? attachment_data = attachment.get_content_object();
             ByteArray byte_array = new ByteArray();
             GMime.StreamMem stream = new GMime.StreamMem.with_byte_array(byte_array);
             stream.set_owner(false);
@@ -1932,14 +1938,16 @@ private class Geary.ImapDB.Folder : BaseObject, Geary.ReferenceSemantics {
             
             // Insert it into the database.
             Db.Statement stmt = cx.prepare("""
-                INSERT INTO MessageAttachmentTable (message_id, filename, mime_type, filesize, disposition)
+                INSERT INTO MessageAttachmentTable (message_id, filename, mime_type, filesize, disposition, content_id, description)
-                VALUES (?, ?, ?, ?, ?)
+                VALUES (?, ?, ?, ?, ?, ?, ?)
                 """);
             stmt.bind_rowid(0, message_id);
             stmt.bind_string(1, filename);
             stmt.bind_string(2, mime_type);
             stmt.bind_uint(3, filesize);
             stmt.bind_int(4, disposition_type);
+            stmt.bind_string(5, content_id);
+            stmt.bind_string(6, description);
             
             int64 attachment_id = stmt.exec_insert(cancellable);
             
@@ -2001,6 +2009,30 @@ private class Geary.ImapDB.Folder : BaseObject, Geary.ReferenceSemantics {
         }
     }
     
+    public static void do_delete_attachments(Db.Connection cx, int64 message_id)
+        throws Error {
+        Gee.List<Geary.Attachment>? attachments = do_list_attachments(cx, message_id, null);
+        if (attachments == null || attachments.size == 0)
+            return;
+        
+        // delete all files
+        foreach (Geary.Attachment attachment in attachments) {
+            try {
+                attachment.file.delete(null);
+            } catch (Error err) {
+                debug("Unable to delete file %s: %s", attachment.file.get_path(), err.message);
+            }
+        }
+        
+        // remove all from attachment table
+        Db.Statement stmt = new Db.Statement(cx, """
+            DELETE FROM MessageAttachmentTable WHERE message_id = ?
+        """);
+        stmt.bind_rowid(0, message_id);
+        
+        stmt.exec();
+    }
+    
     /**
      * Adds a value to the unread count.  If this makes the unread count negative, it will be
      * set to zero.

src/engine/mime/mime-content-type.vala

@@ -18,6 +18,11 @@ public class Geary.Mime.ContentType : Geary.BaseObject {
      */
     public const string WILDCARD = "*";
     
+    /**
+     * Default Content-Type for unknown or unmarked content.
+     */
+    public const string DEFAULT_CONTENT_TYPE = "application/octet-stream";
+    
     /**
      * The type (discrete or concrete) portion of the Content-Type field.
      *

src/engine/rfc822/rfc822-message.vala

@@ -753,24 +753,24 @@ public class Geary.RFC822.Message : BaseObject {
             return;
         }
         
+        // If requested disposition is not UNSPECIFIED, check if this part matches the requested deposition
         Mime.DispositionType part_disposition = Mime.DispositionType.deserialize(part.get_disposition(),
             null);
-        if (part_disposition == Mime.DispositionType.UNSPECIFIED)
+        if (requested_disposition != Mime.DispositionType.UNSPECIFIED && requested_disposition != part_disposition)
             return;
         
+        // skip text/plain and text/html parts that are INLINE or UNSPECIFIED, as they will be used
+        // as part of the body
         if (part.get_content_type() != null) {
             Mime.ContentType content_type = new Mime.ContentType.from_gmime(part.get_content_type());
-            if (part_disposition == Mime.DispositionType.INLINE
+            if ((part_disposition == Mime.DispositionType.INLINE || part_disposition == Mime.DispositionType.UNSPECIFIED)
                 && content_type.has_media_type("text")
                 && (content_type.has_media_subtype("html") || content_type.has_media_subtype("plain"))) {
-                // these are part of the body
                 return;
             }
         }
         
-        // Catch remaining disposition-type matches
+        attachments.add(part);
-        if (requested_disposition == Mime.DispositionType.UNSPECIFIED || part_disposition == requested_disposition)
-            attachments.add(part);
     }
     
     public Gee.List<Geary.RFC822.Message> get_sub_messages() {