From 4abe43298ec81632c3bff6bd15990a8b4755a985 Mon Sep 17 00:00:00 2001
From: Mahathir Mohammad Shuvo <shuvo1728@gmail.com>
Date: Thu, 20 Aug 2026 20:00:51 +0600
Subject: [PATCH] Fix a hole in a sparse array being skipped when serialising

`PDFObject.convert` used `.map` for arrays, which does not call back for
holes, so a sparse array lost the entry entirely and every later index
shifted down. A destination array is `[page /XYZ left top zoom]`, so
building one by index and leaving `left`/`top` unset slid `zoom` two slots
and a reader followed the wrong destination.

Replaced with an indexed loop rather than `Array.from`, which also visits
holes but defers to a subclass's custom iterator. A hole becomes `null`,
matching what #1769 already does for an explicit `undefined` entry.
---
 CHANGELOG.md                   |  1 +
 lib/object.js                  | 17 +++++++-------
 tests/unit/annotations.spec.js | 42 ++++++++++++++++++++++++++++++++++
 tests/unit/object.spec.js      |  7 ++++++
 4 files changed, 59 insertions(+), 8 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index eded8f7e..4f06eae9 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -10,6 +10,7 @@
 - Use `Uint8Array` instead of Node's `Buffer` internally
 - Fix `date` text field formatting emitting invalid JavaScript, so the format was never applied. Fixes #1546
 - Fix `indentAllLines` applying the indent again on every paragraph and every page break, and keep it applied across continued text. Fixes #1606
+- Fix a hole in a sparse array being skipped entirely, which shifted every later entry down one
 
 ### [v0.19.1] - 2026-06-10
 
diff --git a/lib/object.js b/lib/object.js
index 9c4c3aff..7bccec45 100644
--- a/lib/object.js
+++ b/lib/object.js
@@ -127,14 +127,15 @@ class PDFObject {
 
       return `(${string})`;
     } else if (Array.isArray(object)) {
-      // Array entries are positional, so an `undefined` hole cannot simply be
-      // dropped without shifting everything after it. `null` is a real PDF
-      // object (ISO 32000-1, 7.3.9) and is the closest valid stand-in, which
-      // is also what JSON.stringify does.
-      const items = object
-        .map((e) => PDFObject.convert(e === undefined ? null : e, encryptFn))
-        .join(' ');
-      return `[${items}]`;
+      // Indexed, not `.map` (skips holes, shifting later entries) and not
+      // `Array.from` (defers to a subclass's iterator). Positions are
+      // meaningful, so a hole becomes `null` (ISO 32000-1, 7.3.9).
+      const items = [];
+      for (let i = 0; i < object.length; i++) {
+        const e = object[i];
+        items.push(PDFObject.convert(e === undefined ? null : e, encryptFn));
+      }
+      return `[${items.join(' ')}]`;
     } else if ({}.toString.call(object) === '[object Object]') {
       const out = ['<<'];
       for (let key in object) {
diff --git a/tests/unit/annotations.spec.js b/tests/unit/annotations.spec.js
index dd37aa36..3b238f63 100644
--- a/tests/unit/annotations.spec.js
+++ b/tests/unit/annotations.spec.js
@@ -101,6 +101,48 @@ describe('Annotations', () => {
     });
   });
 
+  describe('undefined option values', () => {
+    // `doc.annotate()` passes arbitrary dictionary keys straight through by
+    // design, so unlike the acroform options there is no call site at which an
+    // absent value could be normalised first.
+    test('drops the key but keeps every other entry', () => {
+      const docData = logData(document);
+
+      document.annotate(10, 30, 30, 30, { Subtype: 'Text', CA: undefined });
+
+      const dataStr = docData.join('\n');
+      // the defect
+      expect(dataStr).not.toContain('/CA');
+      expect(dataStr).not.toContain('undefined');
+      // ...and the rest of the dictionary must survive, which a guard that
+      // dropped every key would also satisfy the negative assertions above
+      expect(dataStr).toContain('/Subtype /Text');
+      expect(dataStr).toContain('/Type /Annot');
+      expect(dataStr).toContain('/Rect [10 732 40 762]');
+      expect(dataStr).toContain('/Border [0 0 0]');
+    });
+
+    test('keeps array positions when an entry is absent', () => {
+      // A destination array is [page /XYZ left top zoom], and ISO 32000-1
+      // Table 151 explicitly allows null for left, top and zoom - it means
+      // "retain the current value". So this is a place where the stand-in is
+      // legal, and the point of the test is positional: skipping the hole
+      // instead would slide `3` from the zoom slot into the top slot.
+      const docData = logData(document);
+
+      document.annotate(10, 30, 30, 30, {
+        Subtype: 'Link',
+        // eslint-disable-next-line no-sparse-arrays
+        Dest: [document.page.dictionary, 'XYZ', 1, , 3],
+      });
+
+      const dataStr = docData.join('\n');
+      // asserted on the tail so the page object number stays incidental
+      expect(dataStr).toContain('/XYZ 1 null 3]');
+      expect(dataStr).not.toContain('/XYZ 1  3]');
+    });
+  });
+
   describe('note', () => {
     test.each([
       ['null', null],
diff --git a/tests/unit/object.spec.js b/tests/unit/object.spec.js
index c5396aa8..bae20114 100644
--- a/tests/unit/object.spec.js
+++ b/tests/unit/object.spec.js
@@ -33,6 +33,13 @@ describe('PDFObject', () => {
     test('array converts an undefined entry to null to keep positions', () => {
       expect(PDFObject.convert([1, undefined, 2])).toEqual('[1 null 2]');
     });
+
+    test('an array hole keeps its position', () => {
+      // `.map` skipped holes, so the entry vanished and every later index
+      // shifted down: `[1, , 2]` serialised as a two-element array.
+      // eslint-disable-next-line no-sparse-arrays
+      expect(PDFObject.convert([1, , 2])).toEqual('[1 null 2]');
+    });
   });
 
   describe('escapeName', () => {