fix: preserve object order for PDFs with incremental updates (#951)

This fixes a critical issue where PDFs using the Incremental Update feature
would become corrupted after loading and saving with pdf-lib.

## Problem

When loading and saving PDFs without adding new content, the library would
always sort indirect objects by object number during enumeration. This broke
PDFs with incremental updates because the XRef byte offsets became invalid
when objects were reordered.

## Solution

Introduce a 'needsReordering' flag in PDFContext that tracks whether new
objects have been registered:

- When register() is called to add NEW objects, needsReordering is set to true
- enumerateIndirectObjects() only sorts objects when needsReordering is true
- If only existing objects are modified (no new objects added), the original
  parsing order is preserved

This ensures backward compatibility - PDFs with new content are still properly
sorted, while PDFs with only modifications maintain their original structure.

## Changes

- src/core/PDFContext.ts: Add needsReordering flag and conditional sorting
- tests/core/PDFContext_ordering.spec.ts: Add unit and integration tests
- assets/pdfs/with_incremental_updates.pdf: Test PDF file for the issue

Fixes #951

Author: natinew77-creator

assets/pdfs/with_incremental_updates.pdf

@@ -62,13 +62,24 @@ class PDFContext {
   private pushGraphicsStateContentStreamRef?: PDFRef;
   private popGraphicsStateContentStreamRef?: PDFRef;
 
+  /**
+   * Tracks whether new objects have been registered in this context.
+   * When true, objects will be sorted by object number during enumeration.
+   * When false, objects maintain their original order (important for PDFs
+   * with incremental updates where reordering can cause corruption).
+   * 
+   * @see https://github.com/Hopding/pdf-lib/issues/951
+   */
+  private needsReordering: boolean;
+
   private constructor() {
     this.largestObjectNumber = 0;
     this.header = PDFHeader.forVersion(1, 7);
     this.trailerInfo = {};
 
     this.indirectObjects = new Map();
     this.rng = SimpleRNG.withSeed(1);
+    this.needsReordering = false;
   }
 
   assign(ref: PDFRef, object: PDFObject): void {
@@ -85,6 +96,7 @@ class PDFContext {
 
   register(object: PDFObject): PDFRef {
     const ref = this.nextRef();
+    this.needsReordering = true;
     this.assign(ref, object);
     return ref;
   }
@@ -178,10 +190,24 @@ class PDFContext {
     return undefined;
   }
 
+  /**
+   * Returns all indirect objects in this context.
+   * 
+   * If new objects have been registered (needsReordering=true), objects are
+   * sorted by ascending object number to ensure proper XRef table generation.
+   * 
+   * If no new objects have been registered (needsReordering=false), objects
+   * maintain their original parsing order. This is crucial for PDFs with
+   * incremental updates, where reordering objects can cause corruption.
+   * 
+   * @see https://github.com/Hopding/pdf-lib/issues/951
+   */
   enumerateIndirectObjects(): [PDFRef, PDFObject][] {
-    return Array.from(this.indirectObjects.entries()).sort(
-      byAscendingObjectNumber,
-    );
+    const entries = Array.from(this.indirectObjects.entries());
+    if (this.needsReordering) {
+      return entries.sort(byAscendingObjectNumber);
+    }
+    return entries;
   }
 
   obj(literal: null | undefined): typeof PDFNull;

src/core/PDFContext.ts

@@ -0,0 +1,130 @@
+import fs from 'fs';
+import { PDFDocument } from 'src/index';
+import PDFContext from 'src/core/PDFContext';
+import PDFRef from 'src/core/objects/PDFRef';
+
+/**
+ * Tests for Issue #951 - Corrupted PDF on save with incremental updates
+ * @see https://github.com/Hopding/pdf-lib/issues/951
+ * 
+ * The issue occurs when pdf-lib loads and saves PDFs that use the "Incremental
+ * Update" feature of the PDF format. The previous implementation always sorted
+ * objects by object number during enumeration, which corrupted PDFs by
+ * invalidating the byte offsets in the XRef table.
+ * 
+ * The fix tracks whether new objects have been registered, and only sorts
+ * objects when necessary (when new objects are added that need to be
+ * integrated into the object number sequence).
+ */
+describe('Issue #951 - Corrupted PDF on save with incremental updates', () => {
+    describe('PDFContext.enumerateIndirectObjects ordering', () => {
+        it('should NOT sort objects when no new objects are registered', () => {
+            const context = PDFContext.create();
+
+            // Simulate loading objects from a PDF - assign directly without using register()
+            // This mimics what the parser does when loading an existing PDF
+            const obj1 = context.obj({ Type: 'Font' });
+            const obj2 = context.obj({ Type: 'Page' });
+            const obj3 = context.obj({ Type: 'Catalog' });
+
+            // PDFRef.of creates refs with specific object numbers
+            // Assign in non-sequential order to test ordering behavior
+            // In real parsing, objects may not come in sequential order
+            context.assign(PDFRef.of(5), obj1);
+            context.assign(PDFRef.of(2), obj2);
+            context.assign(PDFRef.of(8), obj3);
+
+            // When NO new objects are registered via register(), 
+            // enumeration should preserve insertion order (not sorted)
+            const entries = context.enumerateIndirectObjects();
+            const objectNumbers = entries.map(([ref]) => ref.objectNumber);
+
+            // Should maintain insertion order: 5, 2, 8 (not sorted as 2, 5, 8)
+            expect(objectNumbers).toEqual([5, 2, 8]);
+        });
+
+        it('should sort objects when new objects ARE registered', () => {
+            const context = PDFContext.create();
+
+            // Assign existing objects
+            const obj1 = context.obj({ Type: 'Font' });
+            const obj2 = context.obj({ Type: 'Page' });
+            context.assign(PDFRef.of(5), obj1);
+            context.assign(PDFRef.of(2), obj2);
+
+            // Register a NEW object (this triggers needsReordering = true)
+            const newObj = context.obj({ Type: 'NewObject' });
+            context.register(newObj);
+
+            // Now enumeration SHOULD sort by object number
+            const entries = context.enumerateIndirectObjects();
+            const objectNumbers = entries.map(([ref]) => ref.objectNumber);
+
+
+            // Should be sorted: 2, 5, 6 (the new one gets objectNumber 6 since largestObjectNumber was 5)
+            expect(objectNumbers).toEqual([2, 5, 6]);
+        });
+    });
+
+    describe('Integration tests with real PDFs', () => {
+        const incrementalUpdatesPdfPath = 'assets/pdfs/with_incremental_updates.pdf';
+        const normalPdfPath = 'assets/pdfs/normal.pdf';
+
+        const incrementalFileExists = fs.existsSync(incrementalUpdatesPdfPath);
+        const normalFileExists = fs.existsSync(normalPdfPath);
+
+        (incrementalFileExists ? it : it.skip)(
+            'should preserve PDF with incremental updates when loading and saving without modifications',
+            async () => {
+                const inputBytes = fs.readFileSync(incrementalUpdatesPdfPath);
+
+                // Load the PDF without making any modifications
+                const pdfDoc = await PDFDocument.load(inputBytes);
+                const pageCount = pdfDoc.getPageCount();
+
+                // Save the PDF
+                const savedBytes = await pdfDoc.save();
+
+                // Verify we can reload the saved PDF
+                const reloadedDoc = await PDFDocument.load(savedBytes);
+                expect(reloadedDoc.getPageCount()).toBe(pageCount);
+            }
+        );
+
+        (incrementalFileExists ? it : it.skip)(
+            'should preserve PDF with incremental updates when saving with useObjectStreams: false',
+            async () => {
+                const inputBytes = fs.readFileSync(incrementalUpdatesPdfPath);
+
+                const pdfDoc = await PDFDocument.load(inputBytes);
+                const pageCount = pdfDoc.getPageCount();
+
+                // Save with useObjectStreams: false (as mentioned in the issue)
+                const savedBytes = await pdfDoc.save({ useObjectStreams: false });
+
+                // Verify reload
+                const reloadedDoc = await PDFDocument.load(savedBytes);
+                expect(reloadedDoc.getPageCount()).toBe(pageCount);
+            }
+        );
+
+        (normalFileExists ? it : it.skip)(
+            'should still correctly save PDFs when new content is added',
+            async () => {
+                const inputBytes = fs.readFileSync(normalPdfPath);
+
+                const pdfDoc = await PDFDocument.load(inputBytes);
+                const originalPageCount = pdfDoc.getPageCount();
+
+                // Add a new page (this should trigger reordering)
+                pdfDoc.addPage();
+
+                const savedBytes = await pdfDoc.save();
+
+                // Verify the new page was added correctly
+                const reloadedDoc = await PDFDocument.load(savedBytes);
+                expect(reloadedDoc.getPageCount()).toBe(originalPageCount + 1);
+            }
+        );
+    });
+});