Tony Finch – blog

We’re upgrading Hermes from a heavily modified Cyrus-2.1 IMAP message store to a somewhat less modified Cyrus-2.3 setup. We’re at the early testing stage, and I’m one of the guinea pigs. Since I’ve moved over almost everything has worked fine, except that my cron email has no longer been filtered into my “root” mailbox. Cyrus-2.3 has a new bytecode interpreter for Sieve scripts so that it only parses a script when it is uploaded instead of on every message delivery, so I suspected a bug in the new code.

The cron mail filtering clause in my sieve script uses an address :regex test. The bad behaviour indicated that the regex was not matching - the sieve script was not bombing out. (When a sieve script fails (e.g. because a mailbox is too full) messages are just delivered to the user’s inbox, but in this case some of my cron mail was being delivered to my mail-support mailbox because later tests were catching it.) Cyrus’s changelog didn’t mention any regex-specific changes, and sieve regexes are just POSIX EREs, so I didn’t think a subtle regex syntax change was causing the match to fail.

The Cyrus Sieve code comes with a test wrapper which is not compiled by default, but a quick cd cyrus-imapd-2.3/sieve && make test built it without a problem. It became much more useful when I toggled the DUMPCODE and VERBOSE flags in bytecode.h and when I removed the UUCP From_ line that was causing it to fail to parse the header of my test message. After adding lots of debug printf()s I found that my sieve script was being handled as I expected except that it was trying to match my regex against the whole contents of the message’s From: line, whereas it should have been matching it against just the address. My regex included pedantic ^$ anchors which detected the unexpected presence of a display name.

The bug is fixed by a one-line patch that passes the parsed out address part to the regex comparator instead of the whole header, as is done for other comparators. It only affects the address :regex test - other tests with the :regex comparator are OK, so are other comparators with the address test.

--- sieve/bc_eval.c     13 Feb 2007 15:06:54 -0000      1.1.1.1
+++ sieve/bc_eval.c     30 Jul 2007 16:03:30 -0000
@@ -591,7 +591,7 @@
                                    goto alldone;
                                }

-                               res |= comp(val[y], strlen(val[y]),
+                               res |= comp(addr, strlen(addr),
                                            (const char *)reg, comprock);
                                free(reg);
                            } else {

(This is not my first Cyrus Sieve bug - I also found a case where it was ignoring backslash escapes before * glob characters. The patch is here.)