From 231597d8ddc313925933aed166d7d10371e83196 Mon Sep 17 00:00:00 2001 From: Mahathir Mohammad Shuvo Date: Wed, 26 Aug 2026 11:07:42 +0600 Subject: [PATCH] Fix addNamedDestination writing the wrong number of parameters MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit addNamedDestination passed its arguments straight through, so a call could write a destination carrying more or fewer parameters than its type takes (ISO 32000-1, Table 151). Surplus parameters are the clearer problem, because a reader does not tolerate them: pdfjs-dist rejects [page /Fit 99] and [page /XYZ left top zoom extra] outright, so the link goes nowhere, while accepting both of them trimmed. Leaving the top off an XYZ destination, or passing it as undefined, was the noisier case. The top is flipped against the page height, and page.height minus undefined is NaN, which threw `unsupported number: NaN` from PDFObject once the destination was serialised — an error naming neither the method nor the argument. Only that one position was affected: an undefined left or zoom already serialised as null, so the method was inconsistent about which missing parameter it would tolerate. Arguments are now trimmed to the type's parameter list, and a short list is filled out with null for the five types whose parameters Table 151 allows to be null: XYZ, FitH, FitV, FitBH and FitBV. FitR has no such allowance, so a short FitR is passed through as it was given, as is a destination type that is not recognised. This resizes the argument list; it does not validate the parameter values. --- CHANGELOG.md | 1 + docs/destinations.md | 3 ++ lib/document.js | 28 +++++++++++- tests/unit/trailer.spec.js | 87 ++++++++++++++++++++++++++++++++++++++ 4 files changed, 117 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9b27cdb02..70c374aab 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,7 @@ ### Unreleased - Fix `doc.file()` throwing when the same in-memory attachment is embedded twice under one name, because the creation and modified dates the deduplication check compares are absent for sources that are not read from disk +- Fix `doc.addNamedDestination()` writing a destination that carries more parameters than its type takes, and throwing `unsupported number: NaN` when the top of an `XYZ` destination was left off or passed as `undefined`. Parameters beyond the type's list are now dropped, and a short list is filled out with null for the types whose parameters may be null (`XYZ`, `FitH`, `FitV`, `FitBH` and `FitBV`) ### [v0.20.1] - 2026-08-23 diff --git a/docs/destinations.md b/docs/destinations.md index 30becbe6f..eefcc9e3c 100644 --- a/docs/destinations.md +++ b/docs/destinations.md @@ -13,6 +13,9 @@ Examples of creating anchor: // Insert anchor to display a portion of the current page, 1/2 inch in from the top and left and zoomed 50% doc.addNamedDestination('LINK', 'XYZ', 36, 36, 50); + // Insert anchor 1/2 inch in from the top and left, leaving the zoom as the reader has it + doc.addNamedDestination('LINK', 'XYZ', 36, 36); + // Insert anchor for this text doc.text('End of paragraph', { destination: 'ENDP' }); diff --git a/lib/document.js b/lib/document.js index 4e76a0a5a..2b9588e72 100644 --- a/lib/document.js +++ b/lib/document.js @@ -24,6 +24,20 @@ import TableMixin from './mixins/table'; import MetadataMixin from './mixins/metadata'; import { fromBinaryString } from './binary'; +// The parameters each destination type takes (ISO 32000-1, Table 151). `nullable` +// marks the types whose parameters may each be null, which tells the reader to keep +// that aspect of its current view. FitR has no such allowance. +const DESTINATION_PARAMETERS = new Map([ + ['XYZ', { count: 3, nullable: true }], + ['Fit', { count: 0, nullable: false }], + ['FitH', { count: 1, nullable: true }], + ['FitV', { count: 1, nullable: true }], + ['FitR', { count: 4, nullable: false }], + ['FitB', { count: 0, nullable: false }], + ['FitBH', { count: 1, nullable: true }], + ['FitBV', { count: 1, nullable: true }], +]); + class PDFDocument extends Readable { constructor(options = {}) { super(options); @@ -221,9 +235,19 @@ class PDFDocument extends Readable { addNamedDestination(name, ...args) { if (args.length === 0) { - args = ['XYZ', null, null, null]; + args = ['XYZ']; + } + const parameters = DESTINATION_PARAMETERS.get(args[0]); + if (parameters !== undefined) { + // Drop anything past the parameters the type takes, and fill out a short + // list only where a null parameter is allowed. The holes this leaves are + // written as null by PDFObject.convert. + const length = parameters.count + 1; + if (args.length > length || parameters.nullable) { + args.length = length; + } } - if (args[0] === 'XYZ' && args[2] !== null) { + if (args[0] === 'XYZ' && args[2] != null) { args[2] = this.page.height - args[2]; } args.unshift(this.page.dictionary); diff --git a/tests/unit/trailer.spec.js b/tests/unit/trailer.spec.js index 95c7566bd..25f1a7671 100644 --- a/tests/unit/trailer.spec.js +++ b/tests/unit/trailer.spec.js @@ -72,6 +72,93 @@ describe('Document trailer', () => { /Resources 6 0 R /UserUnit 1 /Annots [9 0 R] +>>`, + ]); + }); + + test('writes null for XYZ parameters left off, rather than a NaN top', () => { + const docData = logData(document); + document.addNamedDestination('LINK1', 'XYZ', 36); + document.end(); + + expect(docData).toContainChunk([ + '2 0 obj', + `<< +/Dests << + /Names [ + (LINK1) [7 0 R /XYZ 36 null null] +] +>> +>>`, + ]); + }); + + test('fills out a short parameter list for the types that allow null', () => { + const docData = logData(document); + document.addNamedDestination('LINK1', 'XYZ', 36, 36); + document.addNamedDestination('LINK2', 'FitH'); + document.addNamedDestination('LINK3', 'FitV'); + document.addNamedDestination('LINK4', 'FitBH'); + document.addNamedDestination('LINK5', 'FitBV'); + document.end(); + + expect(docData).toContainChunk([ + '2 0 obj', + `<< +/Dests << + /Limits [(LINK1) (LINK5)] + /Names [ + (LINK1) [7 0 R /XYZ 36 756 null] + (LINK2) [7 0 R /FitH null] + (LINK3) [7 0 R /FitV null] + (LINK4) [7 0 R /FitBH null] + (LINK5) [7 0 R /FitBV null] +] +>> +>>`, + ]); + }); + + test('drops parameters beyond the list a destination type takes', () => { + const docData = logData(document); + document.addNamedDestination('LINK1', 'Fit', 99); + document.addNamedDestination('LINK2', 'FitB', 99); + document.addNamedDestination('LINK3', 'XYZ', 1, 2, 3, 4); + document.addNamedDestination('LINK4', 'FitR', 1, 2, 3, 4, 5); + document.end(); + + expect(docData).toContainChunk([ + '2 0 obj', + `<< +/Dests << + /Limits [(LINK1) (LINK4)] + /Names [ + (LINK1) [7 0 R /Fit] + (LINK2) [7 0 R /FitB] + (LINK3) [7 0 R /XYZ 1 790 3] + (LINK4) [7 0 R /FitR 1 2 3 4] +] +>> +>>`, + ]); + }); + + // FitR is the one type whose parameters may not be null, so a short one is left as it + // was given rather than filled out. It is still not a valid destination; resizing it is + // out of scope here. + test('leaves a short FitR destination as it was given', () => { + const docData = logData(document); + document.addNamedDestination('LINK1', 'FitR', 1, 2, 3); + document.end(); + + expect(docData).toContainChunk([ + '2 0 obj', + `<< +/Dests << + /Names [ + (LINK1) [7 0 R /FitR 1 2 3] +] +>> >>`, ]); });