nettika/geary
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Update Geary.Imap.ClientSession connect timeout handling

Browse source 
Allow specifying the connect greeting timeout length, ensure that
any connect errors are in place before releasing the connect waiter,
add unit test to ensure it works properly.
This commit is contained in:
Michael Gratton 2019-12-29 17:01:28 +10:30 • committed by Michael James Gratton
parent 54156003b4
commit 13d43d41b2
5 changed files with 91 additions and 48 deletions
Download patch file Download diff file Expand all files Collapse all files

6
src/engine/api/geary-engine.vala
View file

@ -284,10 +284,12 @@ public class Geary.Engine : BaseObject {
(security, cx) => account.untrusted_host(service, security, cx)
);
Geary.Imap.ClientSession client = new Imap.ClientSession(endpoint);
var client = new Imap.ClientSession(endpoint);
GLib.Error? imap_err = null;
try {
yield client.connect_async(cancellable);
yield client.connect_async(
Imap.ClientSession.DEFAULT_GREETING_TIMEOUT_SEC, cancellable
);
} catch (GLib.Error err) {
imap_err = err;
}

6
src/engine/imap/api/imap-client-service.vala
View file

@ -447,11 +447,13 @@ internal class Geary.Imap.ClientService : Geary.ClientService {
ClientSession new_session = new ClientSession(remote);
new_session.set_logging_parent(this);
yield new_session.connect_async(cancellable);
yield new_session.connect_async(
ClientSession.DEFAULT_GREETING_TIMEOUT_SEC, cancellable
);
try {
yield new_session.initiate_session_async(login, cancellable);
} catch (Error err) {
} catch (GLib.Error err) {
// need to disconnect before throwing error ... don't
// honor Cancellable here, it's important to disconnect
// the client before dropping the ref

82
src/engine/imap/transport/imap-client-session.vala
View file

@ -74,7 +74,9 @@ public class Geary.Imap.ClientSession : BaseObject, Logging.Source {
/** Default keep-alive interval when not in the Selected state. */
public const uint DEFAULT_UNSELECTED_KEEPALIVE_SEC = RECOMMENDED_KEEPALIVE_SEC;
private const uint GREETING_TIMEOUT_SEC = Command.DEFAULT_RESPONSE_TIMEOUT_SEC;
/** Default time to wait for the server greeting when connecting. */
public const uint DEFAULT_GREETING_TIMEOUT_SEC =
Command.DEFAULT_RESPONSE_TIMEOUT_SEC;
/**
@ -659,8 +661,10 @@ public class Geary.Imap.ClientSession : BaseObject, Logging.Source {
* the server (that is, not a network problem). The {@link
* ClientSession} should be discarded.
*/
public async void connect_async(GLib.Cancellable? cancellable)
throws GLib.Error {
public async void connect_async(
uint greeting_timeout_sec = DEFAULT_GREETING_TIMEOUT_SEC,
GLib.Cancellable? cancellable = null
) throws GLib.Error {
MachineParams params = new MachineParams(null);
fsm.issue(Event.CONNECT, null, params);
@ -683,10 +687,13 @@ public class Geary.Imap.ClientSession : BaseObject, Logging.Source {
}
// set up timer to wait for greeting from server
Scheduler.Scheduled timeout = Scheduler.after_sec(GREETING_TIMEOUT_SEC, on_greeting_timeout);
Scheduler.Scheduled timeout = Scheduler.after_sec(
greeting_timeout_sec, on_greeting_timeout
);
// wait for the initial greeting or a timeout ... this prevents the caller from turning
// around and issuing a command while still in CONNECTING state
// wait for the initial greeting or a timeout ... this
// prevents the caller from turning around and issuing a
// command while still in CONNECTING state
try {
yield connect_waiter.wait_async(cancellable);
} catch (GLib.IOError.CANCELLED err) {
@ -783,7 +790,16 @@ public class Geary.Imap.ClientSession : BaseObject, Logging.Source {
private uint on_connecting_recv_status(uint state, uint event, void *user, Object? object) {
StatusResponse status_response = (StatusResponse) object;
// see on_connected() why signals and semaphore are delayed for this event
uint new_state = State.NOAUTH;
if (status_response.status != Status.OK) {
// Don't need to manually disconnect here, by setting
// connect_err here that will be done in connect_async
this.connect_err = new ImapError.UNAVAILABLE(
"Session denied: %s", status_response.get_text()
);
new_state = State.LOGOUT;
}
try {
connect_waiter.notify();
} catch (Error err) {
@ -793,25 +809,16 @@ public class Geary.Imap.ClientSession : BaseObject, Logging.Source {
);
}
if (status_response.status == Status.OK) {
fsm.do_post_transition(() => { connected(); });
return State.NOAUTH;
}
fsm.do_post_transition(() => { session_denied(status_response.get_text()); });
// Don't need to manually disconnect here, by setting
// connect_err here that will be done in connect_async
this.connect_err = new ImapError.UNAVAILABLE(
"Session denied: %s", status_response.get_text()
);
return State.LOGOUT;
return new_state;
}
private uint on_connecting_timeout(uint state, uint event) {
// wake up the waiting task in connect_async
// Don't need to manually disconnect here, by setting
// connect_err here that will be done in connect_async
this.connect_err = new GLib.IOError.TIMED_OUT(
"Session greeting not sent"
);
try {
connect_waiter.notify();
} catch (Error err) {
@ -820,13 +827,6 @@ public class Geary.Imap.ClientSession : BaseObject, Logging.Source {
);
}
// Don't need to manually disconnect here, by setting
// connect_err here that will be done in connect_async
this.connect_err = new IOError.TIMED_OUT(
"Session greeting not seen in %u seconds",
GREETING_TIMEOUT_SEC
);
return State.LOGOUT;
}
@ -1093,18 +1093,24 @@ public class Geary.Imap.ClientSession : BaseObject, Logging.Source {
//
/**
* If seconds is negative or zero, keepalives will be disabled. (This is not recommended.)
* Enables sending keep-alive commands for the sesion.
*
* Although keepalives can be enabled at any time, if they're enabled and trigger sending
* a command prior to connection, error signals may be fired.
* Although keepalives can be enabled at any time, if they're
* enabled and trigger sending a command prior to connection,
* error signals may be fired.
*
* If values are negative or zero, keepalives will be disabled.
* (This is not recommended.)
*/
public void enable_keepalives(uint seconds_while_selected,
uint seconds_while_unselected, uint seconds_while_selected_with_idle) {
selected_keepalive_secs = seconds_while_selected;
selected_with_idle_keepalive_secs = seconds_while_selected_with_idle;
unselected_keepalive_secs = seconds_while_unselected;
uint seconds_while_unselected,
uint seconds_while_selected_with_idle) {
this.selected_keepalive_secs = seconds_while_selected;
this.selected_with_idle_keepalive_secs = seconds_while_selected_with_idle;
this.unselected_keepalive_secs = seconds_while_unselected;
// schedule one now, although will be rescheduled if traffic is received before it fires
// schedule one now, although will be rescheduled if traffic
// is received before it fires
schedule_keepalive();
}

41
test/engine/imap/transport/imap-client-session-test.vala
View file

@ -7,6 +7,7 @@
class Geary.Imap.ClientSessionTest : TestCase {
private const uint CONNECT_TIMEOUT = 2;
private TestServer? server = null;
@ -14,6 +15,9 @@ class Geary.Imap.ClientSessionTest : TestCase {
public ClientSessionTest() {
base("Geary.Imap.ClientSessionTest");
add_test("connect_disconnect", connect_disconnect);
if (GLib.Test.slow()) {
add_test("connect_timeout", connect_timeout);
}
add_test("login", login);
add_test("logout", logout);
add_test("login_logout", login_logout);
@ -37,7 +41,9 @@ class Geary.Imap.ClientSessionTest : TestCase {
var test_article = new ClientSession(new_endpoint());
assert_true(test_article.get_protocol_state(null) == NOT_CONNECTED);
test_article.connect_async.begin(null, this.async_complete_full);
test_article.connect_async.begin(
CONNECT_TIMEOUT, null, this.async_complete_full
);
test_article.connect_async.end(async_result());
assert_true(test_article.get_protocol_state(null) == UNAUTHORIZED);
@ -52,6 +58,27 @@ class Geary.Imap.ClientSessionTest : TestCase {
);
}
public void connect_timeout() throws GLib.Error {
this.server.add_script_line(WAIT_FOR_DISCONNECT, "");
var test_article = new ClientSession(new_endpoint());
GLib.Timer timer = new GLib.Timer();
timer.start();
test_article.connect_async.begin(
CONNECT_TIMEOUT, null, this.async_complete_full
);
try {
test_article.connect_async.end(async_result());
assert_not_reached();
} catch (GLib.IOError.TIMED_OUT err) {
assert_double(timer.elapsed(), CONNECT_TIMEOUT, 0.5);
}
TestServer.Result result = this.server.wait_for_script(this.main_loop);
assert_true(result.succeeded);
}
public void login() throws GLib.Error {
this.server.add_script_line(
SEND_LINE, "* OK localhost test server ready"
@ -63,7 +90,9 @@ class Geary.Imap.ClientSessionTest : TestCase {
var test_article = new ClientSession(new_endpoint());
assert_true(test_article.get_protocol_state(null) == NOT_CONNECTED);
test_article.connect_async.begin(null, this.async_complete_full);
test_article.connect_async.begin(
CONNECT_TIMEOUT, null, this.async_complete_full
);
test_article.connect_async.end(async_result());
assert_true(test_article.get_protocol_state(null) == UNAUTHORIZED);
@ -98,7 +127,9 @@ class Geary.Imap.ClientSessionTest : TestCase {
var test_article = new ClientSession(new_endpoint());
assert_true(test_article.get_protocol_state(null) == NOT_CONNECTED);
test_article.connect_async.begin(null, this.async_complete_full);
test_article.connect_async.begin(
CONNECT_TIMEOUT, null, this.async_complete_full
);
test_article.connect_async.end(async_result());
assert_true(test_article.get_protocol_state(null) == UNAUTHORIZED);
@ -127,7 +158,9 @@ class Geary.Imap.ClientSessionTest : TestCase {
var test_article = new ClientSession(new_endpoint());
assert_true(test_article.get_protocol_state(null) == NOT_CONNECTED);
test_article.connect_async.begin(null, this.async_complete_full);
test_article.connect_async.begin(
CONNECT_TIMEOUT, null, this.async_complete_full
);
test_article.connect_async.end(async_result());
assert_true(test_article.get_protocol_state(null) == UNAUTHORIZED);

4
test/integration/imap/client-session.vala
View file

@ -41,7 +41,7 @@ class Integration.Imap.ClientSession : TestCase {
}
public void session_connect() throws GLib.Error {
this.session.connect_async.begin(null, async_complete_full);
this.session.connect_async.begin(2, null, async_complete_full);
this.session.connect_async.end(async_result());
this.session.disconnect_async.begin(null, async_complete_full);
@ -98,7 +98,7 @@ class Integration.Imap.ClientSession : TestCase {
}
private void do_connect() throws GLib.Error {
this.session.connect_async.begin(null, async_complete_full);
this.session.connect_async.begin(5, null, async_complete_full);
this.session.connect_async.end(async_result());
}