diff --git a/CHANGELOG.md b/CHANGELOG.md index b4ceeb1c..47bc0a49 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,7 @@ - Bump node version requirement to 20+ - Bump minimum supported browsers to Firefox 115, iOS/Safari 16 - Fix text with input x as null +- Fix corrupted PDF when mixing standard and embedded fonts that share postscript name - Fix PDF/UA compliance issues in kitchen-sink-accessible example - Add bbox and placement options to PDFStructureElement for PDF/UA compliance diff --git a/lib/mixins/fonts.js b/lib/mixins/fonts.js index 9b8d915d..bb2da590 100644 --- a/lib/mixins/fonts.js +++ b/lib/mixins/fonts.js @@ -99,10 +99,21 @@ export default { this._fontFamilies[cacheKey] = this._font; } - if (this._font.name) { + if (this._font.name && !this._fontFamilies[this._font.name]) { this._fontFamilies[this._font.name] = this._font; } + // if the font wasn't registered under any key (e.g. loaded via raw buffer + // with no cacheKey and a postscript name that collides with an + // already-registered font), register it under its id so it always gets + // finalized and doesn't leave a dangling references + if ( + !cacheKey && + (!this._font.name || this._fontFamilies[this._font.name] !== this._font) + ) { + this._fontFamilies[this._font.id] = this._font; + } + return this; }, diff --git a/tests/unit/font.spec.js b/tests/unit/font.spec.js index 64695b3e..eec5dc66 100644 --- a/tests/unit/font.spec.js +++ b/tests/unit/font.spec.js @@ -1,7 +1,8 @@ import { vi } from 'vitest'; +import { readFileSync } from 'fs'; import PDFDocument from '../../lib/document'; import PDFFontFactory from '../../lib/font_factory'; -import { logData } from './helpers'; +import { logData, collectPdf, missingObjects } from './helpers'; describe('EmbeddedFont', () => { test('no fontLayoutCache option', () => { @@ -244,3 +245,80 @@ describe('sizeToPoint', () => { expect(doc.sizeToPoint('1rem')).toEqual(12); }); }); + +describe('font name collision', () => { + // fake font with a bogus checksum so isEqualFont always returns false + const makeCachedFont = (name, id) => ({ + name, + id, + font: { _tables: { head: { checkSumAdjustment: -1 } } }, + ref: vi.fn(), + finalize: vi.fn(), + embedded: false, + }); + + afterEach(() => vi.restoreAllMocks()); + + describe('name-alias slot is not overwritten when already occupied', () => { + test('does not overwrite existing font on postscript name collision', () => { + const doc = new PDFDocument({ font: null, compress: false }); + const existingFont = makeCachedFont('Roboto-Regular', 'F0'); + doc._fontFamilies['Roboto-Regular'] = existingFont; + + doc.registerFont('Roboto', 'tests/fonts/Roboto-Regular.ttf'); + doc.font('Roboto'); + + expect(doc._fontFamilies['Roboto-Regular']).toBe(existingFont); + expect(doc._fontFamilies['Roboto']).toBeDefined(); + expect(doc._fontFamilies['Roboto']).not.toBe(existingFont); + }); + }); + + describe('buffer-loaded font with name collision is registered under its id', () => { + test('registers under id when postscript name slot is taken', () => { + const doc = new PDFDocument({ font: null, compress: false }); + const existingFont = makeCachedFont('Roboto-Regular', 'F0'); + doc._fontFamilies['Roboto-Regular'] = existingFont; + + const buf = readFileSync('tests/fonts/Roboto-Regular.ttf'); + doc.font(buf); + + const loaded = doc._font; + expect(doc._fontFamilies[loaded.id]).toBe(loaded); + expect(doc._fontFamilies['Roboto-Regular']).toBe(existingFont); + }); + }); + + describe('document completes without dangling references', () => { + test('standard + registered embedded font', () => { + const doc = new PDFDocument({ compress: false }); + + doc.text('standard helvetica'); + doc.registerFont('Roboto', 'tests/fonts/Roboto-Regular.ttf'); + doc.font('Roboto').text('embedded roboto'); + + const pdf = collectPdf(doc); + + expect(pdf).toContain('startxref'); + expect(pdf).toContain('%%EOF'); + expect(missingObjects(pdf)).toHaveLength(0); + }); + + test('buffer font with name collision', () => { + const doc = new PDFDocument({ compress: false }); + doc._fontFamilies['Roboto-Regular'] = makeCachedFont( + 'Roboto-Regular', + 'F0', + ); + + const buf = readFileSync('tests/fonts/Roboto-Regular.ttf'); + doc.font(buf).text('buffer roboto'); + + const pdf = collectPdf(doc); + + expect(pdf).toContain('startxref'); + expect(pdf).toContain('%%EOF'); + expect(missingObjects(pdf)).toHaveLength(0); + }); + }); +}); diff --git a/tests/unit/helpers.js b/tests/unit/helpers.js index 55829dcb..2c189d66 100644 --- a/tests/unit/helpers.js +++ b/tests/unit/helpers.js @@ -165,4 +165,34 @@ function parseTextStreams(decodedStream) { return results; } -export { logData, joinTokens, parseTextStreams, getObjects }; +/** + * Collect all PDF output from a document into a single binary string. + * @param {PDFDocument} doc + * @return {string} + */ +function collectPdf(doc) { + const data = logData(doc); + doc.end(); + return data.map((d) => d.toString('binary')).join(''); +} + +/** + * Return object ids that are referenced but never defined in a PDF string. + * An empty result means no dangling references. + * @param {string} pdf + * @return {string[]} + */ +function missingObjects(pdf) { + const defined = [...pdf.matchAll(/(\d+) 0 obj/g)].map((m) => m[1]); + const refs = [...new Set([...pdf.matchAll(/(\d+) 0 R/g)].map((m) => m[1]))]; + return refs.filter((r) => !defined.includes(r)); +} + +export { + logData, + joinTokens, + parseTextStreams, + getObjects, + collectPdf, + missingObjects, +};