On Thu, 17 May 2007, Ken Murchison wrote:
Here is an untested patch which SHOULD allow the client to abort the
UPLOAD, with the server successfully accepting the messages transmitted
up to the abort.
Thoughts?
I believe that there is a problem with this patch. Sorry that its taken
this long for me to look at it properly.
A single mailbox event can cover multiple messages, particularly if
sync_client is catching up from a backlog of transactions. Imagine the
second message in a sequence disappearing under sync_client's feet.
1) The first message will be replicated correctly.
2) The second message will be discarded, which is probably correctly.
3) The third (and any subsequent) messages will also be discarded.
The replica will remain out of sync until the next event on that mailbox.
I think that it is important for sync_client to retry immediately.
Here is a patch against 2.3 CVS. This patch also causes the client and
server to resynchronise from an incomplete UPLOAD command by throwing a
syntax error at the server end. I'm uncomfortable about just dropping off
part way through an UPLOAD and pretending that everything is fine.
--
David Carter Email: [EMAIL PROTECTED]
University Computing Service, Phone: (01223) 334502
New Museums Site, Pembroke Street, Fax: (01223) 334679
Cambridge UK. CB2 3QH.
Index: imap/sync_client.c
===================================================================
RCS file: /cvs/src/cyrus/imap/sync_client.c,v
retrieving revision 1.11
diff -u -r1.11 sync_client.c
--- imap/sync_client.c 18 May 2007 13:24:39 -0000 1.11
+++ imap/sync_client.c 2 Jul 2007 16:49:55 -0000
@@ -1132,7 +1132,6 @@
prot_printf(toserver, " COPY");
need_body = 0;
} else {
- sync_msgid_add(msgid_onserver, &record->uuid);
prot_printf(toserver, " PARSED");
need_body = 1;
}
@@ -1177,9 +1176,9 @@
if (r) {
syslog(LOG_ERR, "IOERROR: opening message file %lu of %s: %m",
record->uid, mailbox->name);
- sync_msgid_remove(msgid_onserver, &record->uuid);
return(IMAP_IOERROR);
}
+ sync_msgid_add(msgid_onserver, &record->uuid);
prot_printf(toserver, " %lu %lu %lu {%lu+}\r\n",
record->header_size, record->content_lines,
@@ -1259,6 +1258,10 @@
prot_printf(toserver, "\r\n");
prot_flush(toserver);
+ if (r) {
+ sync_parse_code("UPLOAD", fromserver, SYNC_PARSE_EAT_OKLINE, NULL);
+ return(r);
+ }
r = sync_parse_code("UPLOAD", fromserver, SYNC_PARSE_NOEAT_OKLINE, NULL);
if (r) return(r);
@@ -1331,6 +1334,10 @@
prot_printf(toserver, "\r\n");
prot_flush(toserver);
+ if (r) {
+ sync_parse_code("UPLOAD", fromserver, SYNC_PARSE_EAT_OKLINE, NULL);
+ return(r);
+ }
r = sync_parse_code("UPLOAD", fromserver, SYNC_PARSE_NOEAT_OKLINE, NULL);
if (r) return(r);
Index: imap/sync_server.c
===================================================================
RCS file: /cvs/src/cyrus/imap/sync_server.c,v
retrieving revision 1.6
diff -u -r1.6 sync_server.c
--- imap/sync_server.c 30 May 2007 15:04:12 -0000 1.6
+++ imap/sync_server.c 2 Jul 2007 16:49:55 -0000
@@ -1870,7 +1870,7 @@
switch (msg_type) {
case MSG_SIMPLE:
if (c != ' ') {
- err = "Invalid flags";
+ err = "Invalid flags or missing message";
goto parse_err;
}
@@ -1897,9 +1897,8 @@
break;
case MSG_PARSED:
if (c != ' ') {
- /* Missing message - UPLOAD short-circuited by client */
- sync_upload_list_remove(upload_list, item);
- goto done;
+ err = "Invalid flags or missing message";
+ goto parse_err;
}
message = sync_message_add(message_list, &item->uuid);
