From f1e1973e6fc522bb143be3516decabef699b5429 Mon Sep 17 00:00:00 2001 From: Jonas Jenwald Date: Sat, 14 Mar 2026 09:54:59 +0100 Subject: [PATCH 1/2] Remove the unused `bbox` setter in the `FontFaceObject` class (PR 20427 follow-up) The commit message for the patch in PR 20427 is pretty non-descriptive, being only a single line, however there's a bit more context in https://github.com/mozilla/pdf.js/pull/20427#issue-3597370951 but unfortunately the details there don't really make sense. Note that the PR only changed main-thread code, but all the links are to worker-thread code!? The `FontFaceObject` class is only used on the main-thread, and when encountering a broken font we fallback to the built-in font renderer; see https://github.com/mozilla/pdf.js/blob/820b70eb25b1c6bf74f916e002d11afc49e929cf/src/display/font_loader.js#L135-L143 Hence the `FontFaceObject` class *only* needs a way to set the `disableFontFace` property, however nowhere on the main-thread do we ever update the `bbox` of a font. --- src/display/font_loader.js | 4 ---- 1 file changed, 4 deletions(-) diff --git a/src/display/font_loader.js b/src/display/font_loader.js index 0ace54eb5a559..fbfb2bc8232e7 100644 --- a/src/display/font_loader.js +++ b/src/display/font_loader.js @@ -507,10 +507,6 @@ class FontFaceObject { return this.#fontData.bbox; } - set bbox(bbox) { - shadow(this, "bbox", bbox); - } - get fontMatrix() { return this.#fontData.fontMatrix; } From 0fca64f01e625479c887156096146b0fbac16431 Mon Sep 17 00:00:00 2001 From: Calixte Denizet Date: Fri, 13 Mar 2026 12:19:30 +0100 Subject: [PATCH 2/2] Check for having Ref before adding them in a RefSet (bug 2023106) --- src/core/editor/pdf_editor.js | 7 +- src/core/name_number_tree.js | 14 ++- src/core/primitives.js | 14 +++ test/unit/clitests.json | 1 + test/unit/jasmine-boot.js | 1 + test/unit/name_number_tree_spec.js | 170 +++++++++++++++++++++++++++++ 6 files changed, 200 insertions(+), 7 deletions(-) create mode 100644 test/unit/name_number_tree_spec.js diff --git a/src/core/editor/pdf_editor.js b/src/core/editor/pdf_editor.js index 470913817b57b..eed48c4e63e6d 100644 --- a/src/core/editor/pdf_editor.js +++ b/src/core/editor/pdf_editor.js @@ -1390,13 +1390,16 @@ class PDFEditor { let parent = parentRef; let lastNonNullParent = parentRef; while (true) { - parent = xref.fetchIfRef(parent)?.get("Parent") || null; + parent = xref.fetchIfRef(parent)?.getRaw("Parent") || null; if (!parent) { break; } lastNonNullParent = parent; } - if (!processed.has(lastNonNullParent)) { + if ( + lastNonNullParent instanceof Ref && + !processed.has(lastNonNullParent) + ) { newFields.push(lastNonNullParent); processed.put(lastNonNullParent); } diff --git a/src/core/name_number_tree.js b/src/core/name_number_tree.js index c5b63dc7b6432..8cb825350ab86 100644 --- a/src/core/name_number_tree.js +++ b/src/core/name_number_tree.js @@ -13,7 +13,7 @@ * limitations under the License. */ -import { Dict, RefSet } from "./primitives.js"; +import { Dict, Ref, RefSet } from "./primitives.js"; import { FormatError, unreachable, warn } from "../shared/util.js"; /** @@ -42,7 +42,9 @@ class NameOrNumberTree { const xref = this.xref; // Reading Name/Number tree. const processed = new RefSet(); - processed.put(this.root); + if (this.root instanceof Ref) { + processed.put(this.root); + } const queue = [this.root]; while (queue.length > 0) { const obj = xref.fetchIfRef(queue.shift()); @@ -55,11 +57,13 @@ class NameOrNumberTree { continue; } for (const kid of kids) { - if (processed.has(kid)) { - throw new FormatError(`Duplicate entry in "${this._type}" tree.`); + if (kid instanceof Ref) { + if (processed.has(kid)) { + throw new FormatError(`Duplicate entry in "${this._type}" tree.`); + } + processed.put(kid); } queue.push(kid); - processed.put(kid); } continue; } diff --git a/src/core/primitives.js b/src/core/primitives.js index e862bd5388fdd..8e09845d61a1b 100644 --- a/src/core/primitives.js +++ b/src/core/primitives.js @@ -377,10 +377,24 @@ class RefSet { } has(ref) { + if ( + (typeof PDFJSDev === "undefined" || PDFJSDev.test("TESTING")) && + !(ref instanceof Ref) && + typeof ref !== "string" + ) { + unreachable('RefSet: Invalid "ref" value in has.'); + } return this._set.has(ref.toString()); } put(ref) { + if ( + (typeof PDFJSDev === "undefined" || PDFJSDev.test("TESTING")) && + !(ref instanceof Ref) && + typeof ref !== "string" + ) { + unreachable('RefSet: Invalid "ref" value in put.'); + } this._set.add(ref.toString()); } diff --git a/test/unit/clitests.json b/test/unit/clitests.json index 3fc516e803dc5..c1ca9962308c7 100644 --- a/test/unit/clitests.json +++ b/test/unit/clitests.json @@ -31,6 +31,7 @@ "message_handler_spec.js", "metadata_spec.js", "murmurhash3_spec.js", + "name_number_tree_spec.js", "network_utils_spec.js", "node_stream_spec.js", "obj_bin_transform_spec.js", diff --git a/test/unit/jasmine-boot.js b/test/unit/jasmine-boot.js index 6d359969beb91..f79702bc94d36 100644 --- a/test/unit/jasmine-boot.js +++ b/test/unit/jasmine-boot.js @@ -74,6 +74,7 @@ async function initializePDFJS(callback) { "pdfjs-test/unit/message_handler_spec.js", "pdfjs-test/unit/metadata_spec.js", "pdfjs-test/unit/murmurhash3_spec.js", + "pdfjs-test/unit/name_number_tree_spec.js", "pdfjs-test/unit/network_spec.js", "pdfjs-test/unit/network_utils_spec.js", "pdfjs-test/unit/obj_bin_transform_spec.js", diff --git a/test/unit/name_number_tree_spec.js b/test/unit/name_number_tree_spec.js new file mode 100644 index 0000000000000..b4e57d8cc9631 --- /dev/null +++ b/test/unit/name_number_tree_spec.js @@ -0,0 +1,170 @@ +/* Copyright 2026 Mozilla Foundation + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import { Dict, Ref } from "../../src/core/primitives.js"; +import { NameTree, NumberTree } from "../../src/core/name_number_tree.js"; +import { XRefMock } from "./test_utils.js"; + +describe("NameOrNumberTree", function () { + describe("NameTree", function () { + it("should return an empty map when root is null", function () { + const xref = new XRefMock([]); + const tree = new NameTree(null, xref); + expect(tree.getAll().size).toEqual(0); + }); + + it("should collect all entries from a flat tree", function () { + const root = new Dict(); + root.set("Names", ["alpha", "value_a", "beta", "value_b"]); + + const xref = new XRefMock([]); + const tree = new NameTree(root, xref); + const map = tree.getAll(); + + expect(map.size).toEqual(2); + expect(map.get("alpha")).toEqual("value_a"); + expect(map.get("beta")).toEqual("value_b"); + }); + + it("should collect all entries from a tree with Ref-based Kids", function () { + const leafRef = Ref.get(10, 0); + const leaf = new Dict(); + leaf.set("Names", ["key1", "val1", "key2", "val2"]); + + const root = new Dict(); + root.set("Kids", [leafRef]); + + const xref = new XRefMock([{ ref: leafRef, data: leaf }]); + const tree = new NameTree(root, xref); + const map = tree.getAll(); + + expect(map.size).toEqual(2); + expect(map.get("key1")).toEqual("val1"); + expect(map.get("key2")).toEqual("val2"); + }); + + it("should handle Kids containing inline (non-Ref) Dict nodes without throwing", function () { + // Regression test: before the fix, processed.put() was called on non-Ref + // Dict objects, causing an error in TESTING mode because RefSet only + // accepts Ref instances or ref strings. + const inlineLeaf = new Dict(); + inlineLeaf.set("Names", ["key1", "val1", "key2", "val2"]); + + const root = new Dict(); + root.set("Kids", [inlineLeaf]); + + const xref = new XRefMock([]); + const tree = new NameTree(root, xref); + + // Should not throw even though the kid is an inline Dict (not a Ref). + const map = tree.getAll(); + expect(map.size).toEqual(2); + expect(map.get("key1")).toEqual("val1"); + expect(map.get("key2")).toEqual("val2"); + }); + + it("should throw on duplicate Ref entries in Kids", function () { + const leafRef = Ref.get(20, 0); + const leaf = new Dict(); + leaf.set("Names", ["a", "b"]); + + const root = new Dict(); + root.set("Kids", [leafRef, leafRef]); + + const xref = new XRefMock([{ ref: leafRef, data: leaf }]); + const tree = new NameTree(root, xref); + + expect(() => tree.getAll()).toThrow( + new Error('Duplicate entry in "Names" tree.') + ); + }); + + it("should resolve Ref values when isRaw is false", function () { + const valRef = Ref.get(30, 0); + const valData = "resolved_value"; + + const root = new Dict(); + root.set("Names", ["mykey", valRef]); + + const xref = new XRefMock([{ ref: valRef, data: valData }]); + const tree = new NameTree(root, xref); + + const map = tree.getAll(); + expect(map.get("mykey")).toEqual("resolved_value"); + }); + + it("should keep raw Ref values when isRaw is true", function () { + const valRef = Ref.get(31, 0); + + const root = new Dict(); + root.set("Names", ["mykey", valRef]); + + const xref = new XRefMock([{ ref: valRef, data: "resolved_value" }]); + const tree = new NameTree(root, xref); + + const map = tree.getAll(/* isRaw = */ true); + expect(map.get("mykey")).toBe(valRef); + }); + }); + + describe("NumberTree", function () { + it("should collect all entries from a flat tree", function () { + const root = new Dict(); + root.set("Nums", [1, "one", 2, "two"]); + + const xref = new XRefMock([]); + const tree = new NumberTree(root, xref); + const map = tree.getAll(); + + expect(map.size).toEqual(2); + expect(map.get(1)).toEqual("one"); + expect(map.get(2)).toEqual("two"); + }); + + it("should handle Kids containing inline (non-Ref) Dict nodes without throwing", function () { + // Same regression as NameTree: non-Ref kids must not be passed to + // RefSet.put(). + const inlineLeaf = new Dict(); + inlineLeaf.set("Nums", [0, "zero", 1, "one"]); + + const root = new Dict(); + root.set("Kids", [inlineLeaf]); + + const xref = new XRefMock([]); + const tree = new NumberTree(root, xref); + + const map = tree.getAll(); + expect(map.size).toEqual(2); + expect(map.get(0)).toEqual("zero"); + expect(map.get(1)).toEqual("one"); + }); + + it("should throw on duplicate Ref entries in Kids", function () { + const leafRef = Ref.get(40, 0); + const leaf = new Dict(); + leaf.set("Nums", [5, "five"]); + + const root = new Dict(); + root.set("Kids", [leafRef, leafRef]); + + const xref = new XRefMock([{ ref: leafRef, data: leaf }]); + const tree = new NumberTree(root, xref); + + expect(() => tree.getAll()).toThrow( + new Error('Duplicate entry in "Nums" tree.') + ); + }); + }); +});