Merge pull request #20614 from Snuffleupagus/BasePDFStream-url

Change all relevant `BasePDFStream` implementations to take an actual `URL` instance

Author: Snuffleupagus

src/display/api_utils.js

@@ -25,21 +25,26 @@ function getUrlProp(val) {
     return null; // The 'url' is unused with `PDFDataRangeTransport`.
   }
   if (val instanceof URL) {
-    return val.href;
+    return val;
   }
   if (typeof val === "string") {
     if (
       typeof PDFJSDev !== "undefined" &&
       PDFJSDev.test("GENERIC") &&
       isNodeJS
     ) {
-      return val; // Use the url as-is in Node.js environments.
+      if (/^[a-z][a-z0-9\-+.]+:/i.test(val)) {
+        return new URL(val);
+      }
+      // eslint-disable-next-line no-undef
+      const url = process.getBuiltinModule("url");
+      return new URL(url.pathToFileURL(val));
     }
 
     // The full path is required in the 'url' field.
     const url = URL.parse(val, window.location);
     if (url) {
-      return url.href;
+      return url;
     }
   }
   throw new Error(

src/display/display_utils.js

@@ -464,7 +464,7 @@ function isValidFetchUrl(url, baseUrl) {
   }
   const res = baseUrl ? URL.parse(url, baseUrl) : URL.parse(url);
   // The Fetch API only supports the http/https protocols, and not file/ftp.
-  return res?.protocol === "http:" || res?.protocol === "https:";
+  return /https?:/.test(res?.protocol ?? "");
 }
 
 /**

src/display/fetch_stream.js

@@ -13,7 +13,7 @@
  * limitations under the License.
  */
 
-import { AbortException, warn } from "../shared/util.js";
+import { AbortException, assert, warn } from "../shared/util.js";
 import {
   BasePDFStream,
   BasePDFStreamRangeReader,
@@ -67,8 +67,13 @@ class PDFFetchStream extends BasePDFStream {
 
   constructor(source) {
     super(source, PDFFetchStreamReader, PDFFetchStreamRangeReader);
-    this.isHttp = /^https?:/i.test(source.url);
-    this.headers = createHeaders(this.isHttp, source.httpHeaders);
+    const { httpHeaders, url } = source;
+
+    assert(
+      /https?:/.test(url.protocol),
+      "PDFFetchStream only supports http(s):// URLs."
+    );
+    this.headers = createHeaders(/* isHttp = */ true, httpHeaders);
   }
 }
 
@@ -106,7 +111,7 @@ class PDFFetchStreamReader extends BasePDFStreamReader {
         const { allowRangeRequests, suggestedLength } =
           validateRangeRequestCapabilities({
             responseHeaders,
-            isHttp: stream.isHttp,
+            isHttp: true,
             rangeChunkSize,
             disableRange,
           });

src/display/network.js

@@ -48,9 +48,11 @@ class PDFNetworkStream extends BasePDFStream {
 
   constructor(source) {
     super(source, PDFNetworkStreamReader, PDFNetworkStreamRangeReader);
-    this.url = source.url;
-    this.isHttp = /^https?:/i.test(this.url);
-    this.headers = createHeaders(this.isHttp, source.httpHeaders);
+    const { httpHeaders, url } = source;
+
+    this.url = url;
+    this.isHttp = /https?:/.test(url.protocol);
+    this.headers = createHeaders(this.isHttp, httpHeaders);
   }
 
   /**

src/display/network_utils.js

@@ -101,9 +101,9 @@ function extractFilenameFromHeader(responseHeaders) {
 
 function createResponseError(status, url) {
   return new ResponseException(
-    `Unexpected server response (${status}) while retrieving PDF "${url}".`,
+    `Unexpected server response (${status}) while retrieving PDF "${url.href}".`,
     status,
-    /* missing = */ status === 404 || (status === 0 && url.startsWith("file:"))
+    /* missing = */ status === 404 || (status === 0 && url.protocol === "file:")
   );
 }
 

src/display/node_stream.js

@@ -28,16 +28,6 @@ if (typeof PDFJSDev !== "undefined" && PDFJSDev.test("MOZCENTRAL")) {
   );
 }
 
-const urlRegex = /^[a-z][a-z0-9\-+.]+:/i;
-
-function parseUrlOrPath(sourceUrl) {
-  if (urlRegex.test(sourceUrl)) {
-    return new URL(sourceUrl);
-  }
-  const url = process.getBuiltinModule("url");
-  return new URL(url.pathToFileURL(sourceUrl));
-}
-
 function getReadableStream(readStream) {
   const { Readable } = process.getBuiltinModule("stream");
 
@@ -68,9 +58,10 @@ function getArrayBuffer(val) {
 class PDFNodeStream extends BasePDFStream {
   constructor(source) {
     super(source, PDFNodeStreamReader, PDFNodeStreamRangeReader);
-    this.url = parseUrlOrPath(source.url);
+    const { url } = source;
+
     assert(
-      this.url.protocol === "file:",
+      url.protocol === "file:",
       "PDFNodeStream only supports file:// URLs."
     );
   }
@@ -81,14 +72,13 @@ class PDFNodeStreamReader extends BasePDFStreamReader {
 
   constructor(stream) {
     super(stream);
-    const { disableRange, disableStream, length, rangeChunkSize } =
+    const { disableRange, disableStream, length, rangeChunkSize, url } =
       stream._source;
 
     this._contentLength = length;
     this._isStreamingSupported = !disableStream;
     this._isRangeSupported = !disableRange;
 
-    const url = stream.url;
     const fs = process.getBuiltinModule("fs");
     fs.promises
       .lstat(url)
@@ -117,7 +107,7 @@ class PDFNodeStreamReader extends BasePDFStreamReader {
       })
       .catch(error => {
         if (error.code === "ENOENT") {
-          error = createResponseError(/* status = */ 0, url.href);
+          error = createResponseError(/* status = */ 0, url);
         }
         this._headersCapability.reject(error);
       });
@@ -150,8 +140,8 @@ class PDFNodeStreamRangeReader extends BasePDFStreamRangeReader {
 
   constructor(stream, begin, end) {
     super(stream, begin, end);
+    const { url } = stream._source;
 
-    const url = stream.url;
     const fs = process.getBuiltinModule("fs");
     try {
       const readStream = fs.createReadStream(url, {

test/unit/common_pdfstream_tests.js

@@ -24,7 +24,7 @@ async function testCrossOriginRedirects({
   redirectIfRange,
   testRangeReader,
 }) {
-  const basicApiUrl = TestPdfsServer.resolveURL("basicapi.pdf").href;
+  const basicApiUrl = TestPdfsServer.resolveURL("basicapi.pdf");
   const basicApiFileLength = 105779;
 
   const rangeSize = 32768;
@@ -83,7 +83,7 @@ function getCrossOriginUrlWithRedirects(testserverUrl, redirectIfRange) {
   if (redirectIfRange) {
     url.searchParams.set("redirectIfRange", "1");
   }
-  return url.href;
+  return url;
 }
 
 export { testCrossOriginRedirects };

test/unit/fetch_stream_spec.js

@@ -20,7 +20,7 @@ import { TestPdfsServer } from "./test_utils.js";
 
 describe("fetch_stream", function () {
   function getPdfUrl() {
-    return TestPdfsServer.resolveURL("tracemonkey.pdf").href;
+    return TestPdfsServer.resolveURL("tracemonkey.pdf");
   }
   const pdfLength = 1016315;
 

test/unit/network_spec.js

@@ -19,7 +19,7 @@ import { testCrossOriginRedirects } from "./common_pdfstream_tests.js";
 import { TestPdfsServer } from "./test_utils.js";
 
 describe("network", function () {
-  const pdf1 = new URL("../pdfs/tracemonkey.pdf", window.location).href;
+  const pdf1 = new URL("../pdfs/tracemonkey.pdf", window.location);
   const pdf1Length = 1016315;
 
   it("read without stream and range", async function () {
@@ -124,9 +124,12 @@ describe("network", function () {
     }
 
     async function readRanges(mode) {
+      const pdfUrl = new URL(pdf1);
+      pdfUrl.searchParams.set("test-network-break-ranges", mode);
+
       const rangeSize = 32768;
       const stream = new PDFNetworkStream({
-        url: `${pdf1}?test-network-break-ranges=${mode}`,
+        url: pdfUrl,
         length: pdf1Length,
         rangeChunkSize: rangeSize,
         disableStream: true,

test/unit/network_utils_spec.js

@@ -372,22 +372,22 @@ describe("network_utils", function () {
 
       expect(error instanceof ResponseException).toEqual(true);
       expect(error.message).toEqual(
-        `Unexpected server response (${status}) while retrieving PDF "${url}".`
+        `Unexpected server response (${status}) while retrieving PDF "${url.href}".`
       );
       expect(error.status).toEqual(status);
       expect(error.missing).toEqual(missing);
     }
 
     it("handles missing PDF file responses", function () {
-      testCreateResponseError("https://foo.com/bar.pdf", 404, true);
+      testCreateResponseError(new URL("https://foo.com/bar.pdf"), 404, true);
 
-      testCreateResponseError("file://foo.pdf", 0, true);
+      testCreateResponseError(new URL("file://foo.pdf"), 0, true);
     });
 
     it("handles unexpected responses", function () {
-      testCreateResponseError("https://foo.com/bar.pdf", 302, false);
+      testCreateResponseError(new URL("https://foo.com/bar.pdf"), 302, false);
 
-      testCreateResponseError("https://foo.com/bar.pdf", 0, false);
+      testCreateResponseError(new URL("https://foo.com/bar.pdf"), 0, false);
     });
   });
 });