On Tue, Jan 21, 2003 at 11:55:07AM -0500, Neil Sequeira wrote:
> Thought i'd help out testing the new "NOTYET" feature of qmail-reply
> and found a couple of bugs (i think).
Thanks. I hoped somebody would have a second look at it.
> First, in recent() there looks to be an infinite loop if the
> .qmail-reply.db already contains a sender address and an e-mail
> comes in from a different sender.
>
> Also, recent_update seems to always drop the first address in the db
> when adding a new address.
>
> A couple of patches are attached. The recent() patch (reply-r.patch)
> just adds another 1 to i so the loop can exit, and the
> recent_update() patch (reply-ru.patch) compares timestamps and drops
> any addresses that are past their timeout period.
>
Thanks for the patches, I will add variatioins of both. See attached
patch.
> These have only received minimal testing, so patcher beware ;)
>
So does mine...
The main problem with the NOTYET stuff is that multiple concurrent
accesses to the .qmail-reply.db file will dump some users (by overwriting
a just written file with a new file). I'm not sure if we should use the
maildir++ approach with timestamps or if it is better to use file locking
(which sucks over NFS). Perhaps we can also ignore it and some senders get
more than one reply in the REPLY_TIMEOUT time.
--
:wq Claudio
Index: qmail-reply.c
===================================================================
RCS file: /home/cvs-qmail-ldap/CVS/qmail-ldap/qmail-reply.c,v
retrieving revision 1.19
diff -u -r1.19 qmail-reply.c
--- qmail-reply.c 21 Jan 2003 14:00:27 -0000 1.19
+++ qmail-reply.c 21 Jan 2003 22:35:12 -0000
@@ -211,7 +211,6 @@
}
stralloc rs = {0}; /* recent sender */
-int rsmatch = 0;
datetime_sec timeout;
#ifndef REPLY_TIMEOUT
#define REPLY_TIMEOUT 1209600 /* 2 weeks */
@@ -235,16 +234,16 @@
}
slen = rs.len; s = rs.s;
- for (i = 0; i < slen; i += str_len(s+i)) {
+ for (i = 0; i < slen; i += str_len(s+i) + 1) {
if (case_diffb(buf, len, s+i) == 0) {
/* match found, look at timeval */
- rsmatch = i; i += len;
+ i += len;
if (s[i++] != ':')
strerr_die2x(100, FATAL,
"db file .qmail-reply.db corrupted");
last = get_stamp(s+i);
- if (last + timeout > now()) return 1;
- else return 0;
+ if (last + timeout < now()) return 0;
+ else return 1;
}
}
@@ -263,17 +262,19 @@
void recent_update(char *buf, int len)
{
- char *s, *t;
struct stat st;
- unsigned long pid, time;
- int fd, loop, size, slen, i;
substdio ss;
+ char *s, *t;
+ datetime_sec time, last;
+ unsigned long pid;
+ unsigned int slen, i, n;
+ int fd, loop;
s = rs.s; slen = rs.len;
- size = slen + len + 10;
+ n = slen + len + 10;
for(; size > MAX_SIZE; ) {
i = str_len(s) + 1;
- size -= i;
+ n -= i;
slen -= i;
s += i;
}
@@ -301,7 +302,11 @@
substdio_fdbuf(&ss, write, fd, rsoutbuf, sizeof(rsoutbuf));
for (i = 0; i < slen; i += str_len(s+i) + 1) {
- if (rs.s+rsmatch == s+i) continue;
+ n = byte_chr(s+i, slen, ':');
+ if (n++ != slen) {
+ last = get_stamp(s + i + n);
+ if (last + timeout < time) continue;
+ } /* else file corrupted */
if (substdio_puts(&ss, s+i) == -1) goto fail;
if (substdio_put(&ss, "\n", 1) == -1) goto fail;
}
