From c0f556b19071a4de8d0ad8556acd07ab52445738 Mon Sep 17 00:00:00 2001 From: Lucas Rayan Date: Fri, 16 Oct 2026 16:44:14 -0300 Subject: [PATCH] fix(webmail): a new profile photo shows up everywhere at once The photo endpoint was cached for five minutes with no revalidation, and only the settings page added a version to the URL, so the header, the message list and the reader kept the old photo until it expired. - every reads a per-address version (and refetches when it changes); saving or removing a photo bumps it, in this tab and the other open tabs - the server answers private, max-age=60, must-revalidate, so a stale copy lives for a minute at most and then costs one 304 Co-Authored-By: Claude Sonnet 5.5 --- services/auth/src/router.test.ts | 8 +++ services/auth/src/router.ts | 5 +- .../src/app/(user)/user/settings/page.tsx | 5 +- services/webmail/src/components/Avatar.tsx | 21 +++++- .../webmail/src/lib/avatarVersion.test.ts | 46 +++++++++++++ services/webmail/src/lib/avatarVersion.ts | 65 +++++++++++++++++++ 6 files changed, 146 insertions(+), 4 deletions(-) create mode 100644 services/webmail/src/lib/avatarVersion.test.ts create mode 100644 services/webmail/src/lib/avatarVersion.ts diff --git a/services/auth/src/router.test.ts b/services/auth/src/router.test.ts index b67dfba..1b5febd 100644 --- a/services/auth/src/router.test.ts +++ b/services/auth/src/router.test.ts @@ -946,6 +946,14 @@ describeDb("auth API against a real database", () => { expect(got.headers.get("content-type")).toBe("image/png"); expect(Buffer.from(await got.arrayBuffer())).toEqual(png); + // A new photo has to show up quickly: the browser may reuse the old one + // for at most a minute, and must ask again after that. It was five minutes + // with no revalidation, which is what made a changed photo "take forever". + const cacheControl = got.headers.get("cache-control") ?? ""; + expect(cacheControl).toContain("private"); + expect(cacheControl).toContain("must-revalidate"); + expect(Number(/max-age=(\d+)/.exec(cacheControl)?.[1])).toBeLessThanOrEqual(60); + // A photo appears beside every message from that person; without the // conditional answer it would be re-sent on every one of them. const etag = got.headers.get("etag")!; diff --git a/services/auth/src/router.ts b/services/auth/src/router.ts index 78e29d9..1dd4450 100644 --- a/services/auth/src/router.ts +++ b/services/auth/src/router.ts @@ -343,7 +343,10 @@ async function getAvatar( res.setHeader("ETag", `"${avatar.etag}"`); // Private: this is served to a signed-in reader, and a shared cache must not // hand one person's photo to the next request that comes along. - res.setHeader("Cache-Control", "private, max-age=300"); + // One minute, then revalidate with the ETag (a 304 costs no body). It was five + // minutes with no revalidation, so a new photo took that long to show up + // wherever the page did not add a version to the URL. + res.setHeader("Cache-Control", "private, max-age=60, must-revalidate"); // The bytes were sniffed on the way in, but a browser that ignores the type // and sniffs for itself must not be allowed to find something else. res.setHeader("X-Content-Type-Options", "nosniff"); diff --git a/services/webmail/src/app/(user)/user/settings/page.tsx b/services/webmail/src/app/(user)/user/settings/page.tsx index a7e2085..2e8bdfb 100644 --- a/services/webmail/src/app/(user)/user/settings/page.tsx +++ b/services/webmail/src/app/(user)/user/settings/page.tsx @@ -26,6 +26,7 @@ import { signatureToHtml, sanitizeSignature } from "../../../../lib/signature"; import { buildSieve, parseSieve, type Rule, type RuleField, type RuleCondition, type RuleAction } from "../../../../lib/rules"; import { useFolders } from "../../../../lib/useFolders"; import { AvatarEditor } from "../../../../components/AvatarEditor"; +import { bumpAvatar } from "../../../../lib/avatarVersion"; import { useAccount } from "../../../../lib/useAccount"; import { moveTargets } from "../../../../lib/mailCounts"; @@ -357,8 +358,10 @@ export default function UserSettingsPage() { src={avatarSrc} onSaved={() => { // The query string changes so the browser fetches the new - // photo instead of the one it already has under this URL. + // photo instead of the one it already has under this URL; + // bumpAvatar does the same for every on screen. setAvatarVersion((v) => v + 1); + bumpAvatar(account?.email); }} /> diff --git a/services/webmail/src/components/Avatar.tsx b/services/webmail/src/components/Avatar.tsx index f4b4590..c38f0da 100644 --- a/services/webmail/src/components/Avatar.tsx +++ b/services/webmail/src/components/Avatar.tsx @@ -1,7 +1,8 @@ "use client"; -import React, { useState } from "react"; +import React, { useEffect, useState, useSyncExternalStore } from "react"; import { initialsFor } from "../lib/initials"; +import { avatarVersionOf, subscribeAvatarVersions } from "../lib/avatarVersion"; interface AvatarProps { address: string | null | undefined; @@ -28,7 +29,23 @@ export function Avatar({ address, size, className = "", initials }: AvatarProps) const [loaded, setLoaded] = useState(false); const [failed, setFailed] = useState(false); - const src = address ? `/api/v1/avatar?address=${encodeURIComponent(address)}` : null; + // Bumped when this person's photo is saved or removed, here or in another + // tab, so the browser asks again instead of reusing what it cached + const version = useSyncExternalStore( + subscribeAvatarVersions, + () => avatarVersionOf(address), + () => 0, + ); + + const src = address + ? `/api/v1/avatar?address=${encodeURIComponent(address)}${version ? `&v=${version}` : ""}` + : null; + + // A new version may turn "no photo" (404) into a photo, or the reverse + useEffect(() => { + setLoaded(false); + setFailed(false); + }, [version]); return ( { + it("starts at 0 for a photo nobody changed", () => { + expect(avatarVersionOf("nobody@example.test")).toBe(0); + expect(avatarVersionOf(null)).toBe(0); + }); + + it("changes after a bump, whatever the case of the address", () => { + bumpAvatar("Ana@Example.test", 1000); + expect(avatarVersionOf("ana@example.test")).toBe(1000); + expect(avatarVersionOf(" ANA@example.test ")).toBe(1000); + }); + + it("never repeats a version, even when bumped in the same millisecond", () => { + bumpAvatar("same@example.test", 5000); + const first = avatarVersionOf("same@example.test"); + bumpAvatar("same@example.test", 5000); + expect(avatarVersionOf("same@example.test")).toBeGreaterThan(first); + }); + + it("does not touch other people's photos", () => { + bumpAvatar("one@example.test", 7000); + expect(avatarVersionOf("two@example.test")).toBe(0); + }); + + it("tells subscribers, and stops when they unsubscribe", () => { + const listener = vi.fn(); + const unsubscribe = subscribeAvatarVersions(listener); + bumpAvatar("watch@example.test", 9000); + expect(listener).toHaveBeenCalledTimes(1); + unsubscribe(); + bumpAvatar("watch@example.test", 9001); + expect(listener).toHaveBeenCalledTimes(1); + }); + + it("ignores an empty address", () => { + const listener = vi.fn(); + const unsubscribe = subscribeAvatarVersions(listener); + bumpAvatar("", 1); + bumpAvatar(undefined, 1); + expect(listener).not.toHaveBeenCalled(); + unsubscribe(); + }); +}); diff --git a/services/webmail/src/lib/avatarVersion.ts b/services/webmail/src/lib/avatarVersion.ts new file mode 100644 index 0000000..460d186 --- /dev/null +++ b/services/webmail/src/lib/avatarVersion.ts @@ -0,0 +1,65 @@ +/** + * Which version of each person's photo this browser should ask for. + * + * The photo endpoint is cached by the browser, and the URL is the cache key. + * Only the settings page used to add a version to it, so after a new photo was + * saved the header, the message list and the reader kept showing the old one + * from cache until it expired. Every now reads its version from here, + * so one bump refetches the photo everywhere on screen, and in the other open + * tabs of the same browser. + */ + +type Listener = () => void; + +const CHANNEL = "lambdamail-avatar"; + +const versions = new Map(); +const listeners = new Set(); +let channel: BroadcastChannel | null = null; + +function key(address: string | null | undefined): string { + return (address ?? "").trim().toLowerCase(); +} + +function openChannel(): BroadcastChannel | null { + if (channel) return channel; + if (typeof BroadcastChannel === "undefined") return null; + channel = new BroadcastChannel(CHANNEL); + channel.onmessage = (event: MessageEvent<{ address?: string; version?: number }>) => { + const { address, version } = event.data ?? {}; + if (typeof address === "string" && typeof version === "number") { + apply(address, version); + } + }; + return channel; +} + +function apply(address: string, version: number): void { + const k = key(address); + if (!k || (versions.get(k) ?? 0) >= version) return; + versions.set(k, version); + listeners.forEach((listener) => listener()); +} + +/** 0 until the photo is known to have changed in this session. */ +export function avatarVersionOf(address: string | null | undefined): number { + return versions.get(key(address)) ?? 0; +} + +/** Call after saving or removing a photo: everything showing it refetches. */ +export function bumpAvatar(address: string | null | undefined, now: number = Date.now()): void { + const k = key(address); + if (!k) return; + // Strictly increasing, so two bumps in the same millisecond still differ + const version = Math.max(now, (versions.get(k) ?? 0) + 1); + apply(k, version); + openChannel()?.postMessage({ address: k, version }); +} + +export function subscribeAvatarVersions(listener: Listener): () => void { + openChannel(); + listeners.add(listener); + return () => { + listeners.delete(listener); + }; +}