Fix null dict entries

src/core/PDFContext.ts

@@ -110,15 +110,22 @@ class PDFContext {
   ): PDFString | PDFHexString | undefined;
 
   lookupMaybe(ref: LookupKey, ...types: any[]) {
+    // TODO: `preservePDFNull` is for backwards compatibility. Should be
+    // removed in next breaking API change.
+    const preservePDFNull = types.includes(PDFNull);
+
     const result = ref instanceof PDFRef ? this.indirectObjects.get(ref) : ref;
 
-    if (!result) return undefined;
+    if (!result || (result === PDFNull && !preservePDFNull)) return undefined;
 
     for (let idx = 0, len = types.length; idx < len; idx++) {
       const type = types[idx];
-      if (result instanceof type) return result;
+      if (type === PDFNull) {
+        if (result === PDFNull) return result;
+      } else {
+        if (result instanceof type) return result;
+      }
     }
-
     throw new UnexpectedObjectTypeError(types, result);
   }
 
@@ -146,7 +153,11 @@ class PDFContext {
 
     for (let idx = 0, len = types.length; idx < len; idx++) {
       const type = types[idx];
-      if (result instanceof type) return result;
+      if (type === PDFNull) {
+        if (result === PDFNull) return result;
+      } else {
+        if (result instanceof type) return result;
+      }
     }
 
     throw new UnexpectedObjectTypeError(types, result);

src/core/errors.ts

@@ -18,12 +18,16 @@ export class PrivateConstructorError extends Error {
 
 export class UnexpectedObjectTypeError extends Error {
   constructor(expected: any | any[], actual: any) {
+    const name = (t: any) => t?.name ?? t?.constructor?.name;
+
     const expectedTypes = Array.isArray(expected)
-      ? expected.map(({ name }) => name)
-      : [expected.name];
+      ? expected.map(name)
+      : [name(expected)];
+
     const msg =
       `Expected instance of ${expectedTypes.join(' or ')}, ` +
-      `but got instance of ${actual ? actual.constructor.name : actual}`;
+      `but got instance of ${actual ? name(actual) : actual}`;
+
     super(msg);
   }
 }

src/core/objects/PDFDict.ts

@@ -45,14 +45,18 @@ class PDFDict extends PDFObject {
     this.dict.set(key, value);
   }
 
-  get(key: PDFName): PDFObject | undefined {
-    return this.dict.get(key);
+  get(
+    key: PDFName,
+    // TODO: `preservePDFNull` is for backwards compatibility. Should be
+    // removed in next breaking API change.
+    preservePDFNull = false,
+  ): PDFObject | undefined {
+    const value = this.dict.get(key);
+    if (value === PDFNull && !preservePDFNull) return undefined;
+    return value;
   }
 
   has(key: PDFName): boolean {
-    // NOTE: This might be a bit inconsistent with `PDFDict.get()` - should
-    //       calls to `PDFDict.get()` return `undefined` if the underlying
-    //       value is `PDFNull`? This would be a breaking change.
     const value = this.dict.get(key);
     return value !== undefined && value !== PDFNull;
   }
@@ -88,11 +92,19 @@ class PDFDict extends PDFObject {
   ): PDFString | PDFHexString | PDFArray | undefined;
 
   lookupMaybe(key: PDFName, ...types: any[]) {
-    return this.context.lookupMaybe(
-      this.get(key),
+    // TODO: `preservePDFNull` is for backwards compatibility. Should be
+    // removed in next breaking API change.
+    const preservePDFNull = types.includes(PDFNull);
+
+    const value = this.context.lookupMaybe(
+      this.get(key, preservePDFNull),
       // @ts-ignore
       ...types,
     ) as any;
+
+    if (value === PDFNull && !preservePDFNull) return undefined;
+
+    return value;
   }
 
   lookup(key: PDFName): PDFObject | undefined;
@@ -124,11 +136,19 @@ class PDFDict extends PDFObject {
   ): PDFString | PDFHexString | PDFArray;
 
   lookup(key: PDFName, ...types: any[]) {
-    return this.context.lookup(
-      this.get(key),
+    // TODO: `preservePDFNull` is for backwards compatibility. Should be
+    // removed in next breaking API change.
+    const preservePDFNull = types.includes(PDFNull);
+
+    const value = this.context.lookup(
+      this.get(key, preservePDFNull),
       // @ts-ignore
       ...types,
     ) as any;
+
+    if (value === PDFNull && !preservePDFNull) return undefined;
+
+    return value;
   }
 
   delete(key: PDFName): boolean {

tests/core/objects/PDFDict.spec.ts

@@ -66,7 +66,7 @@ describe(`PDFDict`, () => {
     expect(pdfDict.get(PDFName.of('Boolean'))).toBe(pdfBool);
     expect(pdfDict.get(PDFName.of('HexString'))).toBe(pdfHexString);
     expect(pdfDict.get(PDFName.of('Name'))).toBe(pdfName);
-    expect(pdfDict.get(PDFName.of('Null'))).toBe(pdfNull);
+    expect(pdfDict.get(PDFName.of('Null'))).toBe(undefined);
     expect(pdfDict.get(PDFName.of('Number'))).toBe(pdfNumber);
     expect(pdfDict.get(PDFName.of('String'))).toBe(pdfString);
     expect(pdfDict.get(PDFName.of('Ref'))).toBe(pdfRef);
@@ -137,4 +137,31 @@ describe(`PDFDict`, () => {
       ),
     );
   });
+
+  it(`return "undefined" if the underlying value is "PDFNull"`, () => {
+    const dict = context.obj({ foo: null });
+    dict.set(PDFName.of('Bar'), PDFNull);
+    context.assign(PDFRef.of(21), PDFNull);
+    dict.set(PDFName.of('qux'), PDFRef.of(21));
+
+    expect(dict.get(PDFName.of('foo'))).toBe(undefined);
+    expect(dict.get(PDFName.of('Bar'))).toBe(undefined);
+    expect(dict.get(PDFName.of('qux'))).toBe(PDFRef.of(21));
+
+    expect(dict.lookup(PDFName.of('foo'))).toBe(undefined);
+    expect(dict.lookup(PDFName.of('Bar'))).toBe(undefined);
+    expect(dict.lookup(PDFName.of('qux'))).toBe(undefined);
+
+    expect(dict.lookup(PDFName.of('foo'), PDFNull)).toBe(PDFNull);
+    expect(dict.lookup(PDFName.of('Bar'), PDFNull)).toBe(PDFNull);
+    expect(dict.lookup(PDFName.of('qux'), PDFNull)).toBe(PDFNull);
+
+    expect(dict.lookupMaybe(PDFName.of('foo'), PDFNull)).toBe(PDFNull);
+    expect(dict.lookupMaybe(PDFName.of('Bar'), PDFNull)).toBe(PDFNull);
+    expect(dict.lookupMaybe(PDFName.of('qux'), PDFNull)).toBe(PDFNull);
+
+    expect(dict.lookupMaybe(PDFName.of('foo'), PDFDict)).toBe(undefined);
+    expect(dict.lookupMaybe(PDFName.of('Bar'), PDFDict)).toBe(undefined);
+    expect(dict.lookupMaybe(PDFName.of('qux'), PDFDict)).toBe(undefined);
+  });
 });

tests/core/parser/PDFObjectParser.spec.ts

@@ -518,7 +518,7 @@ describe(`PDFObjectParser`, () => {
       expect(dict.get(PDFName.of('PDFNumber'))).toBeInstanceOf(PDFNumber);
       expect(dict.get(PDFName.of('PDFHexString'))).toBeInstanceOf(PDFHexString);
       expect(dict.get(PDFName.of('PDFBool'))).toBe(PDFBool.True);
-      expect(dict.get(PDFName.of('PDFNull'))).toBe(PDFNull);
+      expect(dict.get(PDFName.of('PDFNull'))).toBe(undefined);
     });
 
     it(`handles dictionaries with no whitespace or comments`, () => {

tests/core/structures/PDFPageTree.spec.ts

@@ -1,3 +1,4 @@
+import fs from 'fs';
 import { TreeNode } from 'src/core/structures/PDFPageTree';
 import {
   PDFArray,
@@ -7,8 +8,13 @@ import {
   PDFPageLeaf,
   PDFPageTree,
   PDFRef,
+  PDFDocument,
 } from 'src/index';
 
+const withNullEntryPdfBytes = fs.readFileSync(
+  'assets/pdfs/with_null_parent_entry.pdf',
+);
+
 const pageUtils = () => {
   const context = PDFContext.create();
 
@@ -569,4 +575,18 @@ describe(`PDFPageTree`, () => {
       expect(tree1.Kids().get(1)).toBe(tree4Ref);
     });
   });
+
+  it(`can be ascended when a "/Parent null" entry exists on a node`, async () => {
+    const pdfDoc = await PDFDocument.load(withNullEntryPdfBytes);
+    const pages = pdfDoc.getPages();
+    const parent = pages[0].node.Parent();
+    expect(parent).toBeInstanceOf(PDFPageTree);
+
+    let ascentions = 0;
+    const handleAscend = () => {
+      ascentions += 1;
+    };
+    expect(() => parent?.ascend(handleAscend)).not.toThrow();
+    expect(ascentions).toBe(1);
+  });
 });