diff --git a/sql/version-022.sql b/sql/version-022.sql new file mode 100644 index 00000000..32d9c63b --- /dev/null +++ b/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; + diff --git a/src/client/conversation-viewer/conversation-viewer.vala b/src/client/conversation-viewer/conversation-viewer.vala index 5ded86b3..bbc11fe5 100644 --- a/src/client/conversation-viewer/conversation-viewer.vala +++ b/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 inlined_content_ids = new Gee.HashSet(); 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)) { diff --git a/src/engine/api/geary-attachment.vala b/src/engine/api/geary-attachment.vala index cd169942..d483352a 100644 --- a/src/engine/api/geary-attachment.vala +++ b/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; } } diff --git a/src/engine/imap-db/imap-db-attachment.vala b/src/engine/imap-db/imap-db-attachment.vala index b8b5226b..def821cf 100644 --- a/src/engine/imap-db/imap-db-attachment.vala +++ b/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) { diff --git a/src/engine/imap-db/imap-db-database.vala b/src/engine/imap-db/imap-db-database.vala index b0292d06..89908e25 100644 --- a/src/engine/imap-db/imap-db-database.vala +++ b/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 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); diff --git a/src/engine/imap-db/imap-db-folder.vala b/src/engine/imap-db/imap-db-folder.vala index 4dbfbb27..32a58d2e 100644 --- a/src/engine/imap-db/imap-db-folder.vala +++ b/src/engine/imap-db/imap-db-folder.vala @@ -1874,7 +1874,7 @@ private class Geary.ImapDB.Folder : BaseObject, Geary.ReferenceSemantics { private static Gee.List? 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(); - string disposition = attachment.get_disposition(); + GMime.ContentType? content_type = attachment.get_content_type(); + 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) - VALUES (?, ?, ?, ?, ?) + INSERT INTO MessageAttachmentTable (message_id, filename, mime_type, filesize, disposition, content_id, description) + 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? 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. diff --git a/src/engine/mime/mime-content-type.vala b/src/engine/mime/mime-content-type.vala index e61ade3a..ec267698 100644 --- a/src/engine/mime/mime-content-type.vala +++ b/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. * diff --git a/src/engine/rfc822/rfc822-message.vala b/src/engine/rfc822/rfc822-message.vala index f38b6942..001038da 100644 --- a/src/engine/rfc822/rfc822-message.vala +++ b/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 - if (requested_disposition == Mime.DispositionType.UNSPECIFIED || part_disposition == requested_disposition) - attachments.add(part); + attachments.add(part); } public Gee.List get_sub_messages() {