cyrusimap / cyrus-imapd

Cyrus IMAP is an email, contacts and calendar server
http://cyrusimap.org
Other
543 stars 149 forks source link

[2.5.9] XLIST on murder frontend does not provide special use folders #41

Open MASHtm opened 8 years ago

MASHtm commented 8 years ago

checking directly on the backend I get on my own mailbox:

. xlist "" "*"
* XLIST (\HasChildren \Inbox) "." INBOX
* XLIST (\HasChildren) "." INBOX.Archiv
.....
* XLIST (\HasNoChildren) "." INBOX.Archiv.2016
* XLIST (\HasNoChildren \Drafts) "." INBOX.Drafts
* XLIST (\HasNoChildren \Junk) "." INBOX.Junk
* XLIST (\HasNoChildren \Sent) "." INBOX.Sent
* XLIST (\HasNoChildren \Trash) "." INBOX.Trash
.....
. OK Completed (0.990 secs 48 calls)

Via murder frontend:

. xlist "" "*"
* XLIST (\HasChildren \Inbox) "." INBOX
* XLIST (\HasChildren) "." INBOX.Archiv
* XLIST (\HasNoChildren) "." INBOX.Archiv.2005
.....
* XLIST (\HasNoChildren) "." INBOX.Archiv.2016
* XLIST (\HasNoChildren) "." INBOX.Drafts
* XLIST (\HasNoChildren) "." INBOX.Junk
* XLIST (\HasNoChildren) "." INBOX.Templates
* XLIST (\HasNoChildren) "." INBOX.Trash
....
. OK Completed (0.080 secs 39 calls)

directly asked for the metadata both front- and backend answer with:

. getmetadata INBOX.Sent "/private/specialuse"
* METADATA INBOX.Sent (/private/specialuse {5}
\Sent)
. OK Completed
elliefm commented 8 years ago

okay, so:

LIST commands (including XLIST) sent to the frontend currently only get proxied to the backend if subscription info is needed (either because we're listing subscribed mailboxes, or we've asked for subscription information to be returned in the results).

XLIST implies RETURN (CHILDREN SPECIAL-USE), plus an XLIST-specific special-use-like \Inbox flag for INBOX, but doesn't imply anything about subscriptions. So it generally doesn't get proxied to the backend, the frontend just answers directly.

The frontend doesn't seem to know about the mailbox's special-use annotations, so it doesn't return these flags.

You can force XLIST to be proxied to the backend by requesting subscription information, e.g. XLIST "" "*" RETURN (SUBSCRIBED). In doing so, the implicit RETURN (CHILDREN SPECIAL-USE) is preserved, but the fact that the command was XLIST gets lost, because the fact that it's also an extended list command overrides that, which will mean no \Inbox flag on the INBOX...

So the fix here seems like it might be in two parts:

But first I need to put together a test that verifies that it behaves the way I think it does (and then test master too, because I think it's affected in the same way).

elliefm commented 8 years ago

Cassandane test: https://github.com/cyrusimap/cassandane/commit/b078fcc5003322dab30abe398dbf22794f9e4e76

and it's confirmed that master is affected by this too.

elliefm commented 8 years ago

This seems to fix it on 2.5 for me: https://github.com/cyrusimap/cyrus-imapd/compare/cyrus-imapd-2.5...elliefm:v25/issue41

How's it look for you?

MASHtm commented 8 years ago

I applied the patch and will test it in the next days. One thing I recognized ... . XLIST "" "*" returns special-use flags now, but it only returns shared mailboxes from the same backend were your personal INBOX lives. Shared folders from other backends are missing.

eg.

. xlist "" "*"
* XLIST (\HasChildren \Inbox) "." INBOX
* XLIST (\HasNoChildren \Drafts) "." INBOX.Drafts
* XLIST (\HasNoChildren \Junk) "." INBOX.Junk
* XLIST (\HasNoChildren \Sent) "." INBOX.Sent
* XLIST (\HasNoChildren) "." INBOX.Templates
* XLIST (\HasNoChildren \Trash) "." INBOX.Trash
* XLIST (\HasNoChildren) "." INBOX.alerts
* XLIST (\HasNoChildren) "." INBOX.bettercrypto
* XLIST (\HasChildren) "." user.dmarcxxx
* XLIST (\HasNoChildren) "." user.dmarcxxx.dmarc_sa

while

. list "" "*"
* LIST (\HasChildren) "." INBOX
* LIST (\HasNoChildren) "." INBOX.Drafts
* LIST (\HasNoChildren) "." INBOX.Junk
* LIST (\HasNoChildren) "." INBOX.Sent
* LIST (\HasNoChildren) "." INBOX.Templates
* LIST (\HasNoChildren) "." INBOX.Trash
* LIST (\HasNoChildren) "." INBOX.alerts
* LIST (\HasNoChildren) "." INBOX.bettercrypto
* LIST (\HasChildren) "." user.dmarcxxx
* LIST (\HasNoChildren) "." user.dmarcxxx.dmarc_sa
* LIST (\HasChildren) "." user.maxxx
* LIST (\HasNoChildren) "." user.maxxx.abuse
* LIST (\HasNoChildren) "." user.mailboxxx.fail

user.dmarcxxx lives on the same backend as my INBOX. user.mailboxxx and user.maxxx live on different backends.

elliefm commented 8 years ago

Oh good catch. I think that's what this ancient comment is talking about:

        /* XXX   If we are in a standard Murder, and are given
           LIST () RETURN (SUBSCRIBED), we need to get the matching
           mailboxes locally (frontend) and the subscriptions remotely
           (INBOX backend).  We can only pass the buck to the INBOX backend
           if its running a unified config */

Which I think means the same problem (that the result only includes mailboxes from the same backend as your inbox) almost certainly occurs for LIST (SUBSCRIBED) ... and LIST ... RETURN (SUBSCRIBED) too?

MASHtm commented 8 years ago

no.

LIST (SUBSCRIBED) "" "*"

works as expected. Most likely because the "INBOX" backend knows about all subscriptions.

surprisingly

LIST "" "*" RETURN (SUBSCRIBED)

doesn't work at all on frontends:

. LIST "" "*" RETURN (SUBSCRIBED)
. OK Completed (0.000 secs)

it returns an empty result.

The INBOX backend returns

. list "" "*" RETURN (SUBSCRIBED)
* LIST (\Subscribed \HasChildren) "." INBOX (CHILDINFO ("SUBSCRIBED"))
* LIST (\Subscribed \HasNoChildren) "." INBOX.Drafts
* LIST (\Subscribed \HasNoChildren) "." INBOX.Junk
* LIST (\Subscribed \HasNoChildren) "." INBOX.Sent
* LIST (\Subscribed \HasNoChildren) "." INBOX.Templates
* LIST (\Subscribed \HasNoChildren) "." INBOX.Trash
* LIST (\Subscribed \HasNoChildren) "." INBOX.bettercrypto
* LIST (\HasNoChildren) "." INBOX.saved-messages
* LIST (\Subscribed \HasChildren) "." user.dmarxxx (CHILDINFO ("SUBSCRIBED"))
* LIST (\Subscribed \HasNoChildren) "." user.dmarxxx.dmarc_sa
. OK Completed (0.080 secs 115 calls)
elliefm commented 8 years ago

That's curious. I take it that this LIST ... RETURN (SUBSCRIBED) being empty on frontends is with the XLIST special-use patch? I wonder if it behaved differently beforehand?

MASHtm commented 7 years ago

I want to follow up on this bug. I updated to 2.5.11 recently and this one catched me again since I didn't apply the patch again. One thing I can tell now is that the answer to your last question if

. LIST "" "*" RETURN (SUBSCRIBED)

behaved differently without the XLIST fix is "no". It behaves exactly the same way on 2.5.11 without the above patch. The frontends return:

. LIST "" "*" RETURN (SUBSCRIBED)
. OK Completed (0.010 secs)
elliefm commented 7 years ago

Thanks for the follow up (and your other recent 2.5 reports/patches). FYI I've been away on medical leave for a while -- I'm returning to work slowly over the next month or two, but it may take longer to review/test/integrate the 2.5 fixes than usual, since I'm the only one who really looks at the 2.5 code at the moment and I'm only doing a few hours a week at this stage.

MASHtm commented 7 years ago

All the best. Take your time. No hurry. My only real concern is this specific bug (or the LIST vs. murder topic at all) which looks even worse on 3.x. Currently I don't even think about upgrading.