[PATCH] LAYOUT=imapdir is broken in v2.2

[Jason Gunthorpe]

The next thing I noticed in my v1.2 -> 2.2 upgrade is that

mail_location = maildir:[..]:LAYOUT=imapdir

is broken, the symptom is dovecot returning this to the client when
requesting any mailbox beyond INBOX:

  Character not allowed in mailbox name: '

Which is actually trying to say "Character not allowed in mailbox name: '\0'",
but since the %c is not escaped it ends up with the truncated string.

This patch fixes it:

diff --git a/src/lib-storage/list/mailbox-list-maildir.c b/src/lib-storage/list/mailbox-list-maildir.c
index c99a2900a6d6..ae5f35d955ac 100644
--- a/src/lib-storage/list/mailbox-list-maildir.c
+++ b/src/lib-storage/list/mailbox-list-maildir.c
@@ -46,6 +46,7 @@ static struct mailbox_list *imapdir_list_alloc(void)
        list = p_new(pool, struct maildir_mailbox_list, 1);
        list->list = imapdir_mailbox_list;
        list->list.pool = pool;
+       list->sep = '.';
 
        list->global_temp_prefix = IMAPDIR_GLOBAL_TEMP_PREFIX;
        list->temp_prefix = p_strconcat(pool, list->global_temp_prefix,

Analysis:

I noticed this while upgrading a dovecot install from 1.2.15 (squeeze) to
2.2.13 (jessie).

This upstream commit

    author	Timo Sirainen <tss at iki.fi>
    Thu Jan 20 20:59:07 2011 +0200 (2011-01-20)
    changeset 12586	a2780b694b2d
    parent 12585	b748c622e896
    child 12587	c3a258ee96c4

    lib-storage: mailbox_alloc() now takes a virtual mailbox name and other related API changes.
    All storage_name <-> vname conversions now go through the same two
    mailbox_list methods. This has many benefits, such as:

    * listescape plugin is now much simpler and bugfree
    * allows changing lib-storage API to use UTF-8 mailbox names in future
    * allows creation of "mailbox aliases" plugin

Restructed the _alloc functions to move the hierarchy_sep from the initializer
into the _alloc call itself:

    @@ -29,6 +30,7 @@ static struct mailbox_list *maildir_list_alloc(void)
	    list = p_new(pool, struct maildir_mailbox_list, 1);
	    list->list = maildir_mailbox_list;
	    list->list.pool = pool;
    +       list->sep = '.';

	    list->global_temp_prefix = MAILDIR_GLOBAL_TEMP_PREFIX;
	    list->temp_prefix = p_strconcat(pool, list->global_temp_prefix,
    [..]
     struct mailbox_list maildir_mailbox_list = {
	    .name = MAILBOX_LIST_NAME_MAILDIRPLUSPLUS,
    -       .hierarchy_sep = '.',
	    .props = MAILBOX_LIST_PROP_NO_MAILDIR_NAME |
		    MAILBOX_LIST_PROP_NO_ALT_DIR |
		    MAILBOX_LIST_PROP_NO_NOSELECT,
    [..]
     struct mailbox_list imapdir_mailbox_list = {
	    .name = MAILBOX_LIST_NAME_IMAPDIR,
    -       .hierarchy_sep = '.',
	    .props = MAILBOX_LIST_PROP_NO_MAILDIR_NAME |
		    MAILBOX_LIST_PROP_NO_ALT_DIR |
		    MAILBOX_LIST_PROP_NO_NOSELECT,

Noting that heierarchy_sep was removed from maildir_mailbox_list and
imapdir_mailbox_list but only added to maildir_list_alloc(), and not
imapdir_list_alloc(). This ultimately results in
mailbox_list_get_hierarchy_sep() returning '\0' and mailbox_verify_name()
failing everything (all strings contain '\0' according to strchr).

This ended up as debian bug #774533

Regards,
Jason