Skip to content

Commit 7db606b

Browse files
JohnMcLearclaude
andcommitted
fix(export): don't serve stale PDF fonts after a font file changes
Review feedback on #8249. `readFontFile()` cached both font buffers and read failures forever, keyed only by path. An operator who corrected a wrong `exportPdfFonts` path, or replaced a face in place, kept getting the old bytes — or the fallback — until Etherpad was restarted. The cache now keys on the file's mtime and size as well as its path, and failures are not cached at all, so a font that appears or changes at a configured path is picked up by the next export. One `stat` per font variant per export. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013S4pYSjwUsiZtdtMMpW7bw
1 parent bc6bff4 commit 7db606b

2 files changed

Lines changed: 57 additions & 5 deletions

File tree

src/node/utils/ExportPdfNative.ts

Lines changed: 20 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -228,18 +228,33 @@ const getFontConfig = (): Map<string, PdfFontFiles> => {
228228

229229
// Font files are read once per process; a pad export can reference the same
230230
// family hundreds of times and every PDFDocument needs its own registration.
231-
const fontFileCache = new Map<string, Buffer | null>();
231+
// The cache is keyed by the file's mtime and size as well as its path, so
232+
// replacing a font in place — or dropping in one whose path was previously
233+
// wrong — takes effect on the next export without restarting Etherpad.
234+
// Failures are deliberately not cached, for the same reason.
235+
const fontFileCache = new Map<string, {stamp: string, buf: Buffer}>();
232236

233237
const readFontFile = (file: string): Buffer | null => {
234-
if (fontFileCache.has(file)) return fontFileCache.get(file)!;
235-
let buf: Buffer | null = null;
238+
let stamp: string;
239+
try {
240+
const st = fs.statSync(file);
241+
stamp = `${st.mtimeMs}:${st.size}`;
242+
} catch (err) {
243+
logger.warn(`PDF export: cannot read font file "${file}": ${(err as Error).message}`);
244+
fontFileCache.delete(file);
245+
return null;
246+
}
247+
const cached = fontFileCache.get(file);
248+
if (cached && cached.stamp === stamp) return cached.buf;
249+
let buf: Buffer;
236250
try {
237251
buf = fs.readFileSync(file);
238252
} catch (err) {
239253
logger.warn(`PDF export: cannot read font file "${file}": ${(err as Error).message}`);
240-
buf = null;
254+
fontFileCache.delete(file);
255+
return null;
241256
}
242-
fontFileCache.set(file, buf);
257+
fontFileCache.set(file, {stamp, buf});
243258
return buf;
244259
};
245260

src/tests/backend/specs/export.ts

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -817,6 +817,43 @@ hello<br>world
817817
['Helvetica']);
818818
});
819819

820+
it('picks up a font file that appears or changes on disk',
821+
async function () {
822+
// The font cache keys on mtime+size, so an operator can correct
823+
// a wrong path or swap a face in place and the next export uses
824+
// it — no restart, and no cached failure to clear.
825+
const os = require('os');
826+
const fs = require('fs');
827+
const path = require('path');
828+
const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'ep8245-'));
829+
const target = path.join(dir, 'face.ttf');
830+
const fontDir = path.join(__dirname, '../../../static/font');
831+
try {
832+
settings.exportPdfFonts = {'Swappable': target};
833+
// Nothing there yet: falls back rather than failing.
834+
assert.deepStrictEqual(
835+
await baseFonts("<p><span style='font-family:Swappable'>t</span></p>"),
836+
['Helvetica'], 'a missing file should fall back');
837+
838+
fs.copyFileSync(path.join(fontDir, 'Quicksand-Regular.ttf'), target);
839+
assert.deepStrictEqual(
840+
await baseFonts("<p><span style='font-family:Swappable'>t</span></p>"),
841+
['Quicksand-Regular'],
842+
'a file appearing at the configured path should be picked up');
843+
844+
fs.copyFileSync(path.join(fontDir, 'RobotoMono-Regular.ttf'), target);
845+
// Guarantee a different mtime even on a coarse-grained clock.
846+
const later = new Date(Date.now() + 2000);
847+
fs.utimesSync(target, later, later);
848+
assert.deepStrictEqual(
849+
await baseFonts("<p><span style='font-family:Swappable'>t</span></p>"),
850+
['RobotoMono-Regular'],
851+
'a replaced file should not serve the previous bytes');
852+
} finally {
853+
fs.rmSync(dir, {recursive: true, force: true, maxRetries: 10, retryDelay: 100});
854+
}
855+
});
856+
820857
it('overrides the built-in mapping for a known family',
821858
async function () {
822859
settings.exportPdfFonts = {

0 commit comments

Comments
 (0)