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.
This commit is contained in:
Michael James Gratton 2016-07-03 19:39:55 +10:00 committed by Michael Gratton
parent f80f52fbe6
commit e9a0b2aa77
2 changed files with 103 additions and 66 deletions

View file

@ -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<ServerData>();
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<ServerData>();
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<ServerData>();
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<ServerData>();
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<StatusData> status_results = new Gee.ArrayList<StatusData>();
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);
}

View file

@ -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<UID, UID>? 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<Command, StatusResponse>? responses = yield exec_commands_async(
Geary.iterate<Command>(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>(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<Command, StatusResponse> responses = yield exec_commands_async(
Geary.iterate<AppendCommand>(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 &&