Bug 344205 - React correctly to NO/BAD tagged response to imap IDLE. r=jorgk
authorGene Smith <gds@chartertn.net>
Tue, 23 Oct 2018 17:32:21 -0400
changeset 33676 a5fe17002ce32b945b3cdba35ef0a2e16f932e4e
parent 33675 e6e4d75191c4f55833b0b8ae7e494344b41c73e9
child 33677 24e632421a17a766666f4653b767781cb0753fa4
push id388
push userclokep@gmail.com
push dateMon, 28 Jan 2019 20:54:56 +0000
reviewersjorgk
bugs344205
Bug 344205 - React correctly to NO/BAD tagged response to imap IDLE. r=jorgk User is notifified and IDLE state is not entered if IDLE command fails.
mailnews/imap/src/nsImapProtocol.cpp
mailnews/imap/src/nsImapProtocol.h
mailnews/imap/src/nsImapServerResponseParser.cpp
--- a/mailnews/imap/src/nsImapProtocol.cpp
+++ b/mailnews/imap/src/nsImapProtocol.cpp
@@ -639,16 +639,17 @@ nsImapProtocol::SetupSinkProxy()
       }
     }
     if (!m_imapServerSink)
     {
       nsCOMPtr<nsIImapServerSink> aImapServerSink;
       res = m_runningUrl->GetImapServerSink(getter_AddRefs(aImapServerSink));
       if (aImapServerSink) {
         m_imapServerSink = new ImapServerSinkProxy(aImapServerSink);
+        m_imapServerSinkLatest =  m_imapServerSink;
       } else {
         return NS_ERROR_ILLEGAL_VALUE;
       }
     }
     if (!m_imapProtocolSink)
     {
       nsCOMPtr<nsIImapProtocolSink> anImapProxyHelper(do_QueryInterface(NS_ISUPPORTS_CAST(nsIImapProtocolSink*, this), &res));
       m_imapProtocolSink = new ImapProtocolSinkProxy(anImapProxyHelper);
@@ -679,16 +680,17 @@ static void SetSecurityCallbacksFromChan
 // before the url gets run - i.e., the url is next in line to run.
 nsresult nsImapProtocol::SetupWithUrl(nsIURI * aURL, nsISupports* aConsumer)
 {
   nsresult rv = NS_ERROR_FAILURE;
   NS_ASSERTION(aURL, "null URL passed into Imap Protocol");
   if (aURL)
   {
     m_runningUrl = do_QueryInterface(aURL, &rv);
+    m_runningUrlLatest = m_runningUrl;
     if (NS_FAILED(rv))
       return rv;
 
     nsCOMPtr<nsIMsgMailNewsUrl> mailnewsUrl = do_QueryInterface(m_runningUrl);
     nsCOMPtr<nsIMsgIncomingServer> server = do_QueryReferent(m_server);
     if (!server)
     {
       rv = mailnewsUrl->GetServer(getter_AddRefs(server));
@@ -1405,16 +1407,21 @@ nsImapProtocol::ImapThreadMainLoop()
       else
       {
         // see if we want to go into idle mode. Might want to check a pref here too.
         if (m_useIdle && !m_urlInProgress && GetServerStateParser().GetCapabilityFlag() & kHasIdleCapability
           && GetServerStateParser().GetIMAPstate()
                 == nsImapServerResponseParser::kFolderSelected)
         {
           Idle(); // for now, lets just do it. We'll probably want to use a timer
+          if (!m_idle)
+          {
+            // Server rejected IDLE. Treat like IDLE not enabled or available.
+            m_imapMailFolderSink = nullptr;
+          }
         }
         else // if not idle, don't need to remember folder sink
           m_imapMailFolderSink = nullptr;
       }
     }
     else if (m_idle && !m_threadShouldDie)
     {
       HandleIdleResponses();
@@ -5217,24 +5224,36 @@ nsImapProtocol::AlertUserEvent(const cha
       mailnewsUrl->GetSuppressErrorMsgs(&suppressErrorMsg);
 
     if (!suppressErrorMsg)
       m_imapServerSink->FEAlert(NS_ConvertASCIItoUTF16(message), mailnewsUrl);
   }
 }
 
 void
-nsImapProtocol::AlertUserEventFromServer(const char * aServerEvent)
-{
-    if (m_imapServerSink && aServerEvent)
+nsImapProtocol::AlertUserEventFromServer(const char * aServerEvent, bool aForIdle)
+{
+  if (aServerEvent)
+  {
+    // If called due to BAD/NO imap IDLE response, the server sink and running url
+    // are typically null when IDLE command is sent. So use the stored latest
+    // values for these so that the error alert notification occurs.
+    if (aForIdle && !m_imapServerSink && !m_runningUrl && m_imapServerSinkLatest)
+    {
+      nsCOMPtr<nsIMsgMailNewsUrl> mailnewsUrl = do_QueryInterface(m_runningUrlLatest);
+      m_imapServerSinkLatest->FEAlertFromServer(nsDependentCString(aServerEvent),
+                                                mailnewsUrl);
+    }
+    else if (m_imapServerSink)
     {
       nsCOMPtr<nsIMsgMailNewsUrl> mailnewsUrl = do_QueryInterface(m_runningUrl);
       m_imapServerSink->FEAlertFromServer(nsDependentCString(aServerEvent),
                                           mailnewsUrl);
     }
+  }
 }
 
 void nsImapProtocol::ResetProgressInfo()
 {
   m_lastProgressTime = 0;
   m_lastPercent = -1;
   m_lastProgressStringName.Truncate();
 }
@@ -7964,27 +7983,34 @@ void nsImapProtocol::Idle()
 
   if (m_urlInProgress)
     return;
   nsAutoCString command (GetServerCommandTag());
   command += " IDLE" CRLF;
   nsresult rv = SendData(command.get());
   if (NS_SUCCEEDED(rv))
   {
+    // we'll just get back a continuation char at first.
+    // + idling...
+    ParseIMAPandCheckForNewMail();
+    if (GetServerStateParser().LastCommandSuccessful())
+    {
       m_idle = true;
-      // we'll just get back a continuation char at first.
-      // + idling...
-      ParseIMAPandCheckForNewMail();
       // this will cause us to get notified of data or the socket getting closed.
       // That notification will occur on the socket transport thread - we just
       // need to poke a monitor so the imap thread will do a blocking read
       // and parse the data.
       nsCOMPtr <nsIAsyncInputStream> asyncInputStream = do_QueryInterface(m_inputStream);
       if (asyncInputStream)
         asyncInputStream->AsyncWait(this, 0, 0, nullptr);
+    }
+    else
+    {
+      m_idle = false;
+    }
   }
 }
 
 // until we can fix the hang on shutdown waiting for server
 // responses, we need to not wait for the server response
 // on shutdown.
 void nsImapProtocol::EndIdle(bool waitForResponse /* = true */)
 {
--- a/mailnews/imap/src/nsImapProtocol.h
+++ b/mailnews/imap/src/nsImapProtocol.h
@@ -254,17 +254,17 @@ public:
   void NotifyMessageFlags(imapMessageFlagsType flags, const nsACString &keywords,
                           nsMsgKey key, uint64_t highestModSeq);
   void NotifySearchHit(const char * hitLine);
 
   // Event handlers for the imap parser.
   void DiscoverMailboxSpec(nsImapMailboxSpec * adoptedBoxSpec);
   void AlertUserEventUsingName(const char* aMessageId);
   void AlertUserEvent(const char * message);
-  void AlertUserEventFromServer(const char * aServerEvent);
+  void AlertUserEventFromServer(const char * aServerEvent, bool aForIdle = false);
 
   void ProgressEventFunctionUsingName(const char* aMsgId);
   void ProgressEventFunctionUsingNameWithString(const char* aMsgName, const char *
     aExtraInfo);
   void PercentProgressUpdateEvent(const char16_t *message, int64_t currentProgress, int64_t maxProgress);
   void ShowProgress();
 
   // utility function calls made by the server
@@ -333,16 +333,17 @@ public:
 
   const nsString &GetEmptyMimePartString() { return m_emptyMimePartString; }
 private:
   virtual ~nsImapProtocol();
   // the following flag is used to determine when a url is currently being run. It is cleared when we
   // finish processng a url and it is set whenever we call Load on a url
   bool m_urlInProgress;
   nsCOMPtr<nsIImapUrl> m_runningUrl; // the nsIImapURL that is currently running
+  nsCOMPtr<nsIImapUrl> m_runningUrlLatest;
   nsImapAction m_imapAction;  // current imap action associated with this connection...
 
   nsCString             m_hostName;
   nsCString             m_userName;
   nsCString             m_serverKey;
   nsCString             m_realHostName;
   char                  *m_dataOutputBuf;
   RefPtr<nsMsgLineStreamBuffer> m_inputStreamBuffer;
@@ -389,16 +390,17 @@ private:
   nsCString   m_connectionType;
 
   bool        m_nextUrlReadyToRun;
   nsWeakPtr   m_server;
 
   RefPtr<ImapMailFolderSinkProxy> m_imapMailFolderSink;
   RefPtr<ImapMessageSinkProxy>    m_imapMessageSink;
   RefPtr<ImapServerSinkProxy>     m_imapServerSink;
+  RefPtr<ImapServerSinkProxy>     m_imapServerSinkLatest;
   RefPtr<ImapProtocolSinkProxy>   m_imapProtocolSink;
 
   // helper function to setup imap sink interface proxies
   nsresult SetupSinkProxy();
   // End thread support stuff
   nsresult LoadImapUrlInternal();
 
   bool GetDeleteIsMoveToTrash();
--- a/mailnews/imap/src/nsImapServerResponseParser.cpp
+++ b/mailnews/imap/src/nsImapServerResponseParser.cpp
@@ -180,16 +180,19 @@ void nsImapServerResponseParser::ParseIM
       if (!fCurrentCommandTag)
         HandleMemoryFailure();
       inIdle = commandToken && !strcmp(commandToken, "IDLE");
     }
 
     if (commandToken && ContinueParse())
       PreProcessCommandToken(commandToken, aCurrentCommand);
 
+    // For checking expected response to IDLE command below.
+    bool untagged = false;
+
     if (ContinueParse())
     {
       ResetLexAnalyzer();
 
       if (aGreetingWithCapability)
       {
         PR_FREEIF(fCurrentLine);
         fCurrentLine = aGreetingWithCapability;
@@ -204,16 +207,17 @@ void nsImapServerResponseParser::ParseIM
           response_data();
           if (ContinueParse())
           {
             if (!fAtEndOfLine)
               SetSyntaxError(true);
             else if (!inIdle && !fCurrentCommandFailed && !aGreetingWithCapability)
               AdvanceToNextToken();
           }
+          untagged = true;
         }
 
         // command continuation request [RFC3501, Sec. 7.5]
         if (ContinueParse() && fNextToken && *fNextToken == '+')  // never pipeline APPEND or AUTHENTICATE
         {
           NS_ASSERTION((fNumberOfTaggedResponsesExpected - numberOfTaggedResponsesReceived) == 1,
             " didn't get the number of tagged responses we expected");
           numberOfTaggedResponsesReceived = fNumberOfTaggedResponsesExpected;
@@ -236,17 +240,39 @@ void nsImapServerResponseParser::ParseIM
       } while (ContinueParse() && !inIdle && (numberOfTaggedResponsesReceived < fNumberOfTaggedResponsesExpected));
 
       // check and see if the server is waiting for more input
       // it's possible that we ate this + while parsing certain responses (like cram data),
       // in these cases, the parsing routine for that specific command will manually set
       // fWaitingForMoreClientInput so we don't lose that information....
       if ((fNextToken && *fNextToken == '+') || inIdle)
       {
-        fWaitingForMoreClientInput = true;
+        if (inIdle && !((fNextToken && *fNextToken == '+') || untagged))
+        {
+          // IDLE "response" + will not be "eaten" as described above since it
+          // is not an authentication response. So if IDLE response does not
+          // begin with '+' (continuation) or '*' (untagged and probably useful
+          // response) then something is wrong and it is probably a tagged
+          // NO or BAD due to transient error or bad configuration of the server.
+          if (!PL_strcmp(fCurrentCommandTag, fNextToken))
+          {
+            response_tagged();
+          }
+          else
+          {
+            // Expected tag doesn't match the received tag. Not good, start over.
+            response_fatal();
+          }
+          // Show an alert notication containing the server response to bad IDLE.
+          fServerConnection.AlertUserEventFromServer(fCurrentLine, true);
+        }
+        else
+        {
+          fWaitingForMoreClientInput = true;
+        }
       }
       // if we aren't still waiting for more input....
       else if (!fWaitingForMoreClientInput && !aGreetingWithCapability)
       {
         if (ContinueParse())
           response_done();
 
         if (ContinueParse() && !CommandFailed())
@@ -254,17 +280,17 @@ void nsImapServerResponseParser::ParseIM
           // a successful command may change the eIMAPstate
           ProcessOkCommand(commandToken);
         }
         else if (CommandFailed())
         {
           // a failed command may change the eIMAPstate
           ProcessBadCommand(commandToken);
           if (fReportingErrors && !aIgnoreBadAndNOResponses)
-            fServerConnection.AlertUserEventFromServer(fCurrentLine);
+            fServerConnection.AlertUserEventFromServer(fCurrentLine, false);
         }
       }
     }
   }
   else
     SetConnected(false);
 }