diff --git a/README.md b/README.md index 35817ed..90bf33f 100644 --- a/README.md +++ b/README.md @@ -92,7 +92,7 @@ Paths are relative to where the host mounts the sub-app. Every route returns 403 | POST | `/me/inbox/:uid/unread` | `mailbox:*` `manage` | Clear `\Seen` on a message in `?folder=` (INBOX by default). | | POST | `/me/inbox/:uid/archive` | `mailbox:*` `manage` | Move from INBOX to Archive. | | POST | `/me/inbox/:uid/trash` | `mailbox:*` `manage` | Move from INBOX to Trash. | -| POST | `/me/inbox/:uid/restore` | `mailbox:*` `manage` | Move back to INBOX from `?folder=` (Archive by default). | +| POST | `/me/inbox/:uid/restore` | `mailbox:*` `manage` | Move back to INBOX from `?folder=`: Archive (default) or Trash. | The grant is `mailbox:*` rather than a per-message resource because every query is already scoped to the caller's own tenant and principal. diff --git a/e2e/mount-native.test.ts b/e2e/mount-native.test.ts index 1fda1d8..2f3cce8 100644 --- a/e2e/mount-native.test.ts +++ b/e2e/mount-native.test.ts @@ -3,8 +3,15 @@ import { beforeEach, describe, expect, test } from "bun:test"; import { createMailboxRoutes } from "../src/mount.js"; import { createInMemoryMailboxEventBus } from "../src/bus.js"; +import { sql } from "drizzle-orm"; import { writeMailboxMessage } from "../src/write.js"; -import { allowAllGrants, mountAs, withTestDb, seedScope } from "./helpers.js"; +import { + allowAllGrants, + handle, + mountAs, + withTestDb, + seedScope, +} from "./helpers.js"; import type { MailboxDb } from "../src/db.js"; let db: MailboxDb; @@ -224,9 +231,115 @@ describe("archive/trash/restore", () => { expect(inbox.messages).toHaveLength(1); }); + test("restore out of anything but Archive or Trash is a 400", async () => { + const uid = await seedMessage("Stay"); + const app = buildApp(); + for (const folder of ["INBOX", "Sent", "junk"]) { + const res = await app.request( + `/me/inbox/${uid}/restore?folder=${folder}`, + { + method: "POST", + }, + ); + expect(res.status).toBe(400); + } + expect( + (await app.request(`/me/inbox/${uid}/read`, { method: "POST" })).status, + ).toBe(200); + }); + + test("an unknown ?folder= on read and unread is a 400 that creates no mailbox", async () => { + const app = buildApp(); + for (const verb of ["read", "unread"]) { + const res = await app.request(`/me/inbox/1/${verb}?folder=junk`, { + method: "POST", + }); + expect(res.status).toBe(400); + } + const rows = await db.execute( + sql`SELECT 1 FROM "mailbox"."mailbox_state" WHERE "folder" = 'junk'`, + ); + expect(rows).toHaveLength(0); + }); + test("trash on an unknown uid is a 404", async () => { const app = buildApp(); const res = await app.request("/me/inbox/999/trash", { method: "POST" }); expect(res.status).toBe(404); }); + + test("a move onto a Message-ID the destination holds is a 409 that burns no uid", async () => { + const message = { + ...SCOPE, + address: "p1@t1.example", + fromAddress: "a@t1.example", + subject: "Twice", + body: "Body", + messageId: "", + }; + await writeMailboxMessage(db, { ...message, folder: "Archive" }); + const written = await writeMailboxMessage(db, message); + const state = () => + db.execute<{ folder: string; uid_next: string }>( + sql`SELECT "folder", "uid_next" FROM "mailbox"."mailbox_state" ORDER BY "folder"`, + ); + const before = await state(); + const app = buildApp(); + const res = await app.request(`/me/inbox/${written!.uid}/archive`, { + method: "POST", + }); + expect(res.status).toBe(409); + expect(await state()).toEqual(before); + }); + + test("interleaved archive and restore moves never deadlock or lose rows", async () => { + for (let i = 0; i < 16; i++) { + await writeMailboxMessage(db, { + ...SCOPE, + folder: i % 2 === 0 ? "INBOX" : "Archive", + address: "p1@t1.example", + fromAddress: "a@t1.example", + subject: `m${i}`, + body: "Body", + }); + } + const app = buildApp(); + const move = async (path: string) => + (await app.request(path, { method: "POST" })).status; + const moves: Promise[] = []; + for (let uid = 1; uid <= 8; uid++) { + moves.push( + move(`/me/inbox/${uid}/archive`), + move(`/me/inbox/${uid}/restore?folder=Archive`), + ); + } + const statuses = await Promise.all(moves); + for (const status of statuses) expect([200, 404, 409]).toContain(status); + + const rows = await db.execute<{ + subject: string; + folder: string; + uid: number; + }>(sql`SELECT "subject", "folder", "uid" FROM "mailbox"."principal_mail"`); + expect(rows).toHaveLength(16); + expect(new Set(rows.map((r) => r.subject)).size).toBe(16); + expect(new Set(rows.map((r) => `${r.folder}/${r.uid}`)).size).toBe(16); + }); + + test("a move while the database is unreachable is a 500, not a 404", async () => { + const { client, db: down } = handle(); + await client.end(); + const app = mountAs( + SCOPE, + createMailboxRoutes({ + db: down, + requireGrant: allowAllGrants, + bus: createInMemoryMailboxEventBus(), + senderAddressFor: () => "p1@t1.example", + deliver: () => {}, + }), + ); + const res = await app.request("/me/inbox/1/archive", { method: "POST" }); + expect(res.status).toBe(500); + }); }); diff --git a/e2e/persist.test.ts b/e2e/persist.test.ts index 73f88f8..7b825c1 100644 --- a/e2e/persist.test.ts +++ b/e2e/persist.test.ts @@ -600,6 +600,24 @@ describe("frame size and recipient hard caps", () => { }); }); +describe("cached columns", () => { + test("an RFC 2047 subject is cached decoded", async () => { + const persist = createMailboxPersist(db, { + upstream: async () => undefined, + authorizeSender: () => ACTIVE, + }); + await persist( + args({ + raw: new TextEncoder().encode( + `From: ${SENDER}\r\nSubject: =?UTF-8?B?w6nDqcOp?=\r\nMessage-ID: \r\n\r\nhi\r\n`, + ), + }), + ); + const [row] = await rowsFor("acme", "user-1"); + expect(row?.subject).toBe("\u00e9\u00e9\u00e9"); + }); +}); + describe("Message-ID dedupe", () => { test("keys on the authorized envelope sender, not the From header", async () => { const persist = createMailboxPersist(db, { diff --git a/src/frame.test.ts b/src/frame.test.ts index 2a7784c..8def5f6 100644 --- a/src/frame.test.ts +++ b/src/frame.test.ts @@ -2,6 +2,7 @@ import { describe, expect, test } from "bun:test"; import { generateMessageId, parseHeaderSection } from "@intx/mime"; import { buildMailFrame, + decodeEncodedWords, decodeMailFrame, generateMailboxMessageId, MESSAGE_ID_FALLBACK_DOMAIN, @@ -191,3 +192,35 @@ describe("Message-ID domain fallback", () => { ); }); }); + +describe("decodeEncodedWords", () => { + test("decodes B and Q words and joins adjacent ones", () => { + expect(decodeEncodedWords("=?UTF-8?B?w6nDqcOp?=")).toBe( + "\u00e9\u00e9\u00e9", + ); + expect(decodeEncodedWords("Re: =?ISO-8859-1?Q?caf=E9_bar?=")).toBe( + "Re: caf\u00e9 bar", + ); + expect(decodeEncodedWords("=?UTF-8?Q?a?= =?UTF-8?Q?b?= c")).toBe("ab c"); + }); + + test("leaves plain text and unknown charsets as written", () => { + expect(decodeEncodedWords("plain ?= text")).toBe("plain ?= text"); + expect(decodeEncodedWords("=?x-nope?B?YQ==?=")).toBe("=?x-nope?B?YQ==?="); + }); + + test("leaves a word that does not decode as written", () => { + expect(decodeEncodedWords("=?UTF-8?B?!!!!?=")).toBe("=?UTF-8?B?!!!!?="); + expect(decodeEncodedWords("=?UTF-8?Q?=C3?= x")).toBe("=?UTF-8?Q?=C3?= x"); + expect(decodeEncodedWords("=?UTF-8?B?!!!!?= =?UTF-8?Q?b?=")).toBe( + "=?UTF-8?B?!!!!?= b", + ); + expect(decodeEncodedWords("=?UTF-8?Q?a?= =?UTF-8?B?!!!!?=")).toBe( + "a =?UTF-8?B?!!!!?=", + ); + }); + + test("keeps whitespace after a literal ?= that is not an encoded-word", () => { + expect(decodeEncodedWords("plain?= =?UTF-8?Q?b?=")).toBe("plain?= b"); + }); +}); diff --git a/src/frame.ts b/src/frame.ts index 740c991..bb73e5a 100644 --- a/src/frame.ts +++ b/src/frame.ts @@ -17,6 +17,76 @@ export function headerValue(value: string): string { .trim(); } +// RFC 2047 encoded-word: =?charset?B|Q?text?=, with the whitespace that +// follows it when another encoded-word comes next. +const WORD = String.raw`=\?[^?\s]+\?[BbQq]\?[^?\s]*\?=`; +const ENCODED_WORD = new RegExp( + String.raw`=\?([^?\s]+)\?([BbQq])\?([^?\s]*)\?=(\s+(?=${WORD}))?`, + "g", +); +const BASE64 = /^(?:[A-Za-z0-9+/]{4})*(?:[A-Za-z0-9+/]{2,3}={0,2})?$/; + +function decodeEncodedWord(charset: string, encoding: string, text: string) { + let bytes: Buffer; + if (encoding.toUpperCase() === "B") { + if (!BASE64.test(text)) throw new Error("invalid base64"); + bytes = Buffer.from(text, "base64"); + } else { + bytes = Buffer.from( + text + .replace(/_/g, " ") + .replace(/=([0-9A-Fa-f]{2})/g, (_, hex: string) => + String.fromCharCode(parseInt(hex, 16)), + ), + "latin1", + ); + } + return new TextDecoder(charset.split("*")[0], { fatal: true }).decode(bytes); +} + +/** + * A header value with its RFC 2047 encoded-words decoded, for the cached + * columns. Whitespace between adjacent encoded-words is dropped, as §6.2 + * requires. A word that does not decode (unknown charset, bad base64, bytes + * invalid in its charset) stays as written, with the whitespace around it. + */ +export function decodeEncodedWords(value: string): string { + return value.replace( + ENCODED_WORD, + ( + word, + charset: string, + encoding: string, + text: string, + gap = "", + at: number, + ) => { + const decoded = tryDecodeEncodedWord(charset, encoding, text); + if (decoded === undefined) return word; + const next = new RegExp(ENCODED_WORD.source, "y"); + next.lastIndex = at + word.length; + const following = next.exec(value); + const joins = + following !== null && + tryDecodeEncodedWord(following[1]!, following[2]!, following[3]!) !== + undefined; + return joins ? decoded : decoded + gap; + }, + ); +} + +function tryDecodeEncodedWord( + charset: string, + encoding: string, + text: string, +): string | undefined { + try { + return decodeEncodedWord(charset, encoding, text); + } catch { + return undefined; + } +} + /** * The domain a minted Message-ID falls back to when the sender address carries * none. Not a cosmetic choice: diff --git a/src/mount.ts b/src/mount.ts index c7a2bb7..c25ed63 100644 --- a/src/mount.ts +++ b/src/mount.ts @@ -16,6 +16,7 @@ import { } from "./bus.js"; import { openNativeMailboxStore, + MailboxMessageNotFoundError, moveNativeMailboxMessage, } from "./native-store.js"; import { @@ -91,6 +92,18 @@ function isListFolder(value: string): value is ListFolder { return (LIST_FOLDERS as readonly string[]).includes(value); } +/** Postgres unique_violation, raw or wrapped by drizzle as the cause. */ +function isUniqueViolation(err: unknown): boolean { + const code = (e: unknown) => (e as { code?: unknown } | null)?.code; + return ( + code(err) === "23505" || + (err instanceof Error && code(err.cause) === "23505") + ); +} + +/** Folders `restore` may move a message out of. */ +const RESTORE_FOLDERS: readonly string[] = ["Archive", "Trash"]; + function parseLimit( raw: string | undefined, ): { limit: number } | { error: string } { @@ -649,7 +662,7 @@ export function createMailboxRoutes( parameters: [ID_PARAM], responses: { 200: { description: "The flag was applied" }, - 400: { description: "uid is not a positive integer" }, + 400: { description: "Bad uid or folder" }, 403: { description: "No resolvable principalId" }, 404: { description: "No message with that uid in this mailbox" }, }, @@ -662,6 +675,9 @@ export function createMailboxRoutes( if (!resolved) return c.json({ error: "No resolvable principalId" }, 403); const folder = c.req.query("folder") ?? DEFAULT_FOLDER; + if (!isListFolder(folder)) { + return c.json({ error: "invalid folder" }, 400); + } const store = await openNativeMailboxStore( db, inFolder(resolved, folder), @@ -687,9 +703,16 @@ export function createMailboxRoutes( parameters: [ID_PARAM], responses: { 200: { description: "The message was moved" }, - 400: { description: "uid is not a positive integer" }, + 400: { + description: + "Bad uid, or a restore folder other than Archive or Trash", + }, 403: { description: "No resolvable principalId" }, 404: { description: "No message with that uid in the source folder" }, + 409: { + description: + "The destination already holds this Message-ID from the same sender", + }, }, }), async (c) => { @@ -702,6 +725,12 @@ export function createMailboxRoutes( // `restore` has no fixed source: a message can be restored out of // either Archive or Trash, named by `?folder=`. const fromFolder = from ?? c.req.query("folder") ?? "Archive"; + if (from === undefined && !RESTORE_FOLDERS.includes(fromFolder)) { + return c.json( + { error: "restore folder must be Archive or Trash" }, + 400, + ); + } let newUid: number; try { newUid = await moveNativeMailboxMessage( @@ -711,8 +740,14 @@ export function createMailboxRoutes( uid, to, ); - } catch { - return c.json({ error: "Message not found" }, 404); + } catch (err) { + if (err instanceof MailboxMessageNotFoundError) { + return c.json({ error: "Message not found" }, 404); + } + if (isUniqueViolation(err)) { + return c.json({ error: `${to} already holds this message` }, 409); + } + throw err; } publish(resolved, `${to}:${newUid}`, op); return c.json({ uid: newUid, ok: true as const }); diff --git a/src/native-store.ts b/src/native-store.ts index 87e0f59..225554d 100644 --- a/src/native-store.ts +++ b/src/native-store.ts @@ -97,7 +97,6 @@ type Row = { uid: string | number; modseq: string | number; flags: string[]; - raw: Uint8Array; subject: string | null; from_address: string | null; message_id: string | null; @@ -143,7 +142,7 @@ function toStoredMessage(row: Row): StoredMessage & { rowId: string } { } async function readState( - db: MailboxDb, + db: Pick, tenantId: string, principalId: string, folder: string, @@ -196,7 +195,7 @@ export async function openNativeMailboxStore( // a naive UTC value, so no AT TIME ZONE: that would re-render it in the // session zone. Never cast inside WHERE — that would break the index. const rows = await db.execute(sql` - SELECT "id", "uid", "modseq", "flags", "raw", "subject", "from_address", + SELECT "id", "uid", "modseq", "flags", "subject", "from_address", "message_id", "in_reply_to", "references", "to_addresses", to_char("created_at", 'YYYY-MM-DD"T"HH24:MI:SS.MS"Z"') AS "created_at" FROM "mailbox"."principal_mail" @@ -443,6 +442,11 @@ export async function openNativeMailboxStore( return store; } +/** `moveNativeMailboxMessage` found no message with that uid. */ +export class MailboxMessageNotFoundError extends Error { + override name = "MailboxMessageNotFoundError"; +} + /** * Move a message from one folder to another for the same (tenant, principal). * Not part of `@intx/mailbox`'s `MailboxStore` interface — IMAP MOVE reassigns a @@ -450,7 +454,9 @@ export async function openNativeMailboxStore( * both folders' `mailbox_state` rows, so this operates directly on the * database rather than through two `NativeMailboxStore` instances (each of * which only knows its own folder's counters). Returns the message's new uid - * in `toFolder`. Any already-open `NativeMailboxStore` for either folder must + * in `toFolder`, in one transaction, so a move that fails (the uid is not in + * `fromFolder`, or `toFolder` already holds its Message-ID from the same + * sender) changes nothing. Any already-open `NativeMailboxStore` for either folder must * be reopened with `openNativeMailboxStore` to see the result. */ export async function moveNativeMailboxMessage( @@ -461,40 +467,54 @@ export async function moveNativeMailboxMessage( toFolder: string, ): Promise { const { tenantId, principalId } = scope; - const rows = await db.execute<{ id: string }>(sql` - SELECT "id" FROM "mailbox"."principal_mail" - WHERE "tenant_id" = ${tenantId} AND "principal_id" = ${principalId} - AND "folder" = ${fromFolder} AND "uid" = ${uid} - `); - const row = rows[0]; - if (row === undefined) { - throw new Error(`Message UID ${uid} not found in mailbox "${fromFolder}"`); - } + return db.transaction(async (tx) => { + // Lock both folders' state rows in one fixed order, so moves in opposite + // directions queue instead of deadlocking. + await readState(tx, tenantId, principalId, toFolder); + await tx.execute(sql` + SELECT 1 FROM "mailbox"."mailbox_state" + WHERE "tenant_id" = ${tenantId} AND "principal_id" = ${principalId} + AND "folder" IN (${fromFolder}, ${toFolder}) + ORDER BY "folder" + FOR UPDATE + `); + const rows = await tx.execute<{ id: string }>(sql` + SELECT "id" FROM "mailbox"."principal_mail" + WHERE "tenant_id" = ${tenantId} AND "principal_id" = ${principalId} + AND "folder" = ${fromFolder} AND "uid" = ${uid} + FOR UPDATE + `); + const row = rows[0]; + if (row === undefined) { + throw new MailboxMessageNotFoundError( + `Message UID ${uid} not found in mailbox "${fromFolder}"`, + ); + } - await readState(db, tenantId, principalId, toFolder); - const bumped = await db.execute<{ - uid_next: string | number; - highest_modseq: string | number; - }>(sql` - UPDATE "mailbox"."mailbox_state" - SET "uid_next" = "uid_next" + 1, "highest_modseq" = "highest_modseq" + 1 - WHERE "tenant_id" = ${tenantId} AND "principal_id" = ${principalId} AND "folder" = ${toFolder} - RETURNING "uid_next" - 1 AS "uid_next", "highest_modseq" - `); - const newUid = Number(bumped[0]!.uid_next); - const newModseq = Number(bumped[0]!.highest_modseq); + const bumped = await tx.execute<{ + uid_next: string | number; + highest_modseq: string | number; + }>(sql` + UPDATE "mailbox"."mailbox_state" + SET "uid_next" = "uid_next" + 1, "highest_modseq" = "highest_modseq" + 1 + WHERE "tenant_id" = ${tenantId} AND "principal_id" = ${principalId} AND "folder" = ${toFolder} + RETURNING "uid_next" - 1 AS "uid_next", "highest_modseq" + `); + const newUid = Number(bumped[0]!.uid_next); + const newModseq = Number(bumped[0]!.highest_modseq); - await db.execute(sql` - UPDATE "mailbox"."principal_mail" - SET "folder" = ${toFolder}, "uid" = ${newUid}, "modseq" = ${newModseq} - WHERE "id" = ${row.id} - `); - await db.execute(sql` - UPDATE "mailbox"."mailbox_state" - SET "highest_modseq" = "highest_modseq" + 1 - WHERE "tenant_id" = ${tenantId} AND "principal_id" = ${principalId} AND "folder" = ${fromFolder} - `); - return newUid; + await tx.execute(sql` + UPDATE "mailbox"."principal_mail" + SET "folder" = ${toFolder}, "uid" = ${newUid}, "modseq" = ${newModseq} + WHERE "id" = ${row.id} + `); + await tx.execute(sql` + UPDATE "mailbox"."mailbox_state" + SET "highest_modseq" = "highest_modseq" + 1 + WHERE "tenant_id" = ${tenantId} AND "principal_id" = ${principalId} AND "folder" = ${fromFolder} + `); + return newUid; + }); } /** diff --git a/src/persist.ts b/src/persist.ts index 8ced5ea..2c8bdce 100644 --- a/src/persist.ts +++ b/src/persist.ts @@ -22,7 +22,11 @@ import { hostPrincipal } from "./schema.js"; import type { MailboxDb } from "./db.js"; import { openNativeMailboxStore } from "./native-store.js"; import { publishMailboxEvent, type MailboxEventBus } from "./bus.js"; -import { decodeMailFrame, parseMsgIdList } from "./frame.js"; +import { + decodeEncodedWords, + decodeMailFrame, + parseMsgIdList, +} from "./frame.js"; import { resolveMailboxRecipients, type ResolvedRecipient, @@ -199,7 +203,7 @@ export function createMailboxPersist( // can infer, matching what `writeMailboxMessage` mints when a caller // supplies none of these fields. const decoded = decodeMailFrame(raw); - const subject = decoded?.headers.get("subject") ?? ""; + const subject = decodeEncodedWords(decoded?.headers.get("subject") ?? ""); const fromAddress = decoded?.headers.get("from") ?? senderAddress; const messageId = decoded?.messageId ?? null; const inReplyTo =