[geary] Show attachments lacking a Content-Disposition: Bug #713830
- From: Jim Nelson <jnelson src gnome org>
- To: commits-list gnome org
- Cc:
- Subject: [geary] Show attachments lacking a Content-Disposition: Bug #713830
- Date: Tue, 12 Aug 2014 21:38:28 +0000 (UTC)
commit a9db6e8c444aabd547a12a77f7962b52205f9a50
Author: Jim Nelson <jim yorba org>
Date: Tue Aug 12 14:22:57 2014 -0700
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 | 10 +++
.../conversation-viewer/conversation-viewer.vala | 14 ++++-
src/engine/api/geary-attachment.vala | 19 +++++-
src/engine/imap-db/imap-db-attachment.vala | 6 +-
src/engine/imap-db/imap-db-database.vala | 69 ++++++++++++++++++++
src/engine/imap-db/imap-db-folder.vala | 46 +++++++++++--
src/engine/mime/mime-content-type.vala | 5 ++
src/engine/rfc822/rfc822-message.vala | 12 ++--
8 files changed, 163 insertions(+), 18 deletions(-)
---
diff --git a/sql/version-022.sql b/sql/version-022.sql
new file mode 100644
index 0000000..32d9c63
--- /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 5ded86b..bbc11fe 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<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)) {
diff --git a/src/engine/api/geary-attachment.vala b/src/engine/api/geary-attachment.vala
index cd16994..d483352 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 b8b5226..def821c 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 b0292d0..89908e2 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<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);
diff --git a/src/engine/imap-db/imap-db-folder.vala b/src/engine/imap-db/imap-db-folder.vala
index 4dbfbb2..32a58d2 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<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();
- 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<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.
diff --git a/src/engine/mime/mime-content-type.vala b/src/engine/mime/mime-content-type.vala
index e61ade3..ec26769 100644
--- a/src/engine/mime/mime-content-type.vala
+++ b/src/engine/mime/mime-content-type.vala
@@ -19,6 +19,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.
*
* It's highly recommended the caller use the various ''has'' and ''is'' methods when performing
diff --git a/src/engine/rfc822/rfc822-message.vala b/src/engine/rfc822/rfc822-message.vala
index f38b694..001038d 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<Geary.RFC822.Message> get_sub_messages() {
[
Date Prev][
Date Next] [
Thread Prev][
Thread Next]
[
Thread Index]
[
Date Index]
[
Author Index]
