[prev in list] [next in list] [prev in thread] [next in thread] 

List:       fetchmail-friends
Subject:    [fetchmail] [PATCH] Re: how to avoid "missed" mail
From:       Sunil Shetye <shetye () bombay ! retortsoft ! com>
Date:       2002-12-23 14:19:03
[Download RAW message or body]

Quoting from Matthias Andree's mail on Fri, Dec 20, 2002 at 05:22:35PM +0100:
> > Quoting from Matthias Andree's mail on Thu, Dec 19, 2002 at 06:55:38PM +0100:
> >> Then there's a driver bug. fetchmail must not mark a mail it could not
> >> deliver "read". However, I only tested against POP3, so I may have
> >> missed that one at that time.
> >
> > I am not sure if this is correct for POP3. It works correctly only for
> > POP3+UIDL (client side tracking).
> 
> UIDL is the only sane way for a reliable retrieval. Eric should make
> that the default in the next version (it should have become the default
> in 6.0.0 already).

Ok.

In that case, the IMAP code indeed has a bug. Here is a patch which
fixes that by adding a mark_seen() function to the driver and always
using BODY.PEEK for imap.

This patch also moves around the POP3/UID code from driver.c to pop3.c
just to keep things consistent.

I have tested this patch by using a script which returns 1 as an mda
for both IMAP and POP3.

-- 
Sunil Shetye.

["fetchmail-6.2.0-markseen.patch" (text/plain)]

diff -Naur fetchmail-6.2.0.orig/driver.c fetchmail-6.2.0/driver.c
--- fetchmail-6.2.0.orig/driver.c	Thu Dec 19 03:38:47 2002
+++ fetchmail-6.2.0/driver.c	Sat Dec 21 15:02:14 2002
@@ -658,25 +658,6 @@
 	 * now.
 	 */
 
-	/*
-	 * Tell the UID code we've seen this.
-	 * Matthias Andree: only register the UID if we could actually
-	 * forward this mail. If we omit this !suppress_delete check,
-	 * fetchmail will never retry mail that the local listener
-	 * refused temporarily.
-	 */
-	if (ctl->newsaved && !suppress_delete)
-	{
-	    struct idlist	*sdp;
-
-	    for (sdp = ctl->newsaved; sdp; sdp = sdp->next)
-		if ((sdp->val.status.num == num) && (msgcodes[num-1] >= 0))
-		{
-		    sdp->val.status.mark = UID_SEEN;
-		    save_str(&ctl->oldsaved, sdp->id,UID_SEEN);
-		}
-	}
-
 	/* maybe we delete this message now? */
 	if (retained)
 	{
@@ -694,11 +675,10 @@
 	    err = (ctl->server.base_protocol->delete)(mailserver_socket, ctl, num);
 	    if (err != 0)
 		return(err);
-#ifdef POP3_ENABLE
-	    delete_str(&ctl->newsaved, num);
-#endif /* POP3_ENABLE */
 	}
-	else if (   (outlevel >= O_VERBOSE) ||
+	else
+	{
+	    if (   (outlevel >= O_VERBOSE) ||
          		/* To avoid flooding the syslog when using --keep,
          		 * report "Skipped message" only when:
          		 *  1) --verbose is on, or
@@ -709,6 +689,17 @@
 	           (outlevel > O_SILENT && (!run.use_syslog || msgcodes[num-1] != MSGLEN_OLD))
 	       )
 	    report_complete(stdout, GT_(" not flushed\n"));
+
+	    /* maybe we mark this message as seen now? */
+	    if (ctl->server.base_protocol->mark_seen
+		&& !suppress_delete
+		&& (msgcodes[num-1] >= 0 && ctl->keep))
+	    {
+		err = (ctl->server.base_protocol->mark_seen)(mailserver_socket, ctl, num);
+		if (err != 0)
+		    return(err);
+	    }
+	}
 
 	/* perhaps this as many as we're ready to handle */
 	if (maxfetch && maxfetch <= *fetches && *fetches < count)
diff -Naur fetchmail-6.2.0.orig/etrn.c fetchmail-6.2.0/etrn.c
--- fetchmail-6.2.0.orig/etrn.c	Mon Mar 11 01:04:50 2002
+++ fetchmail-6.2.0/etrn.c	Sat Dec 21 15:02:14 2002
@@ -140,6 +140,7 @@
     NULL,		/* no way to fetch body */
     NULL,		/* no message trailer */
     NULL,		/* how to delete a message */
+    NULL,		/* how to mark a message as seen */
     etrn_logout,	/* log out, we're done */
     FALSE,		/* no, we can't re-poll */
 };
diff -Naur fetchmail-6.2.0.orig/fetchmail.h fetchmail-6.2.0/fetchmail.h
--- fetchmail-6.2.0.orig/fetchmail.h	Fri Dec 13 10:39:06 2002
+++ fetchmail-6.2.0/fetchmail.h	Sat Dec 21 15:02:14 2002
@@ -191,6 +191,8 @@
 				/* eat trailer of a message */
     int (*delete)(int, struct query *, int);
 				/* delete method */
+    int (*mark_seen)(int, struct query *, int);
+				/* mark as seen method */
     int (*logout_cmd)(int, struct query *);
 				/* logout command */
     flag retry;			/* can getrange poll for new messages? */
diff -Naur fetchmail-6.2.0.orig/imap.c fetchmail-6.2.0/imap.c
--- fetchmail-6.2.0.orig/imap.c	Thu Nov 28 15:59:24 2002
+++ fetchmail-6.2.0/imap.c	Sat Dec 21 15:02:14 2002
@@ -876,11 +876,6 @@
      * craps out during the message, it will still be marked `unseen' on
      * the server.
      *
-     * However...*don't* do this if we're using keep to suppress deletion!
-     * In that case, marking the seen flag is the only way to prevent the
-     * message from being re-fetched on subsequent runs (and according
-     * to RFC2060 p.43 this fetch should set Seen as a side effect).
-     *
      * According to RFC2060, and Mark Crispin the IMAP maintainer,
      * FETCH %d BODY[TEXT] and RFC822.TEXT are "functionally 
      * equivalent".  However, we know of at least one server that
@@ -898,17 +893,11 @@
     switch (imap_version)
     {
     case IMAP4rev1:	/* RFC 2060 */
-	if (!ctl->keep)
-	    gen_send(sock, "FETCH %d BODY.PEEK[TEXT]", number);
-	else
-	    gen_send(sock, "FETCH %d BODY[TEXT]", number);
+	gen_send(sock, "FETCH %d BODY.PEEK[TEXT]", number);
 	break;
 
     case IMAP4:		/* RFC 1730 */
-	if (!ctl->keep)
-	    gen_send(sock, "FETCH %d RFC822.TEXT.PEEK", number);
-	else
-	    gen_send(sock, "FETCH %d RFC822.TEXT", number);
+	gen_send(sock, "FETCH %d RFC822.TEXT.PEEK", number);
 	break;
 
     default:		/* RFC 1176 */
@@ -958,26 +947,6 @@
 	/* UW IMAP returns "OK FETCH", Cyrus returns "OK Completed" */
 	if (strstr(buf, "OK"))
 	    break;
-
-#ifdef __UNUSED__
-	/*
-	 * Any IMAP server that fails to set Seen on a BODY[TEXT]
-	 * fetch violates RFC2060 p.43 (top).  This becomes an issue
-	 * when keep is on, because seen messages aren't deleted and
-	 * get refetched on each poll.  As a workaround, if keep is on
-	 * we can set the Seen flag explicitly.
-	 *
-	 * This code isn't used yet because we don't know of any IMAP
-	 * servers broken in this way.
-	 */
-	if (ctl->keep)
-	    if ((ok = gen_transact(sock,
-			imap_version == IMAP4 
-				? "STORE %d +FLAGS.SILENT (\\Seen)"
-				: "STORE %d +FLAGS (\\Seen)", 
-			number)))
-		return(ok);
-#endif /* __UNUSED__ */
     }
 
     return(PS_SUCCESS);
@@ -1022,6 +991,16 @@
     return(PS_SUCCESS);
 }
 
+static int imap_mark_seen(int sock, struct query *ctl, int number)
+/* mark the given message as seen */
+{
+    return(gen_transact(sock,
+	imap_version == IMAP4
+	? "STORE %d +FLAGS.SILENT (\\Seen)"
+	: "STORE %d +FLAGS (\\Seen)",
+	number));
+}
+
 static int imap_logout(int sock, struct query *ctl)
 /* send logout command */
 {
@@ -1059,6 +1038,7 @@
     imap_fetch_body,	/* request given message body */
     imap_trail,		/* eat message trailer */
     imap_delete,	/* delete the message */
+    imap_mark_seen,	/* how to mark a message as seen */
     imap_logout,	/* expunge and exit */
     TRUE,		/* yes, we can re-poll */
 };
diff -Naur fetchmail-6.2.0.orig/odmr.c fetchmail-6.2.0/odmr.c
--- fetchmail-6.2.0.orig/odmr.c	Fri Oct 18 17:09:00 2002
+++ fetchmail-6.2.0/odmr.c	Sat Dec 21 15:02:14 2002
@@ -229,6 +229,7 @@
     NULL,		/* no way to fetch body */
     NULL,		/* no message trailer */
     NULL,		/* how to delete a message */
+    NULL,		/* how to mark a message as seen */
     odmr_logout,	/* log out, we're done */
     FALSE,		/* no, we can't re-poll */
 };
diff -Naur fetchmail-6.2.0.orig/pop2.c fetchmail-6.2.0/pop2.c
--- fetchmail-6.2.0.orig/pop2.c	Thu Mar 22 11:15:35 2001
+++ fetchmail-6.2.0/pop2.c	Sat Dec 21 15:02:14 2002
@@ -146,6 +146,7 @@
     NULL,				/* no way to fetch body alone */
     pop2_trail,				/* eat message trailer */
     NULL,				/* no POP2 delete method */
+    NULL,				/* how to mark a message as seen */
     pop2_logout,			/* log out, we're done */
     FALSE,				/* no, we can't re-poll */
 };
diff -Naur fetchmail-6.2.0.orig/pop3.c fetchmail-6.2.0/pop3.c
--- fetchmail-6.2.0.orig/pop3.c	Thu Nov 28 15:59:26 2002
+++ fetchmail-6.2.0/pop3.c	Sat Dec 21 15:08:52 2002
@@ -792,11 +792,40 @@
     return(PS_SUCCESS);
 }
 
+static void mark_uid_seen(struct query *ctl, int number)
+/* Tell the UID code we've seen this. */
+{
+    if (ctl->newsaved)
+    {
+	struct idlist	*sdp;
+
+	for (sdp = ctl->newsaved; sdp; sdp = sdp->next)
+	    if (sdp->val.status.num == number)
+	    {
+		sdp->val.status.mark = UID_SEEN;
+		save_str(&ctl->oldsaved, sdp->id,UID_SEEN);
+	    }
+    }
+}
+
 static int pop3_delete(int sock, struct query *ctl, int number)
 /* delete a given message */
 {
+    int ok;
+    mark_uid_seen(ctl, number);
     /* actually, mark for deletion -- doesn't happen until QUIT time */
-    return(gen_transact(sock, "DELE %d", number));
+    ok = gen_transact(sock, "DELE %d", number);
+    if (ok != PS_SUCCESS)
+	return(ok);
+    delete_str(&ctl->newsaved, number);
+    return(PS_SUCCESS);
+}
+
+static int pop3_mark_seen(int sock, struct query *ctl, int number)
+/* mark a given message as seen */
+{
+    mark_uid_seen(ctl, number);
+    return(PS_SUCCESS);
 }
 
 static int pop3_logout(int sock, struct query *ctl)
@@ -856,6 +885,7 @@
     NULL,		/* no way to fetch body alone */
     NULL,		/* no message trailer */
     pop3_delete,	/* how to delete a message */
+    pop3_mark_seen,	/* how to mark a message as seen */
     pop3_logout,	/* log out, we're done */
     FALSE,		/* no, we can't re-poll */
 };


[prev in list] [next in list] [prev in thread] [next in thread]