listmailbox returns empty list for users with dot in their name

[mmatuska]

In cyrus-imapd 3.0, listmailbox does not show any mailboxes for users with dot in their name.

[elliefm]

In your imapd.conf, what is unixhierarchysep set to?

[mmatuska]

I have always been using:
altnamespace: yes
unixhierarchysep: yes

It worked well with 2.5.x. Other operations like create / rename or delete mailbox and ACL management do work. Just listmailbox is broken for me with 3.0 (and only for users with a dot in their name). Dots in names of submailboxes work well, too.I will run it through gdb and tell you more when I have more time.

[brong]

I'd like to have a Cassandane test to prove this, and just looking at this code it's almost certainly a horror show in there already and should probably be simplified rather than made even more complex! Thanks for the patch - if it solves the issue without breaking other cases, we'll definitely add it in, and make a note to refactor that code to simplify everything.

Cheers,

Bron.

[mmatuska]

I wonder that this important issue didn't show up in your tests. Changing unixhierarchysep to yes by default and having a broken listmailbox for users with a dot in their name is a major issue IMHO.

[brong]

Yep! That's totally true. Of course, users with dots in their username didn't even work without unixhierarchysep set to yes, so it's not as if changing the default made any difference. It's definitely a legitimate bug though. I've added two Cassandane tests which prove that the bug exists, and I'm checking your patch to see if it breaks any of the other tests.

…

On Thu, 6 Apr 2017, at 20:42, Martin Matuška wrote: I wonder that this important issue didn't show up in your tests. Changing unixhierarchysep to yes by default and having a broken listmailbox for users with a dot in their name is a major issue IMHO. — You are receiving this because you commented. Reply to this email directly, view it on GitHub[1], or mute the thread[2]. {"api_version":"1.0","publisher":{"api_key":"05dde50f1d1a384dd78767c5- 5493e4bb","name":"GitHub"},"entity":{"external_key":"github/cyrusimap- /cyrus-imapd","title":"cyrusimap/cyrus-imapd","subtitle":"GitHub repository","main_image_url":" https://cloud.githubusercontent.com/assets/143418/17495839/a5054eac-5d88-11e6-95fc-7290892c7bb5.png","avatar_image_url":"https://cloud.githubusercontent.com/assets/143418/15842166/7c72db34-2c0b-11e6-9aed-b52498112777.png","action":{"name":"Open in GitHub","url":" ***@***.*** in #1875: I wonder that this important issue didn't show up in your tests. Changing unixhierarchysep to yes by default and having a broken listmailbox for users with a dot in their name is a major issue IMHO."}],"action":{"name":"View Issue","url":" #1875 (comment)"}}}

-- Bron Gondwana brong@fastmail.fm Links: 1. #1875 (comment) 2. https://github.com/notifications/unsubscribe-auth/AABE7UPXraTi4_z4fzE-lRlBCyZnxafgks5rtMGKgaJpZM4MvEN7

[brong]

Thanks for fixing this for us. Our tests are over in the cassandane repository. You can see the tests I used to confirm the bug and the fix here:

cyrusimap/cassandane@f39411f

Unfortunately, our test coverage is nowhere near comprehensive, and nobody had built a test for this particular case before. At least it won't get broken again now!

If you have any other things about Cyrus behaviour that you particularly want to make sure are never broken, additional tests are very welcome too.

[mmatuska]

Thank you for implementing the tests, they look good. How does your merging policy to the 3.0 branch look like?

Of course, users with dots in their username didn't even work without unixhierarchysep set to yes, so it's not as if changing the default made any difference. It's definitely a legitimate bug though.

Just to clarify - I don't understand your wording:

didn't even work without unixhierarchysep set to yes

Users cannot have dots in names without unixhierarchysep set to yes by definition as dot acts as a hierarchy separator in that case (tom.tester means "submailbox tester of user tom" without unixhierarchysep: yes).

[elliefm]

How does your merging policy to the 3.0 branch look like?

If it fixes stuff that's broken, and doesn't change stuff that's not, it's mergeable. I'm just in the process of merging your patch over to the 3.0 branch now, thanks! It'll be in release 3.0.1.