CORRECT PATCH Re: sync_client bails when encountering a deleted
message
Ken Murchison
murch at andrew.cmu.edu
Thu May 17 12:00:11 EDT 2007
John Capo wrote:
> Quoting Ken Murchison (murch at andrew.cmu.edu):
>> John Capo wrote:
>>> Quoting John Capo (jc at irbs.com):
>>>> Quoting Ken Murchison (murch at andrew.cmu.edu):
>>>>> I don't think I was clear. With my proposal, we're well past "UPDATE".
>>>>> I'm talking about cutting the command short after "(<flags>)". No
>>>>> header_size or anything after.
>>>> I guess I don't understand what you mean.
>>>>
>>>> It sure looks to me like cmd_upload() has to detect that the header
>>>> size it is waiting for will not arrive and recover gracefully rather
>>>> than sending a BAD that the client sees as the response for its
>>>> next command. Patch attached.
>>> The server needs to push the character back except on EOF, grrrr.
>> Here's what I was thinking. We might be able to bump the cache flush
>> down to parse_err, but I'm not sure if this is correct/suboptimal for
>> SIMPLE.
>
> Ths might work if you add an eatline() in the client before returning.
> The BAD response from the server must be discarded somehow or the
> BAD response will be seen by the client as the response to the next
> client command. It seems to me that no response is needed since
> the client is not expecting one.
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?
Index: sync_client.c
===================================================================
RCS file: /afs/andrew/system/cvs/src/cyrus/imap/sync_client.c,v
retrieving revision 1.10
diff -u -r1.10 sync_client.c
--- sync_client.c 7 May 2007 16:23:21 -0000 1.10
+++ sync_client.c 17 May 2007 15:55:35 -0000
@@ -1177,6 +1177,7 @@
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);
}
@@ -1246,7 +1247,7 @@
/* Message with this UUID exists on client but not server */
if ((r=upload_message_work(mailbox, msgno, &record)))
- return(r);
+ break;
if (msg && (msg->uid == record.uid)) /* Overwritten on server */
msg = msg->next;
@@ -1321,7 +1322,7 @@
mailbox->last_uid, mailbox->last_appenddate);
if ((r=upload_message_work(mailbox, msgno, &record)))
- return(r);
+ break;
}
if (count == 0)
Index: sync_server.c
===================================================================
RCS file: /afs/andrew/system/cvs/src/cyrus/imap/sync_server.c,v
retrieving revision 1.4
diff -u -r1.4 sync_server.c
--- sync_server.c 4 Apr 2007 15:22:42 -0000 1.4
+++ sync_server.c 17 May 2007 15:55:35 -0000
@@ -1889,8 +1889,9 @@
break;
case MSG_PARSED:
if (c != ' ') {
- err = "Invalid flags";
- goto parse_err;
+ /* Missing message - UPLOAD short-circuited by client */
+ sync_upload_list_remove(upload_list, item);
+ goto done;
}
message = sync_message_add(message_list, &item->uuid);
@@ -1943,6 +1944,7 @@
/* if we see a SP, we're trying to upload more than one message */
} while (c == ' ');
+ done:
if (c == EOF) {
err = "Unexpected end of sync_in at end of item";
goto parse_err;
Index: sync_support.c
===================================================================
RCS file: /afs/andrew/system/cvs/src/cyrus/imap/sync_support.c,v
retrieving revision 1.7
diff -u -r1.7 sync_support.c
--- sync_support.c 23 Apr 2007 14:15:32 -0000 1.7
+++ sync_support.c 17 May 2007 15:55:35 -0000
@@ -511,6 +511,22 @@
return(result);
}
+void sync_msgid_remove(struct sync_msgid_list *l,
+ struct message_uuid *uuid)
+{
+ int offset = message_uuid_hash(uuid, l->hash_size);
+ struct sync_msgid *msgid;
+
+ if (message_uuid_isnull(uuid)) return;
+
+ for (msgid = l->hash[offset] ; msgid ; msgid = msgid->hash_next) {
+ if (message_uuid_compare(&msgid->uuid, uuid)) {
+ message_uuid_set_null(&msgid->uuid);
+ return;
+ }
+ }
+}
+
void sync_msgid_list_free(struct sync_msgid_list **lp)
{
struct sync_msgid_list *l = *lp;
@@ -1363,6 +1379,27 @@
l->count++;
return(result);
+}
+
+void sync_upload_list_remove(struct sync_upload_list *l,
+ struct sync_upload_item *i)
+{
+ struct sync_upload_item *prev, *current, *next;
+
+ prev = NULL;
+ current = l->head;
+ while (current) {
+ next = current->next;
+ if (current == i) {
+ l->count--;
+ free(current);
+ if (prev) prev->next = next;
+ else l->head = next;
+ return;
+ }
+ prev = current;
+ current = next;
+ }
}
void sync_upload_list_free(struct sync_upload_list **lp)
Index: sync_support.h
===================================================================
RCS file: /afs/andrew/system/cvs/src/cyrus/imap/sync_support.h,v
retrieving revision 1.2
diff -u -r1.2 sync_support.h
--- sync_support.h 30 Nov 2006 17:11:20 -0000 1.2
+++ sync_support.h 17 May 2007 15:55:35 -0000
@@ -144,6 +144,9 @@
struct sync_msgid *sync_msgid_add(struct sync_msgid_list *list,
struct message_uuid *uuid);
+void sync_msgid_remove(struct sync_msgid_list *l,
+ struct message_uuid *uuid);
+
struct sync_msgid *sync_msgid_lookup(struct sync_msgid_list *list,
struct message_uuid *uuid);
@@ -342,6 +345,9 @@
char **flagname);
struct sync_upload_item *sync_upload_list_add(struct sync_upload_list *l);
+
+void sync_upload_list_remove(struct sync_upload_list *l,
+ struct sync_upload_item *i);
void sync_upload_list_free(struct sync_upload_list **lp);
--
Kenneth Murchison
Systems Programmer
Project Cyrus Developer/Maintainer
Carnegie Mellon University
More information about the Cyrus-devel
mailing list