[geary/wip/limit-replay-op-retries] Ignore any IDLE status responses in Imap.ClientSession's handler



commit 57e39084a3522f7074a076d5c18c05b0f7d5a4eb
Author: Michael Gratton <mike vee net>
Date:   Thu Feb 7 13:51:37 2019 +1100

    Ignore any IDLE status responses in Imap.ClientSession's handler
    
    Since Imap.ClientConnection reports all commands and responses,
    including IDLE, and since ClientSession snoops on raw IMAP responses
    from the connection to drive its FSM, ILDE responses may inadvertently
    cause FSM handlers. So ensure we don't have an IDLE reponse before
    doiing something with it.

 .../imap/transport/imap-client-connection.vala     | 32 +++++++-----
 src/engine/imap/transport/imap-client-session.vala | 60 +++++++++++++---------
 2 files changed, 54 insertions(+), 38 deletions(-)
---
diff --git a/src/engine/imap/transport/imap-client-connection.vala 
b/src/engine/imap/transport/imap-client-connection.vala
index 7f3fa6a9..e0789743 100644
--- a/src/engine/imap/transport/imap-client-connection.vala
+++ b/src/engine/imap/transport/imap-client-connection.vala
@@ -327,6 +327,25 @@ public class Geary.Imap.ClientConnection : BaseObject {
         );
     }
 
+    /**
+     * Returns the command that has been sent with the given tag.
+     *
+     * This should be private, but is internal for the
+     * ClientSession.on_received_status_response IDLE workaround.
+     */
+    internal Command? get_sent_command(Tag tag) {
+        Command? sent = null;
+        if (tag.is_tagged()) {
+            foreach (Command queued in this.sent_queue) {
+                if (tag.equal_to(queued.tag)) {
+                    sent = queued;
+                    break;
+                }
+            }
+        }
+        return sent;
+    }
+
     private async void open_channels_async() throws Error {
         assert(ios != null);
         assert(ser == null);
@@ -478,19 +497,6 @@ public class Geary.Imap.ClientConnection : BaseObject {
         }
     }
 
-    private Command? get_sent_command(Tag tag) {
-        Command? sent = null;
-        if (tag.is_tagged()) {
-            foreach (Command queued in this.sent_queue) {
-                if (tag.equal_to(queued.tag)) {
-                    sent = queued;
-                    break;
-                }
-            }
-        }
-        return sent;
-    }
-
     private void check_connection() throws ImapError {
         if (this.cx == null) {
             throw new ImapError.NOT_CONNECTED(
diff --git a/src/engine/imap/transport/imap-client-session.vala 
b/src/engine/imap/transport/imap-client-session.vala
index e296c8db..2207ef7b 100644
--- a/src/engine/imap/transport/imap-client-session.vala
+++ b/src/engine/imap/transport/imap-client-session.vala
@@ -1735,38 +1735,48 @@ public class Geary.Imap.ClientSession : BaseObject {
 
     private void on_received_status_response(StatusResponse status_response) {
         this.last_seen = GLib.get_real_time();
-
-        // reschedule keepalive (traffic seen on channel)
         schedule_keepalive();
 
-        // If a CAPABILITIES ResponseCode, decode and update capabilities ...
-        // some servers do this to prevent a second round-trip
-        ResponseCode? response_code = status_response.response_code;
-        if (response_code != null) {
-            try {
-                if (response_code.get_response_code_type().is_value(ResponseCodeType.CAPABILITY)) {
-                    capabilities = response_code.get_capabilities(ref next_capabilities_revision);
-                    debug("[%s] %s %s", to_string(), status_response.status.to_string(),
-                        capabilities.to_string());
-                    
-                    capability(capabilities);
+        // XXX Need to ignore emitted IDLE status responses. They are
+        // emitted by ClientConnection because it doesn't make any
+        // sense not to, and so they get logged by that class's
+        // default handlers, but because they are snooped on here (and
+        // even worse are used to push FSM transitions, rather relying
+        // on the actual commands themselves), we need to check for
+        // IDLE responses and ignore them.
+        Command? command = this.cx.get_sent_command(status_response.tag);
+        if (command == null || !(command is IdleCommand)) {
+            // If a CAPABILITIES ResponseCode, decode and update
+            // capabilities ...  some servers do this to prevent a
+            // second round-trip
+            ResponseCode? response_code = status_response.response_code;
+            if (response_code != null) {
+                try {
+                    if (response_code.get_response_code_type().is_value(ResponseCodeType.CAPABILITY)) {
+                        capabilities = response_code.get_capabilities(ref next_capabilities_revision);
+                        debug("[%s] %s %s", to_string(), status_response.status.to_string(),
+                              capabilities.to_string());
+
+                        capability(capabilities);
+                    }
+                } catch (Error err) {
+                    debug("[%s] Unable to convert response code to capabilities: %s", to_string(),
+                          err.message);
                 }
-            } catch (Error err) {
-                debug("[%s] Unable to convert response code to capabilities: %s", to_string(),
-                    err.message);
             }
-        }
 
-        // update state machine before notifying subscribers, who may turn around and query ClientSession
-        if (status_response.is_completion) {
-            fsm.issue(Event.RECV_COMPLETION, null, status_response, null);
-        } else {
-            fsm.issue(Event.RECV_STATUS, null, status_response, null);
-        }
+            // update state machine before notifying subscribers, who
+            // may turn around and query ClientSession
+            if (status_response.is_completion) {
+                fsm.issue(Event.RECV_COMPLETION, null, status_response, null);
+            } else {
+                fsm.issue(Event.RECV_STATUS, null, status_response, null);
+            }
 
-        status_response_received(status_response);
+            status_response_received(status_response);
+        }
     }
-    
+
     private void notify_received_data(ServerData server_data) throws ImapError {
         switch (server_data.server_data_type) {
             case ServerDataType.CAPABILITY:


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