From 96d99c383785dec67443ff1b45e2d2f8437398fa Mon Sep 17 00:00:00 2001 From: Carl Worth Date: Thu, 11 Nov 2010 16:36:02 -0800 Subject: [PATCH] tags_to_maildir_flags: Fix to preserve existing, unsupported flags This is to prevent notmuch from destroying any information the user has encoded as flags in the maildir filename. Tests are also added to the test suite to verify the documented behavior. --- lib/message.cc | 333 +++++++++++++++++++++++++++++++--------------- lib/notmuch.h | 12 +- test/maildir-sync | 12 +- 3 files changed, 242 insertions(+), 115 deletions(-) diff --git a/lib/message.cc b/lib/message.cc index 6d8b6c7a..c2da25c9 100644 --- a/lib/message.cc +++ b/lib/message.cc @@ -453,6 +453,41 @@ _notmuch_message_add_filename (notmuch_message_t *message, return NOTMUCH_STATUS_SUCCESS; } +/* Change a particular filename for 'message' from 'old_filename' to + * 'new_filename' + * + * This change will not be reflected in the database until the next + * call to _notmuch_message_sync. + */ +notmuch_status_t +_notmuch_message_rename (notmuch_message_t *message, + const char *old_filename, + const char *new_filename) +{ + void *local = talloc_new (message); + char *direntry; + notmuch_private_status_t private_status; + notmuch_status_t status; + + status = _notmuch_message_add_filename (message, new_filename); + if (status) + return status; + + status = _notmuch_database_filename_to_direntry (local, message->notmuch, + old_filename, &direntry); + if (status) + return status; + + private_status = _notmuch_message_remove_term (message, + "file-direntry", direntry); + status = COERCE_STATUS (private_status, + "Unexpected error from _notmuch_message_remove_term"); + + talloc_free (local); + + return status; +} + char * _notmuch_message_talloc_copy_data (notmuch_message_t *message) { @@ -763,46 +798,6 @@ _notmuch_message_remove_term (notmuch_message_t *message, return NOTMUCH_PRIVATE_STATUS_SUCCESS; } -/* Change the message filename stored in the database. - * - * This change will not be reflected in the database until the next - * call to _notmuch_message_sync. - */ -notmuch_status_t -_notmuch_message_rename (notmuch_message_t *message, - const char *new_filename) -{ - void *local = talloc_new (message); - char *direntry; - Xapian::PostingIterator i, end; - Xapian::Document document; - notmuch_private_status_t private_status; - notmuch_status_t status; - const char *old_filename; - - old_filename = notmuch_message_get_filename(message); - old_filename = talloc_reference(local, old_filename); - if (unlikely (! old_filename)) - return NOTMUCH_STATUS_OUT_OF_MEMORY; - - status = _notmuch_message_add_filename (message, new_filename); - if (status) - return status; - - status = _notmuch_database_filename_to_direntry (local, message->notmuch, - old_filename, &direntry); - if (status) - return status; - - private_status = _notmuch_message_remove_term (message, "file-direntry", direntry); - status = COERCE_STATUS (private_status, - "Unexpected error from _notmuch_message_remove_term"); - - talloc_free (local); - - return status; -} - notmuch_status_t notmuch_message_add_tag (notmuch_message_t *message, const char *tag) { @@ -915,36 +910,6 @@ notmuch_message_maildir_flags_to_tags (notmuch_message_t *message) return status; } -static void -maildir_get_new_flags(notmuch_message_t *message, char *flags) -{ - notmuch_tags_t *tags; - const char *tag; - unsigned i; - char *p; - - for (i = 0; i < ARRAY_SIZE(flag2tag); i++) - flags[i] = flag2tag[i].inverse ? flag2tag[i].flag : '\0'; - - for (tags = notmuch_message_get_tags (message); - notmuch_tags_valid (tags); - notmuch_tags_move_to_next (tags)) - { - tag = notmuch_tags_get (tags); - for (i = 0; i < ARRAY_SIZE(flag2tag); i++) { - if (strcmp(tag, flag2tag[i].tag) == 0) - flags[i] = flag2tag[i].inverse ? '\0' : flag2tag[i].flag; - } - } - - p = flags; - for (i = 0; i < ARRAY_SIZE(flag2tag); i++) { - if (flags[i]) - *p++ = flags[i]; - } - *p = '\0'; -} - /* Is the given filename within a maildir directory? * * Specifically, is the final directory component of 'filename' either @@ -986,20 +951,180 @@ _filename_is_in_maildir (const char *filename) return NULL; } -/* XXX: Needs to ensure that existing, unsupported flags in the - * filename are left unchanged (which also needs a test in the - * test suite). +/* From the set of tags on 'message' and the flag2tag table, compute a + * set of maildir-flag actions to be taken, (flags that should be + * either set or cleared). + * + * The result is returned as two talloced strings: to_set, and to_clear */ +static void +_get_maildir_flag_actions (notmuch_message_t *message, + char **to_set_ret, + char **to_clear_ret) +{ + char *to_set, *to_clear; + notmuch_tags_t *tags; + const char *tag; + unsigned i; + + to_set = talloc_strdup (message, ""); + to_clear = talloc_strdup (message, ""); + + /* First, find flags for all set tags. */ + for (tags = notmuch_message_get_tags (message); + notmuch_tags_valid (tags); + notmuch_tags_move_to_next (tags)) + { + tag = notmuch_tags_get (tags); + + for (i = 0; i < ARRAY_SIZE (flag2tag); i++) { + if (strcmp (tag, flag2tag[i].tag) == 0) { + if (flag2tag[i].inverse) + to_clear = talloc_asprintf_append (to_clear, + "%c", + flag2tag[i].flag); + else + to_set = talloc_asprintf_append (to_set, + "%c", + flag2tag[i].flag); + } + } + } + + /* Then, find the flags for all tags not present. */ + for (i = 0; i < ARRAY_SIZE (flag2tag); i++) { + if (flag2tag[i].inverse) { + if (strchr (to_clear, flag2tag[i].flag) == NULL) + to_set = talloc_asprintf_append (to_set, "%c", flag2tag[i].flag); + } else { + if (strchr (to_set, flag2tag[i].flag) == NULL) + to_clear = talloc_asprintf_append (to_clear, "%c", flag2tag[i].flag); + } + } + + *to_set_ret = to_set; + *to_clear_ret = to_clear; +} + +/* Given 'filename' and a set of maildir flags to set and to clear, + * compute the new maildir filename. + * + * If the existing filename is in the directory "new", the new + * filename will be in the directory "cur". + * + * After a sequence of ":2," in the filename, any subsequent + * single-character flags will be added or removed according to the + * characters in flags_to_set and flags_to_clear. Any existing flags + * not mentioned in either string will remain. The final list of flags + * will be in ASCII order. + * + * If the original flags seem invalid, (repeated characters or + * non-ASCII ordering of flags), this function will return NULL + * (meaning that renaming would not be safe and should not occur). + */ +static char* +_new_maildir_filename (void *ctx, + const char *filename, + const char *flags_to_set, + const char *flags_to_clear) +{ + const char *info, *flags; + unsigned int flag, last_flag; + char *filename_new, *dir; + char flag_map[128]; + int flags_in_map = 0; + unsigned int i; + char *s; + + memset (flag_map, 0, sizeof (flag_map)); + + info = strstr (filename, ":2,"); + + if (info == NULL) { + info = filename + strlen(filename); + } else { + flags = info + 3; + + /* Loop through existing flags in filename. */ + for (flags = info + 3, last_flag = 0; + *flags; + last_flag = flag, flags++) + { + flag = *flags; + + /* Original flags not in ASCII order. Abort. */ + if (flag < last_flag) + return NULL; + + /* Non-ASCII flag. Abort. */ + if (flag > sizeof(flag_map) - 1) + return NULL; + + /* Repeated flag value. Abort. */ + if (flag_map[flag]) + return NULL; + + flag_map[flag] = 1; + flags_in_map++; + } + } + + /* Then set and clear our flags from tags. */ + for (flags = flags_to_set; *flags; flags++) { + flag = *flags; + if (flag_map[flag] == 0) { + flag_map[flag] = 1; + flags_in_map++; + } + } + + for (flags = flags_to_clear; *flags; flags++) { + flag = *flags; + if (flag_map[flag]) { + flag_map[flag] = 0; + flags_in_map--; + } + } + + filename_new = (char *) talloc_size (ctx, + info - filename + + strlen (":2,") + flags_in_map + 1); + if (unlikely (filename_new == NULL)) + return NULL; + + strncpy (filename_new, filename, info - filename); + filename_new[info - filename] = '\0'; + + strcat (filename_new, ":2,"); + + s = filename_new + strlen (filename_new); + for (i = 0; i < sizeof (flag_map); i++) + { + if (flag_map[i]) { + *s = i; + s++; + } + } + *s = '\0'; + + /* If message is in new/ move it under cur/. */ + dir = (char *) _filename_is_in_maildir (filename_new); + if (dir && STRNCMP_LITERAL (dir, "new/") == 0) + memcpy (dir, "cur/", 4); + + return filename_new; +} + notmuch_status_t notmuch_message_tags_to_maildir_flags (notmuch_message_t *message) { notmuch_filenames_t *filenames; - char flags[ARRAY_SIZE(flag2tag)+1]; - const char *filename, *p; - char *filename_new, *dir; - int ret; + const char *filename; + char *filename_new; + char *to_set, *to_clear; + notmuch_status_t status; - maildir_get_new_flags (message, flags); + _get_maildir_flag_actions (message, &to_set, &to_clear); for (filenames = notmuch_message_get_filenames (message); notmuch_filenames_valid (filenames); @@ -1010,46 +1135,36 @@ notmuch_message_tags_to_maildir_flags (notmuch_message_t *message) if (! _filename_is_in_maildir (filename)) continue; - p = strstr(filename, ":2,"); - if ((p && strcmp (p+3, flags) == 0) || - (!p && flags[0] == '\0')) - { + filename_new = _new_maildir_filename (message, filename, + to_set, to_clear); + if (filename_new == NULL) continue; - } - if (!p) - p = filename + strlen(filename); + if (strcmp (filename, filename_new)) { + int err; + notmuch_status_t new_status; - filename_new = (char*) talloc_size (message, - (p-filename) + 3 + sizeof (flags)); - if (unlikely (filename_new == NULL)) - return NOTMUCH_STATUS_OUT_OF_MEMORY; + err = rename (filename, filename_new); + if (err) + continue; - memcpy (filename_new, filename, p-filename); - filename_new[p-filename] = '\0'; - - /* If message is in new/ move it under cur/. */ - dir = (char *) _filename_is_in_maildir (filename_new); - if (dir && STRNCMP_LITERAL (dir, "new/") == 0) - memcpy (dir, "cur/", 4); - - strcpy (filename_new+(p-filename), ":2,"); - strcpy (filename_new+(p-filename)+3, flags); - - if (strcmp (filename, filename_new) != 0) { - notmuch_status_t status = NOTMUCH_STATUS_SUCCESS; - - ret = rename (filename, filename_new); - if (ret == 0) - status = _notmuch_message_rename (message, filename_new); + new_status = _notmuch_message_rename (message, + filename, filename_new); + /* Hold on to only the first error. */ + if (! status && new_status) { + status = new_status; + continue; + } _notmuch_message_sync (message); - - if (status) - return status; } + + talloc_free (filename_new); } + talloc_free (to_set); + talloc_free (to_clear); + return NOTMUCH_STATUS_SUCCESS; } diff --git a/lib/notmuch.h b/lib/notmuch.h index d2deca1e..e508309e 100644 --- a/lib/notmuch.h +++ b/lib/notmuch.h @@ -947,9 +947,11 @@ notmuch_message_maildir_flags_to_tags (notmuch_message_t *message); * * Specifically, for each filename corresponding to this message: * - * If the filename is not in a maildir directory, do nothing. - * (A maildir directory is determined as a directory named "new" or - * "cur".) + * If the filename is not in a maildir directory, do nothing. (A + * maildir directory is determined as a directory named "new" or + * "cur".) Similarly, if the filename has invalid maildir info, + * (repeated or outof-ASCII-order flag characters after ":2,"), then + * do nothing. * * If the filename is in a maildir directory, rename the file so that * its filename ends with the sequence ":2," followed by zero or more @@ -961,8 +963,8 @@ notmuch_message_maildir_flags_to_tags (notmuch_message_t *message); * 'R' iff the message has the "replied" tag * 'S' iff the message does not have the "unread" tag * - * Warning: any existing flags unmentioned in the list above will be - * removed by this renaming. + * Any existing flags unmentioned in the list above will be preserved + * in the renaming. * * Also, if this filename is in a directory named "new", rename it to * be within the neighboring directory named "cur". diff --git a/test/maildir-sync b/test/maildir-sync index df9351de..50a5b8e6 100755 --- a/test/maildir-sync +++ b/test/maildir-sync @@ -133,7 +133,7 @@ expected=$(ls $MAIL_DIR/cur) mv $MAIL_DIR/cur/adding-replied-tag:2,RS $MAIL_DIR/cur/adding-replied-tag:2,S mv $MAIL_DIR/cur/adding-s-flag:2,S $MAIL_DIR/cur/adding-s-flag:2, mv $MAIL_DIR/cur/adding-with-s-flag:2,S $MAIL_DIR/cur/adding-with-s-flag:2,RS -mv $MAIL_DIR/cur/message-to-move-to-cur:2,S $MAIL_DIR/cur/message-to-move-to-cur:2,SD +mv $MAIL_DIR/cur/message-to-move-to-cur:2,S $MAIL_DIR/cur/message-to-move-to-cur:2,DS increment_mtime $MAIL_DIR/cur notmuch dump dump.txt NOTMUCH_NEW >/dev/null @@ -165,4 +165,14 @@ test_expect_equal "$(< actual)" "duplicated-message-another-copy:2,S duplicated-message-copy:2,S duplicated-message:2,S" +test_begin_subtest "Synchronizing tag changes preserves unsupported maildir flags" +add_message [subject]='"Unsupported maildir flags"' [dir]=cur [filename]='unsupported-maildir-flags:2,FSZxyz' +notmuch tag +unread +draft -flagged subject:"Unsupported maildir flags" +test_expect_equal "$(cd $MAIL_DIR/cur/; ls unsupported*)" "unsupported-maildir-flags:2,DZxyz" + +test_begin_subtest "A file with non-compliant maildir info will not be renamed" +add_message [subject]='"Non-compliant maildir info"' [dir]=cur [filename]='non-compliant-maildir-info:2,These-are-not-flags-in-ASCII-order-donottouch' +notmuch tag +unread +draft -flagged subject:"Non-compliant maildir info" +test_expect_equal "$(cd $MAIL_DIR/cur/; ls non-compliant*)" "non-compliant-maildir-info:2,These-are-not-flags-in-ASCII-order-donottouch" + test_done