From e9a0b2aa7736cf607ed54d0f70fa975d8347b4f5 Mon Sep 17 00:00:00 2001 From: Michael James Gratton Date: Sun, 3 Jul 2016 19:39:55 +1000 Subject: [PATCH] Don't crash when LIST on the INBOX returns a NIL delim. Bug 766509. * src/engine/imap/api/imap-account.vala (Account::list_inbox): Check hierarchy_delimiter after doing a LIST on INBOX, if null then list its siblings and use the first returned by it. * src/engine/imap/api/imap-folder.vala (Folder): Add delim property, set it via the constructors, so we don't need to use the possibly- null MailboxInformation.delim property. Update ctor call sites. --- src/engine/imap/api/imap-account.vala | 124 ++++++++++++++++---------- src/engine/imap/api/imap-folder.vala | 45 +++++----- 2 files changed, 103 insertions(+), 66 deletions(-) diff --git a/src/engine/imap/api/imap-account.vala b/src/engine/imap/api/imap-account.vala index f929dc4d..eeb3dd0c 100644 --- a/src/engine/imap/api/imap-account.vala +++ b/src/engine/imap/api/imap-account.vala @@ -116,44 +116,78 @@ private class Geary.Imap.Account : BaseObject { return account_session; } - - private async void list_inbox(ClientSession session, Cancellable? cancellable) throws Error { - // can't use send_command_async() directly because this is called by claim_session_async(), - // which is called by send_command_async() - - int token = yield cmd_mutex.claim_async(cancellable); - - // collect server data directly for direct decoding - server_data_collector = new Gee.ArrayList(); - bool use_xlist = account_session.capabilities.has_capability(Imap.Capabilities.XLIST); - MailboxInformation inbox = null; - Error? throw_err = null; + private async void list_inbox(ClientSession session, Cancellable? cancellable) + throws Error { + // Can't use send_command_async() directly because this is + // called by claim_session_async(), which is called by + // send_command_async(). + int token = yield this.cmd_mutex.claim_async(cancellable); + Error? release_error = null; try { + // collect server data directly for direct decoding + this.server_data_collector = new Gee.ArrayList(); + + MailboxInformation? info = null; Imap.StatusResponse response = yield session.send_command_async( - new ListCommand(MailboxSpecifier.inbox, use_xlist, null), cancellable); - if (response.status == Imap.Status.OK && server_data_collector.size > 0) { - inbox = MailboxInformation.decode(server_data_collector[0], false); + new ListCommand(MailboxSpecifier.inbox, false, null), cancellable); + if (response.status == Imap.Status.OK && !this.server_data_collector.is_empty) { + info = MailboxInformation.decode(this.server_data_collector[0], false); + } + + if (info != null) { + this.inbox_specifier = info.mailbox; + this.hierarchy_delimiter = info.delim; + } + + if (this.hierarchy_delimiter == null) { + // LIST response didn't include a delim for the + // INBOX. Ideally we'd just use NAMESPACE instead (Bug + // 726866) but for now, just list folders alongside the + // INBOX. + this.server_data_collector = new Gee.ArrayList(); + response = yield session.send_command_async( + new ListCommand.wildcarded( + "", new MailboxSpecifier("%"), false, null + ), + cancellable + ); + if (response.status == Imap.Status.OK) { + foreach (ServerData data in this.server_data_collector) { + info = MailboxInformation.decode(data, false); + this.hierarchy_delimiter = info.delim; + if (this.hierarchy_delimiter != null) { + break; + } + } + } + } + + if (this.inbox_specifier == null) { + throw new ImapError.INVALID("Unable to find INBOX"); + } + if (this.hierarchy_delimiter == null) { + throw new ImapError.INVALID("Unable to determine hierarchy delimiter"); + } + debug("[%s] INBOX specifier: %s", to_string(), + this.inbox_specifier.to_string()); + debug("[%s] Hierarchy delimiter: %s", to_string(), + this.hierarchy_delimiter); + } finally { + this.server_data_collector = new Gee.ArrayList(); + try { + this.cmd_mutex.release(ref token); + } catch (Error e) { + // Vala 0.32.1 won't let you throw an exception from a + // finally block :( + release_error = e; } - } catch (Error err) { - throw_err = err; } - - if (inbox != null) { - inbox_specifier = inbox.mailbox; - hierarchy_delimiter = inbox.delim; - debug("[%s] INBOX specifier: %s", to_string(), inbox_specifier.to_string()); - } - - // Cleanup after setting the inbox and delim so it is immediately available for other commands - server_data_collector = null; - cmd_mutex.release(ref token); - - if (inbox == null) { - new ImapError.INVALID("Unable to list INBOX"); + if (release_error != null) { + throw release_error; } } - + private async void drop_session_async(Cancellable? cancellable) { debug("[%s] Dropping account session...", to_string()); @@ -225,7 +259,7 @@ private class Geary.Imap.Account : BaseObject { check_open(); StatusResponse response = yield send_command_async(new CreateCommand( - new MailboxSpecifier.from_folder_path(path, hierarchy_delimiter)), null, null, cancellable); + new MailboxSpecifier.from_folder_path(path, this.hierarchy_delimiter)), null, null, cancellable); if (response.status != Status.OK) { throw new ImapError.SERVER_ERROR("Server reports error creating path %s: %s", path.to_string(), @@ -273,12 +307,12 @@ private class Geary.Imap.Account : BaseObject { status = fallback_status_data; } - - folder = new Imap.Folder(folder_path, session_mgr, status, mailbox_info); + + folder = new Imap.Folder(folder_path, session_mgr, status, mailbox_info, this.hierarchy_delimiter); } else { - folder = new Imap.Folder.unselectable(folder_path, session_mgr, mailbox_info); + folder = new Imap.Folder.unselectable(folder_path, session_mgr, mailbox_info, this.hierarchy_delimiter); } - + folders.set(path, folder); return folder; @@ -304,12 +338,12 @@ private class Geary.Imap.Account : BaseObject { Imap.Folder folder; if (!mailbox_info.attrs.is_no_select) { StatusData status = yield fetch_status_async(folder_path, StatusDataType.all(), cancellable); - - folder = new Imap.Folder(folder_path, session_mgr, status, mailbox_info); + + folder = new Imap.Folder(folder_path, session_mgr, status, mailbox_info, this.hierarchy_delimiter); } else { - folder = new Imap.Folder.unselectable(folder_path, session_mgr, mailbox_info); + folder = new Imap.Folder.unselectable(folder_path, session_mgr, mailbox_info, this.hierarchy_delimiter); } - + return folder; } @@ -346,7 +380,7 @@ private class Geary.Imap.Account : BaseObject { Gee.List status_results = new Gee.ArrayList(); StatusResponse response = yield send_command_async( - new StatusCommand(new MailboxSpecifier.from_folder_path(path, hierarchy_delimiter), status_types), + new StatusCommand(new MailboxSpecifier.from_folder_path(path, this.hierarchy_delimiter), status_types), null, status_results, cancellable); throw_fetch_error(response, path, status_results.size); @@ -390,10 +424,10 @@ private class Geary.Imap.Account : BaseObject { // if new mailbox is unselectable, don't bother doing a STATUS command if (mailbox_info.attrs.is_no_select) { Imap.Folder folder = new Imap.Folder.unselectable(mailbox_info.get_path(inbox_specifier), - session_mgr, mailbox_info); + session_mgr, mailbox_info, this.hierarchy_delimiter); folders.set(folder.path, folder); child_folders.add(folder); - + continue; } @@ -448,10 +482,10 @@ private class Geary.Imap.Account : BaseObject { if (folder != null) { folder.properties.update_status(found_status); } else { - folder = new Imap.Folder(folder_path, session_mgr, found_status, mailbox_info); + folder = new Imap.Folder(folder_path, session_mgr, found_status, mailbox_info, this.hierarchy_delimiter); folders.set(folder.path, folder); } - + child_folders.add(folder); } diff --git a/src/engine/imap/api/imap-folder.vala b/src/engine/imap/api/imap-folder.vala index d74daaf1..f7358c90 100644 --- a/src/engine/imap/api/imap-folder.vala +++ b/src/engine/imap/api/imap-folder.vala @@ -13,11 +13,12 @@ private class Geary.Imap.Folder : BaseObject { private const Geary.Email.Field BASIC_FETCH_FIELDS = Email.Field.ENVELOPE | Email.Field.DATE | Email.Field.ORIGINATORS | Email.Field.RECEIVERS | Email.Field.REFERENCES | Email.Field.SUBJECT | Email.Field.HEADER; - + public bool is_open { get; private set; default = false; } public FolderPath path { get; private set; } public Imap.FolderProperties properties { get; private set; } public MailboxInformation info { get; private set; } + public string delim { get; private set; } public MessageFlags? permanent_flags { get; private set; default = null; } public Trillian readonly { get; private set; default = Trillian.UNKNOWN; } public Trillian accepts_user_flags { get; private set; default = Trillian.UNKNOWN; } @@ -70,8 +71,8 @@ private class Geary.Imap.Folder : BaseObject { * Note that close_async() still needs to be called after this signal is fired. */ public signal void disconnected(ClientSession.DisconnectReason reason); - - internal Folder(FolderPath path, ClientSessionManager session_mgr, StatusData status, MailboxInformation info) { + + internal Folder(FolderPath path, ClientSessionManager session_mgr, StatusData status, MailboxInformation info, string delim) { // Used to assert() here, but that meant that any issue with internationalization/encoding // made Geary unusable for a subset of servers accessed/configured in a non-English language... // this is not the end of the world, but it does suggest an I18N issue, potentially with @@ -80,22 +81,24 @@ private class Geary.Imap.Folder : BaseObject { message("%s: IMAP folder created with differing mailbox names (STATUS=%s LIST=%s)", path.to_string(), status.to_string(), info.to_string()); } - + this.session_mgr = session_mgr; this.info = info; + this.delim = delim; this.path = path; - + properties = new Imap.FolderProperties.status(status, info.attrs); } - - internal Folder.unselectable(FolderPath path, ClientSessionManager session_mgr, MailboxInformation info) { + + internal Folder.unselectable(FolderPath path, ClientSessionManager session_mgr, MailboxInformation info, string delim) { this.session_mgr = session_mgr; this.info = info; + this.delim = delim; this.path = path; - + properties = new Imap.FolderProperties(0, 0, 0, null, null, info.attrs); } - + public async void open_async(Cancellable? cancellable) throws Error { if (is_open) throw new EngineError.ALREADY_OPEN("%s already open", to_string()); @@ -112,18 +115,18 @@ private class Geary.Imap.Folder : BaseObject { session.search.connect(on_search); session.status_response_received.connect(on_status_response); session.disconnected.connect(on_disconnected); - + properties.set_from_session_capabilities(session.capabilities); - + StatusResponse? response = null; Error? select_err = null; try { response = yield session.select_async( - new MailboxSpecifier.from_folder_path(path, info.delim), cancellable); + new MailboxSpecifier.from_folder_path(path, this.delim), cancellable); } catch (Error err) { select_err = err; } - + // if select_err is null, then response can not be null if (select_err != null || response.status != Status.OK) { // don't use user-supplied cancellable; it may be cancelled, and even if not, do not want @@ -678,10 +681,10 @@ private class Geary.Imap.Folder : BaseObject { public async Gee.Map? copy_email_async(MessageSet msg_set, FolderPath destination, Cancellable? cancellable) throws Error { check_open(); - + CopyCommand cmd = new CopyCommand(msg_set, - new MailboxSpecifier.from_folder_path(destination, info.delim)); - + new MailboxSpecifier.from_folder_path(destination, this.delim)); + Gee.Map? responses = yield exec_commands_async( Geary.iterate(cmd).to_array_list(), null, null, cancellable); @@ -1032,17 +1035,17 @@ private class Geary.Imap.Folder : BaseObject { } else { msg_flags = new MessageFlags(Geary.iterate(MessageFlag.SEEN).to_array_list()); } - + InternalDate? internaldate = null; if (date_received != null) internaldate = new InternalDate.from_date_time(date_received); - - AppendCommand cmd = new AppendCommand(new MailboxSpecifier.from_folder_path(path, info.delim), + + AppendCommand cmd = new AppendCommand(new MailboxSpecifier.from_folder_path(path, this.delim), msg_flags, internaldate, message.get_network_buffer(false)); - + Gee.Map responses = yield exec_commands_async( Geary.iterate(cmd).to_array_list(), null, null, null); - + // Grab the response and parse out the UID, if available. StatusResponse response = responses.get(cmd); if (response.status == Status.OK && response.response_code != null &&