From d10eb4a55db7578d7fc9bb032449ce845decfb8b Mon Sep 17 00:00:00 2001 From: Amruth Pillai Date: Sat, 5 Sep 2026 11:22:05 -0700 Subject: [PATCH] fix(pdf): isolate cached glyph character metadata (#3450) * fix(pdf): isolate character metadata for cached font glyphs * test(pdf): verify glyph aliases do not grow cache * test(pdf): assert glyph aliases are unique --- .../pdf/src/glyph-cache.integration.test.tsx | 114 +++++++++++++++++ patches/fontkit@2.0.4.patch | 120 ++++++++++++++++++ pnpm-lock.yaml | 7 +- pnpm-workspace.yaml | 1 + 4 files changed, 239 insertions(+), 3 deletions(-) create mode 100644 packages/pdf/src/glyph-cache.integration.test.tsx create mode 100644 patches/fontkit@2.0.4.patch diff --git a/packages/pdf/src/glyph-cache.integration.test.tsx b/packages/pdf/src/glyph-cache.integration.test.tsx new file mode 100644 index 000000000..4ac4cb192 --- /dev/null +++ b/packages/pdf/src/glyph-cache.integration.test.tsx @@ -0,0 +1,114 @@ +import { describe, expect, it } from "vitest"; +import { Font, renderToBuffer } from "@react-pdf/renderer"; +import { getDocument } from "pdfjs-dist/legacy/build/pdf.mjs"; +import { act } from "react"; +import { getWebFontSource } from "@reactive-resume/fonts"; +import { defaultResumeData } from "@reactive-resume/schema/resume/default"; +import { ResumeDocument } from "./document"; + +async function fontForProbe(alias: string, family = "Noto Serif SC") { + const src = getWebFontSource(family, "400", false); + if (!src) throw new Error(`Missing test font: ${family}`); + Font.register({ family: alias, src }); + await Font.load({ fontFamily: alias }); + const font = Font.getFont({ fontFamily: alias }).data; + if (!font) throw new Error("Font did not load"); + return font; +} + +describe("cached font glyph character identity", () => { + it.each(["\u200c", "\u200d"])( + "keeps invisible %j and visible missing glyphs distinct in both orders", + { timeout: 60_000 }, + async (joiner) => { + const font = await fontForProbe(`Joiner ${joiner.codePointAt(0)}`); + // React PDF's attachment pass can be the first lookup of .notdef. + font.glyphForCodePoint(0xfffc); + const missing = "\u{1f984}"; + expect(font.hasGlyphForCodePoint(0x1f984)).toBe(false); + for (const text of [joiner + missing, missing + joiner]) { + const run = font.layout(text); + expect(run.glyphs).toHaveLength(2); + const missingIndex = text.startsWith(missing) ? 0 : 1; + expect(run.positions[missingIndex]?.xAdvance).toBeGreaterThan(0); + expect(run.positions[1 - missingIndex]?.xAdvance).toBe(0); + expect(run.glyphs[missingIndex]?.codePoints).toEqual([0x1f984]); + } + }, + ); + + it("retains each spelling when a ligature and its Unicode character share an outline", { + timeout: 60_000, + }, async () => { + const font = await fontForProbe("Ligature identity", "IBM Plex Serif"); + const letters = font.layout("fi"); + const character = font.layout("\ufb01"); + expect(letters.glyphs).toHaveLength(1); + expect(character.glyphs).toHaveLength(1); + expect(letters.glyphs[0]?.id).toBe(character.glyphs[0]?.id); + expect(letters.glyphs[0]?.codePoints).toEqual([0x66, 0x69]); + expect(character.glyphs[0]?.codePoints).toEqual([0xfb01]); + expect(letters.glyphs[0]?.isLigature).toBe(true); + expect(character.glyphs[0]?.isLigature).toBe(false); + expect(character.advanceWidth).toBe(letters.advanceWidth); + }); + + it("retains mark metadata when unsupported marks and symbols share .notdef", { timeout: 60_000 }, async () => { + const font = await fontForProbe("Mark identity"); + expect(font.hasGlyphForCodePoint(0x1ab0)).toBe(false); + const symbol = font.glyphForCodePoint(0x1f984); + const mark = font.glyphForCodePoint(0x1ab0); + expect(mark.id).toBe(symbol.id); + expect(symbol.isMark).toBe(false); + expect(mark.isMark).toBe(true); + expect(mark.path.toSVG()).toBe(symbol.path.toSVG()); + expect(mark.bbox).toEqual(symbol.bbox); + }); + + it("does not retain character aliases in the glyph cache", { timeout: 60_000 }, async () => { + const font = await fontForProbe("Alias cache size"); + const cached = font.glyphForCodePoint(0x1f984); + const glyphCache: unknown = Reflect.get(font, "_glyphs"); + if (!glyphCache || typeof glyphCache !== "object") throw new Error("Missing font glyph cache"); + const initialSize = Object.keys(glyphCache).length; + const codePoints = Array.from({ length: 1_000 }, (_, index) => 0xf0000 + index); + expect(codePoints.every((codePoint) => !font.hasGlyphForCodePoint(codePoint))).toBe(true); + + const aliases = codePoints.map((codePoint) => font.glyphForCodePoint(codePoint)); + expect(aliases.every((alias) => alias.id === cached.id && alias !== cached)).toBe(true); + expect(new Set(aliases).size).toBe(codePoints.length); + expect(Object.keys(glyphCache)).toHaveLength(initialSize); + expect(font.glyphForCodePoint(0x1f984)).toBe(cached); + }); + + it("keeps CJK spaces unchanged after a Unicode-only PDF export", { timeout: 60_000 }, async () => { + const data = structuredClone(defaultResumeData); + data.picture.hidden = true; + data.basics.name = "Probe"; + data.metadata.page.locale = "zh-CN"; + data.metadata.typography.body.fontFamily = "Noto Serif SC"; + data.metadata.typography.heading.fontFamily = "Noto Serif SC"; + data.metadata.typography.body.fontSize = 10; + data.metadata.layout.pages = [{ fullWidth: true, main: ["summary"], sidebar: [] }]; + data.summary.title = "Text"; + data.summary.content = '
中\u3000文
'; + await act(() => renderToBuffer()); + data.summary.content = "

中 文 字

"; + for (let attempt = 0; attempt < 2; attempt++) { + const bytes = await act(() => renderToBuffer()); + const loading = getDocument({ data: new Uint8Array(bytes) }); + try { + const document = await loading.promise; + const page = await document.getPage(1); + const items = (await page.getTextContent()).items.filter((item) => "str" in item); + const body = items.filter((item) => item.str.includes("中")); + expect(body).toHaveLength(1); + expect(body[0]?.str).toBe("中 文 字"); + // Three 10pt Chinese glyphs and two 2.56pt ordinary spaces. + expect(body[0]?.width).toBeCloseTo(35.12, 2); + } finally { + await loading.destroy(); + } + } + }); +}); diff --git a/patches/fontkit@2.0.4.patch b/patches/fontkit@2.0.4.patch new file mode 100644 index 000000000..f5f9699a2 --- /dev/null +++ b/patches/fontkit@2.0.4.patch @@ -0,0 +1,120 @@ +diff --git a/dist/browser-module.mjs b/dist/browser-module.mjs +index 4d0a389de8a3db767f1fef99ed186790431aa844..ac3c64de9179d28f0ae10995d7fdda0d13ad9875 100644 +--- a/dist/browser-module.mjs ++++ b/dist/browser-module.mjs +@@ -12673,7 +12673,17 @@ class $4c1709dee528ea76$export$2e2bcd8739ae039 { + else if (this.directory.tables.COLR && this.directory.tables.CPAL) this._glyphs[glyph] = new (0, $0d411f0165859681$export$2e2bcd8739ae039)(glyph, characters, this); + else this._getBaseGlyph(glyph, characters); + } +- return this._glyphs[glyph] || null; ++ const cached = this._glyphs[glyph] || null; ++ // Glyph outlines are cached by id, but shaping needs this call's characters. ++ // Keep aliases local so they cannot mutate earlier runs or grow the font cache. ++ if (cached && characters.length && (cached.codePoints.length !== characters.length || characters.some((code, index) => code !== cached.codePoints[index]))) { ++ const alias = Object.create(Object.getPrototypeOf(cached), Object.getOwnPropertyDescriptors(cached)); ++ alias.codePoints = characters; ++ alias.isMark = characters.every((0, $6uUbQ$isMark)); ++ alias.isLigature = characters.length > 1; ++ return alias; ++ } ++ return cached; + } + /** + * Returns a Subset for this font. +diff --git a/dist/browser.cjs b/dist/browser.cjs +index a5fa901b5608575f84390e3ef0474a7789e8269a..e4811fe39687516950508ce76a488babc1dd145d 100644 +--- a/dist/browser.cjs ++++ b/dist/browser.cjs +@@ -12690,7 +12690,17 @@ class $0a8ef2660a6ce4b6$export$2e2bcd8739ae039 { + else if (this.directory.tables.COLR && this.directory.tables.CPAL) this._glyphs[glyph] = new (0, $42d9dbd2de9ee2d8$export$2e2bcd8739ae039)(glyph, characters, this); + else this._getBaseGlyph(glyph, characters); + } +- return this._glyphs[glyph] || null; ++ const cached = this._glyphs[glyph] || null; ++ // Glyph outlines are cached by id, but shaping needs this call's characters. ++ // Keep aliases local so they cannot mutate earlier runs or grow the font cache. ++ if (cached && characters.length && (cached.codePoints.length !== characters.length || characters.some((code, index) => code !== cached.codePoints[index]))) { ++ const alias = Object.create(Object.getPrototypeOf(cached), Object.getOwnPropertyDescriptors(cached)); ++ alias.codePoints = characters; ++ alias.isMark = characters.every((0, $gfJaN$unicodeproperties.isMark)); ++ alias.isLigature = characters.length > 1; ++ return alias; ++ } ++ return cached; + } + /** + * Returns a Subset for this font. +diff --git a/dist/main.cjs b/dist/main.cjs +index 851b2d2bbcae17e91f9f2468f61112ee2dc8fbd8..5222c2a538f978c3b11511ffa378d58fdd39467d 100644 +--- a/dist/main.cjs ++++ b/dist/main.cjs +@@ -12711,7 +12711,17 @@ class $0a8ef2660a6ce4b6$export$2e2bcd8739ae039 { + else if (this.directory.tables.COLR && this.directory.tables.CPAL) this._glyphs[glyph] = new (0, $42d9dbd2de9ee2d8$export$2e2bcd8739ae039)(glyph, characters, this); + else this._getBaseGlyph(glyph, characters); + } +- return this._glyphs[glyph] || null; ++ const cached = this._glyphs[glyph] || null; ++ // Glyph outlines are cached by id, but shaping needs this call's characters. ++ // Keep aliases local so they cannot mutate earlier runs or grow the font cache. ++ if (cached && characters.length && (cached.codePoints.length !== characters.length || characters.some((code, index) => code !== cached.codePoints[index]))) { ++ const alias = Object.create(Object.getPrototypeOf(cached), Object.getOwnPropertyDescriptors(cached)); ++ alias.codePoints = characters; ++ alias.isMark = characters.every((0, $elh9A$unicodeproperties.isMark)); ++ alias.isLigature = characters.length > 1; ++ return alias; ++ } ++ return cached; + } + /** + * Returns a Subset for this font. +diff --git a/dist/module.mjs b/dist/module.mjs +index fde88fc3f23e6fff533da842186fee8402cbcde1..372fd472dc1c09bb6f5f5ad73499a5be2517224b 100644 +--- a/dist/module.mjs ++++ b/dist/module.mjs +@@ -12694,7 +12694,17 @@ class $4c1709dee528ea76$export$2e2bcd8739ae039 { + else if (this.directory.tables.COLR && this.directory.tables.CPAL) this._glyphs[glyph] = new (0, $0d411f0165859681$export$2e2bcd8739ae039)(glyph, characters, this); + else this._getBaseGlyph(glyph, characters); + } +- return this._glyphs[glyph] || null; ++ const cached = this._glyphs[glyph] || null; ++ // Glyph outlines are cached by id, but shaping needs this call's characters. ++ // Keep aliases local so they cannot mutate earlier runs or grow the font cache. ++ if (cached && characters.length && (cached.codePoints.length !== characters.length || characters.some((code, index) => code !== cached.codePoints[index]))) { ++ const alias = Object.create(Object.getPrototypeOf(cached), Object.getOwnPropertyDescriptors(cached)); ++ alias.codePoints = characters; ++ alias.isMark = characters.every((0, $52ZIf$isMark)); ++ alias.isLigature = characters.length > 1; ++ return alias; ++ } ++ return cached; + } + /** + * Returns a Subset for this font. +diff --git a/src/TTFFont.js b/src/TTFFont.js +index 6aa0937a3b4717d2a23453ebd6ced9494adf6d5e..c1a14b7a3924571fe8114dfe5c088de0669faa7b 100644 +--- a/src/TTFFont.js ++++ b/src/TTFFont.js +@@ -1,3 +1,4 @@ ++import { isMark } from 'unicode-properties'; + import * as r from 'restructure'; + import { cache } from './decorators'; + import * as fontkit from './base'; +@@ -419,7 +420,17 @@ export default class TTFFont { + } + } + +- return this._glyphs[glyph] || null; ++ const cached = this._glyphs[glyph] || null; ++ // Glyph outlines are cached by id, but shaping needs this call's characters. ++ // Keep aliases local so they cannot mutate earlier runs or grow the font cache. ++ if (cached && characters.length && (cached.codePoints.length !== characters.length || characters.some((code, index) => code !== cached.codePoints[index]))) { ++ const alias = Object.create(Object.getPrototypeOf(cached), Object.getOwnPropertyDescriptors(cached)); ++ alias.codePoints = characters; ++ alias.isMark = characters.every(isMark); ++ alias.isLigature = characters.length > 1; ++ return alias; ++ } ++ return cached; + } + + /** diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 0687bb551..7e9162c57 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -14,6 +14,7 @@ overrides: patchedDependencies: '@react-pdf/layout@5.2.0': 75dac7cb8260f9c96cd9ab5522d095a3aa047881a9e9d1d2a597409b45944494 '@react-pdf/textkit': 092a6fe8baf3c472a81cdf2ab52a00ec98ef3364e1b8e744005dbcf219a8a213 + fontkit@2.0.4: 90f4c51c676a88b91dcc397d60a677c77b5e6dc8e15ffb3538310965ef5c05a4 importers: @@ -11243,7 +11244,7 @@ snapshots: '@react-pdf/font@4.1.2': dependencies: '@react-pdf/types': 2.14.0 - fontkit: 2.0.4 + fontkit: 2.0.4(patch_hash=90f4c51c676a88b91dcc397d60a677c77b5e6dc8e15ffb3538310965ef5c05a4) is-url: 1.2.4 pdfkit: 0.20.1 @@ -13390,7 +13391,7 @@ snapshots: dependencies: locate-path: 3.0.0 - fontkit@2.0.4: + fontkit@2.0.4(patch_hash=90f4c51c676a88b91dcc397d60a677c77b5e6dc8e15ffb3538310965ef5c05a4): dependencies: '@swc/helpers': 0.5.23 brotli: 1.3.3 @@ -14868,7 +14869,7 @@ snapshots: '@noble/ciphers': 1.3.0 '@noble/hashes': 1.8.0 fflate: 0.8.3 - fontkit: 2.0.4 + fontkit: 2.0.4(patch_hash=90f4c51c676a88b91dcc397d60a677c77b5e6dc8e15ffb3538310965ef5c05a4) linebreak: 1.1.0 png-js: 2.0.0 diff --git a/pnpm-workspace.yaml b/pnpm-workspace.yaml index 5d5b6d2b2..621308968 100644 --- a/pnpm-workspace.yaml +++ b/pnpm-workspace.yaml @@ -19,3 +19,4 @@ overrides: patchedDependencies: '@react-pdf/layout@5.2.0': patches/@react-pdf__layout@5.2.0.patch '@react-pdf/textkit': patches/@react-pdf__textkit.patch + fontkit@2.0.4: patches/fontkit@2.0.4.patch