From 0db795371e95458c45ea9dccfed0d0b9952c4e19 Mon Sep 17 00:00:00 2001 From: John Coffey Date: Tue, 1 Sep 2026 21:57:11 -0700 Subject: [PATCH] Forward a message as an attachment Forwarding quoted the original into a new message, which is the right thing for passing on something to be read and the wrong thing for passing on something to be looked at. Quoting rewrites the body, drops the headers, and re-parents the attachments, so a bounce, a phishing report or anything else where the message itself is the evidence arrived altered. Forward as attachment sends the message whole, as a message/rfc822 part. It costs no upload at all: a message's own blobId is its RFC822 blob and already lives in the account, so this goes through the same by-reference path as attach-from-Files and a 40 MB message attaches as fast as a small one. It is in the message's own menu, the list's right-click menu, and the overflow on the reply strip at the foot of a thread, which is the one a thumb finds on a phone. Two things fixed on the way, both exposed rather than introduced by this. The filename rule was subject.replace(/[^\w.-]+/g, "_"), and \w without the u flag is ASCII: every character of a Russian, Japanese or Chinese subject failed the class, so those messages downloaded as a row of underscores. What is actually unsafe in a filename is much shorter than "not ASCII" -- path separators, the names Windows reserves, the control range -- so the rule now keeps letters from any script and drops only those. It lives in one place and the .eml download uses it too. And the composer's attachment chip set overflow/text-overflow on a span, where neither does anything, so the name never truncated and the size ran on after it on the same line. Only long names showed it, which is every .eml named from a subject. --- FEATURES.md | 13 +++ web/src/lib/__tests__/emlName.test.ts | 55 +++++++++++ web/src/lib/emlName.ts | 50 ++++++++++ .../__tests__/forward-as-attachment.test.ts | 93 +++++++++++++++++++ web/src/store/compose.ts | 26 ++++++ web/src/styles/app.css | 8 +- web/src/views/mail/MessageList.tsx | 1 + web/src/views/mail/MessageView.tsx | 6 +- web/src/views/mail/ThreadView.tsx | 3 +- 9 files changed, 251 insertions(+), 4 deletions(-) create mode 100644 web/src/lib/__tests__/emlName.test.ts create mode 100644 web/src/lib/emlName.ts create mode 100644 web/src/store/__tests__/forward-as-attachment.test.ts diff --git a/FEATURES.md b/FEATURES.md index 17b8ce0..a55403f 100644 --- a/FEATURES.md +++ b/FEATURES.md @@ -229,6 +229,19 @@ same query string — so what it builds can be read, edited and learned from. - **Attachments** listed with type and size: download, open in a new tab, and an inline preview for images and PDFs. - **Show original**, **Show headers**, **Download (.eml)** and **Print**. +- **Forward as attachment** sends the message itself rather than a quotation of + it — headers, structure and every attachment intact, which is what a bounce + or a phishing report needs and what quoting destroys. It costs **no upload at + all**: a message's `blobId` is its own RFC822 blob and already lives in the + account, so a 40 MB message attaches by reference as fast as a small one. In + the message's ⋮ menu, the list's right-click menu, and the overflow on the + reply strip at the foot of a thread, which is the one a thumb finds on a + phone. +- Saved and attached `.eml` files are **named from the subject in whatever + script it is written in**. The rule keeps letters and drops only what a + filesystem cannot take — path separators, the names Windows reserves, control + characters — so a Russian or Japanese subject keeps its own name instead of + becoming a row of underscores. - **Unsubscribe** where the message carries `List-Unsubscribe`. - **Sender details** expand to the full From/To/Cc/Reply-To with addresses. - **Message body theming** is off by default — sender HTML is left exactly as it diff --git a/web/src/lib/__tests__/emlName.test.ts b/web/src/lib/__tests__/emlName.test.ts new file mode 100644 index 0000000..9d3bd7d --- /dev/null +++ b/web/src/lib/__tests__/emlName.test.ts @@ -0,0 +1,55 @@ +import { describe, expect, it } from "vitest"; +import { emlFilename, sanitizeFilename } from "@/lib/emlName"; + +describe("emlFilename", () => { + it("keeps an ordinary subject, with spaces as underscores", () => { + expect(emlFilename("Quarterly report")).toBe("Quarterly_report.eml"); + }); + + it("keeps letters from any script, which the ASCII rule threw away", () => { + // The whole point: none of these may come out as a row of underscores. + expect(emlFilename("Квартальный отчёт")).toBe("Квартальный_отчёт.eml"); + expect(emlFilename("四半期報告")).toBe("四半期報告.eml"); + expect(emlFilename("Rapport trimestriel été")).toBe("Rapport_trimestriel_été.eml"); + }); + + it("keeps the punctuation that is fine in a filename", () => { + expect(emlFilename("Re- budget (v3) [final]")).toBe("Re-_budget_(v3)_[final].eml"); + }); + + it("drops path separators and the characters Windows reserves", () => { + expect(emlFilename("a/b\\c:d*e?f\"gi|j")).toBe("abcdefghij.eml"); + }); + + it("drops control characters", () => { + expect(emlFilename("a\u0007b\u0000c")).toBe("abc.eml"); + expect(emlFilename("a\u007fb")).toBe("ab.eml"); + }); + + it("falls back when there is no subject, or nothing survives", () => { + expect(emlFilename("")).toBe("message.eml"); + expect(emlFilename(null)).toBe("message.eml"); + expect(emlFilename(undefined)).toBe("message.eml"); + expect(emlFilename("///")).toBe("message.eml"); + expect(emlFilename(" ")).toBe("message.eml"); + }); + + it("does not end in a dot or a space, which Windows refuses", () => { + expect(emlFilename("Report.")).toBe("Report.eml"); + expect(emlFilename("Report ")).toBe("Report.eml"); + expect(emlFilename("...Report...")).toBe("Report.eml"); + }); + + it("does not start with a dot, which would hide the file on Unix", () => { + expect(emlFilename(".hidden")).toBe("hidden.eml"); + }); + + it("caps the length so it survives a filesystem limit", () => { + const name = emlFilename("x".repeat(500)); + expect(name).toBe(`${"x".repeat(80)}.eml`); + }); + + it("exposes the stem on its own", () => { + expect(sanitizeFilename("Quarterly report")).toBe("Quarterly_report"); + }); +}); diff --git a/web/src/lib/emlName.ts b/web/src/lib/emlName.ts new file mode 100644 index 0000000..14509f9 --- /dev/null +++ b/web/src/lib/emlName.ts @@ -0,0 +1,50 @@ +/** + * A filename for a message saved or attached as `.eml`. + * + * The rule this replaces was `subject.replace(/[^\w.-]+/g, "_")`, and `\w` + * without the `u` flag is ASCII: every character of a Russian, Japanese or + * Chinese subject failed the class, so those messages downloaded as a row of + * underscores. ihasmail ships in nine languages besides English, so the + * subjects it handled worst were most of the world's. + * + * What is actually unsafe in a filename is a much shorter list than "not + * ASCII": the path separators, the characters Windows reserves, and the + * control range. Everything else is a letter to somebody. + * + * The test is written by code point rather than as a character class because + * the escaping in one of those is its own small trap, and this says plainly + * what it means. + */ + +/** Reserved on Windows, or a path separator. */ +const RESERVED = '<>:"/\\|?*'; + +function unsafe(ch: string): boolean { + const c = ch.codePointAt(0) ?? 0; + // C0 controls, and DEL. + if (c < 0x20 || c === 0x7f) return true; + return RESERVED.includes(ch); +} + +/** + * Long enough to stay recognisable, short enough to survive a 255-*byte* limit + * once a CJK subject is three bytes a character. + */ +const MAX = 80; + +/** The stem only, so a caller can put another extension on it. */ +export function sanitizeFilename(subject: string | null | undefined): string { + const kept = [...(subject ?? "")].filter((ch) => !unsafe(ch)).join(""); + return kept + // Whitespace becomes an underscore rather than being kept: it is what the + // previous rule did, and it saves a quoting question in a shell later. + .replace(/\s+/g, "_") + .slice(0, MAX) + // Windows refuses a name ending in a dot or a space, and a leading dot + // hides the file on Unix. Neither is worth inheriting from a subject. + .replace(/^[.\s_]+|[.\s_]+$/g, ""); +} + +export function emlFilename(subject: string | null | undefined): string { + return `${sanitizeFilename(subject) || "message"}.eml`; +} diff --git a/web/src/store/__tests__/forward-as-attachment.test.ts b/web/src/store/__tests__/forward-as-attachment.test.ts new file mode 100644 index 0000000..7b1a454 --- /dev/null +++ b/web/src/store/__tests__/forward-as-attachment.test.ts @@ -0,0 +1,93 @@ +import { beforeEach, describe, expect, it } from "vitest"; +import { useCompose } from "@/store/compose"; +import { useMail } from "@/store/mail"; +import type { Email } from "@/jmap/types"; + +/** + * Forwarding a message whole rather than quoted. The point of the + * implementation is that it costs no upload: a message's own `blobId` is its + * RFC822 blob and already lives in this account, so the attachment references + * it directly. + */ +function email(over: Partial = {}): Email { + return { + id: "e1", + blobId: "b-raw-1", + threadId: "t1", + mailboxIds: { mb1: true }, + keywords: {}, + size: 40 * 1024 * 1024, + receivedAt: "2026-03-04T10:00:00Z", + sentAt: "2026-03-04T10:00:00Z", + subject: "Quarterly report", + from: [{ name: "Ada Lovelace", email: "ada@example.com" }], + to: [{ name: null, email: "john@example.org" }], + ...over, + } as Email; +} + +beforeEach(() => { + useCompose.setState({ drafts: [], activeKey: null, pendingSends: {} }); + useMail.setState({ + accountId: "a1", + identities: [{ id: "i1", name: "John", email: "john@example.org", replyTo: null }] as never, + }); +}); + +const draftFor = (key: string) => useCompose.getState().drafts.find((d) => d.key === key)!; + +describe("forwardAsAttachment", () => { + it("attaches the message itself, by reference, with no upload", () => { + const key = useCompose.getState().forwardAsAttachment(email()); + const d = draftFor(key); + expect(d.attachments).toHaveLength(1); + const a = d.attachments[0]!; + expect(a.type).toBe("message/rfc822"); + // The message's own blob, carried straight across: nothing was uploaded, + // and the attachment is complete the moment the composer opens. + expect(a.blobId).toBe("b-raw-1"); + expect(a.progress).toBe(100); + expect(a.error).toBeNull(); + }); + + it("names the attachment from the subject", () => { + expect(draftFor(useCompose.getState().forwardAsAttachment(email())).attachments[0]!.name).toBe("Quarterly_report.eml"); + }); + + it("names it from a subject in any script, not a row of underscores", () => { + const key = useCompose.getState().forwardAsAttachment(email({ subject: "四半期報告" })); + expect(draftFor(key).attachments[0]!.name).toBe("四半期報告.eml"); + }); + + it("falls back to a name when there is no subject", () => { + const key = useCompose.getState().forwardAsAttachment(email({ subject: null })); + expect(draftFor(key).attachments[0]!.name).toBe("message.eml"); + }); + + it("prefixes the subject once, and does not double it on a forward of a forward", () => { + expect(draftFor(useCompose.getState().forwardAsAttachment(email())).subject).toBe("Fwd: Quarterly report"); + const again = useCompose.getState().forwardAsAttachment(email({ subject: "Fwd: Quarterly report" })); + expect(draftFor(again).subject).toBe("Fwd: Quarterly report"); + }); + + it("marks the original forwarded, and starts no reply thread", () => { + const d = draftFor(useCompose.getState().forwardAsAttachment(email())); + expect(d.relatedEmailId).toBe("e1"); + expect(d.relatedKeyword).toBe("$forwarded"); + // A forward is not a reply: it must not join the original's thread. + expect(d.inReplyTo).toBeNull(); + expect(d.references).toBeNull(); + }); + + it("addresses nobody, since a forward chooses its own recipient", () => { + const d = draftFor(useCompose.getState().forwardAsAttachment(email())); + expect(d.to).toEqual([]); + expect(d.cc).toEqual([]); + }); + + it("does not quote the message into the body as well as attaching it", () => { + const d = draftFor(useCompose.getState().forwardAsAttachment(email())); + expect(d.html).not.toContain("Forwarded message"); + expect(d.text).not.toContain("Forwarded message"); + }); +}); diff --git a/web/src/store/compose.ts b/web/src/store/compose.ts index 1df1ba0..a546e54 100644 --- a/web/src/store/compose.ts +++ b/web/src/store/compose.ts @@ -11,6 +11,7 @@ import { ensureScheduledMailbox, useScheduled } from "./scheduled"; import { formatScheduleTime, holdUntil } from "@/lib/schedule"; import { t as translate } from "@/lib/i18n"; import { settings } from "./settings"; +import { emlFilename } from "@/lib/emlName"; export interface ComposeAttachment { id: string; @@ -84,6 +85,8 @@ interface ComposeState { /** Open a message again as a mail that has not been sent yet. */ composeAsNew(email: Email): Promise; reply(email: Email, mode: "reply" | "replyAll" | "forward", opts?: { all?: boolean }): Promise; + /** Forward the message whole, as an attachment, rather than quoted into a new one. */ + forwardAsAttachment(email: Email): string; update(key: string, patch: Partial): void; close(key: string, opts?: { discard?: boolean }): Promise; focus(key: string): void; @@ -381,6 +384,29 @@ export const useCompose = create((set, get) => ({ return d.key; }, + forwardAsAttachment(email) { + const accountId = useMail.getState().accountId; + const key = get().open({ + subject: replySubject(email.subject, "Fwd"), + relatedEmailId: email.id, + relatedKeyword: "$forwarded", + replyMode: "forward", + }); + // A message's own blobId *is* its RFC822 blob, and it already lives in this + // account -- so this goes through the same path as attach-from-Files and + // uploads nothing at all, however large the message. + // + // It inherits that path's size check as well, which is measured against + // `maxSizeUpload` even though nothing is being uploaded. That is worth + // knowing rather than working around here: the check belongs to + // `addFromFiles` and applies to every by-reference attachment, so if it is + // wrong it is wrong in one place and should be fixed there. + if (accountId) { + void get().addFromFiles(key, [{ accountId, name: emlFilename(email.subject), type: "message/rfc822", size: email.size, blobId: email.blobId }]); + } + return key; + }, + update(key, patch) { set((s) => ({ drafts: s.drafts.map((d) => (d.key === key ? { ...d, ...patch, dirty: patch.dirty ?? (d.dirty || isContentPatch(patch)) } : d)) })); if (isContentPatch(patch)) scheduleAutosave(key, get); diff --git a/web/src/styles/app.css b/web/src/styles/app.css index dc349c6..62b0ba3 100644 --- a/web/src/styles/app.css +++ b/web/src/styles/app.css @@ -666,8 +666,12 @@ a.menu-item:hover { color: var(--fg); } .attachment .att-icon { width: 36px; height: 36px; border-radius: 8px; display: flex; align-items: center; justify-content: center; background: var(--accent-soft); color: var(--accent-soft-fg); flex: 0 0 auto; overflow: hidden; } .attachment .att-icon img { width: 100%; height: 100%; object-fit: cover; } .attachment .att-text { flex: 1; min-width: 0; } -.attachment .att-name { font-weight: 600; white-space: nowrap; overflow: hidden; text-overflow: ellipsis; font-size: .92em; } -.attachment .att-size { color: var(--fg-muted); font-size: .8em; } +/* Both are spans, and `overflow`/`text-overflow` do nothing on an inline + element -- so the name never truncated and the size ran on after it on the + same line. Only long names showed it, which is every .eml named from a + subject. */ +.attachment .att-name { display: block; font-weight: 600; white-space: nowrap; overflow: hidden; text-overflow: ellipsis; font-size: .92em; } +.attachment .att-size { display: block; color: var(--fg-muted); font-size: .8em; } .attachment .att-actions { display: none; gap: 0; } .attachment:hover .att-actions { display: flex; } .attachment:hover .att-size { display: none; } diff --git a/web/src/views/mail/MessageList.tsx b/web/src/views/mail/MessageList.tsx index 38e8b60..949095c 100644 --- a/web/src/views/mail/MessageList.tsx +++ b/web/src/views/mail/MessageList.tsx @@ -475,6 +475,7 @@ export function MessageList({ title, list, openThreadId, focusId, setFocusId, on } label={t("Reply")} onClick={() => { const e = ctxRow ? emails[ctxRow] : undefined; if (e) void useCompose.getState().reply(e, "reply"); }} /> } label={t("Forward")} onClick={() => { const e = ctxRow ? emails[ctxRow] : undefined; if (e) void useCompose.getState().reply(e, "forward"); }} /> + } label={t("Forward as attachment")} onClick={() => { const e = ctxRow ? emails[ctxRow] : undefined; if (e) useCompose.getState().forwardAsAttachment(e); }} /> } label={t("Compose as new")} onClick={() => { const e = ctxRow ? emails[ctxRow] : undefined; if (e) void useCompose.getState().composeAsNew(e); }} /> } label={t("Archive")} kbd="e" onClick={() => void actions.archive(ctxTargets)} /> diff --git a/web/src/views/mail/MessageView.tsx b/web/src/views/mail/MessageView.tsx index d269827..8c35927 100644 --- a/web/src/views/mail/MessageView.tsx +++ b/web/src/views/mail/MessageView.tsx @@ -10,6 +10,7 @@ import { useContacts } from "@/store/contacts"; import { useCalendar } from "@/store/calendar"; import { startAppointment } from "@/lib/appointment"; import { client } from "@/jmap/client"; +import { emlFilename } from "@/lib/emlName"; import { formatFullDate, formatListDate, formatSize } from "@/lib/format"; import { displayName, formatAddress } from "@/lib/address"; import { EMAIL_BASE_CSS, TEXT_EMAIL_CSS, htmlDeclaresColors, sanitizeEmailHtml } from "@/lib/html"; @@ -126,7 +127,7 @@ export const MessageView = memo(function MessageView({ email: e, expanded, wasUn const downloadEml = () => { const a = document.createElement("a"); - a.href = client.downloadUrl(accountId, e.blobId, `${(e.subject || "message").replace(/[^\w.-]+/g, "_")}.eml`, "message/rfc822"); + a.href = client.downloadUrl(accountId, e.blobId, emlFilename(e.subject), "message/rfc822"); a.download = ""; a.click(); }; @@ -229,6 +230,9 @@ export const MessageView = memo(function MessageView({ email: e, expanded, wasUn } label={translate("Reply")} onClick={() => void reply(e, "reply")} /> } label={translate("Reply all")} onClick={() => void reply(e, "replyAll")} /> } label={translate("Forward")} onClick={() => void reply(e, "forward")} /> + {/* The same message rather than a quotation of it: headers, attachments + and all, for passing one on to be looked at rather than read. */} + } label={translate("Forward as attachment")} onClick={() => useCompose.getState().forwardAsAttachment(e)} /> {/* Sends the same mail again rather than passing it on, so it sits with the other three rather than down among the read-only actions. */} } label={translate("Compose as new")} onClick={() => void useCompose.getState().composeAsNew(e)} /> diff --git a/web/src/views/mail/ThreadView.tsx b/web/src/views/mail/ThreadView.tsx index df8bb5f..5fb4574 100644 --- a/web/src/views/mail/ThreadView.tsx +++ b/web/src/views/mail/ThreadView.tsx @@ -1,5 +1,5 @@ import { useCallback, useEffect, useMemo, useRef, useState } from "react"; -import { AlertOctagon, Archive, ArrowLeft, ChevronDown, ChevronUp, FolderInput, Forward, Mail, MailOpen, MailPlus, MoreVertical, Printer, Reply, ReplyAll, ShieldCheck, Star, Tag, Trash2, Download } from "lucide-react"; +import { AlertOctagon, Archive, ArrowLeft, ChevronDown, ChevronUp, FolderInput, Forward, Mail, MailOpen, MailPlus, MoreVertical, Printer, Reply, ReplyAll, ShieldCheck, Star, Tag, Trash2, Download , Paperclip} from "lucide-react"; import { useMail } from "@/store/mail"; import { useSettings } from "@/store/settings"; import { useCompose } from "@/store/compose"; @@ -302,6 +302,7 @@ export function ThreadView({ threadId, mailboxId, onBack, actions, onNavigate, h + } label={t("Forward as attachment")} onClick={() => useCompose.getState().forwardAsAttachment(last)} /> } label={t("Compose as new")} onClick={() => void useCompose.getState().composeAsNew(last)} />