Add an abstract BasePDFStreamRangeReader class, that all the old IPDFStreamRangeReader implementations inherit from

Given that there's no less than *five* different, but very similar, implementations this helps reduce code duplication and simplifies maintenance.

src/core/worker_stream.js

@@ -15,6 +15,7 @@
 
 import {
   BasePDFStream,
+  BasePDFStreamRangeReader,
   BasePDFStreamReader,
 } from "../shared/base_pdf_stream.js";
 
@@ -58,9 +59,11 @@ class PDFWorkerStreamReader extends BasePDFStreamReader {
   }
 }
 
-/** @implements {IPDFStreamRangeReader} */
-class PDFWorkerStreamRangeReader {
+class PDFWorkerStreamRangeReader extends BasePDFStreamRangeReader {
+  _reader = null;
+
   constructor(stream, begin, end) {
+    super(stream, begin, end);
     const { msgHandler } = stream._source;
 
     const readableStream = msgHandler.sendWithStream("GetRangeReader", {

src/display/fetch_stream.js

@@ -16,15 +16,16 @@
 import { AbortException, warn } from "../shared/util.js";
 import {
   BasePDFStream,
+  BasePDFStreamRangeReader,
   BasePDFStreamReader,
 } from "../shared/base_pdf_stream.js";
 import {
   createHeaders,
   createResponseError,
+  ensureResponseOrigin,
   extractFilenameFromHeader,
   getResponseOrigin,
   validateRangeRequestCapabilities,
-  validateResponseStatus,
 } from "./network_utils.js";
 
 if (typeof PDFJSDev !== "undefined" && PDFJSDev.test("MOZCENTRAL")) {
@@ -44,6 +45,12 @@ function fetchUrl(url, headers, withCredentials, abortController) {
   });
 }
 
+function ensureResponseStatus(status, url) {
+  if (status !== 200 && status !== 206) {
+    throw createResponseError(status, url);
+  }
+}
+
 function getArrayBuffer(val) {
   if (val instanceof Uint8Array) {
     return val.buffer;
@@ -91,9 +98,7 @@ class PDFFetchStreamReader extends BasePDFStreamReader {
       .then(response => {
         stream._responseOrigin = getResponseOrigin(response.url);
 
-        if (!validateResponseStatus(response.status)) {
-          throw createResponseError(response.status, url);
-        }
+        ensureResponseStatus(response.status, url);
         this._reader = response.body.getReader();
 
         const responseHeaders = response.headers;
@@ -144,35 +149,30 @@ class PDFFetchStreamReader extends BasePDFStreamReader {
   }
 }
 
-/** @implements {IPDFStreamRangeReader} */
-class PDFFetchStreamRangeReader {
-  constructor(stream, begin, end) {
-    this._stream = stream;
-    this._reader = null;
-    const source = stream._source;
-    this._withCredentials = source.withCredentials || false;
-    this._readCapability = Promise.withResolvers();
+class PDFFetchStreamRangeReader extends BasePDFStreamRangeReader {
+  _abortController = new AbortController();
+
+  _readCapability = Promise.withResolvers();
+
+  _reader = null;
+
+  constructor(stream, begin, end) {
+    super(stream, begin, end);
+    const { url, withCredentials } = stream._source;
 
-    this._abortController = new AbortController();
     // Always create a copy of the headers.
     const headers = new Headers(stream.headers);
     headers.append("Range", `bytes=${begin}-${end - 1}`);
 
-    const url = source.url;
-    fetchUrl(url, headers, this._withCredentials, this._abortController)
+    fetchUrl(url, headers, withCredentials, this._abortController)
       .then(response => {
         const responseOrigin = getResponseOrigin(response.url);
 
-        if (responseOrigin !== stream._responseOrigin) {
-          throw new Error(
-            `Expected range response-origin "${responseOrigin}" to match "${stream._responseOrigin}".`
-          );
-        }
-        if (!validateResponseStatus(response.status)) {
-          throw createResponseError(response.status, url);
-        }
-        this._readCapability.resolve();
+        ensureResponseOrigin(responseOrigin, stream._responseOrigin);
+        ensureResponseStatus(response.status, url);
         this._reader = response.body.getReader();
+
+        this._readCapability.resolve();
       })
       .catch(this._readCapability.reject);
   }

src/display/network.js

@@ -16,11 +16,13 @@
 import { assert, stringToBytes, warn } from "../shared/util.js";
 import {
   BasePDFStream,
+  BasePDFStreamRangeReader,
   BasePDFStreamReader,
 } from "../shared/base_pdf_stream.js";
 import {
   createHeaders,
   createResponseError,
+  ensureResponseOrigin,
   extractFilenameFromHeader,
   getResponseOrigin,
   validateRangeRequestCapabilities,
@@ -307,57 +309,59 @@ class PDFNetworkStreamReader extends BasePDFStreamReader {
   }
 }
 
-/** @implements {IPDFStreamRangeReader} */
-class PDFNetworkStreamRangeReader {
+class PDFNetworkStreamRangeReader extends BasePDFStreamRangeReader {
   onClosed = null;
 
+  _done = false;
+
+  _queuedChunk = null;
+
+  _requests = [];
+
+  _storedError = null;
+
   constructor(stream, begin, end) {
-    this._stream = stream;
+    super(stream, begin, end);
 
     this._requestXhr = stream._request({
       begin,
       end,
-      onHeadersReceived: this._onHeadersReceived.bind(this),
-      onDone: this._onDone.bind(this),
-      onError: this._onError.bind(this),
+      onHeadersReceived: this.#onHeadersReceived.bind(this),
+      onDone: this.#onDone.bind(this),
+      onError: this.#onError.bind(this),
       onProgress: null,
     });
-    this._requests = [];
-    this._queuedChunk = null;
-    this._done = false;
-    this._storedError = undefined;
   }
 
-  _onHeadersReceived() {
+  #onHeadersReceived() {
     const responseOrigin = getResponseOrigin(this._requestXhr?.responseURL);
-
-    if (responseOrigin !== this._stream._responseOrigin) {
-      this._storedError = new Error(
-        `Expected range response-origin "${responseOrigin}" to match "${this._stream._responseOrigin}".`
-      );
-      this._onError(0);
+    try {
+      ensureResponseOrigin(responseOrigin, this._stream._responseOrigin);
+    } catch (ex) {
+      this._storedError = ex;
+      this.#onError(0);
     }
   }
 
-  _onDone(chunk) {
+  #onDone(chunk) {
     if (this._requests.length > 0) {
-      const requestCapability = this._requests.shift();
-      requestCapability.resolve({ value: chunk, done: false });
+      const capability = this._requests.shift();
+      capability.resolve({ value: chunk, done: false });
     } else {
       this._queuedChunk = chunk;
     }
     this._done = true;
-    for (const requestCapability of this._requests) {
-      requestCapability.resolve({ value: undefined, done: true });
+    for (const capability of this._requests) {
+      capability.resolve({ value: undefined, done: true });
     }
     this._requests.length = 0;
     this.onClosed?.();
   }
 
-  _onError(status) {
+  #onError(status) {
     this._storedError ??= createResponseError(status, this._stream.url);
-    for (const requestCapability of this._requests) {
-      requestCapability.reject(this._storedError);
+    for (const capability of this._requests) {
+      capability.reject(this._storedError);
     }
     this._requests.length = 0;
     this._queuedChunk = null;
@@ -375,15 +379,15 @@ class PDFNetworkStreamRangeReader {
     if (this._done) {
       return { value: undefined, done: true };
     }
-    const requestCapability = Promise.withResolvers();
-    this._requests.push(requestCapability);
-    return requestCapability.promise;
+    const capability = Promise.withResolvers();
+    this._requests.push(capability);
+    return capability.promise;
   }
 
   cancel(reason) {
     this._done = true;
-    for (const requestCapability of this._requests) {
-      requestCapability.resolve({ value: undefined, done: true });
+    for (const capability of this._requests) {
+      capability.resolve({ value: undefined, done: true });
     }
     this._requests.length = 0;
 

src/display/network_utils.js

@@ -107,15 +107,19 @@ function createResponseError(status, url) {
   );
 }
 
-function validateResponseStatus(status) {
-  return status === 200 || status === 206;
+function ensureResponseOrigin(rangeOrigin, origin) {
+  if (rangeOrigin !== origin) {
+    throw new Error(
+      `Expected range response-origin "${rangeOrigin}" to match "${origin}".`
+    );
+  }
 }
 
 export {
   createHeaders,
   createResponseError,
+  ensureResponseOrigin,
   extractFilenameFromHeader,
   getResponseOrigin,
   validateRangeRequestCapabilities,
-  validateResponseStatus,
 };

src/display/node_stream.js

@@ -17,6 +17,7 @@
 import { AbortException, assert, warn } from "../shared/util.js";
 import {
   BasePDFStream,
+  BasePDFStreamRangeReader,
   BasePDFStreamReader,
 } from "../shared/base_pdf_stream.js";
 import { createResponseError } from "./network_utils.js";
@@ -66,7 +67,7 @@ function getArrayBuffer(val) {
 
 class PDFNodeStream extends BasePDFStream {
   constructor(source) {
-    super(source, PDFNodeStreamReader, PDFNodeStreamFsRangeReader);
+    super(source, PDFNodeStreamReader, PDFNodeStreamRangeReader);
     this.url = parseUrlOrPath(source.url);
     assert(
       this.url.protocol === "file:",
@@ -142,12 +143,14 @@ class PDFNodeStreamReader extends BasePDFStreamReader {
   }
 }
 
-class PDFNodeStreamFsRangeReader {
+class PDFNodeStreamRangeReader extends BasePDFStreamRangeReader {
   _readCapability = Promise.withResolvers();
 
   _reader = null;
 
   constructor(stream, begin, end) {
+    super(stream, begin, end);
+
     const url = stream.url;
     const fs = process.getBuiltinModule("fs");
     try {

src/display/transport_stream.js

@@ -13,11 +13,9 @@
  * limitations under the License.
  */
 
-// eslint-disable-next-line max-len
-/** @typedef {import("../interfaces").IPDFStreamRangeReader} IPDFStreamRangeReader */
-
 import {
   BasePDFStream,
+  BasePDFStreamRangeReader,
   BasePDFStreamReader,
 } from "../shared/base_pdf_stream.js";
 import { assert } from "../shared/util.js";
@@ -187,17 +185,20 @@ class PDFDataTransportStreamReader extends BasePDFStreamReader {
   }
 }
 
-/** @implements {IPDFStreamRangeReader} */
-class PDFDataTransportStreamRangeReader {
+class PDFDataTransportStreamRangeReader extends BasePDFStreamRangeReader {
   onDone = null;
 
+  _begin = -1;
+
+  _done = false;
+
+  _queuedChunk = null;
+
+  _requests = [];
+
   constructor(stream, begin, end) {
-    this._stream = stream;
+    super(stream, begin, end);
     this._begin = begin;
-    this._end = end;
-    this._queuedChunk = null;
-    this._requests = [];
-    this._done = false;
   }
 
   _enqueue(chunk) {
@@ -207,10 +208,11 @@ class PDFDataTransportStreamRangeReader {
     if (this._requests.length === 0) {
       this._queuedChunk = chunk;
     } else {
-      const requestsCapability = this._requests.shift();
-      requestsCapability.resolve({ value: chunk, done: false });
-      for (const requestCapability of this._requests) {
-        requestCapability.resolve({ value: undefined, done: true });
+      const firstCapability = this._requests.shift();
+      firstCapability.resolve({ value: chunk, done: false });
+
+      for (const capability of this._requests) {
+        capability.resolve({ value: undefined, done: true });
       }
       this._requests.length = 0;
     }
@@ -227,15 +229,15 @@ class PDFDataTransportStreamRangeReader {
     if (this._done) {
       return { value: undefined, done: true };
     }
-    const requestCapability = Promise.withResolvers();
-    this._requests.push(requestCapability);
-    return requestCapability.promise;
+    const capability = Promise.withResolvers();
+    this._requests.push(capability);
+    return capability.promise;
   }
 
   cancel(reason) {
     this._done = true;
-    for (const requestCapability of this._requests) {
-      requestCapability.resolve({ value: undefined, done: true });
+    for (const capability of this._requests) {
+      capability.resolve({ value: undefined, done: true });
     }
     this._requests.length = 0;
     this.onDone?.();

src/shared/base_pdf_stream.js

@@ -198,10 +198,20 @@ class BasePDFStreamReader {
 
 /**
  * Interface for a PDF binary data fragment reader.
- *
- * @interface
  */
-class IPDFStreamRangeReader {
+class BasePDFStreamRangeReader {
+  _stream = null;
+
+  constructor(stream, begin, end) {
+    if (
+      (typeof PDFJSDev === "undefined" || PDFJSDev.test("TESTING")) &&
+      this.constructor === BasePDFStreamRangeReader
+    ) {
+      unreachable("Cannot initialize BasePDFStreamRangeReader.");
+    }
+    this._stream = stream;
+  }
+
   /**
    * Requests a chunk of the binary data. The method returns the promise, which
    * is resolved into object with properties "value" and "done". If the done
@@ -210,13 +220,17 @@ class IPDFStreamRangeReader {
    * set to true.
    * @returns {Promise}
    */
-  async read() {}
+  async read() {
+    unreachable("Abstract method `read` called");
+  }
 
   /**
    * Cancels all pending read requests and closes the stream.
    * @param {Object} reason
    */
-  cancel(reason) {}
+  cancel(reason) {
+    unreachable("Abstract method `cancel` called");
+  }
 }
 
-export { BasePDFStream, BasePDFStreamReader, IPDFStreamRangeReader };
+export { BasePDFStream, BasePDFStreamRangeReader, BasePDFStreamReader };

test/unit/network_utils_spec.js

@@ -18,7 +18,6 @@ import {
   createResponseError,
   extractFilenameFromHeader,
   validateRangeRequestCapabilities,
-  validateResponseStatus,
 } from "../../src/display/network_utils.js";
 import { ResponseException } from "../../src/shared/util.js";
 
@@ -391,18 +390,4 @@ describe("network_utils", function () {
       testCreateResponseError("https://foo.com/bar.pdf", 0, false);
     });
   });
-
-  describe("validateResponseStatus", function () {
-    it("accepts valid response statuses", function () {
-      expect(validateResponseStatus(200)).toEqual(true);
-      expect(validateResponseStatus(206)).toEqual(true);
-    });
-
-    it("rejects invalid response statuses", function () {
-      expect(validateResponseStatus(302)).toEqual(false);
-      expect(validateResponseStatus(404)).toEqual(false);
-      expect(validateResponseStatus(null)).toEqual(false);
-      expect(validateResponseStatus(undefined)).toEqual(false);
-    });
-  });
 });