Apply the upload limit only where something is uploaded

FEATURES has always said attach-from-Files works "however large", because a
blob the account already holds is attached by reference and nothing is
sent. The code checked every file against maxSizeUpload regardless, so the
two disagreed and the code was the one that was wrong.

maxSizeUpload is what the server will accept for a single upload (RFC
8620). It bears on a file that is about to be uploaded and on nothing
else. Applying it to a by-reference attachment refused a 60 MB message the
server was already storing, on the grounds that it could not have been
uploaded -- which it was not being. Forwarding a large message as an
attachment hit exactly that.

A file from somebody else's account is fetched and re-uploaded into this
one, because a message can only carry blobs from the account sending it.
That upload is real and the limit is real for it, so it still applies
there.

FEATURES.md

@@ -459,6 +459,13 @@ minimisable and maximisable; full-screen on mobile.
   upload at all**, however large. A file from someone else's shared folder is
   copied to your account first, because a message can only carry blobs from the
   account sending it; the picker says so before it does.
+
+  The upload limit applies to that copy and to nothing else. `maxSizeUpload` is
+  what the server will accept for a single *upload*, so it bears only on a file
+  that is about to be uploaded — a blob this account already holds is attached
+  by reference and never sent. Checking it in both cases refused a 60 MB message
+  the server was already storing, on the grounds that it could not have been
+  uploaded, which it was not being.
 - **Attachment reminder** when the text mentions an attachment and none is there.
 - **Spell check** toggle.
 - **Drafts** save as you type and on close, with the save state shown.

web/src/store/__tests__/attach-size-limit.test.ts

@@ -0,0 +1,100 @@
+import { beforeEach, describe, expect, it, vi } from "vitest";
+import { CAP, client } from "@/jmap/client";
+import { useCompose, type AttachableFile } from "@/store/compose";
+import { useMail } from "@/store/mail";
+import type { JmapSession } from "@/jmap/types";
+
+/**
+ * `maxSizeUpload` is what the server will accept for a single *upload*
+ * (RFC 8620), so it bears only on a file that is about to be uploaded.
+ *
+ * FEATURES has always said attach-from-Files works "however large" because a
+ * blob the account already holds is attached by reference. The code applied
+ * the limit to those as well, which refused a message the server was already
+ * storing on the grounds that it could not have been uploaded — which it was
+ * not being.
+ */
+
+const MAX = 50_000_000;
+const OURS = "a1";
+const THEIRS = "a2";
+
+const file = (over: Partial<AttachableFile> = {}): AttachableFile => ({
+  accountId: OURS,
+  name: "big.bin",
+  type: "application/octet-stream",
+  size: MAX * 2,
+  blobId: "b-big",
+  ...over,
+});
+
+const attachments = (key: string) => useCompose.getState().drafts.find((d) => d.key === key)!.attachments;
+
+beforeEach(() => {
+  client.session = {
+    capabilities: { [CAP.core]: { maxSizeUpload: MAX }, [CAP.mail]: {} },
+    accounts: {},
+    primaryAccounts: {},
+    state: "s1",
+  } as unknown as JmapSession;
+  useCompose.setState({ drafts: [], activeKey: null, pendingSends: {} });
+  useMail.setState({
+    accountId: OURS,
+    identities: [{ id: "i1", name: "John", email: "[email protected]", replyTo: null }] as never,
+  });
+  // Nothing here should reach the network; a call would mean an upload was
+  // attempted for a file that is only being referenced.
+  vi.stubGlobal("fetch", vi.fn(async () => {
+    throw new Error("no upload should happen");
+  }));
+});
+
+describe("attaching a blob this account already holds", () => {
+  it("takes it however large, because nothing is uploaded", async () => {
+    const key = useCompose.getState().open();
+    await useCompose.getState().addFromFiles(key, [file()]);
+    const a = attachments(key)[0]!;
+    expect(a.error).toBeNull();
+    expect(a.blobId).toBe("b-big");
+    expect(a.progress).toBe(100);
+  });
+
+  it("is complete the moment it is added, with no request made", async () => {
+    const key = useCompose.getState().open();
+    await useCompose.getState().addFromFiles(key, [file({ size: MAX * 10 })]);
+    expect(attachments(key)[0]!.error).toBeNull();
+    expect(fetch).not.toHaveBeenCalled();
+  });
+});
+
+describe("attaching a blob from somebody else's account", () => {
+  it("refuses one larger than the server will accept, since it must be uploaded", async () => {
+    const key = useCompose.getState().open();
+    await useCompose.getState().addFromFiles(key, [file({ accountId: THEIRS, size: MAX + 1 })]);
+    const a = attachments(key)[0]!;
+    expect(a.error).toMatch(/Larger than/);
+    expect(a.blobId).toBeNull();
+  });
+
+  it("allows one within the limit, and marks it as still needing the upload", async () => {
+    const key = useCompose.getState().open();
+    await useCompose.getState().addFromFiles(key, [file({ accountId: THEIRS, size: 1000, blobId: "b-small" })]);
+    const a = attachments(key)[0]!;
+    // The upload itself fails here because fetch is stubbed to throw; what
+    // matters is that it was attempted rather than refused up front.
+    expect(a.progress).not.toBe(100);
+  });
+});
+
+describe("a mixture in one drop", () => {
+  it("judges each file by whether it will actually be uploaded", async () => {
+    const key = useCompose.getState().open();
+    await useCompose.getState().addFromFiles(key, [
+      file({ name: "ours.bin", size: MAX * 3 }),
+      file({ name: "theirs.bin", accountId: THEIRS, size: MAX * 3, blobId: "b-theirs" }),
+    ]);
+    const [ours, theirs] = attachments(key);
+    expect(ours!.error).toBeNull();
+    expect(theirs!.error).toMatch(/Larger than/);
+  });
+});

web/src/store/compose.ts

@@ -487,15 +487,33 @@ export const useCompose = create<ComposeState>((set, get) => ({
     const accountId = useMail.getState().accountId;
     if (!accountId || !nodes.length) return;
     const max = client.maxSizeUpload;
-    const atts: ComposeAttachment[] = nodes.map((n) => ({
+    const atts: ComposeAttachment[] = nodes.map((n) => {
-      id: uid("a"),
+      /*
-      name: n.name,
+       * `maxSizeUpload` is what the server will accept for a single *upload*
-      type: n.type || "application/octet-stream",
+       * (RFC 8620), so it only bears on a file that is about to be uploaded.
-      size: n.size ?? 0,
+       *
-      blobId: n.accountId === accountId ? n.blobId : null,
+       * A blob already in this account is attached by reference and nothing is
-      progress: n.accountId === accountId ? 100 : 0,
+       * sent, however large it is -- which is the whole point of attaching from
-      error: (n.size ?? 0) > max ? translate("Larger than {size} MB limit", { size: Math.round(max / 1048576) }) : null,
+       * Files, and of forwarding a message as an attachment. Applying the limit
-    }));
+       * to those refused a 60 MB message the server was already holding, on the
+       * grounds that it could not have been uploaded, which it was not being.
+       *
+       * A file from somebody else's account is fetched and re-uploaded into
+       * this one, because a message can only carry blobs from the account
+       * sending it. That upload is real, and the limit is real for it.
+       */
+      const byReference = n.accountId === accountId;
+      const tooLargeToUpload = !byReference && (n.size ?? 0) > max;
+      return {
+        id: uid("a"),
+        name: n.name,
+        type: n.type || "application/octet-stream",
+        size: n.size ?? 0,
+        blobId: byReference ? n.blobId : null,
+        progress: byReference ? 100 : 0,
+        error: tooLargeToUpload ? translate("Larger than {size} MB limit", { size: Math.round(max / 1048576) }) : null,
+      };
+    });
     get().update(key, { attachments: [...(get().drafts.find((d) => d.key === key)?.attachments ?? []), ...atts] });
 
     for (const [i, a] of atts.entries()) {