diff --git a/src/engine/api/geary-engine.vala b/src/engine/api/geary-engine.vala index 3fee1063..157eb2b1 100644 --- a/src/engine/api/geary-engine.vala +++ b/src/engine/api/geary-engine.vala @@ -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; } diff --git a/src/engine/imap/api/imap-client-service.vala b/src/engine/imap/api/imap-client-service.vala index 26d4aa6d..adbd9b4d 100644 --- a/src/engine/imap/api/imap-client-service.vala +++ b/src/engine/imap/api/imap-client-service.vala @@ -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 diff --git a/src/engine/imap/transport/imap-client-session.vala b/src/engine/imap/transport/imap-client-session.vala index 3e95fc5b..d20d65fb 100644 --- a/src/engine/imap/transport/imap-client-session.vala +++ b/src/engine/imap/transport/imap-client-session.vala @@ -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(); } diff --git a/test/engine/imap/transport/imap-client-session-test.vala b/test/engine/imap/transport/imap-client-session-test.vala index 329e9aca..63358ed0 100644 --- a/test/engine/imap/transport/imap-client-session-test.vala +++ b/test/engine/imap/transport/imap-client-session-test.vala @@ -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); diff --git a/test/integration/imap/client-session.vala b/test/integration/imap/client-session.vala index e2d38a63..7bafd9f4 100644 --- a/test/integration/imap/client-session.vala +++ b/test/integration/imap/client-session.vala @@ -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()); }