[geary/wip/714104-refine-account-dialog] Modernise how Endpoints are constructed, used and managed.



commit d942abac5c468adf9654374b5e197ee72094337a
Author: Michael Gratton <mike vee net>
Date:   Sat Sep 1 22:42:51 2018 +1000

    Modernise how Endpoints are constructed, used and managed.
    
    Use the new TlsNegotiationMethod enum rather than Endpoint's flags
    and simply require StartTLS negotiation if requested. Move the
    endpoint cache into the Engine, since that actually uses it and is
    looking for something to do these days. Have service-specific
    providers setup a ServiceInformation instead of endpoints, so their
    service info shows up in the accounts editor.
    
    Update call sites etc.

 src/client/accounts/accounts-editor-add-pane.vala  |   4 +-
 src/client/accounts/accounts-manager.vala          |   3 +
 src/client/application/geary-controller.vala       |  10 +-
 src/console/main.vala                              |  22 ++--
 src/engine/api/geary-account-information.vala      | 123 +++----------------
 src/engine/api/geary-endpoint.vala                 | 133 ++++++++-------------
 src/engine/api/geary-engine.vala                   | 112 +++++++++++++++--
 src/engine/api/geary-service-information.vala      |   4 +-
 src/engine/api/geary-service-provider.vala         |  14 +++
 .../gmail/imap-engine-gmail-account.vala           |  28 +++--
 .../outlook/imap-engine-outlook-account.vala       |  37 +++---
 .../yahoo/imap-engine-yahoo-account.vala           |  31 ++---
 src/engine/imap/transport/imap-client-session.vala |  75 ++++++------
 src/engine/smtp/smtp-client-connection.vala        |  48 ++++----
 src/mailer/main.vala                               |  19 +--
 15 files changed, 316 insertions(+), 347 deletions(-)
---
diff --git a/src/client/accounts/accounts-editor-add-pane.vala 
b/src/client/accounts/accounts-editor-add-pane.vala
index 72523db7..3e746be0 100644
--- a/src/client/accounts/accounts-editor-add-pane.vala
+++ b/src/client/accounts/accounts-editor-add-pane.vala
@@ -223,6 +223,7 @@ internal class Accounts.EditorAddPane : Gtk.Grid, EditorPane {
             service.host = address.hostname;
             service.port = (uint16) address.port;
         } else {
+            this.provider.setup_service(service);
             service.credentials = new Geary.Credentials(
                 Geary.Credentials.Method.PASSWORD,
                 this.email.value.get_text().strip(),
@@ -268,8 +269,7 @@ internal class Accounts.EditorAddPane : Gtk.Grid, EditorPane {
             service.host = address.hostname;
             service.port = (uint16) address.port;
         } else {
-            service.smtp_noauth = false;
-            service.smtp_use_imap_credentials = true;
+            this.provider.setup_service(service);
         }
 
         return service;
diff --git a/src/client/accounts/accounts-manager.vala b/src/client/accounts/accounts-manager.vala
index 87701de0..8a6fed26 100644
--- a/src/client/accounts/accounts-manager.vala
+++ b/src/client/accounts/accounts-manager.vala
@@ -785,6 +785,9 @@ public class Accounts.Manager : GLib.Object {
         if (provider == Geary.ServiceProvider.OTHER) {
             imap.load_settings(imap_config);
             smtp.load_settings(smtp_config);
+        } else {
+            provider.setup_service(imap);
+            provider.setup_service(smtp);
         }
 
         return new Geary.AccountInformation(
diff --git a/src/client/application/geary-controller.vala b/src/client/application/geary-controller.vala
index 066efbce..fdba25b3 100644
--- a/src/client/application/geary-controller.vala
+++ b/src/client/application/geary-controller.vala
@@ -631,15 +631,15 @@ public class GearyController : Geary.BaseObject {
 
     private void on_untrusted_host(Geary.AccountInformation account,
                                    Geary.ServiceInformation service,
-                                   Geary.Endpoint.SecurityType security,
+                                   Geary.TlsNegotiationMethod method,
                                    TlsConnection cx) {
-        this.prompt_untrusted_host_async.begin(account, service, security, cx);
+        this.prompt_untrusted_host_async.begin(account, service, method, cx);
     }
 
     private async void
         prompt_untrusted_host_async(Geary.AccountInformation account,
                                     Geary.ServiceInformation service,
-                                    Geary.Endpoint.SecurityType security,
+                                    Geary.TlsNegotiationMethod method,
                                     TlsConnection cx) {
         // use a mutex to prevent multiple dialogs popping up at the same time
         int token = Geary.Nonblocking.Mutex.INVALID_TOKEN;
@@ -650,7 +650,7 @@ public class GearyController : Geary.BaseObject {
             return;
         }
 
-        yield locked_prompt_untrusted_host_async(account, service, security, cx);
+        yield locked_prompt_untrusted_host_async(account, service, method, cx);
 
         try {
             untrusted_host_prompt_mutex.release(ref token);
@@ -663,7 +663,7 @@ public class GearyController : Geary.BaseObject {
     private async void
         locked_prompt_untrusted_host_async(Geary.AccountInformation account,
                                            Geary.ServiceInformation service,
-                                           Geary.Endpoint.SecurityType security,
+                                           Geary.TlsNegotiationMethod method,
                                            TlsConnection cx) {
         // possible while waiting on mutex that this endpoint became trusted or untrusted
         if (service.endpoint.trust_untrusted_host != Geary.Trillian.UNKNOWN)
diff --git a/src/console/main.vala b/src/console/main.vala
index f7b7037a..0d1b3fe6 100644
--- a/src/console/main.vala
+++ b/src/console/main.vala
@@ -308,15 +308,21 @@ class ImapConsole : Gtk.Window {
             throw new CommandException.STATE("'logout' required");
         
         check_args(cmd, args, 1, "hostname[:port]");
-        
-        Geary.Endpoint.Flags flags = Geary.Endpoint.Flags.NONE;
-        if (cmd != "unsecure")
-            flags |= Geary.Endpoint.Flags.SSL;
-        
+
+        Geary.TlsNegotiationMethod method = Geary.TlsNegotiationMethod.TRANSPORT;
+        if (cmd == "unsecure") {
+            method = Geary.TlsNegotiationMethod.START_TLS;
+        }
+
         cx = new Geary.Imap.ClientConnection(
-            new Geary.Endpoint(args[0], Geary.Imap.ClientConnection.DEFAULT_PORT_SSL,
-                flags, Geary.Imap.ClientConnection.DEFAULT_TIMEOUT_SEC));
-        
+            new Geary.Endpoint(
+                args[0],
+                Geary.Imap.ClientConnection.DEFAULT_PORT_SSL,
+                method,
+                Geary.Imap.ClientConnection.DEFAULT_TIMEOUT_SEC
+            )
+        );
+
         cx.sent_command.connect(on_sent_command);
         cx.received_status_response.connect(on_received_status_response);
         cx.received_server_data.connect(on_received_server_data);
diff --git a/src/engine/api/geary-account-information.vala b/src/engine/api/geary-account-information.vala
index 9285441a..886c071f 100644
--- a/src/engine/api/geary-account-information.vala
+++ b/src/engine/api/geary-account-information.vala
@@ -18,10 +18,6 @@ public class Geary.AccountInformation : BaseObject {
     public static int next_ordinal = 0;
 
 
-    private static Gee.HashMap<string,weak Endpoint> known_endpoints =
-        new Gee.HashMap<string,weak Endpoint>();
-
-
     /** Comparator for account info objects based on their ordinals. */
     public static int compare_ascending(AccountInformation a, AccountInformation b) {
         int diff = a.ordinal - b.ordinal;
@@ -32,25 +28,6 @@ public class Geary.AccountInformation : BaseObject {
         return a.display_name.collate(b.display_name);
     }
 
-    private static Geary.Endpoint get_shared_endpoint(ServiceInformation service,
-                                                      Endpoint endpoint) {
-        string key = "%s/%s:%u".printf(
-            service.protocol.to_value(),
-            endpoint.remote_address.hostname,
-            endpoint.remote_address.port
-        );
-
-        weak Endpoint? cached = AccountInformation.known_endpoints.get(key);
-        weak Endpoint? shared = endpoint;
-        if (cached != null) {
-            shared = cached;
-            AccountInformation.known_endpoints.set(key, shared);
-        }
-
-        return shared;
-    }
-
-
     /** Location of the account information's settings key file. */
     public File? settings_file {
         owned get {
@@ -203,8 +180,8 @@ public class Geary.AccountInformation : BaseObject {
      * the future.
      */
     public signal void untrusted_host(ServiceInformation service,
-                                      Endpoint.SecurityType security,
-                                      TlsConnection cx);
+                                      TlsNegotiationMethod method,
+                                      GLib.TlsConnection cx);
 
     /** Indicates that properties contained herein have changed. */
     public signal void information_changed();
@@ -248,7 +225,7 @@ public class Geary.AccountInformation : BaseObject {
     }
 
     ~AccountInformation() {
-        disconnect_endpoints();
+        disconnect_service_endpoints();
     }
 
 
@@ -584,25 +561,25 @@ public class Geary.AccountInformation : BaseObject {
         );
     }
 
-    internal void connect_imap_endpoint() {
+    internal void connect_imap_service(Endpoint service) {
         if (this.imap.endpoint == null) {
-            this.imap.endpoint = get_imap_endpoint();
+            this.imap.endpoint = service;
             this.imap.endpoint.untrusted_host.connect(
                 on_imap_untrusted_host
             );
         }
     }
 
-    internal void connect_smtp_endpoint() {
+    internal void connect_smtp_service(Endpoint service) {
         if (this.smtp.endpoint == null) {
-            this.smtp.endpoint = get_smtp_endpoint();
+            this.smtp.endpoint = service;
             this.smtp.endpoint.untrusted_host.connect(
                 on_smtp_untrusted_host
             );
         }
     }
 
-    internal void disconnect_endpoints() {
+    internal void disconnect_service_endpoints() {
         if (this.imap.endpoint != null) {
             this.imap.endpoint.untrusted_host.disconnect(
                 on_imap_untrusted_host
@@ -617,78 +594,6 @@ public class Geary.AccountInformation : BaseObject {
         }
     }
 
-    private Endpoint get_imap_endpoint() {
-        Endpoint? imap_endpoint = null;
-        switch (this.service_provider) {
-            case ServiceProvider.GMAIL:
-                imap_endpoint = ImapEngine.GmailAccount.generate_imap_endpoint();
-            break;
-
-            case ServiceProvider.YAHOO:
-                imap_endpoint = ImapEngine.YahooAccount.generate_imap_endpoint();
-            break;
-
-            case ServiceProvider.OUTLOOK:
-                imap_endpoint = ImapEngine.OutlookAccount.generate_imap_endpoint();
-            break;
-
-            case ServiceProvider.OTHER:
-                Endpoint.Flags imap_flags = Endpoint.Flags.NONE;
-                if (this.imap.use_ssl)
-                    imap_flags |= Endpoint.Flags.SSL;
-                if (this.imap.use_starttls)
-                    imap_flags |= Endpoint.Flags.STARTTLS;
-
-                imap_endpoint = new Endpoint(this.imap.host, this.imap.port,
-                    imap_flags, Imap.ClientConnection.RECOMMENDED_TIMEOUT_SEC);
-            break;
-
-            default:
-                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
-        return get_shared_endpoint(this.imap, imap_endpoint);
-    }
-
-    private Endpoint get_smtp_endpoint() {
-        Endpoint? smtp_endpoint = null;
-        switch (service_provider) {
-            case ServiceProvider.GMAIL:
-                smtp_endpoint = ImapEngine.GmailAccount.generate_smtp_endpoint();
-            break;
-
-            case ServiceProvider.YAHOO:
-                smtp_endpoint = ImapEngine.YahooAccount.generate_smtp_endpoint();
-            break;
-
-            case ServiceProvider.OUTLOOK:
-                smtp_endpoint = ImapEngine.OutlookAccount.generate_smtp_endpoint();
-            break;
-
-            case ServiceProvider.OTHER:
-                Endpoint.Flags smtp_flags = Endpoint.Flags.NONE;
-                if (this.smtp.use_ssl)
-                    smtp_flags |= Endpoint.Flags.SSL;
-                if (this.smtp.use_starttls)
-                    smtp_flags |= Endpoint.Flags.STARTTLS;
-
-                smtp_endpoint = new Endpoint(this.smtp.host, this.smtp.port,
-                    smtp_flags, Smtp.ClientConnection.DEFAULT_TIMEOUT_SEC);
-            break;
-
-            default:
-                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
-        return get_shared_endpoint(this.smtp, smtp_endpoint);
-    }
-
     public static Geary.FolderPath? build_folder_path(Gee.List<string>? parts) {
         if (parts == null || parts.size == 0)
             return null;
@@ -699,14 +604,14 @@ public class Geary.AccountInformation : BaseObject {
         return path;
     }
 
-    private void on_imap_untrusted_host(Endpoint.SecurityType security,
-                                        TlsConnection cx) {
-        untrusted_host(this.imap, security, cx);
+    private void on_imap_untrusted_host(TlsNegotiationMethod method,
+                                        GLib.TlsConnection cx) {
+        untrusted_host(this.imap, method, cx);
     }
 
-    private void on_smtp_untrusted_host(Endpoint.SecurityType security,
-                                        TlsConnection cx) {
-        untrusted_host(this.smtp, security, cx);
+    private void on_smtp_untrusted_host(TlsNegotiationMethod method,
+                                        GLib.TlsConnection cx) {
+        untrusted_host(this.smtp, method, cx);
     }
 
 }
diff --git a/src/engine/api/geary-endpoint.vala b/src/engine/api/geary-endpoint.vala
index 1be7b856..6ea42eea 100644
--- a/src/engine/api/geary-endpoint.vala
+++ b/src/engine/api/geary-endpoint.vala
@@ -10,39 +10,13 @@
  */
 
 public class Geary.Endpoint : BaseObject {
+
     public const string PROP_TRUST_UNTRUSTED_HOST = "trust-untrusted-host";
-    
-    [Flags]
-    public enum Flags {
-        NONE = 0,
-        SSL,
-        STARTTLS;
-        
-        public inline bool is_all_set(Flags flags) {
-            return (this & flags) == flags;
-        }
-        
-        public inline bool is_any_set(Flags flags) {
-            return (this & flags) != 0;
-        }
-    }
-    
-    public enum SecurityType {
-        NONE,
-        SSL,
-        STARTTLS
-    }
-    
-    public enum AttemptStarttls {
-        YES,
-        NO,
-        HALT
-    }
-    
+
     public NetworkAddress remote_address { get; private set; }
     public ConnectivityManager connectivity { get; private set; }
-    public Flags flags { get; private set; }
     public uint timeout_sec { get; private set; }
+    public TlsNegotiationMethod tls_method { get; private set; }
     public TlsCertificateFlags tls_validation_flags { get; set; default = TlsCertificateFlags.VALIDATE_ALL; }
 
     /**
@@ -98,17 +72,8 @@ public class Geary.Endpoint : BaseObject {
                 : trust_untrusted_host.is_possible();
         }
     }
-    
-    public bool is_ssl { get {
-        return flags.is_all_set(Flags.SSL);
-    } }
-    
-    public bool use_starttls { get {
-        return flags.is_all_set(Flags.STARTTLS);
-    } }
-    
     private SocketClient? socket_client = null;
-    
+
     /**
      * Fired when TLS certificate warnings are detected and the caller has not marked this
      * {@link Endpoint} as trusted via {@link trust_untrusted_host}.
@@ -119,28 +84,32 @@ public class Geary.Endpoint : BaseObject {
      *
      * @see tls_validation_warnings
      */
-    public signal void untrusted_host(SecurityType security, TlsConnection cx);
+    public signal void untrusted_host(TlsNegotiationMethod method,
+                                      GLib.TlsConnection cx);
 
 
-    public Endpoint(string host_specifier, uint16 default_port, Flags flags, uint timeout_sec) {
-        this.remote_address = new NetworkAddress(host_specifier, default_port);
-        this.flags = flags;
-        this.timeout_sec = timeout_sec;
+    public Endpoint(string host_specifier,
+                    uint16 port,
+                    TlsNegotiationMethod method,
+                    uint timeout_sec) {
+        this.remote_address = new NetworkAddress(host_specifier, port);
         this.connectivity = new ConnectivityManager(this.remote_address);
+        this.timeout_sec = timeout_sec;
+        this.tls_method = method;
     }
 
     private SocketClient get_socket_client() {
         if (socket_client != null)
             return socket_client;
-        
+
         socket_client = new SocketClient();
-        
-        if (is_ssl) {
+
+        if (this.tls_method == TlsNegotiationMethod.TRANSPORT) {
             socket_client.set_tls(true);
             socket_client.set_tls_validation_flags(tls_validation_flags);
             socket_client.event.connect(on_socket_client_event);
         }
-        
+
         socket_client.set_timeout(timeout_sec);
         
         return socket_client;
@@ -177,31 +146,44 @@ public class Geary.Endpoint : BaseObject {
             tls_cx.accept_certificate.connect(on_accept_ssl_certificate);
     }
 
-    private bool on_accept_starttls_certificate(TlsConnection cx, TlsCertificate cert, TlsCertificateFlags 
flags) {
-        return report_tls_warnings(SecurityType.STARTTLS, cx, cert, flags);
+    private bool on_accept_starttls_certificate(GLib.TlsConnection cx,
+                                                GLib.TlsCertificate cert,
+                                                GLib.TlsCertificateFlags flags) {
+        return report_tls_warnings(
+            TlsNegotiationMethod.START_TLS, cx, cert, flags
+        );
     }
-    
-    private bool on_accept_ssl_certificate(TlsConnection cx, TlsCertificate cert, TlsCertificateFlags flags) 
{
-        return report_tls_warnings(SecurityType.SSL, cx, cert, flags);
+
+    private bool on_accept_ssl_certificate(GLib.TlsConnection cx,
+                                           GLib.TlsCertificate cert,
+                                           GLib.TlsCertificateFlags flags) {
+        return report_tls_warnings(
+            TlsNegotiationMethod.TRANSPORT, cx, cert, flags
+        );
     }
-    
-    private bool report_tls_warnings(SecurityType security, TlsConnection cx, TlsCertificate cert,
-        TlsCertificateFlags warnings) {
-        // TODO: Report or verify flags with user, but for now merely log for informational/debugging
-        // reasons and accede
-        message("%s TLS warnings connecting to %s: %Xh (%s)", security.to_string(), to_string(), warnings,
-            tls_flags_to_string(warnings));
-        
+
+    private bool report_tls_warnings(TlsNegotiationMethod method,
+                                     GLib.TlsConnection cx,
+                                     GLib.TlsCertificate cert,
+                                     GLib.TlsCertificateFlags warnings) {
+        // TODO: Report or verify flags with user, but for now merely
+        // log for informational/debugging reasons and accede
+        message(
+            "%s TLS warnings connecting to %s: %Xh (%s)",
+            method.to_string(), to_string(), warnings,
+            tls_flags_to_string(warnings)
+        );
+
         tls_validation_warnings = warnings;
         untrusted_certificate = cert;
         
         // if user has marked this untrusted host as trusted already, accept warnings and move on
         if (trust_untrusted_host == Trillian.TRUE)
             return true;
-        
+
         // signal an issue has been detected and return false to deny the connection
-        untrusted_host(security, cx);
-        
+        untrusted_host(this.tls_method, cx);
+
         return false;
     }
     
@@ -250,29 +232,8 @@ public class Geary.Endpoint : BaseObject {
                 return "(unknown=%Xh)".printf(flag);
         }
     }
-    
-    /**
-     * Returns true if a STARTTLS command should be attempted on the connection:
-     * (a) STARTTLS is reported available (a parameter specified by the caller to this method),
-     * (b) not using SSL (so TLS is not required), and (c) STARTTLS is specified as a flag on
-     * the Endpoint.
-     *
-     * If AttemptStarttls.HALT is returned, the caller should not proceed to pass any
-     * authentication information down the connection; this situation indicates the connection is
-     * insecure and the Endpoint is configured otherwise.
-     */
-    public AttemptStarttls attempt_starttls(bool starttls_available) {
-        if (is_ssl || !use_starttls)
-            return AttemptStarttls.NO;
-        
-        if (!starttls_available)
-            return AttemptStarttls.HALT;
-        
-        return AttemptStarttls.YES;
-    }
-    
+
     public string to_string() {
         return "%s/default:%u".printf(remote_address.hostname, remote_address.port);
     }
 }
-
diff --git a/src/engine/api/geary-engine.vala b/src/engine/api/geary-engine.vala
index cba9b3d6..5d5f60b3 100644
--- a/src/engine/api/geary-engine.vala
+++ b/src/engine/api/geary-engine.vala
@@ -1,4 +1,6 @@
-/* Copyright 2016 Software Freedom Conservancy Inc.
+/*
+ * Copyright 2016 Software Freedom Conservancy Inc.
+ * Copyright 2018 Michael Gratton <mike vee net>
  *
  * This software is licensed under the GNU Lesser General Public License
  * (version 2.1 or later).  See the COPYING file in this distribution.
@@ -49,6 +51,22 @@ public class Geary.Engine : BaseObject {
         }
     }
 
+    // Workaround for Vala issue #659. See shared_endpoints below.
+    private class EndpointWeakRef {
+
+        GLib.WeakRef weak_ref;
+
+        public EndpointWeakRef(Endpoint endpoint) {
+            this.weak_ref = GLib.WeakRef(endpoint);
+        }
+
+        public Endpoint? get() {
+            return this.weak_ref.get() as Endpoint;
+        }
+
+    }
+
+
     /** Location of the directory containing shared resource files. */
     public File? resource_dir { get; private set; default = null; }
 
@@ -57,6 +75,13 @@ public class Geary.Engine : BaseObject {
     private bool is_initialized = false;
     private bool is_open = false;
 
+    // Would use a `weak Endpoint` value type for this map instead of
+    // the custom class, but we can't currently reassign built-in
+    // weak refs back to a strong ref at the moment, nor use a
+    // GLib.WeakRef as a generics param. See Vala issue #659.
+    private Gee.Map<string,EndpointWeakRef?> shared_endpoints =
+        new Gee.HashMap<string,EndpointWeakRef?>();
+
     /**
      * Fired when the engine is opened and all the existing accounts are loaded.
      */
@@ -92,8 +117,8 @@ public class Geary.Engine : BaseObject {
      */
     public signal void untrusted_host(AccountInformation account,
                                       ServiceInformation service,
-                                      Endpoint.SecurityType security,
-                                      TlsConnection cx);
+                                      TlsNegotiationMethod method,
+                                      GLib.TlsConnection cx);
 
     // Public so it can be tested
     public Engine() {
@@ -215,7 +240,9 @@ public class Geary.Engine : BaseObject {
         }
 
         account.untrusted_host.connect(on_untrusted_host);
-        account.connect_imap_endpoint();
+        account.connect_imap_service(
+            get_shared_endpoint(account.service_provider, account.imap)
+        );
 
         // validate IMAP, which requires logging in and establishing
         // an AUTHORIZED cx state
@@ -241,7 +268,7 @@ public class Geary.Engine : BaseObject {
             } catch {
                 // Oh well
             }
-            account.disconnect_endpoints();
+            account.disconnect_service_endpoints();
             account.untrusted_host.disconnect(on_untrusted_host);
         }
 
@@ -272,7 +299,9 @@ public class Geary.Engine : BaseObject {
         }
 
         account.untrusted_host.connect(on_untrusted_host);
-        account.connect_smtp_endpoint();
+        account.connect_smtp_service(
+            get_shared_endpoint(account.service_provider, account.smtp)
+        );
 
         Geary.Smtp.ClientSession? smtp_session = new Geary.Smtp.ClientSession(
             account.smtp.endpoint
@@ -288,7 +317,7 @@ public class Geary.Engine : BaseObject {
             } catch {
                 // Oh well
             }
-            account.disconnect_endpoints();
+            account.disconnect_service_endpoints();
             account.untrusted_host.disconnect(on_untrusted_host);
         }
     }
@@ -361,8 +390,12 @@ public class Geary.Engine : BaseObject {
 
         accounts.set(account.id, account);
 
-        account.connect_imap_endpoint();
-        account.connect_smtp_endpoint();
+        account.connect_imap_service(
+            get_shared_endpoint(account.service_provider, account.imap)
+        );
+        account.connect_smtp_service(
+            get_shared_endpoint(account.service_provider, account.smtp)
+        );
         account.untrusted_host.connect(on_untrusted_host);
         account_available(account);
     }
@@ -384,7 +417,7 @@ public class Geary.Engine : BaseObject {
 
         if (this.accounts.has_key(account.id)) {
             account.untrusted_host.disconnect(on_untrusted_host);
-            account.disconnect_endpoints();
+            account.disconnect_service_endpoints();
 
             // Removal *MUST* be done in the following order:
             // 1. Send the account-unavailable signal.
@@ -397,10 +430,63 @@ public class Geary.Engine : BaseObject {
         }
     }
 
+    private Geary.Endpoint get_shared_endpoint(ServiceProvider provider,
+                                               ServiceInformation service) {
+        string key = "%s/%s:%u".printf(
+            service.protocol.to_value(),
+            service.host,
+            service.port
+        );
+
+        Endpoint? shared = null;
+        EndpointWeakRef? cached = this.shared_endpoints.get(key);
+        if (cached != null) {
+            shared = cached.get() as Endpoint;
+        }
+        if (shared == null) {
+            // Prefer SSL by RFC 8314
+            TlsNegotiationMethod method = TlsNegotiationMethod.NONE;
+            if (service.use_ssl) {
+                method = TlsNegotiationMethod.TRANSPORT;
+            } else if (service.use_starttls) {
+                method = TlsNegotiationMethod.START_TLS;
+            }
+
+            uint timeout = service.protocol == Protocol.IMAP
+                ? Imap.ClientConnection.RECOMMENDED_TIMEOUT_SEC
+                : Smtp.ClientConnection.DEFAULT_TIMEOUT_SEC;
+
+            shared = new Endpoint(
+                service.host,
+                service.port,
+                method,
+                timeout
+            );
+
+            // XXX this is pretty hacky, move this back into the
+            // OutlookAccount somehow
+            if (provider == ServiceProvider.OUTLOOK) {
+                // As of June 2016, outlook.com's IMAP servers have a bug
+                // where a large number (~50) of pipelined STATUS commands on
+                // mailboxes with many messages will eventually cause it to
+                // break command parsing and return a BAD response, causing us
+                // to drop the connection. Limit the number of pipelined
+                // commands per batch to work around this.  See b.g.o Bug
+                // 766552
+                shared.max_pipeline_batch_size = 25;
+            }
+
+            this.shared_endpoints.set(key, new EndpointWeakRef(shared));
+        }
+
+        return shared;
+    }
+
+
     private void on_untrusted_host(AccountInformation account,
                                    ServiceInformation service,
-                                   Endpoint.SecurityType security,
-                                   TlsConnection cx) {
-        untrusted_host(account, service, security, cx);
+                                   TlsNegotiationMethod method,
+                                   GLib.TlsConnection cx) {
+        untrusted_host(account, service, method, cx);
     }
 }
diff --git a/src/engine/api/geary-service-information.vala b/src/engine/api/geary-service-information.vala
index fbdbe8bf..0f758377 100644
--- a/src/engine/api/geary-service-information.vala
+++ b/src/engine/api/geary-service-information.vala
@@ -140,7 +140,7 @@ public abstract class Geary.ServiceInformation : GLib.Object {
      * This only makes sense with providers that support saving the
      * password.
      */
-    public bool remember_password { get; set; default = false; }
+    public bool remember_password { get; set; default = true; }
 
     /**
      * Whether we should NOT authenticate with the server.
@@ -154,7 +154,7 @@ public abstract class Geary.ServiceInformation : GLib.Object {
      *
      * Only valid if this instance represents an SMTP server.
      */
-    public bool smtp_use_imap_credentials { get; set; default = false; }
+    public bool smtp_use_imap_credentials { get; set; default = true; }
 
     /**
      * The network endpoint for this service.
diff --git a/src/engine/api/geary-service-provider.vala b/src/engine/api/geary-service-provider.vala
index c795591d..f778b100 100644
--- a/src/engine/api/geary-service-provider.vala
+++ b/src/engine/api/geary-service-provider.vala
@@ -38,4 +38,18 @@ public enum Geary.ServiceProvider {
         return value.substring(value.last_index_of("_") + 1);
     }
 
+    public void setup_service(ServiceInformation service) {
+        switch (this) {
+        case GMAIL:
+            ImapEngine.GmailAccount.setup_service(service);
+            break;
+        case YAHOO:
+            ImapEngine.YahooAccount.setup_service(service);
+            break;
+        case OUTLOOK:
+            ImapEngine.OutlookAccount.setup_service(service);
+            break;
+        }
+    }
+
 }
diff --git a/src/engine/imap-engine/gmail/imap-engine-gmail-account.vala 
b/src/engine/imap-engine/gmail/imap-engine-gmail-account.vala
index ea43d616..e132e6eb 100644
--- a/src/engine/imap-engine/gmail/imap-engine-gmail-account.vala
+++ b/src/engine/imap-engine/gmail/imap-engine-gmail-account.vala
@@ -14,22 +14,24 @@ private class Geary.ImapEngine.GmailAccount : Geary.ImapEngine.GenericAccount {
         Geary.SpecialFolderType.TRASH,
     };
 
-    public static Geary.Endpoint generate_imap_endpoint() {
-        return new Geary.Endpoint(
-            "imap.gmail.com",
-            Imap.ClientConnection.DEFAULT_PORT_SSL,
-            Geary.Endpoint.Flags.SSL,
-            Imap.ClientConnection.RECOMMENDED_TIMEOUT_SEC);
-    }
 
-    public static Geary.Endpoint generate_smtp_endpoint() {
-        return new Geary.Endpoint(
-            "smtp.gmail.com",
-            Smtp.ClientConnection.DEFAULT_PORT_SSL,
-            Geary.Endpoint.Flags.SSL,
-            Smtp.ClientConnection.DEFAULT_TIMEOUT_SEC);
+    public static void setup_service(ServiceInformation service) {
+        switch (service.protocol) {
+        case Protocol.IMAP:
+            service.host = "imap.gmail.com";
+            service.port = Imap.ClientConnection.DEFAULT_PORT_SSL;
+            service.use_ssl = true;
+            break;
+
+        case Protocol.SMTP:
+            service.host = "smtp.gmail.com";
+            service.port = Smtp.ClientConnection.DEFAULT_PORT_SSL;
+            service.use_ssl = true;
+            break;
+        }
     }
 
+
     public GmailAccount(string name,
                         Geary.AccountInformation account_information,
                         ImapDB.Account local) {
diff --git a/src/engine/imap-engine/outlook/imap-engine-outlook-account.vala 
b/src/engine/imap-engine/outlook/imap-engine-outlook-account.vala
index f8e6f9f4..4d446b98 100644
--- a/src/engine/imap-engine/outlook/imap-engine-outlook-account.vala
+++ b/src/engine/imap-engine/outlook/imap-engine-outlook-account.vala
@@ -6,31 +6,24 @@
 
 private class Geary.ImapEngine.OutlookAccount : Geary.ImapEngine.GenericAccount {
 
-    public static Geary.Endpoint generate_imap_endpoint() {
-        Geary.Endpoint endpoint = new Geary.Endpoint(
-            "imap-mail.outlook.com",
-            Imap.ClientConnection.DEFAULT_PORT_SSL,
-            Geary.Endpoint.Flags.SSL,
-            Imap.ClientConnection.RECOMMENDED_TIMEOUT_SEC);
-        // As of June 2016, outlook.com's IMAP servers have a bug
-        // where a large number (~50) of pipelined STATUS commands on
-        // mailboxes with many messages will eventually cause it to
-        // break command parsing and return a BAD response, causing us
-        // to drop the connection. Limit the number of pipelined
-        // commands per batch to work around this.  See b.g.o Bug
-        // 766552
-        endpoint.max_pipeline_batch_size = 25;
-        return endpoint;
-    }
 
-    public static Geary.Endpoint generate_smtp_endpoint() {
-        return new Geary.Endpoint(
-            "smtp-mail.outlook.com",
-            Smtp.ClientConnection.DEFAULT_PORT_STARTTLS,
-            Geary.Endpoint.Flags.STARTTLS,
-            Smtp.ClientConnection.DEFAULT_TIMEOUT_SEC);
+    public static void setup_service(ServiceInformation service) {
+        switch (service.protocol) {
+        case Protocol.IMAP:
+            service.host = "imap-mail.outlook.com";
+            service.port = Imap.ClientConnection.DEFAULT_PORT_SSL;
+            service.use_ssl = true;
+            break;
+
+        case Protocol.SMTP:
+            service.host = "smtp-mail.outlook.com";
+            service.port = Smtp.ClientConnection.DEFAULT_PORT_STARTTLS;
+            service.use_starttls = true;
+            break;
+        }
     }
 
+
     public OutlookAccount(string name,
                           AccountInformation account_information,
                           ImapDB.Account local) {
diff --git a/src/engine/imap-engine/yahoo/imap-engine-yahoo-account.vala 
b/src/engine/imap-engine/yahoo/imap-engine-yahoo-account.vala
index e2524e2f..b817249e 100644
--- a/src/engine/imap-engine/yahoo/imap-engine-yahoo-account.vala
+++ b/src/engine/imap-engine/yahoo/imap-engine-yahoo-account.vala
@@ -6,23 +6,26 @@
 
 private class Geary.ImapEngine.YahooAccount : Geary.ImapEngine.GenericAccount {
 
-    public static Geary.Endpoint generate_imap_endpoint() {
-        return new Geary.Endpoint(
-            "imap.mail.yahoo.com",
-            Imap.ClientConnection.DEFAULT_PORT_SSL,
-            Geary.Endpoint.Flags.SSL,
-            Imap.ClientConnection.RECOMMENDED_TIMEOUT_SEC);
-    }
 
-    public static Geary.Endpoint generate_smtp_endpoint() {
-        return new Geary.Endpoint(
-            "smtp.mail.yahoo.com",
-            Smtp.ClientConnection.DEFAULT_PORT_SSL,
-            Geary.Endpoint.Flags.SSL,
-            Smtp.ClientConnection.DEFAULT_TIMEOUT_SEC);
+    private static Gee.HashMap<Geary.FolderPath, Geary.SpecialFolderType>? special_map = null;
+
+
+    public static void setup_service(ServiceInformation service) {
+        switch (service.protocol) {
+        case Protocol.IMAP:
+            service.host = "imap.mail.yahoo.com";
+            service.port = Imap.ClientConnection.DEFAULT_PORT_SSL;
+            service.use_ssl = true;
+            break;
+
+        case Protocol.SMTP:
+            service.host = "smtp.mail.yahoo.com";
+            service.port = Smtp.ClientConnection.DEFAULT_PORT_SSL;
+            service.use_ssl = true;
+            break;
+        }
     }
 
-    private static Gee.HashMap<Geary.FolderPath, Geary.SpecialFolderType>? special_map = null;
 
     public YahooAccount(string name,
                         AccountInformation account_information,
diff --git a/src/engine/imap/transport/imap-client-session.vala 
b/src/engine/imap/transport/imap-client-session.vala
index 1e44a750..92a8f580 100644
--- a/src/engine/imap/transport/imap-client-session.vala
+++ b/src/engine/imap/transport/imap-client-session.vala
@@ -911,48 +911,45 @@ public class Geary.Imap.ClientSession : BaseObject {
         // If no capabilities available, get them now
         if (capabilities.is_empty())
             yield send_command_async(new CapabilityCommand());
-        
+
         // store them for comparison later
         Imap.Capabilities caps = capabilities;
-        
-        debug("[%s] use_starttls=%s is_ssl=%s starttls=%s", to_string(), 
imap_endpoint.use_starttls.to_string(),
-            imap_endpoint.is_ssl.to_string(), caps.has_capability(Capabilities.STARTTLS).to_string());
-        switch (imap_endpoint.attempt_starttls(caps.has_capability(Capabilities.STARTTLS))) {
-            case Endpoint.AttemptStarttls.YES:
-                debug("[%s] Attempting STARTTLS...", to_string());
-                StatusResponse resp;
-                try {
-                    resp = yield send_command_async(new StarttlsCommand());
-                } catch (Error err) {
-                    debug("Error attempting STARTTLS command on %s: %s", to_string(), err.message);
-                    
-                    throw err;
-                }
-                
-                if (resp.status == Status.OK) {
-                    yield cx.starttls_async(cancellable);
-                    debug("[%s] STARTTLS completed", to_string());
-                } else {
-                    debug("[%s} STARTTLS refused: %s", to_string(), resp.status.to_string());
-                    
-                    // throw an exception and fail rather than send credentials under suspect
-                    // conditions
-                    throw new ImapError.NOT_SUPPORTED("STARTTLS refused by %s: %s", to_string(),
-                        resp.status.to_string());
-                }
-            break;
-            
-            case Endpoint.AttemptStarttls.NO:
-                debug("[%s] No STARTTLS attempted", to_string());
-            break;
-            
-            case Endpoint.AttemptStarttls.HALT:
-                throw new ImapError.NOT_SUPPORTED("STARTTLS unavailable for %s", to_string());
-            
-            default:
-                assert_not_reached();
+
+        if (imap_endpoint.tls_method == TlsNegotiationMethod.START_TLS) {
+            if (!caps.has_capability(Capabilities.STARTTLS)) {
+                throw new ImapError.NOT_SUPPORTED(
+                    "STARTTLS unavailable for %s", to_string());
+            }
+
+            debug("[%s] Attempting STARTTLS...", to_string());
+            StatusResponse resp;
+            try {
+                resp = yield send_command_async(new StarttlsCommand());
+            } catch (Error err) {
+                debug(
+                    "Error attempting STARTTLS command on %s: %s",
+                    to_string(), err.message
+                );
+                throw err;
+            }
+
+            if (resp.status == Status.OK) {
+                yield cx.starttls_async(cancellable);
+                debug("[%s] STARTTLS completed", to_string());
+            } else {
+                debug(
+                    "[%s} STARTTLS refused: %s",
+                    to_string(), resp.status.to_string()
+                );
+                // Throw an exception and fail rather than send
+                // credentials under suspect conditions
+                throw new ImapError.NOT_SUPPORTED(
+                    "STARTTLS refused by %s: %s", to_string(),
+                    resp.status.to_string()
+                );
+            }
         }
-        
+
         // Login after STARTTLS
         StatusResponse login_resp = yield login_async(credentials, cancellable);
         if (login_resp.status != Status.OK) {
diff --git a/src/engine/smtp/smtp-client-connection.vala b/src/engine/smtp/smtp-client-connection.vala
index 26074ab1..a49a9ee8 100644
--- a/src/engine/smtp/smtp-client-connection.vala
+++ b/src/engine/smtp/smtp-client-connection.vala
@@ -244,38 +244,34 @@ public class Geary.Smtp.ClientConnection {
      */
     public async Response establish_connection_async(Cancellable? cancellable = null) throws Error {
         check_connected();
-        
+
         // issue first HELO/EHLO, which will generate a set of capabiltiies
         Smtp.Response response = yield say_hello_async(cancellable);
-        
+
         // STARTTLS, if required
-        switch (endpoint.attempt_starttls(capabilities.has_capability(Capabilities.STARTTLS))) {
-            case Endpoint.AttemptStarttls.YES:
-                Response starttls_response = yield transaction_async(new Request(Command.STARTTLS));
-                if (!starttls_response.code.is_starttls_ready())
-                    throw new SmtpError.STARTTLS_FAILED("STARTTLS failed: %s", response.to_string());
-                
-                TlsClientConnection tls_cx = yield endpoint.starttls_handshake_async(cx, cancellable);
-                cx = tls_cx;
-                set_data_streams(tls_cx);
-                
-                // Now that we are on an encrypted line we need to say hello again in order to get the
-                // updated capabilities.
-                response = yield say_hello_async(cancellable);
-            break;
-            
-            case Endpoint.AttemptStarttls.NO:
-                // do nothing
-            break;
-            
-            case Endpoint.AttemptStarttls.HALT:
-            default:
-                throw new SmtpError.NOT_SUPPORTED("STARTTLS not available for %s", endpoint.to_string());
+        if (endpoint.tls_method == TlsNegotiationMethod.START_TLS) {
+            if (!capabilities.has_capability(Capabilities.STARTTLS)) {
+                throw new SmtpError.NOT_SUPPORTED(
+                    "STARTTLS not available for %s", endpoint.to_string()
+                );
+            }
+
+            Response starttls_response = yield transaction_async(new Request(Command.STARTTLS));
+            if (!starttls_response.code.is_starttls_ready())
+                throw new SmtpError.STARTTLS_FAILED("STARTTLS failed: %s", response.to_string());
+
+            TlsClientConnection tls_cx = yield endpoint.starttls_handshake_async(cx, cancellable);
+            cx = tls_cx;
+            set_data_streams(tls_cx);
+
+            // Now that we are on an encrypted line we need to say hello again in order to get the
+            // updated capabilities.
+            response = yield say_hello_async(cancellable);
         }
-        
+
         return response;
     }
-    
+
     public async Response quit_async(Cancellable? cancellable = null) throws Error {
         capabilities = null;
         return yield transaction_async(new Request(Command.QUIT), cancellable);
diff --git a/src/mailer/main.vala b/src/mailer/main.vala
index e33c6c30..969d1e4b 100644
--- a/src/mailer/main.vala
+++ b/src/mailer/main.vala
@@ -137,15 +137,18 @@ int main(string[] args) {
         arg_count = 1;
     
     if (arg_gmail) {
-        endpoint = new Geary.Endpoint("smtp.gmail.com", Geary.Smtp.ClientConnection.DEFAULT_PORT_STARTTLS,
-            Geary.Endpoint.Flags.STARTTLS,
-            Geary.Smtp.ClientConnection.DEFAULT_TIMEOUT_SEC);
+        endpoint = new Geary.Endpoint(
+            "smtp.gmail.com",
+            Geary.Smtp.ClientConnection.DEFAULT_PORT_STARTTLS,
+            Geary.TlsNegotiationMethod.START_TLS,
+            Geary.Smtp.ClientConnection.DEFAULT_TIMEOUT_SEC
+        );
     } else {
-        Geary.Endpoint.Flags flags = Geary.Endpoint.Flags.NONE;
-        if (!arg_no_tls)
-            flags |= Geary.Endpoint.Flags.SSL;
-        
-        endpoint = new Geary.Endpoint(arg_hostname, (uint16) arg_port, flags,
+        Geary.TlsNegotiationMethod method = Geary.TlsNegotiationMethod.TRANSPORT;
+        if (arg_no_tls) {
+            method = Geary.TlsNegotiationMethod.START_TLS;
+        }
+        endpoint = new Geary.Endpoint(arg_hostname, (uint16) arg_port, method,
             Geary.Smtp.ClientConnection.DEFAULT_TIMEOUT_SEC);
     }
 



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