Fix cache key collision for fonts with bad TTF metadata

CHANGELOG.md

@@ -2,6 +2,7 @@
 
 ### Unreleased
 
+- Fix cache key collisions for fonts with bad TTF metadata
 - Add support for PDF/A-1b and PDF/A-1a
 
 ### [v0.13.0] - 2021-10-24

lib/metadata.js

@@ -1,4 +1,3 @@
-
 class PDFMetadata {
     constructor() {
         this._metadata = `
@@ -7,7 +6,7 @@ class PDFMetadata {
                 <rdf:RDF xmlns:rdf="http://www.w3.org/1999/02/22-rdf-syntax-ns#">
         `;
     }
-    
+
     _closeTags() {
         this._metadata = this._metadata.concat(`
                 </rdf:RDF>
@@ -17,9 +16,9 @@ class PDFMetadata {
     }
 
     append(xml, newline=true) {
-        this._metadata = this._metadata.concat(xml); 
+        this._metadata = this._metadata.concat(xml);
         if (newline)
-            this._metadata = this._metadata.concat('\n'); 
+            this._metadata = this._metadata.concat('\n');
     }
 
     getXML() { return this._metadata; }
@@ -30,6 +29,6 @@ class PDFMetadata {
         this._closeTags();
         this._metadata = this._metadata.trim();
     }
-};
+}
 
-export default PDFMetadata;
+export default PDFMetadata;

lib/mixins/fonts.js

@@ -19,20 +19,20 @@ export default {
   },
 
   font(src, family, size) {
-    let cacheKey, font;
+    let name, font;
     if (typeof family === 'number') {
       size = family;
       family = null;
     }
 
     // check registered fonts if src is a string
     if (typeof src === 'string' && this._registeredFonts[src]) {
-      cacheKey = src;
+      name = src;
       ({ src, family } = this._registeredFonts[src]);
     } else {
-      cacheKey = family || src;
-      if (typeof cacheKey !== 'string') {
-        cacheKey = null;
+      name = family || src;
+      if (typeof name !== 'string') {
+        name = null;
       }
     }
 
@@ -41,7 +41,7 @@ export default {
     }
 
     // fast path: check if the font is already in the PDF
-    if ((font = this._fontFamilies[cacheKey])) {
+    if ((font = this._fontFamilies[name])) {
       this._font = font;
       return this;
     }
@@ -50,21 +50,21 @@ export default {
     const id = `F${++this._fontCount}`;
     this._font = PDFFontFactory.open(this, src, family, id);
 
+    // get the font name from the loaded font object (usually TTF metadata)
+    // useful if the font was passed as a buffer
+    if (!name) {
+      name = this._font.name || id;
+    }
+
     // check for existing font familes with the same name already in the PDF
     // useful if the font was passed as a buffer
-    if ((font = this._fontFamilies[this._font.name])) {
+    if ((font = this._fontFamilies[name])) {
       this._font = font;
       return this;
     }
 
-    // save the font for reuse later
-    if (cacheKey) {
-      this._fontFamilies[cacheKey] = this._font;
-    }
-
-    if (this._font.name) {
-      this._fontFamilies[this._font.name] = this._font;
-    }
+    // save the font for reuse later and for finalization in doc.end()
+    this._fontFamilies[name] = this._font;
 
     return this;
   },

tests/unit/document.spec.js

@@ -1,4 +1,5 @@
 import PDFDocument from '../../lib/document';
+import fs from 'fs';
 import { logData } from './helpers';
 
 describe('PDFDocument', () => {
@@ -54,12 +55,12 @@ describe('PDFDocument', () => {
     doc.end();
 
     let catalog = data[data.length-28];
-    
+
     expect(catalog).toContain('/Metadata');
   });
 
   test('metadata is NOT present for PDF 1.3', () => {
-    let doc = new PDFDocument({pdfVersion: '1.3'}); 
+    let doc = new PDFDocument({pdfVersion: '1.3'});
     const data = logData(doc);
     doc.end();
 
@@ -68,4 +69,62 @@ describe('PDFDocument', () => {
     expect(catalog).not.toContain('/Metadata');
   });
 
+  describe('FontsMixin', () => {
+    let roboto;
+
+    beforeAll(() => {
+      roboto = fs.readFileSync('tests/fonts/Roboto-Regular.ttf');
+    });
+
+    describe('font', () => {
+      test('saves a default font to _fontFamilies and reuses the same font object', () => {
+        const doc = new PDFDocument();
+
+        doc.font('Times-Italic');
+        const fontObj1 = doc._fontFamilies['Times-Italic'];
+
+        doc.font('Times-Italic');
+        const fontObj2 = doc._fontFamilies['Times-Italic'];
+
+        expect(Object.keys(doc._fontFamilies)).toEqual([
+          'Helvetica',
+          'Times-Italic'
+        ]);
+        expect(fontObj1).toBe(fontObj2);
+      });
+
+      test('saves a registered font to _fontFamilies and reuses the same font object', () => {
+        const doc = new PDFDocument();
+
+        doc.registerFont('My Roboto', roboto);
+        doc.font('My Roboto');
+        const fontObj1 = doc._fontFamilies['My Roboto'];
+
+        doc.font('My Roboto');
+        const fontObj2 = doc._fontFamilies['My Roboto'];
+
+        expect(Object.keys(doc._fontFamilies)).toEqual([
+          'Helvetica',
+          'My Roboto'
+        ]);
+        expect(fontObj1).toBe(fontObj2);
+      });
+
+      test('saves a font passed as a buffer to _fontFamilies and reuses the same font object', () => {
+        const doc = new PDFDocument();
+
+        doc.font(roboto);
+        const fontObj1 = doc._fontFamilies['Roboto-Regular'];
+
+        doc.font(roboto);
+        const fontObj2 = doc._fontFamilies['Roboto-Regular'];
+
+        expect(Object.keys(doc._fontFamilies)).toEqual([
+          'Helvetica',
+          'Roboto-Regular'
+        ]);
+        expect(fontObj1).toBe(fontObj2);
+      });
+    });
+  });
 });