[geary/wip/713247-tls] Fix first-account validation and retry issues



commit dfe28494bfea5e3df673be90497a39b03a0204d1
Author: Jim Nelson <jim yorba org>
Date:   Thu Aug 28 14:40:33 2014 -0700

    Fix first-account validation and retry issues

 src/client/application/geary-controller.vala      |   26 ++++++++++++++++++
 src/engine/api/geary-account-information.vala     |   30 +++++++++++++++++++++
 src/engine/api/geary-engine.vala                  |    3 +-
 src/engine/imap-db/outbox/smtp-outbox-folder.vala |    2 +-
 src/engine/smtp/smtp-client-connection.vala       |    2 +-
 src/engine/smtp/smtp-client-session.vala          |    5 ++-
 src/mailer/main.vala                              |    2 +-
 7 files changed, 64 insertions(+), 6 deletions(-)
---
diff --git a/src/client/application/geary-controller.vala b/src/client/application/geary-controller.vala
index 17dc6ca..21a2a7f 100644
--- a/src/client/application/geary-controller.vala
+++ b/src/client/application/geary-controller.vala
@@ -667,6 +667,11 @@ public class GearyController : Geary.BaseObject {
         if (result == Geary.Engine.ValidationResult.OK)
             return null;
         
+        // Because TLS warnings need cycles to process, sleep and give 'em a chance to do their
+        // thing ... note that the signal handler does *not* invoke the user prompt dialog when the
+        // login dialog is in play, so this sleep does not need to worry about user input
+        yield Geary.Scheduler.sleep_ms_async(100);
+        
         // check Endpoints for trust issues
         
         // use LoginDialog for parent only if available and visible
@@ -677,17 +682,34 @@ public class GearyController : Geary.BaseObject {
             parent = main_window;
         
         bool prompted = false;
+        bool retry = false;
         
+        // If IMAP had unresolved TLS issues, prompt user about them
         Geary.Endpoint endpoint = account_information.get_imap_endpoint();
         if (endpoint.tls_validation_warnings != 0 && endpoint.trust_untrusted_host != Geary.Trillian.TRUE) {
             prompt_for_untrusted_host(parent, account_information, endpoint, Geary.Service.IMAP);
             prompted = true;
+        } else if (endpoint.tls_validation_warnings != 0 && endpoint.trust_untrusted_host == 
Geary.Trillian.TRUE) {
+            // If there were TLS connection issues that caused the connection to fail (happens on the
+            // first attempt), clear those errors and retry
+            if ((result & Geary.Engine.ValidationResult.IMAP_CONNECTION_FAILED) != 0) {
+                result &= ~Geary.Engine.ValidationResult.IMAP_CONNECTION_FAILED;
+                retry = true;
+            }
         }
         
+        // If SMTP had unresolved TLS issues, prompt user about them
         endpoint = account_information.get_smtp_endpoint();
         if (endpoint.tls_validation_warnings != 0 && endpoint.trust_untrusted_host != Geary.Trillian.TRUE) {
             prompt_for_untrusted_host(parent, account_information, endpoint, Geary.Service.SMTP);
             prompted = true;
+        } else if (endpoint.tls_validation_warnings != 0 && endpoint.trust_untrusted_host == 
Geary.Trillian.TRUE) {
+            // If there were TLS connection issues that caused the connection to fail (happens on the
+            // first attempt), clear those errors and retry
+            if ((result & Geary.Engine.ValidationResult.SMTP_CONNECTION_FAILED) != 0) {
+                result &= ~Geary.Engine.ValidationResult.SMTP_CONNECTION_FAILED;
+                retry = true;
+            }
         }
         
         // if prompted for user acceptance of bad certificates and they agreed to both, try again;
@@ -698,6 +720,10 @@ public class GearyController : Geary.BaseObject {
             return account_information;
         }
         
+        // if retry required, do it now
+        if (retry)
+            return account_information;
+        
         debug("Validation failed. Prompting user for revised account information");
         Geary.AccountInformation? new_account_information =
             request_account_information(account_information, result);
diff --git a/src/engine/api/geary-account-information.vala b/src/engine/api/geary-account-information.vala
index ee9c76a..2783df4 100644
--- a/src/engine/api/geary-account-information.vala
+++ b/src/engine/api/geary-account-information.vala
@@ -50,6 +50,8 @@ public class Geary.AccountInformation : BaseObject {
     
     public static int default_ordinal = 0;
     
+    private static Gee.HashMap<string, Geary.Endpoint>? known_endpoints = null;
+    
     internal File? settings_dir = null;
     internal File? file = null;
     
@@ -201,6 +203,24 @@ public class Geary.AccountInformation : BaseObject {
             smtp_endpoint.untrusted_host.disconnect(on_smtp_untrusted_host);
     }
     
+    internal static void init() {
+        known_endpoints = new Gee.HashMap<string, Geary.Endpoint>();
+    }
+    
+    private static Geary.Endpoint get_shared_endpoint(Service service, Endpoint endpoint) {
+        string key = "%s/%s:%u".printf(service.user_label(), endpoint.remote_address.hostname,
+            endpoint.remote_address.port);
+        
+        // if already known, prefer it over this one
+        if (known_endpoints.has_key(key))
+            return known_endpoints.get(key);
+        
+        // save for future use and return this one
+        known_endpoints.set(key, endpoint);
+        
+        return endpoint;
+    }
+    
     // Copies all data from the "from" object into this one.
     public void copy_from(AccountInformation from) {
         real_name = from.real_name;
@@ -501,6 +521,11 @@ public class Geary.AccountInformation : BaseObject {
                 assert_not_reached();
         }
         
+        // look for existing one in the global pool; want to use that because Endpoint is mutable
+        // and signalled in such a way that it's better to share them
+        imap_endpoint = get_shared_endpoint(Service.IMAP, imap_endpoint);
+        
+        // bind shared Endpoint signal to this AccountInformation's signal
         imap_endpoint.untrusted_host.connect(on_imap_untrusted_host);
         
         return imap_endpoint;
@@ -550,6 +575,11 @@ public class Geary.AccountInformation : BaseObject {
                 assert_not_reached();
         }
         
+        // look for existing one in the global pool; want to use that because Endpoint is mutable
+        // and signalled in such a way that it's better to share them
+        smtp_endpoint = get_shared_endpoint(Service.SMTP, smtp_endpoint);
+        
+        // bind shared Endpoint signal to this AccountInformation's signal
         smtp_endpoint.untrusted_host.connect(on_smtp_untrusted_host);
         
         return smtp_endpoint;
diff --git a/src/engine/api/geary-engine.vala b/src/engine/api/geary-engine.vala
index 7415253..58ce176 100644
--- a/src/engine/api/geary-engine.vala
+++ b/src/engine/api/geary-engine.vala
@@ -111,6 +111,7 @@ public class Geary.Engine : BaseObject {
         
         is_initialized = true;
         
+        AccountInformation.init();
         Logging.init();
         RFC822.init();
         ImapEngine.init();
@@ -298,7 +299,7 @@ public class Geary.Engine : BaseObject {
         }
         
         try {
-            yield smtp_session.logout_async(cancellable);
+            yield smtp_session.logout_async(true, cancellable);
         } catch (Error err) {
             // ignored
         } finally {
diff --git a/src/engine/imap-db/outbox/smtp-outbox-folder.vala 
b/src/engine/imap-db/outbox/smtp-outbox-folder.vala
index 69fd30a..745967b 100644
--- a/src/engine/imap-db/outbox/smtp-outbox-folder.vala
+++ b/src/engine/imap-db/outbox/smtp-outbox-folder.vala
@@ -644,7 +644,7 @@ private class Geary.SmtpOutboxFolder : Geary.AbstractLocalFolder, Geary.FolderSu
         
         // always logout
         try {
-            yield smtp.logout_async(cancellable);
+            yield smtp.logout_async(false, cancellable);
         } catch (Error err) {
             debug("Unable to disconnect from SMTP server %s: %s", smtp.to_string(), err.message);
         }
diff --git a/src/engine/smtp/smtp-client-connection.vala b/src/engine/smtp/smtp-client-connection.vala
index 1010eed..6d4bf1c 100644
--- a/src/engine/smtp/smtp-client-connection.vala
+++ b/src/engine/smtp/smtp-client-connection.vala
@@ -9,7 +9,7 @@ public class Geary.Smtp.ClientConnection {
     public const uint16 DEFAULT_PORT_SSL = 465;
     public const uint16 DEFAULT_PORT_STARTTLS = 587;
     
-    public const uint DEFAULT_TIMEOUT_SEC = 60;
+    public const uint DEFAULT_TIMEOUT_SEC = 20;
     
     public Geary.Smtp.Capabilities? capabilities { get; private set; default = null; }
     
diff --git a/src/engine/smtp/smtp-client-session.vala b/src/engine/smtp/smtp-client-session.vala
index a3b96a1..db950f6 100644
--- a/src/engine/smtp/smtp-client-session.vala
+++ b/src/engine/smtp/smtp-client-session.vala
@@ -109,10 +109,11 @@ public class Geary.Smtp.ClientSession {
         throw new SmtpError.AUTHENTICATION_FAILED("Unable to authenticate with %s", to_string());
     }
     
-    public async Response? logout_async(Cancellable? cancellable = null) throws Error {
+    public async Response? logout_async(bool force, Cancellable? cancellable = null) throws Error {
         Response? response = null;
         try {
-            response = yield cx.quit_async(cancellable);
+            if (!force)
+                response = yield cx.quit_async(cancellable);
         } catch (Error err) {
             // catch because although error occurred, still attempt to close the connection
             message("Unable to QUIT: %s", err.message);
diff --git a/src/mailer/main.vala b/src/mailer/main.vala
index 50daab2..8fac48f 100644
--- a/src/mailer/main.vala
+++ b/src/mailer/main.vala
@@ -53,7 +53,7 @@ async void main_async() throws Error {
         stdout.printf("Sent email #%d\n", ctr);
     }
     
-    Geary.Smtp.Response? logout = yield session.logout_async();
+    Geary.Smtp.Response? logout = yield session.logout_async(false);
     stdout.printf("%s\n", logout.to_string());
 }
 


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]