From 3d29b3b95a1a87530c05314b46fb1c252e6f72bc Mon Sep 17 00:00:00 2001 From: Brendan Cully Date: Thu, 28 Aug 2008 02:31:14 -0700 Subject: [PATCH] Split long IMAP commands for the benefit of lazy servers (closes #3000). Also touches lots of old, hairy code. Likely to wake sleeping dogs. --- imap/command.c | 1 + imap/imap.c | 217 +++++++++++++++++++++++++------------------- imap/imap_private.h | 7 +- imap/message.c | 145 +++++++++++++++-------------- 4 files changed, 211 insertions(+), 159 deletions(-) diff --git a/imap/command.c b/imap/command.c index 95269bbe7..c041336ea 100644 --- a/imap/command.c +++ b/imap/command.c @@ -196,6 +196,7 @@ int imap_code (const char *s) * IMAP_CMD_FAIL_OK: the calling procedure can handle failure. This is used * for checking for a mailbox on append and login * IMAP_CMD_PASS: command contains a password. Suppress logging. + * IMAP_CMD_QUEUE: only queue command, do not execute. * Return 0 on success, -1 on Failure, -2 on OK Failure */ int imap_exec (IMAP_DATA* idata, const char* cmdstr, int flags) diff --git a/imap/imap.c b/imap/imap.c index 8cf174c61..b1f80c644 100644 --- a/imap/imap.c +++ b/imap/imap.c @@ -856,40 +856,23 @@ int imap_has_flag (LIST* flag_list, const char* flag) return 0; } -/* imap_make_msg_set: make an IMAP4rev1 UID message set out of a set of - * headers, given a flag enum to filter on. - * Params: idata: IMAP_DATA containing context containing header set - * buf: to write message set into - * flag: enum of flag type on which to filter - * changed: include only changed messages in message set - * invert: invert sense of flag, eg M_READ matches unread messages - * Returns: number of messages in message set (0 if no matches) */ -int imap_make_msg_set (IMAP_DATA* idata, BUFFER* buf, int flag, int changed, - int invert) +/* Note: headers must be in SORT_ORDER. See imap_exec_msgset for args. + * Pos is an opaque pointer a la strtok. It should be 0 at first call. */ +static int imap_make_msg_set (IMAP_DATA* idata, BUFFER* buf, int flag, + int changed, int invert, int* pos) { - HEADER** hdrs; /* sorted local copy */ + HEADER** hdrs = idata->ctx->hdrs; int count = 0; /* number of messages in message set */ int match = 0; /* whether current message matches flag condition */ unsigned int setstart = 0; /* start of current message range */ int n; - short oldsort; /* we clobber reverse, must restore it */ int started = 0; - if (Sort != SORT_ORDER) - { - hdrs = safe_calloc (idata->ctx->msgcount, sizeof (HEADER*)); - memcpy (hdrs, idata->ctx->hdrs, idata->ctx->msgcount * sizeof (HEADER*)); - - oldsort = Sort; - Sort = SORT_ORDER; - qsort ((void*) hdrs, idata->ctx->msgcount, sizeof (HEADER*), - mutt_get_sort_func (SORT_ORDER)); - Sort = oldsort; - } - else - hdrs = idata->ctx->hdrs; + hdrs = idata->ctx->hdrs; - for (n = 0; n < idata->ctx->msgcount; n++) + for (n = *pos; + n < idata->ctx->msgcount && buf->dptr - buf->data < IMAP_MAX_CMDLEN; + n++) { match = 0; /* don't include pending expunged messages */ @@ -951,10 +934,82 @@ int imap_make_msg_set (IMAP_DATA* idata, BUFFER* buf, int flag, int changed, } } + *pos = n; + + return count; +} + +/* Prepares commands for all messages matching conditions (must be flushed + * with imap_exec) + * Params: + * idata: IMAP_DATA containing context containing header set + * pre, post: commands are of the form "%s %s %s %s", tag, + * pre, message set, post + * flag: enum of flag type on which to filter + * changed: include only changed messages in message set + * invert: invert sense of flag, eg M_READ matches unread messages + * Returns: number of matched messages, or -1 on failure */ +int imap_exec_msgset (IMAP_DATA* idata, const char* pre, const char* post, + int flag, int changed, int invert) +{ + HEADER** hdrs; /* sorted local copy */ + short oldsort; /* we clobber reverse, must restore it */ + BUFFER* cmd; + int pos; + int rc; + int count = 0; + + if (! (cmd = mutt_buffer_init (NULL))) + { + dprint (1, (debugfile, "imap_exec_msgset: unable to allocate buffer\n")); + return -1; + } + + /* We make a copy of the headers just in case resorting doesn't give + exactly the original order (duplicate messages?), because other parts of + the ctx are tied to the header order. This may be overkill. */ + if (Sort != SORT_ORDER) + { + hdrs = safe_calloc (idata->ctx->msgcount, sizeof (HEADER*)); + memcpy (hdrs, idata->ctx->hdrs, idata->ctx->msgcount * sizeof (HEADER*)); + + oldsort = Sort; + Sort = SORT_ORDER; + qsort ((void*) hdrs, idata->ctx->msgcount, sizeof (HEADER*), + mutt_get_sort_func (Sort)); + Sort = oldsort; + } + else + hdrs = idata->ctx->hdrs; + + pos = 0; + + do + { + cmd->dptr = cmd->data; + mutt_buffer_printf (cmd, "%s ", pre); + rc = imap_make_msg_set (idata, cmd, flag, changed, invert, &pos); + if (rc > 0) + { + mutt_buffer_printf (cmd, " %s", post); + if (imap_exec (idata, cmd->data, IMAP_CMD_QUEUE)) + { + rc = -1; + goto out; + } + count += rc; + } + } + while (rc > 0); + + rc = count; + +out: + mutt_buffer_free (&cmd); if (Sort != SORT_ORDER) FREE (&hdrs); - return count; + return rc; } /* returns 0 if mutt's flags match cached server flags */ @@ -977,7 +1032,6 @@ static int compare_flags (HEADER* h) } /* Update the IMAP server to reflect the flags a single message. */ - int imap_sync_message (IMAP_DATA *idata, HEADER *hdr, BUFFER *cmd, int *err_continue) { @@ -1055,35 +1109,30 @@ int imap_sync_message (IMAP_DATA *idata, HEADER *hdr, BUFFER *cmd, return 0; } -static int sync_helper (IMAP_DATA* idata, BUFFER* buf, int right, int flag, - const char* name) +static int sync_helper (IMAP_DATA* idata, int right, int flag, const char* name) { - int rc = 0; + int count = 0; + int rc; + + char buf[LONG_STRING]; if (!mutt_bit_isset (idata->ctx->rights, right)) return 0; - + if (right == M_ACL_WRITE && !imap_has_flag (idata->flags, name)) return 0; - buf->dptr = buf->data; - mutt_buffer_addstr (buf, "UID STORE "); - if (imap_make_msg_set (idata, buf, flag, 1, 0)) - { - rc++; - mutt_buffer_printf (buf, " +FLAGS.SILENT (%s)", name); - imap_exec (idata, buf->data, IMAP_CMD_QUEUE); - } - buf->dptr = buf->data; - mutt_buffer_addstr (buf, "UID STORE "); - if (imap_make_msg_set (idata, buf, flag, 1, 1)) - { - rc++; - mutt_buffer_printf (buf, " -FLAGS.SILENT (%s)", name); - imap_exec (idata, buf->data, IMAP_CMD_QUEUE); - } - - return rc; + snprintf (buf, sizeof(buf), "+FLAGS.SILENT (%s)", name); + if ((rc = imap_exec_msgset (idata, "UID STORE", buf, flag, 1, 0)) < 0) + return rc; + count += rc; + + buf[0] = '-'; + if ((rc = imap_exec_msgset (idata, "UID STORE", buf, flag, 1, 1)) < 0) + return rc; + count += rc; + + return count; } /* update the IMAP server to reflect message changes done within mutt. @@ -1095,11 +1144,9 @@ int imap_sync_mailbox (CONTEXT* ctx, int expunge, int* index_hint) { IMAP_DATA* idata; CONTEXT* appendctx = NULL; - BUFFER cmd; HEADER* h; HEADER** hdrs = NULL; int oldsort; - int deleted; int n; int rc; @@ -1118,31 +1165,25 @@ int imap_sync_mailbox (CONTEXT* ctx, int expunge, int* index_hint) if ((rc = imap_check_mailbox (ctx, index_hint, 0)) != 0) return rc; - memset (&cmd, 0, sizeof (cmd)); - /* if we are expunging anyway, we can do deleted messages very quickly... */ if (expunge && mutt_bit_isset (idata->ctx->rights, M_ACL_DELETE)) { - mutt_buffer_addstr (&cmd, "UID STORE "); - deleted = imap_make_msg_set (idata, &cmd, M_DELETED, 1, 0); + if ((rc = imap_exec_msgset (idata, "UID STORE", "+FLAGS.SILENT (\\Deleted)", + M_DELETED, 1, 0)) < 0) + { + mutt_error (_("Expunge failed")); + mutt_sleep (1); + goto out; + } - /* if we have a message set, then let's delete */ - if (deleted) + if (rc > 0) { - mutt_message (_("Marking %d messages deleted..."), deleted); - mutt_buffer_addstr (&cmd, " +FLAGS.SILENT (\\Deleted)"); /* mark these messages as unchanged so second pass ignores them. Done * here so BOGUS UW-IMAP 4.7 SILENT FLAGS updates are ignored. */ for (n = 0; n < ctx->msgcount; n++) - if (ctx->hdrs[n]->deleted && ctx->hdrs[n]->changed) - ctx->hdrs[n]->active = 0; - if (imap_exec (idata, cmd.data, 0) != 0) - { - mutt_error (_("Expunge failed")); - mutt_sleep (1); - rc = -1; - goto out; - } + if (ctx->hdrs[n]->deleted && ctx->hdrs[n]->changed) + ctx->hdrs[n]->active = 0; + mutt_message (_("Marking %d messages deleted..."), rc); } } @@ -1176,9 +1217,7 @@ int imap_sync_mailbox (CONTEXT* ctx, int expunge, int* index_hint) if (!appendctx) appendctx = mx_open_mailbox (ctx->path, M_APPEND | M_QUIET, NULL); if (!appendctx) - { dprint (1, (debugfile, "imap_sync_mailbox: Error opening mailbox in append mode\n")); - } else _mutt_save_message (h, appendctx, 1, 0, 0); } @@ -1192,7 +1231,7 @@ int imap_sync_mailbox (CONTEXT* ctx, int expunge, int* index_hint) /* sync +/- flags for the five flags mutt cares about */ rc = 0; - /* presort here to avoid doing 10 resorts in imap_make_msg_set */ + /* presort here to avoid doing 10 resorts in imap_exec_msgset */ oldsort = Sort; if (Sort != SORT_ORDER) { @@ -1206,11 +1245,11 @@ int imap_sync_mailbox (CONTEXT* ctx, int expunge, int* index_hint) mutt_get_sort_func (SORT_ORDER)); } - rc += sync_helper (idata, &cmd, M_ACL_DELETE, M_DELETED, "\\Deleted"); - rc += sync_helper (idata, &cmd, M_ACL_WRITE, M_FLAG, "\\Flagged"); - rc += sync_helper (idata, &cmd, M_ACL_WRITE, M_OLD, "Old"); - rc += sync_helper (idata, &cmd, M_ACL_SEEN, M_READ, "\\Seen"); - rc += sync_helper (idata, &cmd, M_ACL_WRITE, M_REPLIED, "\\Answered"); + rc += sync_helper (idata, M_ACL_DELETE, M_DELETED, "\\Deleted"); + rc += sync_helper (idata, M_ACL_WRITE, M_FLAG, "\\Flagged"); + rc += sync_helper (idata, M_ACL_WRITE, M_OLD, "Old"); + rc += sync_helper (idata, M_ACL_SEEN, M_READ, "\\Seen"); + rc += sync_helper (idata, M_ACL_WRITE, M_REPLIED, "\\Answered"); if (oldsort != Sort) { @@ -1219,24 +1258,22 @@ int imap_sync_mailbox (CONTEXT* ctx, int expunge, int* index_hint) ctx->hdrs = hdrs; } - if (rc) + if (rc && (imap_exec (idata, NULL, 0) != IMAP_CMD_OK)) { - if ((rc = imap_exec (idata, NULL, 0)) != IMAP_CMD_OK) + if (ctx->closing) { - if (ctx->closing) + if (mutt_yesorno (_("Error saving flags. Close anyway?"), 0) == M_YES) { - if (mutt_yesorno (_("Error saving flags. Close anyway?"), 0) == M_YES) - { - rc = 0; - idata->state = IMAP_AUTHENTICATED; - goto out; - } + rc = 0; + idata->state = IMAP_AUTHENTICATED; + goto out; } - else - mutt_error _("Error saving flags"); - goto out; } + else + mutt_error _("Error saving flags"); + goto out; } + for (n = 0; n < ctx->msgcount; n++) ctx->hdrs[n]->changed = 0; ctx->changed = 0; @@ -1268,8 +1305,6 @@ int imap_sync_mailbox (CONTEXT* ctx, int expunge, int* index_hint) rc = 0; out: - if (cmd.data) - FREE (&cmd.data); if (appendctx) { mx_fastclose_mailbox (appendctx); diff --git a/imap/imap_private.h b/imap/imap_private.h index 330136fd2..122eff4c6 100644 --- a/imap/imap_private.h +++ b/imap/imap_private.h @@ -55,6 +55,9 @@ #define IMAP_CACHE_LEN 10 #define SEQLEN 5 +/* maximum length of command lines before they must be split (for + * lazy servers) */ +#define IMAP_MAX_CMDLEN 1024 #define IMAP_REOPEN_ALLOW (1<<0) #define IMAP_EXPUNGE_EXPECTED (1<<1) @@ -225,8 +228,8 @@ int imap_rename_mailbox (IMAP_DATA* idata, IMAP_MBOX* mx, const char* newname); IMAP_STATUS* imap_mboxcache_get (IMAP_DATA* idata, const char* mbox, int create); void imap_mboxcache_free (IMAP_DATA* idata); -int imap_make_msg_set (IMAP_DATA* idata, BUFFER* buf, int flag, int changed, - int invert); +int imap_exec_msgset (IMAP_DATA* idata, const char* pre, const char* post, + int flag, int changed, int invert); int imap_open_connection (IMAP_DATA* idata); void imap_close_connection (IMAP_DATA* idata); IMAP_DATA* imap_conn_find (const ACCOUNT* account, int flags); diff --git a/imap/message.c b/imap/message.c index e7df7bb18..e4536ae0e 100644 --- a/imap/message.c +++ b/imap/message.c @@ -688,13 +688,14 @@ int imap_copy_messages (CONTEXT* ctx, HEADER* h, char* dest, int delete) { IMAP_DATA* idata; BUFFER cmd, sync_cmd; - char uid[11]; char mbox[LONG_STRING]; char mmbox[LONG_STRING]; + char prompt[LONG_STRING]; int rc; int n; IMAP_MBOX mx; int err_continue = M_NO; + int triedcreate = 0; idata = (IMAP_DATA*) ctx->data; @@ -719,89 +720,101 @@ int imap_copy_messages (CONTEXT* ctx, HEADER* h, char* dest, int delete) } imap_fix_path (idata, mx.mbox, mbox, sizeof (mbox)); + imap_munge_mbox_name (mmbox, sizeof (mmbox), mbox); - memset (&sync_cmd, 0, sizeof (sync_cmd)); - memset (&cmd, 0, sizeof (cmd)); - mutt_buffer_addstr (&cmd, "UID COPY "); - - /* Null HEADER* means copy tagged messages */ - if (!h) + /* loop in case of TRYCREATE */ + do { - /* if any messages have attachments to delete, fall through to FETCH - * and APPEND. TODO: Copy what we can with COPY, fall through for the - * remainder. */ - for (n = 0; n < ctx->msgcount; n++) + memset (&sync_cmd, 0, sizeof (sync_cmd)); + memset (&cmd, 0, sizeof (cmd)); + + /* Null HEADER* means copy tagged messages */ + if (!h) { - if (ctx->hdrs[n]->tagged && ctx->hdrs[n]->attach_del) + /* if any messages have attachments to delete, fall through to FETCH + * and APPEND. TODO: Copy what we can with COPY, fall through for the + * remainder. */ + for (n = 0; n < ctx->msgcount; n++) { - dprint (3, (debugfile, "imap_copy_messages: Message contains attachments to be deleted\n")); - return 1; + if (ctx->hdrs[n]->tagged && ctx->hdrs[n]->attach_del) + { + dprint (3, (debugfile, "imap_copy_messages: Message contains attachments to be deleted\n")); + return 1; + } + + if (ctx->hdrs[n]->tagged && ctx->hdrs[n]->active && + ctx->hdrs[n]->changed) + { + rc = imap_sync_message (idata, ctx->hdrs[n], &sync_cmd, &err_continue); + if (rc < 0) + { + dprint (1, (debugfile, "imap_copy_messages: could not sync\n")); + goto fail; + } + } } - if (ctx->hdrs[n]->tagged && ctx->hdrs[n]->active && - ctx->hdrs[n]->changed) + rc = imap_exec_msgset (idata, "UID COPY", mmbox, M_TAG, 0, 0); + if (!rc) { - rc = imap_sync_message (idata, ctx->hdrs[n], &sync_cmd, &err_continue); - if (rc < 0) - { - dprint (1, (debugfile, "imap_copy_messages: could not sync\n")); - goto fail; - } + dprint (1, (debugfile, "imap_copy_messages: No messages tagged\n")); + goto fail; } + else if (rc < 0) + { + dprint (1, (debugfile, "could not queue copy\n")); + goto fail; + } + else + mutt_message (_("Copying %d messages to %s..."), rc, mbox); } - - rc = imap_make_msg_set (idata, &cmd, M_TAG, 0, 0); - if (!rc) + else { - dprint (1, (debugfile, "imap_copy_messages: No messages tagged\n")); - goto fail; - } - mutt_message (_("Copying %d messages to %s..."), rc, mbox); - } - else - { - mutt_message (_("Copying message %d to %s..."), h->index+1, mbox); - snprintf (uid, sizeof (uid), "%u", HEADER_DATA (h)->uid); - mutt_buffer_addstr (&cmd, uid); + mutt_message (_("Copying message %d to %s..."), h->index+1, mbox); + mutt_buffer_printf (&cmd, "UID COPY %u %s", HEADER_DATA (h)->uid, mmbox); - if (h->active && h->changed) - { - rc = imap_sync_message (idata, h, &sync_cmd, &err_continue); - if (rc < 0) + if (h->active && h->changed) { - dprint (1, (debugfile, "imap_copy_messages: could not sync\n")); - goto fail; + rc = imap_sync_message (idata, h, &sync_cmd, &err_continue); + if (rc < 0) + { + dprint (1, (debugfile, "imap_copy_messages: could not sync\n")); + goto fail; + } + } + if ((rc = imap_exec (idata, cmd.data, IMAP_CMD_QUEUE)) < 0) + { + dprint (1, (debugfile, "could not queue copy\n")); + goto fail; } } - } - /* let's get it on */ - mutt_buffer_addstr (&cmd, " "); - imap_munge_mbox_name (mmbox, sizeof (mmbox), mbox); - mutt_buffer_addstr (&cmd, mmbox); - - rc = imap_exec (idata, cmd.data, IMAP_CMD_FAIL_OK); - if (rc == -2) - { - /* bail out if command failed for reasons other than nonexistent target */ - if (ascii_strncasecmp (imap_get_qualifier (idata->buf), "[TRYCREATE]", 11)) - { - imap_error ("imap_copy_messages", idata->buf); - goto fail; - } - dprint (2, (debugfile, "imap_copy_messages: server suggests TRYCREATE\n")); - snprintf (mmbox, sizeof (mmbox), _("Create %s?"), mbox); - if (option (OPTCONFIRMCREATE) && mutt_yesorno (mmbox, 1) < 1) + /* let's get it on */ + rc = imap_exec (idata, NULL, IMAP_CMD_FAIL_OK); + if (rc == -2) { - mutt_clear_error (); - goto fail; + if (triedcreate) + { + dprint (1, (debugfile, "Already tried to create mailbox %s\n", mbox)); + break; + } + /* bail out if command failed for reasons other than nonexistent target */ + if (ascii_strncasecmp (imap_get_qualifier (idata->buf), "[TRYCREATE]", 11)) + break; + dprint (3, (debugfile, "imap_copy_messages: server suggests TRYCREATE\n")); + snprintf (prompt, sizeof (prompt), _("Create %s?"), mbox); + if (option (OPTCONFIRMCREATE) && mutt_yesorno (prompt, 1) < 1) + { + mutt_clear_error (); + break; + } + if (imap_create_mailbox (idata, mbox) < 0) + break; + triedcreate = 1; } - if (imap_create_mailbox (idata, mbox) < 0) - goto fail; - - /* try again */ - rc = imap_exec (idata, cmd.data, 0); } + while (rc == -2); + if (rc != 0) { imap_error ("imap_copy_messages", idata->buf); -- 2.49.0