diff --git a/app/api/admin/branding/route.ts b/app/api/admin/branding/route.ts index 955b3f713..3ba4e6691 100644 --- a/app/api/admin/branding/route.ts +++ b/app/api/admin/branding/route.ts @@ -12,6 +12,7 @@ import { logger } from '@/lib/logger'; import path from 'node:path'; import { configReadOnlyResponse } from '@/lib/admin/config-read-only-response'; import { brandingDocs } from '@/lib/admin/extension-store'; +import type { PutResult } from '@/lib/shared-store'; import { announceChange } from '@/lib/shared-store/changes'; // Uploads live in C/branding, or the shared store in shared mode (#1066), @@ -111,6 +112,10 @@ function clearDomainKeys( return next; } +function brandingUrl(name: string, written: PutResult): string { + return `/api/admin/branding/${name}${written.ok ? `?v=${written.version}` : ''}`; +} + /** * POST /api/admin/branding - Upload a branding image file * @@ -177,9 +182,12 @@ export async function POST(request: NextRequest) { } const buffer = Buffer.from(await file.arrayBuffer()); - await docs.put(safeName, buffer); + const written = await docs.put(safeName, buffer); - const servedUrl = `/api/admin/branding/${safeName}`; + // The version in the URL changes with the bytes, so browsers (which may + // keep the file for an hour) and the rendered icons and previews pick up + // a re-upload of the same slot and type at once. + const servedUrl = brandingUrl(safeName, written); await configManager.ensureLoaded(); if (host) { diff --git a/app/api/og-image/route.ts b/app/api/og-image/route.ts index 3482ee631..03464bc1c 100644 --- a/app/api/og-image/route.ts +++ b/app/api/og-image/route.ts @@ -2,6 +2,7 @@ import { NextRequest, NextResponse } from 'next/server'; import sharp from 'sharp'; import { configManager } from '@/lib/admin/config-manager'; import { fetchBrandingAsset } from '@/lib/admin/branding-asset'; +import { renderCache } from '@/lib/admin/render-cache'; import { matchDomainBranding, parseDomainBranding, @@ -63,7 +64,9 @@ function isDarkColor({ r, g, b }: Rgb): boolean { // Cache rendered cards keyed by (source logo, background) so a branding change // invalidates the previous render instead of serving stale bytes forever. -const cache = new Map(); +// An uploaded logo's URL carries its version. Capped, and dropped on a +// branding change. +const cache = renderCache('og-image'); export async function GET(req: NextRequest) { await configManager.ensureLoaded(); diff --git a/app/api/pwa-icon/[size]/route.ts b/app/api/pwa-icon/[size]/route.ts index 9f99e1a60..262f42cb2 100644 --- a/app/api/pwa-icon/[size]/route.ts +++ b/app/api/pwa-icon/[size]/route.ts @@ -2,6 +2,7 @@ import { NextRequest, NextResponse } from 'next/server'; import sharp from 'sharp'; import { configManager } from '@/lib/admin/config-manager'; import { fetchBrandingAsset } from '@/lib/admin/branding-asset'; +import { renderCache } from '@/lib/admin/render-cache'; import { matchDomainBranding, parseDomainBranding, @@ -10,9 +11,11 @@ import { const VALID_SIZES = new Set([192, 512]); -// Cache resized images keyed by (size, source URL) so admin re-uploads or URL -// changes invalidate the prior render instead of serving stale bytes forever. -const cache = new Map(); +// Cache resized images keyed by (size, source URL) so admin re-uploads (the +// URL carries the upload's version) or URL changes invalidate the prior +// render instead of serving stale bytes forever. Capped, and dropped on a +// branding change. +const cache = renderCache('pwa-icon'); export async function GET( req: NextRequest, diff --git a/app/api/pwa-screenshot/[variant]/route.ts b/app/api/pwa-screenshot/[variant]/route.ts index 5744321af..e08194e88 100644 --- a/app/api/pwa-screenshot/[variant]/route.ts +++ b/app/api/pwa-screenshot/[variant]/route.ts @@ -2,6 +2,7 @@ import { NextRequest, NextResponse } from 'next/server'; import sharp from 'sharp'; import { configManager } from '@/lib/admin/config-manager'; import { fetchBrandingAsset } from '@/lib/admin/branding-asset'; +import { renderCache } from '@/lib/admin/render-cache'; import { matchDomainBranding, parseDomainBranding, @@ -20,8 +21,9 @@ const VARIANTS = { type Variant = keyof typeof VARIANTS; -// Cache resized images keyed by (variant, source URL). -const cache = new Map(); +// Cache resized images keyed by (variant, source URL); an uploaded source's +// URL carries its version. Capped, and dropped on a branding change. +const cache = renderCache('pwa-screenshot'); export async function GET( req: NextRequest, diff --git a/app/api/setup/branding/route.ts b/app/api/setup/branding/route.ts index f21bdcd85..d16874a69 100644 --- a/app/api/setup/branding/route.ts +++ b/app/api/setup/branding/route.ts @@ -102,9 +102,10 @@ export async function POST(request: NextRequest) { } const buffer = Buffer.from(await file.arrayBuffer()); - await docs.put(safeName, buffer); + const written = await docs.put(safeName, buffer); - const servedUrl = `/api/admin/branding/${safeName}`; + // Versioned like the admin upload's URL (app/api/admin/branding/route.ts). + const servedUrl = `/api/admin/branding/${safeName}${written.ok ? `?v=${written.version}` : ''}`; await configManager.ensureLoaded(); await configManager.setAdminConfig({ [slot]: servedUrl }); await announceChange('branding'); diff --git a/lib/admin/__tests__/branding-reupload.test.ts b/lib/admin/__tests__/branding-reupload.test.ts new file mode 100644 index 000000000..15bb32fe0 --- /dev/null +++ b/lib/admin/__tests__/branding-reupload.test.ts @@ -0,0 +1,136 @@ +// @vitest-environment node +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +import { mkdtemp, rm } from 'node:fs/promises'; +import { tmpdir } from 'node:os'; +import path from 'node:path'; +import sharp from 'sharp'; +import { NextRequest } from 'next/server'; + +// A re-upload of the same branding slot with the same file type kept its +// URL. Browsers kept showing the old file for up to an hour, and the PWA +// icon, screenshot and link-preview renders, cached by that URL, kept the +// old logo until the server restarted. The URL now carries the upload's +// version, and the render caches are capped and dropped on a branding or +// config change (also one made on another instance, #1066). + +vi.mock('@/lib/admin/session', () => ({ + requireAdminAuth: async () => ({ payload: { role: 'admin' } }), + getClientIP: () => '127.0.0.1', +})); +vi.mock('@/lib/admin/audit', () => ({ auditLog: async () => {} })); + +import { configManager } from '../config-manager'; +import { POST as upload } from '@/app/api/admin/branding/route'; +import { GET as pwaIcon } from '@/app/api/pwa-icon/[size]/route'; +import { GET as serveFile } from '@/app/api/admin/branding/[filename]/route'; +import { MAX_ENTRIES, clearRenderCaches, renderCache } from '../render-cache'; +import { setStoreForTests } from '@/lib/shared-store'; +import { resetLocalStoreForTests } from '@/lib/shared-store/local'; +import { INIT_EPOCH, markEpochsSeen, pollChanges, resetChangesForTests } from '@/lib/shared-store/changes'; +import { separateFakeStore } from './shared-store-fake'; + +let dir: string; + +const square = (r: number, g: number, b: number) => + sharp({ create: { width: 64, height: 64, channels: 4, background: { r, g, b, alpha: 1 } } }).png().toBuffer(); + +async function uploadIcon(png: Buffer): Promise { + const form = new FormData(); + form.set('file', new File([new Uint8Array(png)], 'icon.png', { type: 'image/png' })); + form.set('slot', 'pwaIconUrl'); + const res = await upload(new NextRequest('https://mail.example/api/admin/branding', { method: 'POST', body: form })); + expect(res.status).toBe(200); + return (await res.json()).url; +} + +async function renderedIconColour(route = pwaIcon): Promise { + const res = await route(new NextRequest('https://mail.example/api/pwa-icon/192'), { params: Promise.resolve({ size: '192' }) }); + const { data } = await sharp(Buffer.from(await res.arrayBuffer())).raw().toBuffer({ resolveWithObject: true }); + // The centre pixel. + const at = (96 * 192 + 96) * 4; + return [data[at], data[at + 1], data[at + 2]]; +} + +beforeEach(async () => { + dir = await mkdtemp(path.join(tmpdir(), 'bw-branding-reupload-')); + vi.stubEnv('ADMIN_CONFIG_DIR', path.join(dir, 'config')); + vi.stubEnv('ADMIN_STATE_DIR', path.join(dir, 'state')); + resetLocalStoreForTests(); + resetChangesForTests(); + clearRenderCaches(); + await configManager.load(); +}); + +afterEach(async () => { + setStoreForTests(null); + resetChangesForTests(); + resetLocalStoreForTests(); + vi.unstubAllEnvs(); + await rm(dir, { recursive: true, force: true }); +}); + +describe('re-uploading a branding image', () => { + it('gets a new URL, and the rendered PWA icon shows the new image at once', async () => { + const first = await uploadIcon(await square(255, 0, 0)); + expect(first).toMatch(/^\/api\/admin\/branding\/pwaIconUrl\.png\?v=[\w-]+$/); + expect(await renderedIconColour()).toEqual([255, 0, 0]); + + const second = await uploadIcon(await square(0, 0, 255)); + expect(second).not.toBe(first); + expect(second.split('?')[0]).toBe(first.split('?')[0]); + expect(configManager.get('pwaIconUrl')).toBe(second); + expect(await renderedIconColour()).toEqual([0, 0, 255]); + + // The file itself is served under its name, whatever the version says. + const res = await serveFile(new NextRequest(`https://mail.example${second}`), { params: Promise.resolve({ filename: 'pwaIconUrl.png' }) }); + expect(res.status).toBe(200); + }); + + it('renders anew after a branding change even when the URL stayed the same', async () => { + // The route as loaded at process start, listening for changes. + vi.resetModules(); + const { GET: route } = await import('@/app/api/pwa-icon/[size]/route'); + await uploadIcon(await square(255, 0, 0)); + expect(await renderedIconColour(route)).toEqual([255, 0, 0]); + // The file behind the URL changes (as a linked remote logo can) and the + // branding change is announced. + const { brandingDocs } = await import('../extension-store'); + await brandingDocs().put('pwaIconUrl.png', await square(0, 255, 0)); + const { announceChange } = await import('@/lib/shared-store/changes'); + await announceChange('branding'); + expect(await renderedIconColour(route)).toEqual([0, 255, 0]); + }); + + it('keeps the same URL for the same bytes', async () => { + const png = await square(0, 255, 0); + expect(await uploadIcon(png)).toBe(await uploadIcon(png)); + }); +}); + +describe('render caches', () => { + it('hold at most a fixed number of images, dropping the least recently used', () => { + const cache = renderCache('test-cap'); + for (let i = 0; i < MAX_ENTRIES; i++) cache.set(`k${i}`, new Blob([String(i)])); + expect(cache.get('k0')).toBeDefined(); // k0 used again + for (let i = MAX_ENTRIES; i < MAX_ENTRIES + 8; i++) cache.set(`k${i}`, new Blob([String(i)])); + expect(cache.size).toBe(MAX_ENTRIES); + expect(cache.has('k0')).toBe(true); + expect(cache.has('k1')).toBe(false); + }); + + it('are dropped when another instance changes the branding', async () => { + const store = separateFakeStore(path.join(dir, 'store')); + setStoreForTests(store); + await store.epochs.bump(INIT_EPOCH); + await markEpochsSeen(); + // A fresh copy of the module listens, as each route bundle's does. + vi.resetModules(); + const fresh = await import('../render-cache'); + const cache = fresh.renderCache('og-image'); + cache.set('logo', new Blob(['old render'])); + + await store.epochs.bump('branding'); + await pollChanges(); + expect(cache.has('logo')).toBe(false); + }); +}); diff --git a/lib/admin/branding-asset.ts b/lib/admin/branding-asset.ts index 9efbbea3c..4aa70f527 100644 --- a/lib/admin/branding-asset.ts +++ b/lib/admin/branding-asset.ts @@ -57,7 +57,8 @@ export async function fetchBrandingAsset(url: string, label = 'branding asset'): } if (url.startsWith(ADMIN_BRANDING_PREFIX)) { - const filename = path.basename(url.slice(ADMIN_BRANDING_PREFIX.length)); + // The URL carries the upload's version (?v=…) for caches; the file is the path. + const filename = path.basename(url.slice(ADMIN_BRANDING_PREFIX.length).split(/[?#]/)[0]); const doc = await brandingDocs().get(filename); if (!doc) throw new Error(`Branding file ${filename} not found`); return doc.data; diff --git a/lib/admin/render-cache.ts b/lib/admin/render-cache.ts new file mode 100644 index 000000000..f34050cb3 --- /dev/null +++ b/lib/admin/render-cache.ts @@ -0,0 +1,66 @@ +import { onSharedChange } from '@/lib/shared-store/changes'; + +/** + * Images rendered from the branding (og-image, pwa-icon, pwa-screenshot), + * kept so each is made once. Keyed by the source URL, which for an upload + * carries the file's version (`?v=`), so a re-upload renders anew. Each + * cache holds at most MAX_ENTRIES (the least recently used goes first; a + * deployment with many domains used to grow it without bound), and all of + * them are dropped when the branding or config changes, on this instance or + * another one (#1066). + * + * On globalThis: one set of caches per process, whichever module copy asks. + */ + +export const MAX_ENTRIES = 32; + +export interface RenderCache { + has(key: string): boolean; + get(key: string): Blob | undefined; + set(key: string, value: Blob): void; + readonly size: number; +} + +const KEY = Symbol.for('bulwark.admin.renderCaches'); + +function caches(): Map> { + const g = globalThis as typeof globalThis & { [KEY]?: Map> }; + return (g[KEY] ??= new Map()); +} + +export function clearRenderCaches(): void { + for (const cache of caches().values()) cache.clear(); +} + +for (const area of ['branding', 'config']) onSharedChange(area, () => clearRenderCaches()); + +export function renderCache(name: string): RenderCache { + const entries = () => { + const all = caches(); + let cache = all.get(name); + if (!cache) all.set(name, (cache = new Map())); + return cache; + }; + return { + has: (key) => entries().has(key), + get(key) { + const cache = entries(); + const value = cache.get(key); + if (value !== undefined) { + // Most recently used last. + cache.delete(key); + cache.set(key, value); + } + return value; + }, + set(key, value) { + const cache = entries(); + cache.delete(key); + cache.set(key, value); + while (cache.size > MAX_ENTRIES) cache.delete(cache.keys().next().value!); + }, + get size() { + return entries().size; + }, + }; +}