[geary/mjog/imap-connection-fixes: 33/34] Geary.Imap.{Acount, Folder, Client}Session: Handle NO responses better
- From: Michael Gratton <mjog src gnome org>
- To: commits-list gnome org
- Cc:
- Subject: [geary/mjog/imap-connection-fixes: 33/34] Geary.Imap.{Acount, Folder, Client}Session: Handle NO responses better
- Date: Thu, 26 Mar 2020 21:33:58 +0000 (UTC)
commit f7a5679002b608dcb8a891e40c8b152c7efa3204
Author: Michael Gratton <mike vee net>
Date: Fri Mar 27 08:23:13 2020 +1100
Geary.Imap.{Acount,Folder,Client}Session: Handle NO responses better
Instead of treading `NO` and `BAD` responses by throwing a
`SERVER_ERROR`, call `Command.throw_on_error` so that NO throws an
`OPERATIONAL_ERROR` and any IMAP response codes returned are also
observed.
In ClientSession's high-level protocol methods (`login_async`,
`select_async`, etc), just check the error as well rather than passing
the response back and expecting the caller to do it.
Fixes #502
src/engine/imap/api/imap-account-session.vala | 52 +++++------------
src/engine/imap/api/imap-folder-session.vala | 27 +--------
src/engine/imap/transport/imap-client-session.vala | 66 +++++++---------------
3 files changed, 37 insertions(+), 108 deletions(-)
---
diff --git a/src/engine/imap/api/imap-account-session.vala b/src/engine/imap/api/imap-account-session.vala
index f0bdfef1..2131c844 100644
--- a/src/engine/imap/api/imap-account-session.vala
+++ b/src/engine/imap/api/imap-account-session.vala
@@ -101,16 +101,7 @@ internal class Geary.Imap.AccountSession : Geary.Imap.SessionObject {
? new CreateCommand.special_use(mailbox, type)
: new CreateCommand(mailbox);
- StatusResponse response = yield send_command_async(
- session, cmd, null, null, cancellable
- );
-
- if (response.status != Status.OK) {
- throw new ImapError.SERVER_ERROR(
- "Server reports error creating folder %s: %s",
- mailbox.to_string(), response.to_string()
- );
- }
+ yield send_command_async(session, cmd, null, null, cancellable);
}
/**
@@ -339,11 +330,7 @@ internal class Geary.Imap.AccountSession : Geary.Imap.SessionObject {
}
Gee.List<MailboxInformation> list_results = new Gee.ArrayList<MailboxInformation>();
- StatusResponse response = yield send_command_async(session, cmd, list_results, null, cancellable);
- if (response.status != Status.OK) {
- throw new ImapError.SERVER_ERROR("Unable to list children of %s: %s",
- (folder != null) ? folder.to_string() : "root", response.to_string());
- }
+ yield send_command_async(session, cmd, list_results, null, cancellable);
// See note at ListCommand about some servers returning the
// parent's name alongside their children ... this filters
@@ -369,36 +356,26 @@ internal class Geary.Imap.AccountSession : Geary.Imap.SessionObject {
StatusDataType[] status_types,
Cancellable? cancellable)
throws Error {
- Gee.List<StatusData> status_results = new Gee.ArrayList<StatusData>();
- StatusResponse response = yield send_command_async(
- session,
- new StatusCommand(mailbox, status_types),
- null,
- status_results,
- cancellable
+ var status_results = new Gee.ArrayList<StatusData>();
+ var cmd = new StatusCommand(mailbox, status_types);
+ yield send_command_async(
+ session, cmd, null, status_results, cancellable
);
-
- if (response.status != Status.OK) {
- throw new ImapError.SERVER_ERROR("Error fetching \"%s\" STATUS: %s",
- mailbox.to_string(),
- response.to_string());
- }
-
if (status_results.size != 1) {
throw new ImapError.INVALID("Invalid result count (%d) \"%s\" STATUS: %s",
status_results.size,
mailbox.to_string(),
- response.to_string());
+ cmd.status.to_string());
}
-
return status_results[0];
}
- private async StatusResponse send_command_async(ClientSession session,
- Command cmd,
- Gee.List<MailboxInformation>? list_results,
- Gee.List<StatusData>? status_results,
- Cancellable? cancellable) throws Error {
+ private async void send_command_async(ClientSession session,
+ Command cmd,
+ Gee.List<MailboxInformation>? list_results,
+ Gee.List<StatusData>? status_results,
+ GLib.Cancellable? cancellable)
+ throws GLib.Error {
Gee.Map<Command, StatusResponse> responses = yield send_multiple_async(
session,
Geary.iterate<Command>(cmd).to_array_list(),
@@ -413,7 +390,8 @@ internal class Geary.Imap.AccountSession : Geary.Imap.SessionObject {
"No status response received from server"
);
}
- return response;
+
+ cmd.throw_on_error();
}
private async Gee.Map<Command, StatusResponse>
diff --git a/src/engine/imap/api/imap-folder-session.vala b/src/engine/imap/api/imap-folder-session.vala
index c94da4bc..1963468e 100644
--- a/src/engine/imap/api/imap-folder-session.vala
+++ b/src/engine/imap/api/imap-folder-session.vala
@@ -113,10 +113,7 @@ private class Geary.Imap.FolderSession : Geary.Imap.SessionObject {
session.status_response_received.connect(on_status_response);
MailboxSpecifier mailbox = session.get_mailbox_for_path(folder.path);
- StatusResponse? response = yield session.select_async(
- mailbox, cancellable
- );
- throw_on_not_ok(response, "SELECT " + this.folder.path.to_string());
+ yield session.select_async(mailbox, cancellable);
// if at end of SELECT command accepts_user_flags is still
// UNKKNOWN, treat as TRUE because, according to IMAP spec, if
@@ -319,7 +316,7 @@ private class Geary.Imap.FolderSession : Geary.Imap.SessionObject {
}
foreach (Command cmd in responses.keys) {
- throw_on_not_ok(responses.get(cmd), cmd.to_string());
+ cmd.throw_on_error();
}
return responses;
@@ -1130,26 +1127,6 @@ private class Geary.Imap.FolderSession : Geary.Imap.SessionObject {
return false;
}
- private void throw_on_not_ok(StatusResponse response, string cmd)
- throws ImapError {
- switch (response.status) {
- case Status.OK:
- // All good
- break;
-
- case Status.NO:
- throw new ImapError.NOT_SUPPORTED(
- "Request %s failed: %s", cmd.to_string(), response.to_string()
- );
-
- default:
- throw new ImapError.SERVER_ERROR(
- "Unknown response status to %s: %s",
- cmd.to_string(), response.to_string()
- );
- }
- }
-
private static bool required_but_not_set(Geary.Email.Field check, Geary.Email.Field users_fields,
Geary.Email email) {
return users_fields.require(check) ? !email.fields.is_all_set(check) : false;
}
diff --git a/src/engine/imap/transport/imap-client-session.vala
b/src/engine/imap/transport/imap-client-session.vala
index fabee175..27dcd9ee 100644
--- a/src/engine/imap/transport/imap-client-session.vala
+++ b/src/engine/imap/transport/imap-client-session.vala
@@ -921,40 +921,12 @@ public class Geary.Imap.ClientSession : BaseObject, Logging.Source {
// should always proceed; only an Error could change this
assert(params.proceed);
- StatusResponse response = yield command_transaction_async(
- cmd, cancellable
- );
-
- if (response.status != Status.OK) {
- // Throw an error indicating auth failed here, unless
- // there is a status response and it indicates that the
- // server is merely reporting login as being unavailable,
- // then don't since the creds might actually be fine.
- ResponseCode? code = response.response_code;
- if (code != null) {
- ResponseCodeType? code_type = code.get_response_code_type();
- if (code_type != null) {
- switch (code_type.value) {
- case ResponseCodeType.UNAVAILABLE:
- throw new ImapError.UNAVAILABLE(
- "Login restricted: %s: ", response.to_string()
- );
-
- case ResponseCodeType.AUTHENTICATIONFAILED:
- // pass through to the error being thrown below
- break;
-
- default:
- throw new ImapError.SERVER_ERROR(
- "Login error: %s: ", response.to_string()
- );
- }
- }
- }
-
- throw new ImapError.UNAUTHENTICATED(
- "Bad credentials: %s: ", response.to_string()
- );
+ yield command_transaction_async(cmd, cancellable);
+ try {
+ cmd.throw_on_error();
+ } catch (ImapError.OPERATIONAL_ERROR err) {
+ // Catch any plain errors because of a NO
+ throw new ImapError.UNAUTHENTICATED(err.message);
}
return cmd.status;
@@ -1368,21 +1340,21 @@ public class Geary.Imap.ClientSession : BaseObject, Logging.Source {
// select/examine
//
- public async StatusResponse select_async(MailboxSpecifier mailbox,
- GLib.Cancellable? cancellable)
+ public async void select_async(MailboxSpecifier mailbox,
+ GLib.Cancellable? cancellable)
throws GLib.Error {
- return yield select_examine_async(mailbox, true, cancellable);
+ yield select_examine_async(mailbox, true, cancellable);
}
- public async StatusResponse examine_async(MailboxSpecifier mailbox,
- GLib.Cancellable? cancellable)
+ public async void examine_async(MailboxSpecifier mailbox,
+ GLib.Cancellable? cancellable)
throws GLib.Error {
- return yield select_examine_async(mailbox, false, cancellable);
+ yield select_examine_async(mailbox, false, cancellable);
}
- public async StatusResponse select_examine_async(MailboxSpecifier mailbox,
- bool is_select,
- GLib.Cancellable? cancellable)
+ public async void select_examine_async(MailboxSpecifier mailbox,
+ bool is_select,
+ GLib.Cancellable? cancellable)
throws GLib.Error {
// Ternary troubles
Command cmd;
@@ -1399,7 +1371,8 @@ public class Geary.Imap.ClientSession : BaseObject, Logging.Source {
assert(params.proceed);
- return yield command_transaction_async(cmd, cancellable);
+ yield command_transaction_async(cmd, cancellable);
+ cmd.throw_on_error();
}
private uint on_select(uint state, uint event, void *user, Object? object) {
@@ -1454,7 +1427,7 @@ public class Geary.Imap.ClientSession : BaseObject, Logging.Source {
// close mailbox
//
- public async StatusResponse close_mailbox_async(GLib.Cancellable? cancellable)
+ public async void close_mailbox_async(GLib.Cancellable? cancellable)
throws GLib.Error {
CloseCommand cmd = new CloseCommand();
@@ -1464,7 +1437,8 @@ public class Geary.Imap.ClientSession : BaseObject, Logging.Source {
if (params.err != null)
throw params.err;
- return yield command_transaction_async(cmd, cancellable);
+ yield command_transaction_async(cmd, cancellable);
+ cmd.throw_on_error();
}
private uint on_close_mailbox(uint state, uint event, void *user, Object? object) {
[
Date Prev][
Date Next] [
Thread Prev][
Thread Next]
[
Thread Index]
[
Date Index]
[
Author Index]