diff --git a/src/client/conversation-list/conversation-list-view.vala b/src/client/conversation-list/conversation-list-view.vala index 158708ca..89db534e 100644 --- a/src/client/conversation-list/conversation-list-view.vala +++ b/src/client/conversation-list/conversation-list-view.vala @@ -541,12 +541,15 @@ public class ConversationListView : Gtk.TreeView, Geary.BaseInterface { public void select_conversations(Gee.Collection new_selection) { if (this.selected.size != new_selection.size || !this.selected.contains_all(new_selection)) { - Gtk.TreeSelection selection = get_selection(); + var selection = get_selection(); selection.unselect_all(); - foreach (var conversation in new_selection) { - Gtk.TreePath path = get_model().get_path_for_conversation(conversation); - if (path != null) { - selection.select_path(path); + var model = get_model(); + if (model != null) { + foreach (var conversation in new_selection) { + var path = model.get_path_for_conversation(conversation); + if (path != null) { + selection.select_path(path); + } } } } diff --git a/src/engine/imap-db/imap-db-message-row.vala b/src/engine/imap-db/imap-db-message-row.vala index 03b49b0d..29dbd0b6 100644 --- a/src/engine/imap-db/imap-db-message-row.vala +++ b/src/engine/imap-db/imap-db-message-row.vala @@ -104,13 +104,7 @@ private class Geary.ImapDB.MessageRow { Geary.Email email = new Geary.Email(id); if (fields.is_all_set(Geary.Email.Field.DATE)) { - try { - email.set_send_date( - !String.is_empty(date) ? new RFC822.Date.from_rfc822_string(date) : null - ); - } catch (GLib.Error err) { - debug("Error loading message date from db: %s", err.message); - } + email.set_send_date(unflatten_date(date)); } if (fields.is_all_set(Geary.Email.Field.ORIGINATORS)) { @@ -127,9 +121,10 @@ private class Geary.ImapDB.MessageRow { if (fields.is_all_set(Geary.Email.Field.REFERENCES)) { email.set_full_references( - (message_id != null) ? new RFC822.MessageID(message_id) : null, - (in_reply_to != null) ? new RFC822.MessageIDList.from_rfc822_string(in_reply_to) : null, - (references != null) ? new RFC822.MessageIDList.from_rfc822_string(references) : null); + unflatten_message_id(message_id), + unflatten_message_id_list(in_reply_to), + unflatten_message_id_list(references) + ); } if (fields.is_all_set(Geary.Email.Field.SUBJECT)) @@ -274,22 +269,79 @@ private class Geary.ImapDB.MessageRow { return (addrs == null || addrs.size == 0) ? null : addrs.to_rfc822_string(); } - private RFC822.MailboxAddress? unflatten_address(string? str) - throws RFC822.Error { - return ( - String.is_empty_or_whitespace(str) - ? null - : new RFC822.MailboxAddress.from_rfc822_string(str) - ); + private RFC822.Date? unflatten_date(string? str) { + RFC822.Date? date = null; + if (!String.is_empty_or_whitespace(str)) { + try { + date = new RFC822.Date.from_rfc822_string(str); + } catch (RFC822.Error err) { + // There's not much we can do here aside from logging + // the error, since a lot of email just contain + // invalid addresses + debug("Invalid RFC822 date \"%s\": %s", str, err.message); + } + } + return date; } - private RFC822.MailboxAddresses? unflatten_addresses(string? str) - throws RFC822.Error { - return ( - String.is_empty_or_whitespace(str) - ? null - : new RFC822.MailboxAddresses.from_rfc822_string(str) - ); + private RFC822.MailboxAddress? unflatten_address(string? str) { + RFC822.MailboxAddress? address = null; + if (!String.is_empty_or_whitespace(str)) { + try { + address = new RFC822.MailboxAddress.from_rfc822_string(str); + } catch (RFC822.Error err) { + // There's not much we can do here aside from logging + // the error, since a lot of email just contain + // invalid addresses + debug("Invalid RFC822 mailbox address \"%s\": %s", str, err.message); + } + } + return address; + } + + private RFC822.MailboxAddresses? unflatten_addresses(string? str) { + RFC822.MailboxAddresses? addresses = null; + if (!String.is_empty_or_whitespace(str)) { + try { + addresses = new RFC822.MailboxAddresses.from_rfc822_string(str); + } catch (RFC822.Error err) { + // There's not much we can do here aside from logging + // the error, since a lot of email just contain + // invalid addresses + debug("Invalid RFC822 mailbox addresses \"%s\": %s", str, err.message); + } + } + return addresses; + } + + private RFC822.MessageID? unflatten_message_id(string? str) { + RFC822.MessageID? id = null; + if (!String.is_empty_or_whitespace(str)) { + try { + id = new RFC822.MessageID.from_rfc822_string(str); + } catch (RFC822.Error err) { + // There's not much we can do here aside from logging + // the error, since a lot of email just contain + // invalid addresses + debug("Invalid RFC822 message id \"%s\": %s", str, err.message); + } + } + return id; + } + + private RFC822.MessageIDList? unflatten_message_id_list(string? str) { + RFC822.MessageIDList? ids = null; + if (!String.is_empty_or_whitespace(str)) { + try { + ids = new RFC822.MessageIDList.from_rfc822_string(str); + } catch (RFC822.Error err) { + // There's not much we can do here aside from logging + // the error, since a lot of email just contain + // invalid addresses + debug("Invalid RFC822 message id \"%s\": %s", str, err.message); + } + } + return ids; } } diff --git a/src/engine/imap-engine/imap-engine-account-synchronizer.vala b/src/engine/imap-engine/imap-engine-account-synchronizer.vala index 1880b150..21ea1c74 100644 --- a/src/engine/imap-engine/imap-engine-account-synchronizer.vala +++ b/src/engine/imap-engine/imap-engine-account-synchronizer.vala @@ -121,7 +121,7 @@ private class Geary.ImapEngine.RefreshFolderSync : FolderOperation { } ~RefreshFolderSync() { - Geary.Folder? folder = this.folder; + weak Geary.Folder? folder = this.folder; if (folder != null) { this.folder.closed.disconnect(on_folder_close); } diff --git a/src/engine/imap/api/imap-folder-session.vala b/src/engine/imap/api/imap-folder-session.vala index b7c42a86..56b5c435 100644 --- a/src/engine/imap/api/imap-folder-session.vala +++ b/src/engine/imap/api/imap-folder-session.vala @@ -1,9 +1,9 @@ /* - * Copyright 2016 Software Freedom Conservancy Inc. - * Copyright 2018 Michael Gratton . + * Copyright © 2016 Software Freedom Conservancy Inc. + * Copyright © 2018, 2020 Michael Gratton . * * This software is licensed under the GNU Lesser General Public License - * (version 2.1 or later). See the COPYING file in this distribution. + * (version 2.1 or later). See the COPYING file in this distribution. */ @@ -873,6 +873,10 @@ private class Geary.Imap.FolderSession : Geary.Imap.SessionObject { if (header_specifiers != null) { // Header fields are case insensitive, so use a // case-insensitive map. + // + // XXX this is bogus because it doesn't take into the + // presence of multiple headers. It's not common, but it's + // possible for there to be two To headers, for example Gee.Map headers = new Gee.HashMap( String.stri_hash, String.stri_equal ); @@ -882,7 +886,7 @@ private class Geary.Imap.FolderSession : Geary.Imap.SessionObject { if (fetched_headers != null) { RFC822.Header parsed_headers = new RFC822.Header(fetched_headers); foreach (string name in parsed_headers.get_header_names()) { - headers.set(name, parsed_headers.get_header(name)); + headers.set(name, parsed_headers.get_raw_header(name)); } } else { warning( @@ -896,60 +900,34 @@ private class Geary.Imap.FolderSession : Geary.Imap.SessionObject { } // DATE - if (required_but_not_set(Geary.Email.Field.DATE, required_fields, email)) { - string? value = headers.get("Date"); - RFC822.Date? date = null; - if (!String.is_empty(value)) { - try { - date = new RFC822.Date.from_rfc822_string(value); - } catch (GLib.Error err) { - warning( - "Error parsing date from FETCH response: %s", - err.message - ); - } + if (required_but_not_set(DATE, required_fields, email)) { + RFC822.Date? date = unflatten_date(headers.get("Date")); + if (date != null) { + email.set_send_date(date); } - email.set_send_date(date); } // ORIGINATORS - if (required_but_not_set(Geary.Email.Field.ORIGINATORS, required_fields, email)) { - RFC822.MailboxAddresses? from = null; - string? value = headers.get("From"); - if (!String.is_empty(value)) - from = new RFC822.MailboxAddresses.from_rfc822_string(value); - - RFC822.MailboxAddress? sender = null; - value = headers.get("Sender"); - if (!String.is_empty(value)) - sender = new RFC822.MailboxAddress.from_rfc822_string(value); - - RFC822.MailboxAddresses? reply_to = null; - value = headers.get("Reply-To"); - if (!String.is_empty(value)) - reply_to = new RFC822.MailboxAddresses.from_rfc822_string(value); - - email.set_originators(from, sender, reply_to); + if (required_but_not_set(ORIGINATORS, required_fields, email)) { + // Allow sender to be a list (contra to the RFC), but + // only take the first from it + RFC822.MailboxAddresses? sender = unflatten_addresses( + headers.get("Sender") + ); + email.set_originators( + unflatten_addresses(headers.get("From")), + (sender != null && !sender.is_empty) ? sender.get(0) : null, + unflatten_addresses(headers.get("Reply-To")) + ); } // RECEIVERS - if (required_but_not_set(Geary.Email.Field.RECEIVERS, required_fields, email)) { - RFC822.MailboxAddresses? to = null; - string? value = headers.get("To"); - if (!String.is_empty(value)) - to = new RFC822.MailboxAddresses.from_rfc822_string(value); - - RFC822.MailboxAddresses? cc = null; - value = headers.get("Cc"); - if (!String.is_empty(value)) - cc = new RFC822.MailboxAddresses.from_rfc822_string(value); - - RFC822.MailboxAddresses? bcc = null; - value = headers.get("Bcc"); - if (!String.is_empty(value)) - bcc = new RFC822.MailboxAddresses.from_rfc822_string(value); - - email.set_receivers(to, cc, bcc); + if (required_but_not_set(RECEIVERS, required_fields, email)) { + email.set_receivers( + unflatten_addresses(headers.get("To")), + unflatten_addresses(headers.get("Cc")), + unflatten_addresses(headers.get("Bcc")) + ); } // REFERENCES @@ -957,21 +935,17 @@ private class Geary.Imap.FolderSession : Geary.Imap.SessionObject { // References header will be present if REFERENCES were required, which is why // REFERENCES is set at the bottom of the method, when all information has been gathered if (message_id == null) { - string? value = headers.get("Message-ID"); - if (!String.is_empty(value)) - message_id = new RFC822.MessageID(value); + message_id = unflatten_message_id(headers.get("Message-ID")); } - if (in_reply_to == null) { - string? value = headers.get("In-Reply-To"); - if (!String.is_empty(value)) - in_reply_to = new RFC822.MessageIDList.from_rfc822_string(value); + in_reply_to = unflatten_message_id_list( + headers.get("In-Reply-To") + ); } - if (references == null) { - string? value = headers.get("References"); - if (!String.is_empty(value)) - references = new RFC822.MessageIDList.from_rfc822_string(value); + references = unflatten_message_id_list( + headers.get("References") + ); } // SUBJECT @@ -993,13 +967,20 @@ private class Geary.Imap.FolderSession : Geary.Imap.SessionObject { // if preview was requested, get it now ... both identifiers // must be supplied if one is if (preview_specifier != null || preview_charset_specifier != null) { - assert(preview_specifier != null && preview_charset_specifier != null); + Memory.Buffer? preview_headers = fetched_data.body_data_map.get( + preview_charset_specifier + ); + Memory.Buffer? preview_body = fetched_data.body_data_map.get( + preview_specifier + ); - if (fetched_data.body_data_map.has_key(preview_specifier) - && fetched_data.body_data_map.has_key(preview_charset_specifier)) { - email.set_message_preview(new RFC822.PreviewText.with_header( - fetched_data.body_data_map.get(preview_charset_specifier), - fetched_data.body_data_map.get(preview_specifier))); + if (preview_headers != null && preview_headers.size > 0 && + preview_body != null && preview_body.size > 0) { + email.set_message_preview( + new RFC822.PreviewText.with_header( + preview_headers, preview_body + ) + ); } else { warning("No preview specifiers \"%s\" and \"%s\" found", preview_specifier.to_string(), preview_charset_specifier.to_string()); @@ -1154,4 +1135,64 @@ private class Geary.Imap.FolderSession : Geary.Imap.SessionObject { return users_fields.require(check) ? !email.fields.is_all_set(check) : false; } + private RFC822.Date? unflatten_date(string? str) { + RFC822.Date? date = null; + if (!String.is_empty_or_whitespace(str)) { + try { + date = new RFC822.Date.from_rfc822_string(str); + } catch (RFC822.Error err) { + // There's not much we can do here aside from logging + // the error, since a lot of email just contain + // invalid addresses + debug("Invalid RFC822 date \"%s\": %s", str, err.message); + } + } + return date; + } + + private RFC822.MailboxAddresses? unflatten_addresses(string? str) { + RFC822.MailboxAddresses? addresses = null; + if (!String.is_empty_or_whitespace(str)) { + try { + addresses = new RFC822.MailboxAddresses.from_rfc822_string(str); + } catch (RFC822.Error err) { + // There's not much we can do here aside from logging + // the error, since a lot of email just contain + // invalid addresses + debug("Invalid RFC822 mailbox addresses \"%s\": %s", str, err.message); + } + } + return addresses; + } + + private RFC822.MessageID? unflatten_message_id(string? str) { + RFC822.MessageID? id = null; + if (!String.is_empty_or_whitespace(str)) { + try { + id = new RFC822.MessageID.from_rfc822_string(str); + } catch (RFC822.Error err) { + // There's not much we can do here aside from logging + // the error, since a lot of email just contain + // invalid addresses + debug("Invalid RFC822 message id \"%s\": %s", str, err.message); + } + } + return id; + } + + private RFC822.MessageIDList? unflatten_message_id_list(string? str) { + RFC822.MessageIDList? ids = null; + if (!String.is_empty_or_whitespace(str)) { + try { + ids = new RFC822.MessageIDList.from_rfc822_string(str); + } catch (RFC822.Error err) { + // There's not much we can do here aside from logging + // the error, since a lot of email just contain + // invalid addresses + debug("Invalid RFC822 message id \"%s\": %s", str, err.message); + } + } + return ids; + } + } diff --git a/src/engine/rfc822/rfc822-mailbox-addresses.vala b/src/engine/rfc822/rfc822-mailbox-addresses.vala index 40ba2462..25591d89 100644 --- a/src/engine/rfc822/rfc822-mailbox-addresses.vala +++ b/src/engine/rfc822/rfc822-mailbox-addresses.vala @@ -83,7 +83,10 @@ public class Geary.RFC822.MailboxAddresses : public MailboxAddresses.from_rfc822_string(string rfc822) throws Error { - var list = GMime.InternetAddressList.parse(null, rfc822); + var list = GMime.InternetAddressList.parse( + Geary.RFC822.get_parser_options(), + rfc822 + ); if (list == null) { throw new Error.INVALID("Not a RFC822 mailbox address list"); } diff --git a/src/engine/rfc822/rfc822-message-data.vala b/src/engine/rfc822/rfc822-message-data.vala index 48b1d6f7..01a854e9 100644 --- a/src/engine/rfc822/rfc822-message-data.vala +++ b/src/engine/rfc822/rfc822-message-data.vala @@ -285,7 +285,7 @@ public class Geary.RFC822.Subject : } public Subject.from_rfc822_string(string rfc822) { - base(GMime.utils_header_decode_text(get_parser_options(), rfc822)); + base(GMime.utils_header_decode_text(get_parser_options(), rfc822).strip()); this.rfc822 = rfc822; } @@ -391,6 +391,15 @@ public class Geary.RFC822.Header : return value; } + public string? get_raw_header(string name) { + string? value = null; + var header = this.message.get_header_list().get_header(name); + if (header != null) { + value = header.get_raw_value(); + } + return value; + } + public string[] get_header_names() { if (this.names == null) { GMime.HeaderList headers = this.message.get_header_list(); diff --git a/src/engine/rfc822/rfc822.vala b/src/engine/rfc822/rfc822.vala index 3d800508..d25df3da 100644 --- a/src/engine/rfc822/rfc822.vala +++ b/src/engine/rfc822/rfc822.vala @@ -24,7 +24,9 @@ namespace Geary.RFC822 { */ public const string ASCII_CHARSET = "US-ASCII"; - internal Regex? invalid_filename_character_re = null; + internal GMime.ParserOptions gmime_parser_options; + + internal Regex? invalid_filename_character_re; private int init_count = 0; @@ -34,7 +36,12 @@ namespace Geary.RFC822 { return; GMime.init(); - GMime.ParserOptions.get_default().set_allow_addresses_without_domain(true); + + gmime_parser_options = GMime.ParserOptions.get_default(); + gmime_parser_options.set_allow_addresses_without_domain(true); + gmime_parser_options.set_address_compliance_mode(LOOSE); + gmime_parser_options.set_parameter_compliance_mode(LOOSE); + gmime_parser_options.set_rfc2047_compliance_mode(LOOSE); try { invalid_filename_character_re = new Regex("[/\\0]"); @@ -44,11 +51,11 @@ namespace Geary.RFC822 { } public GMime.FormatOptions get_format_options() { - return GMime.FormatOptions.get_default().clone(); + return GMime.FormatOptions.get_default(); } public GMime.ParserOptions get_parser_options() { - return GMime.ParserOptions.get_default().clone(); + return Geary.RFC822.gmime_parser_options; } public string? get_charset() { diff --git a/src/engine/util/util-logging.vala b/src/engine/util/util-logging.vala index c4c517cd..c0356862 100644 --- a/src/engine/util/util-logging.vala +++ b/src/engine/util/util-logging.vala @@ -54,6 +54,7 @@ namespace Geary.Logging { private GLib.Mutex writer_lock; private unowned FileStream? stream = null; + private GLib.LogLevelFlags set_breakpoint_on = 0; private Gee.Set suppressed_domains; @@ -71,6 +72,18 @@ namespace Geary.Logging { Logging.record_lock = GLib.Mutex(); Logging.writer_lock = GLib.Mutex(); Logging.max_log_length = DEFAULT_MAX_LOG_BUFFER_LENGTH; + + var debug_var = GLib.Environment.get_variable("G_DEBUG"); + if (debug_var != null) { + var parts = debug_var.split(","); + if ("fatal-warnings" in parts) { + Logging.set_breakpoint_on |= GLib.LogLevelFlags.LEVEL_WARNING; + } + if ("fatal-criticals" in parts) { + Logging.set_breakpoint_on |= GLib.LogLevelFlags.LEVEL_WARNING; + Logging.set_breakpoint_on |= GLib.LogLevelFlags.LEVEL_CRITICAL; + } + } } } @@ -299,6 +312,10 @@ namespace Geary.Logging { out.puts(record.format()); out.putc('\n'); Logging.writer_lock.unlock(); + + if (levels in Logging.set_breakpoint_on) { + GLib.breakpoint(); + } } } @@ -538,12 +555,14 @@ public interface Geary.Logging.Source : GLib.Object { va_list args) { Context context = Context(this.logging_domain, levels, fmt, args); - // Don't attempt to this object if it is in the middle of - // being destructed, which can happen when logging from - // the destructor. - Source? decorated = (this.ref_count > 0) ? this : this.logging_parent; + weak Source? decorated = this; while (decorated != null) { - context.append_source(decorated); + // Check ref counts of logged objects and don't log them + // if they are or have been being destroyed, which can + // happen when e.g. logging from the destructor. + if (decorated.ref_count > 0) { + context.append_source(decorated); + } decorated = decorated.logging_parent; }