From 4ca205bac3bec369ae88818e7271ae019b2c0cd9 Mon Sep 17 00:00:00 2001 From: Jonas Jenwald Date: Fri, 30 Jan 2026 08:02:07 +0100 Subject: [PATCH] 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 | 7 ++-- src/display/fetch_stream.js | 48 ++++++++++++------------- src/display/network.js | 64 +++++++++++++++++---------------- src/display/network_utils.js | 10 ++++-- src/display/node_stream.js | 7 ++-- src/display/transport_stream.js | 40 +++++++++++---------- src/shared/base_pdf_stream.js | 26 ++++++++++---- test/unit/network_utils_spec.js | 15 -------- 8 files changed, 116 insertions(+), 101 deletions(-) diff --git a/src/core/worker_stream.js b/src/core/worker_stream.js index becc0de6f..66db66322 100644 --- a/src/core/worker_stream.js +++ b/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", { diff --git a/src/display/fetch_stream.js b/src/display/fetch_stream.js index 3972eb595..c1b1b636d 100644 --- a/src/display/fetch_stream.js +++ b/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); } diff --git a/src/display/network.js b/src/display/network.js index 5ba4fb1af..018df2024 100644 --- a/src/display/network.js +++ b/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; diff --git a/src/display/network_utils.js b/src/display/network_utils.js index 5af942a9f..0e8a7cffc 100644 --- a/src/display/network_utils.js +++ b/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, }; diff --git a/src/display/node_stream.js b/src/display/node_stream.js index 188481f4e..35dfaefdf 100644 --- a/src/display/node_stream.js +++ b/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 { diff --git a/src/display/transport_stream.js b/src/display/transport_stream.js index 46c3cf897..57423932b 100644 --- a/src/display/transport_stream.js +++ b/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?.(); diff --git a/src/shared/base_pdf_stream.js b/src/shared/base_pdf_stream.js index c003a6640..4f3ee4dfe 100644 --- a/src/shared/base_pdf_stream.js +++ b/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 }; diff --git a/test/unit/network_utils_spec.js b/test/unit/network_utils_spec.js index 2b68c4dbf..ae84d9c27 100644 --- a/test/unit/network_utils_spec.js +++ b/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); - }); - }); });