Re-use the isValidExplicitDest helper function in the worker/viewer

Currently we re-implement the same helper function twice, which in hindsight seems like the wrong decision since that way it's quite easy for the implementations to accidentally diverge. The reason for doing it this way was because the code in the worker-thread is able to check for `Ref`- and `Name`-instances directly, which obviously isn't possible in the viewer but can be solved by passing validation-functions to the helper.
mozilla · Mar 1, 2025 · 165d90f · 165d90f
1 parent 5f4d923
commit 165d90f
Show file tree

Hide file tree

Showing 7 changed files with 90 additions and 119 deletions.
diff --git a/src/core/catalog.js b/src/core/catalog.js
@@ -14,15 +14,7 @@
  */
 
 import {
-  collectActions,
-  isNumberArray,
-  MissingDataException,
-  PDF_VERSION_REGEXP,
-  recoverJsURL,
-  toRomanNumerals,
-  XRefEntryException,
-} from "./core_utils.js";
-import {
+  _isValidExplicitDest,
   createValidAbsoluteUrl,
   DocumentActionEventType,
   FormatError,
@@ -34,6 +26,15 @@ import {
   stringToUTF8String,
   warn,
 } from "../shared/util.js";
+import {
+  collectActions,
+  isNumberArray,
+  MissingDataException,
+  PDF_VERSION_REGEXP,
+  recoverJsURL,
+  toRomanNumerals,
+  XRefEntryException,
+} from "./core_utils.js";
 import {
   Dict,
   isDict,
@@ -53,52 +54,13 @@ import { GlobalImageCache } from "./image_utils.js";
 import { MetadataParser } from "./metadata_parser.js";
 import { StructTreeRoot } from "./struct_tree.js";
 
-function isValidExplicitDest(dest) {
-  if (!Array.isArray(dest) || dest.length < 2) {
-    return false;
-  }
-  const [page, zoom, ...args] = dest;
-  if (!(page instanceof Ref) && !Number.isInteger(page)) {
-    return false;
-  }
-  if (!(zoom instanceof Name)) {
-    return false;
-  }
-  const argsLen = args.length;
-  let allowNull = true;
-  switch (zoom.name) {
-    case "XYZ":
-      if (argsLen < 2 || argsLen > 3) {
-        return false;
-      }
-      break;
-    case "Fit":
-    case "FitB":
-      return argsLen === 0;
-    case "FitH":
-    case "FitBH":
-    case "FitV":
-    case "FitBV":
-      if (argsLen > 1) {
-        return false;
-      }
-      break;
-    case "FitR":
-      if (argsLen !== 4) {
-        return false;
-      }
-      allowNull = false;
-      break;
-    default:
-      return false;
-  }
-  for (const arg of args) {
-    if (!(typeof arg === "number" || (allowNull && arg === null))) {
-      return false;
-    }
-  }
-  return true;
-}
+const isRef = v => v instanceof Ref;
+
+const isValidExplicitDest = _isValidExplicitDest.bind(
+  null,
+  /* validRef = */ isRef,
+  /* validName = */ isName
+);
 
 function fetchDest(dest) {
   if (dest instanceof Dict) {

diff --git a/src/display/api.js b/src/display/api.js
@@ -18,6 +18,7 @@
  */
 
 import {
+  _isValidExplicitDest,
   AbortException,
   AnnotationMode,
   assert,
@@ -595,15 +596,20 @@ function getFactoryUrlProp(val) {
   throw new Error(`Invalid factory url: "${val}" must include trailing slash.`);
 }
 
-function isRefProxy(ref) {
-  return (
-    typeof ref === "object" &&
-    Number.isInteger(ref?.num) &&
-    ref.num >= 0 &&
-    Number.isInteger(ref?.gen) &&
-    ref.gen >= 0
-  );
-}
+const isRefProxy = v =>
+  typeof v === "object" &&
+  Number.isInteger(v?.num) &&
+  v.num >= 0 &&
+  Number.isInteger(v?.gen) &&
+  v.gen >= 0;
+
+const isNameProxy = v => typeof v === "object" && typeof v?.name === "string";
+
+const isValidExplicitDest = _isValidExplicitDest.bind(
+  null,
+  /* validRef = */ isRefProxy,
+  /* validName = */ isNameProxy
+);
 
 /**
  * @typedef {Object} OnProgressParameters
@@ -3484,6 +3490,7 @@ const build =
 export {
   build,
   getDocument,
+  isValidExplicitDest,
   LoopbackPort,
   PDFDataRangeTransport,
   PDFDocumentLoadingTask,

diff --git a/src/pdf.js b/src/pdf.js
@@ -45,6 +45,7 @@ import {
 import {
   build,
   getDocument,
+  isValidExplicitDest,
   PDFDataRangeTransport,
   PDFWorker,
   version,
@@ -117,6 +118,7 @@ export {
   InvalidPDFException,
   isDataScheme,
   isPdfFile,
+  isValidExplicitDest,
   noContextMenu,
   normalizeUnicode,
   OPS,

diff --git a/src/shared/util.js b/src/shared/util.js
@@ -1080,6 +1080,54 @@ function getUuid() {
 
 const AnnotationPrefix = "pdfjs_internal_id_";
 
+function _isValidExplicitDest(validRef, validName, dest) {
+  if (!Array.isArray(dest) || dest.length < 2) {
+    return false;
+  }
+  const [page, zoom, ...args] = dest;
+  if (!validRef(page) && !Number.isInteger(page)) {
+    return false;
+  }
+  if (!validName(zoom)) {
+    return false;
+  }
+  const argsLen = args.length;
+  let allowNull = true;
+  switch (zoom.name) {
+    case "XYZ":
+      if (argsLen < 2 || argsLen > 3) {
+        return false;
+      }
+      break;
+    case "Fit":
+    case "FitB":
+      return argsLen === 0;
+    case "FitH":
+    case "FitBH":
+    case "FitV":
+    case "FitBV":
+      if (argsLen > 1) {
+        return false;
+      }
+      break;
+    case "FitR":
+      if (argsLen !== 4) {
+        return false;
+      }
+      allowNull = false;
+      break;
+    default:
+      return false;
+  }
+  for (const arg of args) {
+    if (typeof arg === "number" || (allowNull && arg === null)) {
+      continue;
+    }
+    return false;
+  }
+  return true;
+}
+
 // TODO: Remove this once `Uint8Array.prototype.toHex` is generally available.
 function toHexUtil(arr) {
   if (Uint8Array.prototype.toHex) {
@@ -1119,6 +1167,7 @@ if (
 }
 
 export {
+  _isValidExplicitDest,
   AbortException,
   AnnotationActionEventType,
   AnnotationBorderStyleType,

diff --git a/test/unit/pdf_spec.js b/test/unit/pdf_spec.js
@@ -36,6 +36,7 @@ import {
 import {
   build,
   getDocument,
+  isValidExplicitDest,
   PDFDataRangeTransport,
   PDFWorker,
   version,
@@ -94,6 +95,7 @@ const expectedAPI = Object.freeze({
   InvalidPDFException,
   isDataScheme,
   isPdfFile,
+  isValidExplicitDest,
   noContextMenu,
   normalizeUnicode,
   OPS,

diff --git a/web/pdf_link_service.js b/web/pdf_link_service.js
@@ -16,6 +16,7 @@
 /** @typedef {import("./event_utils").EventBus} EventBus */
 /** @typedef {import("./interfaces").IPDFLinkService} IPDFLinkService */
 
+import { isValidExplicitDest } from "pdfjs-lib";
 import { parseQueryString } from "./ui_utils.js";
 
 const DEFAULT_LINK_REL = "noopener noreferrer nofollow";
@@ -415,7 +416,7 @@ class PDFLinkService {
       }
     } catch {}
 
-    if (typeof dest === "string" || PDFLinkService.#isValidExplicitDest(dest)) {
+    if (typeof dest === "string" || isValidExplicitDest(dest)) {
       this.goToDestination(dest);
       return;
     }
@@ -486,60 +487,6 @@ class PDFLinkService {
       optionalContentConfig
     );
   }
-
-  static #isValidExplicitDest(dest) {
-    if (!Array.isArray(dest) || dest.length < 2) {
-      return false;
-    }
-    const [page, zoom, ...args] = dest;
-    if (
-      !(
-        typeof page === "object" &&
-        Number.isInteger(page?.num) &&
-        Number.isInteger(page?.gen)
-      ) &&
-      !Number.isInteger(page)
-    ) {
-      return false;
-    }
-    if (!(typeof zoom === "object" && typeof zoom?.name === "string")) {
-      return false;
-    }
-    const argsLen = args.length;
-    let allowNull = true;
-    switch (zoom.name) {
-      case "XYZ":
-        if (argsLen < 2 || argsLen > 3) {
-          return false;
-        }
-        break;
-      case "Fit":
-      case "FitB":
-        return argsLen === 0;
-      case "FitH":
-      case "FitBH":
-      case "FitV":
-      case "FitBV":
-        if (argsLen > 1) {
-          return false;
-        }
-        break;
-      case "FitR":
-        if (argsLen !== 4) {
-          return false;
-        }
-        allowNull = false;
-        break;
-      default:
-        return false;
-    }
-    for (const arg of args) {
-      if (!(typeof arg === "number" || (allowNull && arg === null))) {
-        return false;
-      }
-    }
-    return true;
-  }
 }
 
 /**

diff --git a/web/pdfjs.js b/web/pdfjs.js
@@ -39,6 +39,7 @@ const {
   InvalidPDFException,
   isDataScheme,
   isPdfFile,
+  isValidExplicitDest,
   noContextMenu,
   normalizeUnicode,
   OPS,
@@ -90,6 +91,7 @@ export {
   InvalidPDFException,
   isDataScheme,
   isPdfFile,
+  isValidExplicitDest,
   noContextMenu,
   normalizeUnicode,
   OPS,