]> Pileus Git - ~andy/fetchmail/commitdiff
Fix IMAP IDLE and untagged (* ...) response parser, by Sunil Shetye.
authorMatthias Andree <matthias.andree@gmx.de>
Fri, 22 Jan 2010 01:15:18 +0000 (01:15 -0000)
committerMatthias Andree <matthias.andree@gmx.de>
Fri, 22 Jan 2010 01:15:18 +0000 (01:15 -0000)
The IMAP client no longer skips messages from several IMAP servers including
Dovecot if fetchmail's "idle" is in use.  Causes were that fetchmail (a)
ignored some untagged responses when it should not (b) relied on EXISTS
messages in response to EXPUNGE, which aren't mandated by RFC-3501 (the IMAP
standard) and aren't sent by Dovecot either.
  Fix by Sunil Shetye (the fix also consolidates IMAP response handling,
improving overall robustness of the IMAP client), bug report and testing by
Matt Doran, with further hints from Timo Sirainen.

svn path=/branches/BRANCH_6-3/; revision=5459

NEWS
fetchmail.h
imap.c

diff --git a/NEWS b/NEWS
index 1ff64180be4002019ac2dfb9325c38e5da50044f..d39ea062a12e1d6ee72301c8f8cfb52466d67e14 100644 (file)
--- a/NEWS
+++ b/NEWS
@@ -49,6 +49,16 @@ removed from a 6.4.0 or newer release.)
 
 fetchmail 6.3.14 (not yet released):
 
+# BUG FIXES
+* The IMAP client no longer skips messages from several IMAP servers including
+  Dovecot if fetchmail's "idle" is in use.  Causes were that fetchmail (a)
+  ignored some untagged responses when it should not (b) relied on EXISTS
+  messages in response to EXPUNGE, which aren't mandated by RFC-3501 (the IMAP
+  standard) and aren't sent by Dovecot either.
+    Fix by Sunil Shetye (the fix also consolidates IMAP response handling,
+  improving overall robustness of the IMAP client), bug report and testing by
+  Matt Doran, with further hints from Timo Sirainen.
+
 # TRANSLATION UPDATES
 * [it]    Italian, by Vincenzo Campanella
 
index 702fcae5e2eaf935e2154e4b964dc69db678487f..c459bdca1f2fe5a39d4e83299c549f2052929021 100644 (file)
@@ -145,6 +145,7 @@ char *strstr(const char *, const char *);
 #define                PS_RETAINED     26      /* message retained (internal use) */
 #define                PS_REPOLL       28      /* repoll immediately with changed parameters (internal use) */
 #define                PS_IDLETIMEOUT  29      /* timeout on imap IDLE (internal use) */
+#define                PS_UNTAGGED     30      /* untagged response on imap command (internal use) */
 
 /* output noise level */
 #define         O_SILENT       0       /* mute, max squelch, etc. */
diff --git a/imap.c b/imap.c
index 54309f7dc7393ed4379546ceaf9fc9c4527620bc..14566d43f957366f1f54c6f68fc0586468ccb465 100644 (file)
--- a/imap.c
+++ b/imap.c
@@ -49,7 +49,122 @@ static int actual_deletions = 0;
 static int saved_timeout = 0, idle_timeout = 0;
 static time_t idle_start_time = 0;
 
-static int imap_ok(int sock, char *argbuf)
+static int imap_untagged_response(int sock, const char *buf)
+/* interpret untagged status responses */
+{
+    /* For each individual check, use a BLANK before the word to avoid
+     * confusion with the \Recent flag or similar */
+    if (stage == STAGE_GETAUTH
+           && !strncmp(buf, "* CAPABILITY", 12))
+    {
+       strlcpy(capabilities, buf + 12, sizeof(capabilities));
+    }
+    else if (stage == STAGE_GETAUTH
+           && !strncmp(buf, "* PREAUTH", 9))
+    {
+       preauth = TRUE;
+    }
+    else if (stage != STAGE_LOGOUT
+           && !strncmp(buf, "* BYE", 5))
+    {
+       /* log the unexpected bye from server as we expect the
+        * connection to be cut-off after this */
+       if (outlevel > O_SILENT)
+           report(stderr, GT_("bye from imap server: %s"), buf + 5);
+    }
+    else if (strstr(buf, " EXISTS"))
+    {
+       count = atoi(buf+2);
+       /*
+        * Don't trust the message count passed by the server.
+        * Without this check, it might be possible to do a
+        * DNS-spoofing attack that would pass back a ridiculous 
+        * count, and allocate a malloc area that would overlap
+        * a portion of the stack.
+        */
+       if ((unsigned)count > INT_MAX/sizeof(int))
+       {
+           report(stderr, GT_("bogus message count!"));
+           return(PS_PROTOCOL);
+       }
+       if ((recentcount = count - oldcount) < 0)
+           recentcount = 0;
+
+       /*
+        * Nasty kluge to handle RFC2177 IDLE.  If we know we're idling
+        * we can't wait for the tag matching the IDLE; we have to tell the
+        * server the IDLE is finished by shipping back a DONE when we
+        * see an EXISTS.  Only after that will a tagged response be
+        * shipped.  The idling flag also gets cleared on a timeout.
+        */
+       if (stage == STAGE_IDLE)
+       {
+           /* If IDLE isn't supported, we were only sending NOOPs anyway. */
+           if (has_idle)
+           {
+               /* we do our own write and report here to disable tagging */
+               SockWrite(sock, "DONE\r\n", 6);
+               if (outlevel >= O_MONITOR)
+                   report(stdout, "IMAP> DONE\n");
+           }
+
+           mytimeout = saved_timeout;
+           stage = STAGE_GETRANGE;
+       }
+    }
+    /* we now compute recentcount as a difference between
+     * new and old EXISTS, hence disable RECENT check */
+# if 0
+    else if (strstr(buf, " RECENT"))
+    {
+       recentcount = atoi(buf+2);
+    }
+# endif
+    else if (strstr(buf, " EXPUNGE"))
+    {
+       /* the response "* 10 EXPUNGE" means that the currently
+        * tenth (i.e. only one) message has been deleted */
+       if (atoi(buf+2) > 0)
+       {
+           if (count > 0)
+               count--;
+           if (oldcount > 0)
+               oldcount--;
+           /* We do expect an EXISTS response immediately
+            * after this, so this updation of recentcount is
+            * just a precaution! */
+           if ((recentcount = count - oldcount) < 0)
+               recentcount = 0;
+           actual_deletions++;
+       }
+    }
+    /*
+     * The server may decide to make the mailbox read-only, 
+     * which causes fetchmail to go into a endless loop
+     * fetching the same message over and over again. 
+     * 
+     * However, for check_only, we use EXAMINE which will
+     * mark the mailbox read-only as per the RFC.
+     * 
+     * This checks for the condition and aborts if 
+     * the mailbox is read-only. 
+     *
+     * See RFC 2060 section 6.3.1 (SELECT).
+     * See RFC 2060 section 6.3.2 (EXAMINE).
+     */ 
+    else if (stage == STAGE_GETRANGE
+           && !check_only && strstr(buf, "[READ-ONLY]"))
+    {
+       return(PS_LOCKBUSY);
+    }
+    else
+    {
+       return(PS_UNTAGGED);
+    }
+    return(PS_SUCCESS);
+}
+
+static int imap_response(int sock, char *argbuf)
 /* parse command response */
 {
     char buf[MSGBUFSIZE+1];
@@ -66,103 +181,23 @@ static int imap_ok(int sock, char *argbuf)
            if (islower((unsigned char)*cp))
                *cp = toupper((unsigned char)*cp);
 
-       /* interpret untagged status responses
-        * First check if we really have an untagged response, starting
-        * with "*" SPACE. Then, for each individual check, use a BLANK
-        * before the word to avoid confusion with the \Recent flag or
-        * similar */
+       /* untagged responses start with "* " */
        if (buf[0] == '*' && buf[1] == ' ') {
-           if (strstr(buf, " CAPABILITY")) {
-               strlcpy(capabilities, buf + 12, sizeof(capabilities));
-           }
-           else if (strstr(buf, " EXISTS"))
+           ok = imap_untagged_response(sock, buf);
+           if (ok == PS_UNTAGGED)
            {
-               count = atoi(buf+2);
-               /*
-                * Don't trust the message count passed by the server.
-                * Without this check, it might be possible to do a
-                * DNS-spoofing attack that would pass back a ridiculous 
-                * count, and allocate a malloc area that would overlap
-                * a portion of the stack.
-                */
-               if ((unsigned)count > INT_MAX/sizeof(int))
+               if (argbuf && stage != STAGE_IDLE && tag[0] != '\0')
                {
-                   report(stderr, GT_("bogus message count!"));
-                   return(PS_PROTOCOL);
-               }
-               if ((recentcount = count - oldcount) < 0)
-                   recentcount = 0;
-
-               /*
-                * Nasty kluge to handle RFC2177 IDLE.  If we know we're idling
-                * we can't wait for the tag matching the IDLE; we have to tell the
-                * server the IDLE is finished by shipping back a DONE when we
-                * see an EXISTS.  Only after that will a tagged response be
-                * shipped.  The idling flag also gets cleared on a timeout.
-                */
-               if (stage == STAGE_IDLE)
-               {
-                   /* If IDLE isn't supported, we were only sending NOOPs anyway. */
-                   if (has_idle)
-                   {
-                       /* we do our own write and report here to disable tagging */
-                       SockWrite(sock, "DONE\r\n", 6);
-                       if (outlevel >= O_MONITOR)
-                           report(stdout, "IMAP> DONE\n");
-                   }
-
-                   mytimeout = saved_timeout;
-                   stage = STAGE_FETCH;
-               }
-           }
-           /* we now compute recentcount as a difference between
-            * new and old EXISTS, hence disable RECENT check */
-# if 0
-           else if (strstr(buf, " RECENT"))
-           {
-               recentcount = atoi(buf+2);
-           }
-# endif
-           else if (strstr(buf, " EXPUNGE"))
-           {
-               /* the response "* 10 EXPUNGE" means that the currently
-                * tenth (i.e. only one) message has been deleted */
-               if (atoi(buf+2) > 0)
-               {
-                   if (count > 0)
-                       count--;
-                   if (oldcount > 0)
-                       oldcount--;
-                   /* We do expect an EXISTS response immediately
-                    * after this, so this updation of recentcount is
-                    * just a precaution! */
-                   if (recentcount > 0)
-                       recentcount--;
-                   actual_deletions++;
+                   /* if there is an unmatched response, pass it back to
+                    * the calling function for further analysis. The
+                    * calling function should call imap_response() again
+                    * to read the remaining response */
+                   strcpy(argbuf, buf);
+                   return(ok);
                }
            }
-           else if (strstr(buf, " PREAUTH"))
-           {
-               preauth = TRUE;
-           }
-               /*
-                * The server may decide to make the mailbox read-only, 
-                * which causes fetchmail to go into a endless loop
-                * fetching the same message over and over again. 
-                * 
-                * However, for check_only, we use EXAMINE which will
-                * mark the mailbox read-only as per the RFC.
-                * 
-                * This checks for the condition and aborts if 
-                * the mailbox is read-only. 
-                *
-                * See RFC 2060 section 6.3.1 (SELECT).
-                * See RFC 2060 section 6.3.2 (EXAMINE).
-                */ 
-           else if (!check_only && strstr(buf, "[READ-ONLY]"))
-           {
-               return(PS_LOCKBUSY);
-           }
+           else if (ok != PS_SUCCESS)
+               return(ok);
        }
 
        if (stage == STAGE_IDLE)
@@ -204,6 +239,8 @@ static int imap_ok(int sock, char *argbuf)
        {
            if (stage == STAGE_GETAUTH) 
                return(PS_AUTHFAIL);    /* RFC2060, 6.2.2 */
+           else if (stage == STAGE_GETSIZES)
+               return(PS_SUCCESS);     /* see comments in imap_getpartialsizes() */
            else
                return(PS_ERROR);
        }
@@ -212,6 +249,16 @@ static int imap_ok(int sock, char *argbuf)
     }
 }
 
+static int imap_ok(int sock, char *argbuf)
+/* parse command response */
+{
+    int ok;
+
+    while ((ok = imap_response(sock, argbuf)) == PS_UNTAGGED)
+       ; /* wait for the tagged response */
+    return(ok);
+}
+
 #ifdef NTLM_ENABLE
 #include "ntlm.h"
 
@@ -713,7 +760,7 @@ static int imap_idle(int sock)
                report(stdout, "IMAP> DONE\n");
            /* reset stage and timeout here: we are not idling any more */
            mytimeout = saved_timeout;
-           stage = STAGE_FETCH;
+           stage = STAGE_GETRANGE;
            /* get OK IDLE message */
            ok = imap_ok(sock, NULL);
        }
@@ -746,7 +793,7 @@ static int imap_idle(int sock)
     /* restore normal timeout value */
     set_timeout(0);
     mytimeout = saved_timeout;
-    stage = STAGE_FETCH;
+    stage = STAGE_GETRANGE;
 
     return(ok);
 }
@@ -864,14 +911,9 @@ static int imap_getrange(int sock,
 
        /* don't count deleted messages, in case user enabled keep last time */
        gen_send(sock, "SEARCH UNSEEN NOT DELETED");
-       do {
-           ok = gen_recv(sock, buf, sizeof(buf));
-           if (ok != 0)
-           {
-               report(stderr, GT_("search for unseen messages failed\n"));
-               return(PS_PROTOCOL);
-           }
-           else if ((cp = strstr(buf, "* SEARCH")))
+       while ((ok = imap_response(sock, buf)) == PS_UNTAGGED)
+       {
+           if ((cp = strstr(buf, "* SEARCH")))
            {
                char    *ep;
 
@@ -904,8 +946,12 @@ static int imap_getrange(int sock,
                    }
                }
            }
-       } while
-           (tag[0] != '\0' && strncmp(buf, tag, strlen(tag)));
+       }
+       if (ok != 0)
+       {
+           report(stderr, GT_("search for unseen messages failed\n"));
+           return(ok);
+       }
 
        if (outlevel >= O_DEBUG && unseen > 0)
            report(stdout, GT_("%u is first unseen\n"), startcount);
@@ -923,6 +969,7 @@ static int imap_getpartialsizes(int sock, int first, int last, int *sizes)
 /* capture the sizes of messages #first-#last */
 {
     char buf [MSGBUFSIZE+1];
+    int ok;
 
     /*
      * Some servers (as in, PMDF5.1-9.1 under OpenVMS 6.1)
@@ -954,7 +1001,7 @@ static int imap_getpartialsizes(int sock, int first, int last, int *sizes)
      * * 1 FETCH (BODY ("TEXT" "PLAIN" ("CHARSET" "US-ASCII") NIL NIL "7BIT" 35 3))
      * A006 OK FETCH completed.
      *
-     * To get around this, we terminate the read loop on a NO and count
+     * To get around this, we treat the final NO as success and count
      * on the fact that the sizes array has been preinitialized with a
      * known-bad size value.
      */
@@ -969,23 +1016,12 @@ static int imap_getpartialsizes(int sock, int first, int last, int *sizes)
        gen_send(sock, "FETCH %d:%d RFC822.SIZE", first, last);
     else /* no unseen messages! */
        return(PS_SUCCESS);
-    for (;;)
+    while ((ok = imap_response(sock, buf)) == PS_UNTAGGED)
     {
        unsigned int size;
-       int ok, num;
-       char *cp;
+       int num;
 
-       if ((ok = gen_recv(sock, buf, sizeof(buf))))
-           return(ok);
-       /* we want response matching to be case-insensitive */
-       for (cp = buf; *cp; cp++)
-           *cp = toupper((unsigned char)*cp);
-       /* an untagged NO means that a message was not readable */
-       if (strstr(buf, "* NO"))
-           ;
-       else if (strstr(buf, "OK") || strstr(buf, "NO"))
-           break;
-       else if (sscanf(buf, "* %d FETCH (RFC822.SIZE %u)", &num, &size) == 2
+       if (sscanf(buf, "* %d FETCH (RFC822.SIZE %u)", &num, &size) == 2
        /* some servers (like mail.internode.on.net bld-mail04) return UID information here
         *
         * IMAP> A0005 FETCH 1 RFC822.SIZE
@@ -1002,8 +1038,7 @@ static int imap_getpartialsizes(int sock, int first, int last, int *sizes)
                        GT_("Warning: ignoring bogus data for message sizes returned by the server.\n"));
        }
     }
-
-    return(PS_SUCCESS);
+    return(ok);
 }
 
 static int imap_getsizes(int sock, int count, int *sizes)
@@ -1194,24 +1229,9 @@ static int imap_trail(int sock, struct query *ctl, const char *tag)
     /* number -= expunged; */
 
     (void)ctl;
-    for (;;)
-    {
-       char buf[MSGBUFSIZE+1], *t;
-       int ok;
-
-       if ((ok = gen_recv(sock, buf, sizeof(buf))))
-           return(ok);
-
-       /* UW IMAP returns "OK FETCH", Cyrus returns "OK Completed" */
-       if (strncmp(buf, tag, strlen(tag)) == 0) {
-           t = buf + strlen(tag);
-           t += strspn(t, " \t");
-           if (strncmp(t, "OK", 2) == 0)
-               break;
-       }
-    }
+    (void)tag;
 
-    return(PS_SUCCESS);
+    return imap_ok(sock, NULL);
 }
 
 static int imap_delete(int sock, struct query *ctl, int number)