From 326df1f711c831296cf8853d78f2bdea508ed292 Mon Sep 17 00:00:00 2001 From: Jonas Jenwald Date: Mon, 11 May 2026 12:44:57 +0200 Subject: [PATCH] Update the `deepCompare` helper function to handle `Ref`s and `Name`s correctly Note that `Ref`s and `Name`s are cached globally[1], since that helps reduce object creation (a lot) during parsing. That cache will be cleared after a period of inactivity in the viewer[2], which is why those primitives cannot *safely* be compared with just `===`/`!==` and also (partially) why abstractions such as `RefSet`/`RefSetCache` are necessary. Currently `deepCompare` doesn't handle `Ref`s and `Name`s correctly, which may lead to future *intermittent* bugs in any code using the `deepCompare` helper function. --- [1] This applies to `Cmd` as well, however that doesn't matter in the context of this patch. [2] Currently, and for more than a decade, set to 30 seconds. --- src/core/core_utils.js | 9 +++++++- test/unit/core_utils_spec.js | 45 +++++++++++++++++++++++++++++++++++- 2 files changed, 52 insertions(+), 2 deletions(-) diff --git a/src/core/core_utils.js b/src/core/core_utils.js index 56fa0236f..02c9adae6 100644 --- a/src/core/core_utils.js +++ b/src/core/core_utils.js @@ -23,7 +23,7 @@ import { Util, warn, } from "../shared/util.js"; -import { Dict, isName, Ref, RefSet } from "./primitives.js"; +import { Dict, isName, isRefsEqual, Name, Ref, RefSet } from "./primitives.js"; import { BaseStream } from "./base_stream.js"; const PDF_VERSION_REGEXP = /^[1-9]\.\d$/; @@ -210,6 +210,13 @@ function deepCompare(a, b) { if (a === b) { return true; } + if (a instanceof Ref && b instanceof Ref) { + return isRefsEqual(a, b); + } + if (a instanceof Name && b instanceof Name) { + return a.name === b.name; + } + if (a instanceof Dict && b instanceof Dict) { if (a.size !== b.size) { return false; diff --git a/test/unit/core_utils_spec.js b/test/unit/core_utils_spec.js index 788e70eab..7f60f4ac8 100644 --- a/test/unit/core_utils_spec.js +++ b/test/unit/core_utils_spec.js @@ -31,7 +31,12 @@ import { toRomanNumerals, validateCSSFont, } from "../../src/core/core_utils.js"; -import { Dict, Ref } from "../../src/core/primitives.js"; +import { + clearPrimitiveCaches, + Dict, + Name, + Ref, +} from "../../src/core/primitives.js"; import { XRefMock } from "./test_utils.js"; describe("core_utils", function () { @@ -498,6 +503,20 @@ describe("core_utils", function () { expect(deepCompare(a, b)).toBeTrue(); }); + it("should return true for Dicts with same Ref values, after clearing cached Refs", function () { + const refA = Ref.get(10, 0); + clearPrimitiveCaches(); + const refB = Ref.get(10, 0); + // Ensure that Ref-objects are not identical, after clearing the cache. + expect(refA).not.toBe(refB); + + const a = new Dict(); + a.set("Foo", refA); + const b = new Dict(); + b.set("Foo", refB); + expect(deepCompare(a, b)).toBeTrue(); + }); + it("should return false for Dicts with different Ref values", function () { const a = new Dict(); a.set("Foo", Ref.get(10, 0)); @@ -555,6 +574,30 @@ describe("core_utils", function () { it("should return false for arrays with different values", function () { expect(deepCompare([Ref.get(1, 0)], [Ref.get(2, 0)])).toBeFalse(); }); + + it("should return true for equal Names", function () { + const name1 = Name.get("name"), + name2 = Name.get("name"); + expect(name1).toBe(name2); // Names are cached. + + expect(deepCompare(name1, name2)).toBeTrue(); + }); + + it("should return false for different Names", function () { + const name1 = Name.get("name"), + name2 = Name.get("otherName"); + expect(deepCompare(name1, name2)).toBeFalse(); + }); + + it("should return true for equal Names, after clearing cached Names", function () { + const name1 = Name.get("name"); + clearPrimitiveCaches(); + const name2 = Name.get("name"); + // Ensure that Name-objects are not identical, after clearing the cache. + expect(name1).not.toBe(name2); + + expect(deepCompare(name1, name2)).toBeTrue(); + }); }); describe("getSizeInBytes", function () {