mirror of
https://github.com/bulwarkmail/webmail.git
synced 2026-10-31 00:12:39 +00:00
fix: drop folder and filter fetches that finish after an account switch
A folder or filter fetch started for one account and answered after a switch wrote its result into the stores the next account had just filled. The sidebar then showed the old account's folders, and the filter store kept the old account's rules and script id, so the next save uploaded them - a redirect to someone else's address included - to the new account's server. Clearing the stores for another account now bumps a store epoch. `fetchMailboxes` and the email list's view check drop results from an older epoch, and the filter store drops any fetch that is no longer the latest one.
This commit is contained in:
@@ -12,6 +12,7 @@ import { DEFAULT_SEARCH_FILTERS } from '@/lib/jmap/search-utils';
|
||||
import { useIdentityStore } from '@/stores/identity-store';
|
||||
import { useVacationStore } from '@/stores/vacation-store';
|
||||
import { useFileStore } from '@/stores/file-store';
|
||||
import { bumpStoreEpoch } from '@/lib/store-epoch';
|
||||
|
||||
// Minimal snapshot shapes - we only capture what we need
|
||||
// eslint-disable-next-line @typescript-eslint/no-explicit-any
|
||||
@@ -105,6 +106,9 @@ export function restoreAccount(accountId: string): boolean {
|
||||
|
||||
/** Clear all stores (used before restoring a different account) */
|
||||
export function clearAllStores(): void {
|
||||
// In-flight fetches of the outgoing account must not land in the stores
|
||||
// the next account is about to fill.
|
||||
bumpStoreEpoch();
|
||||
useEmailStore.setState({
|
||||
emails: [],
|
||||
mailboxes: [],
|
||||
|
||||
@@ -0,0 +1,16 @@
|
||||
/**
|
||||
* Bumped whenever the account-scoped stores are emptied for another account
|
||||
* (an account switch, a sign-out). A fetch captures it before awaiting the
|
||||
* server and drops its result if it has changed since: the data belongs to
|
||||
* an account that is no longer on screen, and writing it would show - and
|
||||
* let the user act on - one account's folders or filters in another.
|
||||
*/
|
||||
let epoch = 0;
|
||||
|
||||
export function currentStoreEpoch(): number {
|
||||
return epoch;
|
||||
}
|
||||
|
||||
export function bumpStoreEpoch(): void {
|
||||
epoch++;
|
||||
}
|
||||
@@ -0,0 +1,80 @@
|
||||
import { expect, it } from 'vitest';
|
||||
|
||||
// Store fetches had no account guard: a request started for account A that
|
||||
// resolved after the switch to B wrote A's folders into B's sidebar, and A's
|
||||
// filter rules and script id into B's filter store - so B's next save
|
||||
// uploaded A's rules (a redirect to A's bookkeeper included) to B's server.
|
||||
import { useEmailStore } from '@/stores/email-store';
|
||||
import { useFilterStore } from '@/stores/filter-store';
|
||||
import { useAuthStore } from '@/stores/auth-store';
|
||||
import { clearAllStores } from '@/lib/account-state-manager';
|
||||
|
||||
function deferred<T>() {
|
||||
let resolve!: (v: T) => void;
|
||||
const promise = new Promise<T>((r) => { resolve = r; });
|
||||
return { promise, resolve };
|
||||
}
|
||||
|
||||
it('fetchMailboxes for A landing after the switch leaves B folder tree alone', async () => {
|
||||
const gate = deferred<void>();
|
||||
const aMailboxes = [
|
||||
{ id: 'a', name: 'Inbox', role: 'inbox', accountId: 'A', totalEmails: 1, unreadEmails: 0, sortOrder: 0 },
|
||||
{ id: 'g', name: 'A-Invoices', role: null, accountId: 'A', totalEmails: 1, unreadEmails: 0, sortOrder: 1 },
|
||||
];
|
||||
const clientA = {
|
||||
getAllMailboxesWithState: async () => { await gate.promise; return { mailboxes: aMailboxes, states: { A: 's1' } }; },
|
||||
};
|
||||
useAuthStore.setState({ activeAccountId: 'acct-A' });
|
||||
useEmailStore.setState({ mailboxes: [{ id: 'a', name: 'Inbox', role: 'inbox', accountId: 'A' }] as never, selectedMailbox: 'a' });
|
||||
const p = useEmailStore.getState().fetchMailboxes(clientA as never); // e.g. push-driven refresh for A
|
||||
|
||||
// User switches to B: stores reset and B's folders restored.
|
||||
clearAllStores();
|
||||
useAuthStore.setState({ activeAccountId: 'acct-B' });
|
||||
const bMailboxes = [
|
||||
{ id: 'a', name: 'Inbox', role: 'inbox', accountId: 'B' },
|
||||
{ id: 'g', name: 'B-Family', role: null, accountId: 'B' },
|
||||
];
|
||||
useEmailStore.setState({ mailboxes: bMailboxes as never, selectedMailbox: 'a' });
|
||||
|
||||
gate.resolve();
|
||||
await p;
|
||||
const names = useEmailStore.getState().mailboxes.map((m) => `${m.id}:${m.name}`);
|
||||
expect(names).toEqual(['a:Inbox', 'g:B-Family']);
|
||||
});
|
||||
|
||||
it('filter fetch for A landing after B loaded is dropped', async () => {
|
||||
const gateA = deferred<void>();
|
||||
const scriptA = 'require ["fileinto"];\r\nif header :contains "subject" "invoice" { redirect "bookkeeper@a.example"; }\r\n';
|
||||
const scriptB = 'require ["fileinto"];\r\nif header :contains "from" "mom@b.example" { fileinto "Family"; }\r\n';
|
||||
const mk = (sieveAcct: string, script: string, gate?: Promise<void>) => {
|
||||
const uploads: Array<{ id: string; content: string; account: string | undefined }> = [];
|
||||
return {
|
||||
uploads,
|
||||
client: {
|
||||
getSieveAccounts: () => [{ id: sieveAcct, name: sieveAcct, isPrimary: true }],
|
||||
getSieveAccountId: () => sieveAcct,
|
||||
getSieveCapabilities: () => ({ sieveExtensions: ['fileinto', 'include'] }),
|
||||
getSieveScripts: async () => { if (gate) await gate; return [{ id: sieveAcct === 'sA' ? 'a' : 'c', name: 'filters', blobId: 'blob-' + sieveAcct, isActive: true }]; },
|
||||
getSieveScriptContent: async () => script,
|
||||
updateSieveScript: async (id: string, content: string, _act: boolean, account?: string) => { uploads.push({ id, content, account }); },
|
||||
createSieveScript: async () => ({ id: 'new' }),
|
||||
},
|
||||
};
|
||||
};
|
||||
const A = mk('sA', scriptA, gateA.promise);
|
||||
const B = mk('sB', scriptB);
|
||||
|
||||
const fA = useFilterStore.getState().fetchFilters(A.client as never); // in flight for A
|
||||
useFilterStore.getState().clearState(); // switch → clearAllStores
|
||||
await useFilterStore.getState().selectAccount(B.client as never, 'sB'); // B's filters page loads
|
||||
gateA.resolve();
|
||||
await fA;
|
||||
const s = useFilterStore.getState();
|
||||
expect(s.selectedAccountId).toBe('sB');
|
||||
expect(s.activeScriptId).toBe('c');
|
||||
expect(s.rawScript).not.toContain('bookkeeper@a.example');
|
||||
await useFilterStore.getState().saveFilters(B.client as never); // user toggles a rule in B
|
||||
expect(B.uploads[0].account).toBe('sB');
|
||||
expect(B.uploads[0].content).not.toContain('bookkeeper@a.example');
|
||||
});
|
||||
+11
-1
@@ -12,6 +12,7 @@ import { threadKeyFor, threadIdFromKey } from "@/lib/thread-utils";
|
||||
import type { ExternalSearchResult } from "@/lib/plugin-types";
|
||||
import { fetchUnifiedEmails, fetchUnifiedMailboxCounts, searchUnifiedEmails, advancedSearchUnifiedEmails, fetchCrossViewEmails, searchCrossViewEmails, advancedSearchCrossViewEmails, fetchTagEmails, searchAcrossAccounts, advancedSearchAcrossAccounts, getCrossUnreadTotal, type UnifiedAccountClient, type UnifiedMailboxCounts } from "@/lib/unified-mailbox";
|
||||
import { useAuthStore } from "@/stores/auth-store";
|
||||
import { currentStoreEpoch } from "@/lib/store-epoch";
|
||||
import { useAccountStore } from "@/stores/account-store";
|
||||
import { useMessageListTabsStore } from "@/stores/message-list-tabs-store";
|
||||
|
||||
@@ -699,9 +700,11 @@ function resolveActionMailboxes(): Mailbox[] {
|
||||
function captureEmailListView(): () => boolean {
|
||||
const view = useEmailStore.getState();
|
||||
const activeAccountId = useAuthStore.getState().activeAccountId;
|
||||
const epoch = currentStoreEpoch();
|
||||
return () => {
|
||||
const current = useEmailStore.getState();
|
||||
return current.selectedMailbox === view.selectedMailbox
|
||||
return currentStoreEpoch() === epoch
|
||||
&& current.selectedMailbox === view.selectedMailbox
|
||||
&& current.selectedKeyword === view.selectedKeyword
|
||||
&& current.viewingAccountId === view.viewingAccountId
|
||||
&& current.isUnifiedView === view.isUnifiedView
|
||||
@@ -1662,6 +1665,10 @@ export const useEmailStore = create<EmailStore>((set, get) => ({
|
||||
|
||||
// JMAP operations
|
||||
fetchMailboxes: (client) => coalesceRefresh(client, 'mailboxes', async () => {
|
||||
// An account switch while this is in flight empties the stores for the
|
||||
// next account; this account's folders must not land in them.
|
||||
const epoch = currentStoreEpoch();
|
||||
const stale = () => currentStoreEpoch() !== epoch;
|
||||
// Only toggle the email list's isLoading on the initial load. Background
|
||||
// refreshes (after a move/archive that may have created new folders) must
|
||||
// not flash the list's loading state, which hides the results-count bar
|
||||
@@ -1674,6 +1681,7 @@ export const useEmailStore = create<EmailStore>((set, get) => ({
|
||||
const fetched = client.getAllMailboxesWithState
|
||||
? await client.getAllMailboxesWithState()
|
||||
: { mailboxes: await client.getAllMailboxes(), states: {} };
|
||||
if (stale()) return;
|
||||
const mailboxes = fetched.mailboxes;
|
||||
|
||||
// Guard against a transient fetch returning an empty list (e.g. a server
|
||||
@@ -1688,6 +1696,7 @@ export const useEmailStore = create<EmailStore>((set, get) => ({
|
||||
// fetch. This is also emitted for Mailbox-only state changes (for example
|
||||
// an empty provider label), where no Email fetch follows.
|
||||
await emailHooks.onMailboxesRefresh.emit(mailboxes);
|
||||
if (stale()) return;
|
||||
|
||||
// Auto-select inbox if no mailbox is selected or the current selection
|
||||
// doesn't exist in the fetched list (e.g. after an account switch)
|
||||
@@ -1715,6 +1724,7 @@ export const useEmailStore = create<EmailStore>((set, get) => ({
|
||||
set({ mailboxes, ...loadingPatch });
|
||||
}
|
||||
} catch (error) {
|
||||
if (stale()) return;
|
||||
// The existing folder list is deliberately left untouched: a failed
|
||||
// background refresh must never blank the sidebar (#780).
|
||||
set({
|
||||
|
||||
+32
-16
@@ -43,6 +43,14 @@ interface FilterStore {
|
||||
clearState: () => void;
|
||||
}
|
||||
|
||||
/**
|
||||
* Bumped by every fetch, account selection and reset. A fetch that is no
|
||||
* longer the latest when its answer arrives drops it: an account switch
|
||||
* during the fetch would otherwise leave one account's rules and script id
|
||||
* in place for the next account, and the next save would upload them there.
|
||||
*/
|
||||
let fetchGeneration = 0;
|
||||
|
||||
export const useFilterStore = create<FilterStore>()((set, get) => ({
|
||||
rules: [],
|
||||
isLoading: false,
|
||||
@@ -62,6 +70,8 @@ export const useFilterStore = create<FilterStore>()((set, get) => ({
|
||||
setSupported: (supported) => set({ isSupported: supported }),
|
||||
|
||||
fetchFilters: async (client, accountId) => {
|
||||
const generation = ++fetchGeneration;
|
||||
const stale = () => generation !== fetchGeneration;
|
||||
set({ isLoading: true, error: null });
|
||||
try {
|
||||
const accounts = client.getSieveAccounts();
|
||||
@@ -73,6 +83,7 @@ export const useFilterStore = create<FilterStore>()((set, get) => ({
|
||||
set({ sieveCapabilities: capabilities });
|
||||
|
||||
const allScripts = await client.getSieveScripts(resolvedId);
|
||||
if (stale()) return;
|
||||
debug.log('filters', 'Sieve scripts fetched:', allScripts.length);
|
||||
|
||||
// Skip the server-managed 'vacation' script (RFC 9661 §4) - it can only
|
||||
@@ -101,6 +112,7 @@ export const useFilterStore = create<FilterStore>()((set, get) => ({
|
||||
set({ activeScriptId: activeScript.id });
|
||||
|
||||
const content = await client.getSieveScriptContent(activeScript.blobId, resolvedId);
|
||||
if (stale()) return;
|
||||
set({ rawScript: content });
|
||||
|
||||
const result = parseScript(content);
|
||||
@@ -127,6 +139,7 @@ export const useFilterStore = create<FilterStore>()((set, get) => ({
|
||||
});
|
||||
}
|
||||
} catch (error) {
|
||||
if (stale()) return;
|
||||
debug.error('Failed to fetch filters:', error);
|
||||
set({
|
||||
isLoading: false,
|
||||
@@ -252,22 +265,25 @@ export const useFilterStore = create<FilterStore>()((set, get) => ({
|
||||
|
||||
resetToVisualBuilder: () => set({ isOpaque: false, rawScript: '', rules: [], externalRequires: [] }),
|
||||
|
||||
clearState: () => set({
|
||||
rules: [],
|
||||
isLoading: false,
|
||||
isSaving: false,
|
||||
error: null,
|
||||
isSupported: false,
|
||||
sieveCapabilities: null,
|
||||
activeScriptId: null,
|
||||
isOpaque: false,
|
||||
rawScript: '',
|
||||
vacationSettings: null,
|
||||
externalRequires: [],
|
||||
includeVacation: false,
|
||||
availableAccounts: [],
|
||||
selectedAccountId: null,
|
||||
}),
|
||||
clearState: () => {
|
||||
fetchGeneration++;
|
||||
set({
|
||||
rules: [],
|
||||
isLoading: false,
|
||||
isSaving: false,
|
||||
error: null,
|
||||
isSupported: false,
|
||||
sieveCapabilities: null,
|
||||
activeScriptId: null,
|
||||
isOpaque: false,
|
||||
rawScript: '',
|
||||
vacationSettings: null,
|
||||
externalRequires: [],
|
||||
includeVacation: false,
|
||||
availableAccounts: [],
|
||||
selectedAccountId: null,
|
||||
});
|
||||
},
|
||||
}));
|
||||
|
||||
function supportsInclude(capabilities: SieveCapabilities | null): boolean {
|
||||
|
||||
Reference in New Issue
Block a user