Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix shared mailbox subscription bug when one user is prefix of other #5155

Open
wants to merge 18 commits into
base: master
Choose a base branch
from

Conversation

elliefm
Copy link
Contributor

@elliefm elliefm commented Dec 2, 2024

Reviewers: I'd suggest stepping through this one commit by commit

  • Fixes Cannot subscribe shared folder if destination mailbox name is a prefix of source mailbox name #5146 by more strictly checking boundaries when converting between mboxlist dbnames and keys.
  • Adds a new Cassandane suite "Shared" with tests for subscribing to shared mailboxes. I expect we'll accumulate other shared mailbox tests here over time.
  • Bumps the subscriptions database version number from 2 to 3, and makes the upgrade-on-open path fix any broken v2 entries it finds. It fixes the bad subscriptions by string manipulation -- it does not check whether those mailboxes actually exist, nor whether the subscribed user has access to them. I don't think this is a problem, as I believe you can ordinarily be subscribed to mailboxes that don't exist, or which you cannot see, and you just won't see them, or they'll have the \Nonexistent flag if you LSUB or LIST (SUBSCRIBED) ... them.
  • Renames the existing Cassandane "Lsub" test suite to "Subscriptions", and adds tests that create a v2 subscriptions db with good and bad records, and verify that it's correctly upgraded and fixed on first open.
  • Extends the old and barely-used Cassandane::Mboxname class to better support non-admin namespace mailboxes, because I needed that functionality for these tests. These improvements should make this class useful in many other tests too.
  • Also improves Cassandane::Util::Words a little, cause I needed lists of random words for the tests.

@elliefm elliefm force-pushed the v311/cyr-1410-shared-subs branch 4 times, most recently from 9c28409 to 9ae5781 Compare December 9, 2024 03:05
@elliefm elliefm force-pushed the v311/cyr-1410-shared-subs branch 5 times, most recently from b1073c8 to aea8a0c Compare December 18, 2024 00:19
@elliefm elliefm marked this pull request as ready for review December 18, 2024 00:42
cassandane/Cassandane/Util/Words.pm Outdated Show resolved Hide resolved
cassandane/Cassandane/Instance.pm Outdated Show resolved Hide resolved
cassandane/Cassandane/Mboxname.pm Show resolved Hide resolved
cassandane/Cassandane/Mboxname.pm Outdated Show resolved Hide resolved
imap/mboxlist.c Show resolved Hide resolved
@elliefm elliefm force-pushed the v311/cyr-1410-shared-subs branch from aea8a0c to b56690d Compare December 20, 2024 03:06
@elliefm
Copy link
Contributor Author

elliefm commented Dec 22, 2024

Even though it's just a niche bug fix, I think the sub.db version bump and autoupgrade makes this consequential enough to need a changes file

@elliefm elliefm force-pushed the v311/cyr-1410-shared-subs branch from b56690d to 43996bb Compare December 22, 2024 23:41
imap/mboxlist.c Outdated Show resolved Hide resolved
@elliefm elliefm force-pushed the v311/cyr-1410-shared-subs branch from 43996bb to bb951f4 Compare December 26, 2024 23:04
@elliefm elliefm requested a review from wolfsage December 26, 2024 23:20
@elliefm
Copy link
Contributor Author

elliefm commented Jan 3, 2025

@rsto I think you're doing the FM builds next week, so FYI I've tagged this include-in-fastmail

@elliefm elliefm added the backport-to-3.12 for PRs that are to be backported to 3.12 label Jan 3, 2025
@elliefm elliefm removed the request for review from rsto January 7, 2025 23:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport-to-3.12 for PRs that are to be backported to 3.12 include-in-fastmail
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Cannot subscribe shared folder if destination mailbox name is a prefix of source mailbox name
3 participants