Dovecot-based servers (e.g. Migadu) volunteer a CONDSTORE MODSEQ
attribute in FETCH responses even when CONDSTORE was not requested,
in the RFC 4551 form `MODSEQ (n)`. The FETCH response parser did not
recognise MODSEQ and aborted with "unexpected attribute", which broke
all flag synchronisation and expunge propagation for affected accounts.
Recognise and skip the MODSEQ attribute and its parenthesised value.
mbsync does not consume per-message modseqs, so ignoring it is safe.
Repro: sync any mailbox on imap.migadu.com; a FLAGS/expunge sync emits
IMAP error: malformed FETCH response ...: unexpected attribute
on a `* N FETCH (UID n FLAGS (...) MODSEQ (m))` line.
---
Hi,
The offending exchange (mbsync -D), reduced:
F: * 2 FETCH (UID 2 FLAGS (\Seen \Deleted) MODSEQ (359812601))
IMAP error: malformed FETCH response from imap.migadu.com:
unexpected attribute
The server is RFC-compliant: RFC 4551 (CONDSTORE) section 3.3 defines
fetch-mod-resp = "MODSEQ" SP "(" permsg-modsequence ")"
i.e. the parenthesised `MODSEQ (n)` form is exactly what the spec
mandates. The issue is purely client-side: fetch_rsp_atom() has no
branch for MODSEQ, so it falls into the "unexpected attribute" error
and aborts the whole sync. Because the abort happens during the
FLAGS/expunge pass, no flag changes or deletions propagate at all.
The server volunteers MODSEQ unsolicited -- mbsync never sends
CONDSTORE/CHANGEDSINCE. Dovecot advertises CONDSTORE in its CAPABILITY
and attaches the per-message modseq to FETCH replies regardless. Since
DisableExtension does not accept CONDSTORE, there is no config-level
workaround; the parser is the right layer to fix.
Approach: I took the targeted route -- recognise MODSEQ explicitly and
skip its value, mirroring the existing FLAGS handling exactly (atom ->
FetchModSeq, "(" -> FetchModSeqVal on enter, value atom ignored, ")" ->
FetchAttrib on leave), so the parser cursor ends up positioned just like
every other attribute. I deliberately did not broaden the terminal
"unexpected attribute" else-branch into a general "skip any unknown
attribute" tolerance, to avoid masking real protocol errors -- but I'm
happy to switch to the more general form if you'd prefer it.
I only see the one per-message FETCH path that needs this in 1.5.x;
there is no HIGHESTMODSEQ/VANISHED/QRESYNC handling in the tree that
would need the same treatment.
Testing:
- make check: 3/3 pass (tst_imap_msgs, tst_imap_utf7, tst_msg_cvt).
- Against live Migadu: full sync now completes with no error;
flag changes and expunges propagate in both directions.
- The change is inert for servers that don't emit MODSEQ: no new
attribute name is ever produced, so non-CONDSTORE servers hit none
of the new code paths.
Patch is against current master (1.5.1-8-g0949519).
Thanks,
Tyler
src/drv_imap.c | 18 ++++++++++++++++++
1 file changed, 18 insertions(+)
diff --git a/src/drv_imap.c b/src/drv_imap.c
index 9b170ab..800e786 100644
--- a/src/drv_imap.c
+++ b/src/drv_imap.c
@@ -93,6 +93,8 @@ typedef enum {
FetchFlagVal,
FetchDate,
FetchSize,
+ FetchModSeq,
+ FetchModSeqVal,
FetchBody,
FetchBodyChunk,
FetchHeaders,
@@ -1124,6 +1126,9 @@ fetch_rsp_enter( imap_store_t *ctx )
case FetchFlags:
ctx->fetch_state = FetchFlagVal;
return LIST_OK;
+ case FetchModSeq:
+ ctx->fetch_state = FetchModSeqVal;
+ return LIST_OK;
case FetchHeaders:
ctx->fetch_state = FetchHeaderFields;
return LIST_OK;
@@ -1141,6 +1146,9 @@ fetch_rsp_leave( imap_store_t *ctx )
case FetchFlagVal:
ctx->fetch_state = FetchAttrib;
return LIST_OK;
+ case FetchModSeqVal:
+ ctx->fetch_state = FetchAttrib;
+ return LIST_OK;
case FetchHeaderFields:
ctx->fetch_state = FetchHeaderBracket;
return LIST_OK;
@@ -1155,6 +1163,7 @@ fetch_rsp_atom( imap_store_t *ctx, char *val, uint len,
int type )
switch (ctx->fetch_state) {
case FetchInit:
case FetchFlags:
+ case FetchModSeq:
case FetchHeaders:
return LIST_BAD;
case FetchAttrib:
@@ -1176,6 +1185,12 @@ fetch_rsp_atom( imap_store_t *ctx, char *val, uint len,
int type )
} else if (equals( val, len, "RFC822.SIZE", 11 )) {
ctx->fetch_state = FetchSize;
field = M_SIZE;
+ } else if (equals( val, len, "MODSEQ", 6 )) {
+ // RFC 4551: CONDSTORE-capable servers (e.g.
Dovecot/Migadu) may
+ // volunteer a MODSEQ (n) attribute even when it was
not requested.
+ // We don't use per-message modseqs, so consume and
ignore it.
+ ctx->fetch_state = FetchModSeq;
+ return LIST_OK;
} else if (equals( val, len, "BODY[]", 6 ) || equals( val, len,
"BODY[HEADER]", 12 )) {
ctx->parse_list_sts.chunked = 1;
ctx->fetch_state = FetchBody;
@@ -1206,6 +1221,9 @@ fetch_rsp_atom( imap_store_t *ctx, char *val, uint len,
int type )
}
parse_fetched_flag( val, len, &ctx->fetch.flags,
&ctx->fetch.status );
return LIST_OK;
+ case FetchModSeqVal:
+ // Ignore the parenthesised modseq value.
+ return LIST_OK;
case FetchDate:
if (type != AtomString) {
dfail:
--
2.50.1 (Apple Git-155)
_______________________________________________
isync-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/isync-devel
