diff --git a/assets/pdfs/with_null_parent_entry.pdf b/assets/pdfs/with_null_parent_entry.pdf new file mode 100644 index 000000000..fdcf323c2 Binary files /dev/null and b/assets/pdfs/with_null_parent_entry.pdf differ diff --git a/src/core/PDFContext.ts b/src/core/PDFContext.ts index 1bbb1d242..fa9053136 100644 --- a/src/core/PDFContext.ts +++ b/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); diff --git a/src/core/errors.ts b/src/core/errors.ts index d3ad47e72..f29a3e4f9 100644 --- a/src/core/errors.ts +++ b/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); } } diff --git a/src/core/objects/PDFDict.ts b/src/core/objects/PDFDict.ts index ef8fb2682..d855d9010 100644 --- a/src/core/objects/PDFDict.ts +++ b/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 { diff --git a/tests/core/objects/PDFDict.spec.ts b/tests/core/objects/PDFDict.spec.ts index f6ba94c6b..f14faff9b 100644 --- a/tests/core/objects/PDFDict.spec.ts +++ b/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); + }); }); diff --git a/tests/core/parser/PDFObjectParser.spec.ts b/tests/core/parser/PDFObjectParser.spec.ts index 86c46bb29..576dfe14a 100644 --- a/tests/core/parser/PDFObjectParser.spec.ts +++ b/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`, () => { diff --git a/tests/core/structures/PDFPageTree.spec.ts b/tests/core/structures/PDFPageTree.spec.ts index 0a70160ac..57df5a619 100644 --- a/tests/core/structures/PDFPageTree.spec.ts +++ b/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); + }); });