Merge pull request #19866 from Snuffleupagus/updateUrlHash

Avoid (most) string parsing when removing/replacing the hash property of a URL

Author: timvandermeij

extensions/chromium/extension-router.js

@@ -46,6 +46,7 @@ limitations under the License.
     }
     var scheme = url.slice(0, schemeIndex).toLowerCase();
     if (schemes.includes(scheme)) {
+      // NOTE: We cannot use the `updateUrlHash` function in this context.
       url = url.split("#", 1)[0];
       if (url.charAt(schemeIndex) === ":") {
         url = encodeURIComponent(url);

src/core/catalog.js

@@ -1607,6 +1607,9 @@ class Catalog {
           // NOTE: the destination is relative to the *remote* document.
           const remoteDest = fetchRemoteDest(action);
           if (remoteDest && typeof url === "string") {
+            // NOTE: We don't use the `updateUrlHash` function here, since
+            // the `createValidAbsoluteUrl` function (see below) already
+            // handles parsing and validation of the final URL.
             url = /* baseUrl = */ url.split("#", 1)[0] + "#" + remoteDest;
           }
           // The 'NewWindow' property, equal to `LinkTarget.BLANK`.

src/display/filter_factory.js

@@ -14,7 +14,7 @@
  */
 
 import { getRGB, isDataScheme, SVG_NS } from "./display_utils.js";
-import { unreachable, Util, warn } from "../shared/util.js";
+import { unreachable, updateUrlHash, Util, warn } from "../shared/util.js";
 
 class BaseFilterFactory {
   constructor() {
@@ -143,7 +143,7 @@ class DOMFilterFactory extends BaseFilterFactory {
         if (isDataScheme(url)) {
           warn('#createUrl: ignore "data:"-URL for performance reasons.');
         } else {
-          this.#baseUrl = url.split("#", 1)[0];
+          this.#baseUrl = updateUrlHash(url, "");
         }
       }
     }

src/pdf.js

@@ -40,6 +40,7 @@ import {
   PermissionFlag,
   ResponseException,
   shadow,
+  updateUrlHash,
   Util,
   VerbosityLevel,
 } from "./shared/util.js";
@@ -140,6 +141,7 @@ globalThis.pdfjsLib = {
   SupportedImageMimeTypes,
   TextLayer,
   TouchManager,
+  updateUrlHash,
   Util,
   VerbosityLevel,
   version,
@@ -193,6 +195,7 @@ export {
   SupportedImageMimeTypes,
   TextLayer,
   TouchManager,
+  updateUrlHash,
   Util,
   VerbosityLevel,
   version,

src/shared/util.js

@@ -445,6 +445,28 @@ function createValidAbsoluteUrl(url, baseUrl = null, options = null) {
   return _isValidProtocol(absoluteUrl) ? absoluteUrl : null;
 }
 
+/**
+ * Remove, or replace, the hash property of the URL.
+ *
+ * @param {URL|string} url - The absolute, or relative, URL.
+ * @param {string} hash - The hash property (use an empty string to remove it).
+ * @param {boolean} [allowRel] - Allow relative URLs.
+ * @returns {string} The resulting URL string.
+ */
+function updateUrlHash(url, hash, allowRel = false) {
+  const res = URL.parse(url);
+  if (res) {
+    res.hash = hash;
+    return res.href;
+  }
+  // Support well-formed relative URLs, necessary for `web/app.js` in GENERIC
+  // builds, by optionally falling back to string parsing.
+  if (allowRel && createValidAbsoluteUrl(url, "http://example.com")) {
+    return url.split("#", 1)[0] + `${hash ? `#${hash}` : ""}`;
+  }
+  return "";
+}
+
 function shadow(obj, prop, value, nonSerializable = false) {
   if (typeof PDFJSDev === "undefined" || PDFJSDev.test("TESTING")) {
     assert(
@@ -1319,6 +1341,7 @@ export {
   toHexUtil,
   UnknownErrorException,
   unreachable,
+  updateUrlHash,
   utf8StringToString,
   Util,
   VerbosityLevel,

test/unit/pdf_spec.js

@@ -31,6 +31,7 @@ import {
   PermissionFlag,
   ResponseException,
   shadow,
+  updateUrlHash,
   Util,
   VerbosityLevel,
 } from "../../src/shared/util.js";
@@ -117,6 +118,7 @@ const expectedAPI = Object.freeze({
   SupportedImageMimeTypes,
   TextLayer,
   TouchManager,
+  updateUrlHash,
   Util,
   VerbosityLevel,
   version,

web/app.js

@@ -57,6 +57,7 @@ import {
   shadow,
   stopEvent,
   TouchManager,
+  updateUrlHash,
   version,
 } from "pdfjs-lib";
 import { AppOptions, OptionKind } from "./app_options.js";
@@ -943,10 +944,18 @@ const PDFViewerApplication = {
 
   setTitleUsingUrl(url = "", downloadUrl = null) {
     this.url = url;
-    this.baseUrl = url.split("#", 1)[0];
+    this.baseUrl =
+      typeof PDFJSDev === "undefined" || PDFJSDev.test("GENERIC")
+        ? updateUrlHash(url, "", /* allowRel = */ true)
+        : updateUrlHash(url, "");
     if (downloadUrl) {
       this._downloadUrl =
-        downloadUrl === url ? this.baseUrl : downloadUrl.split("#", 1)[0];
+        // eslint-disable-next-line no-nested-ternary
+        downloadUrl === url
+          ? this.baseUrl
+          : typeof PDFJSDev === "undefined" || PDFJSDev.test("GENERIC")
+            ? updateUrlHash(downloadUrl, "", /* allowRel = */ true)
+            : updateUrlHash(downloadUrl, "");
     }
     if (isDataScheme(url)) {
       this._hideViewBookmark();
@@ -1309,7 +1318,7 @@ const PDFViewerApplication = {
     this.secondaryToolbar?.setPagesCount(pdfDocument.numPages);
 
     if (typeof PDFJSDev !== "undefined" && PDFJSDev.test("CHROME")) {
-      const baseUrl = location.href.split("#", 1)[0];
+      const baseUrl = updateUrlHash(location, "");
       // Ignore "data:"-URLs for performance reasons, even though it may cause
       // internal links to not work perfectly in all cases (see bug 1803050).
       this.pdfLinkService.setDocument(

web/app_options.js

@@ -393,7 +393,13 @@ const defaultOptions = {
   },
   docBaseUrl: {
     /** @type {string} */
-    value: typeof PDFJSDev === "undefined" ? document.URL.split("#", 1)[0] : "",
+    value:
+      typeof PDFJSDev === "undefined"
+        ? // NOTE: We cannot use the `updateUrlHash` function here, because of
+          // the default preferences generation (see `gulpfile.mjs`).
+          // However, the following line is *only* used in development mode.
+          document.URL.split("#", 1)[0]
+        : "",
     kind: OptionKind.API,
   },
   enableHWA: {

web/generic_scripting.js

@@ -17,7 +17,7 @@ import { getPdfFilenameFromUrl } from "pdfjs-lib";
 
 async function docProperties(pdfDocument) {
   const url = "",
-    baseUrl = url.split("#", 1)[0];
+    baseUrl = "";
   const { info, metadata, contentDispositionFilename, contentLength } =
     await pdfDocument.getMetadata();
 

web/pdf_history.js

@@ -17,6 +17,7 @@
 /** @typedef {import("./interfaces").IPDFLinkService} IPDFLinkService */
 
 import { isValidRotation, parseQueryString } from "./ui_utils.js";
+import { updateUrlHash } from "pdfjs-lib";
 import { waitOnEventOrTimeout } from "./event_utils.js";
 
 // Heuristic value used when force-resetting `this._blockHashChange`.
@@ -383,10 +384,9 @@ class PDFHistory {
 
     let newUrl;
     if (this._updateUrl && destination?.hash) {
-      const baseUrl = document.location.href.split("#", 1)[0];
-      // Prevent errors in Firefox.
-      if (!baseUrl.startsWith("file://")) {
-        newUrl = `${baseUrl}#${destination.hash}`;
+      const { href, protocol } = document.location;
+      if (protocol !== "file:") {
+        newUrl = updateUrlHash(href, destination.hash);
       }
     }
     if (shouldReplace) {