From d1e27e61e11675111332307def3110e5f1cd9be9 Mon Sep 17 00:00:00 2001 From: Jonas Jenwald Date: Sun, 10 Nov 2024 16:00:38 +0100 Subject: [PATCH] Ensure that the response-origin of range requests match the full request (issue 12744) The following cases are excluded in the patch: - The Firefox PDF Viewer, since it has been fixed on the platform side already; please see https://bugzilla.mozilla.org/show_bug.cgi?id=1683940 - The `PDFNodeStream`-implementation, used in Node.js environments, since after recent changes that code only supports `file://`-URLs. Also updates the `read`-methods, on both `PDFNetworkStreamFullRequestReader` and `PDFNetworkStreamRangeRequestReader`, to await the headers before returning any data. This is similar to the implementation in `src/display/fetch_stream.js` and removes the need to manually track errors. *Note:* The relevant unit-tests are updated to await the `headersReady` Promise before dispatching range requests, since that's consistent with the actual usage in the `src/`-folder. --- src/display/fetch_stream.js | 12 +++++++++ src/display/network.js | 49 +++++++++++++++++++++++++--------- src/display/network_utils.js | 11 ++++++++ test/unit/fetch_stream_spec.js | 7 +++-- test/unit/network_spec.js | 7 +++-- 5 files changed, 65 insertions(+), 21 deletions(-) diff --git a/src/display/fetch_stream.js b/src/display/fetch_stream.js index 1d025ce8bee685..24604e43b68df6 100644 --- a/src/display/fetch_stream.js +++ b/src/display/fetch_stream.js @@ -18,6 +18,7 @@ import { createHeaders, createResponseStatusError, extractFilenameFromHeader, + getResponseOrigin, validateRangeRequestCapabilities, validateResponseStatus, } from "./network_utils.js"; @@ -52,6 +53,8 @@ function getArrayBuffer(val) { /** @implements {IPDFStream} */ class PDFFetchStream { + _responseOrigin = null; + constructor(source) { this.source = source; this.isHttp = /^https?:/i.test(source.url); @@ -121,6 +124,8 @@ class PDFFetchStreamReader { createFetchOptions(headers, this._withCredentials, this._abortController) ) .then(response => { + stream._responseOrigin = getResponseOrigin(response.url); + if (!validateResponseStatus(response.status)) { throw createResponseStatusError(response.status, url); } @@ -217,6 +222,13 @@ class PDFFetchStreamRangeReader { createFetchOptions(headers, this._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 createResponseStatusError(response.status, url); } diff --git a/src/display/network.js b/src/display/network.js index 9bf7aec9610db5..4542e47e01bfc4 100644 --- a/src/display/network.js +++ b/src/display/network.js @@ -18,6 +18,7 @@ import { createHeaders, createResponseStatusError, extractFilenameFromHeader, + getResponseOrigin, validateRangeRequestCapabilities, } from "./network_utils.js"; @@ -39,6 +40,8 @@ function getArrayBuffer(xhr) { } class NetworkManager { + _responseOrigin = null; + constructor({ url, httpHeaders, withCredentials }) { this.url = url; this.isHttp = /^https?:/i.test(url); @@ -263,7 +266,6 @@ class PDFNetworkStreamFullRequestReader { this._cachedChunks = []; this._requests = []; this._done = false; - this._storedError = undefined; this._filename = null; this.onProgress = null; @@ -273,6 +275,10 @@ class PDFNetworkStreamFullRequestReader { const fullRequestXhrId = this._fullRequestId; const fullRequestXhr = this._manager.getRequestXhr(fullRequestXhrId); + this._manager._responseOrigin = getResponseOrigin( + fullRequestXhr.responseURL + ); + const rawResponseHeaders = fullRequestXhr.getAllResponseHeaders(); const responseHeaders = new Headers( rawResponseHeaders @@ -333,10 +339,10 @@ class PDFNetworkStreamFullRequestReader { } _onError(status) { - this._storedError = createResponseStatusError(status, this._url); - this._headersCapability.reject(this._storedError); + const error = createResponseStatusError(status, this._url); + this._headersCapability.reject(error); for (const requestCapability of this._requests) { - requestCapability.reject(this._storedError); + requestCapability.reject(error); } this._requests.length = 0; this._cachedChunks.length = 0; @@ -370,9 +376,8 @@ class PDFNetworkStreamFullRequestReader { } async read() { - if (this._storedError) { - throw this._storedError; - } + await this._headersCapability.promise; + if (this._cachedChunks.length > 0) { const chunk = this._cachedChunks.shift(); return { value: chunk, done: false }; @@ -405,21 +410,36 @@ class PDFNetworkStreamRangeRequestReader { this._manager = manager; const args = { + onHeadersReceived: this._onHeadersReceived.bind(this), onDone: this._onDone.bind(this), onError: this._onError.bind(this), onProgress: this._onProgress.bind(this), }; this._url = manager.url; this._requestId = manager.requestRange(begin, end, args); + this._readCapability = Promise.withResolvers(); this._requests = []; this._queuedChunk = null; this._done = false; - this._storedError = undefined; this.onProgress = null; this.onClosed = null; } + _onHeadersReceived() { + const responseOrigin = getResponseOrigin( + this._manager.getRequestXhr(this._requestId)?.responseURL + ); + + if (responseOrigin !== this._manager._responseOrigin) { + this._readCapability.reject( + new Error( + `Expected range response-origin "${responseOrigin}" to match "${this._manager._responseOrigin}".` + ) + ); + } + } + _close() { this.onClosed?.(this); } @@ -438,15 +458,19 @@ class PDFNetworkStreamRangeRequestReader { } this._requests.length = 0; this._close(); + + this._readCapability.resolve(); } _onError(status) { - this._storedError = createResponseStatusError(status, this._url); + const error = createResponseStatusError(status, this._url); for (const requestCapability of this._requests) { - requestCapability.reject(this._storedError); + requestCapability.reject(error); } this._requests.length = 0; this._queuedChunk = null; + + this._readCapability.reject(error); } _onProgress(evt) { @@ -460,9 +484,8 @@ class PDFNetworkStreamRangeRequestReader { } async read() { - if (this._storedError) { - throw this._storedError; - } + await this._readCapability.promise; + if (this._queuedChunk !== null) { const chunk = this._queuedChunk; this._queuedChunk = null; diff --git a/src/display/network_utils.js b/src/display/network_utils.js index 80f89588566c4b..98ba103fdba438 100644 --- a/src/display/network_utils.js +++ b/src/display/network_utils.js @@ -36,6 +36,16 @@ function createHeaders(isHttp, httpHeaders) { return headers; } +function getResponseOrigin(url) { + try { + return new URL(url).origin; + } catch { + // `new URL()` will throw on incorrect data. + } + // Notably, null is distinct from "null" string (e.g. from file:-URLs). + return null; +} + function validateRangeRequestCapabilities({ responseHeaders, isHttp, @@ -116,6 +126,7 @@ export { createHeaders, createResponseStatusError, extractFilenameFromHeader, + getResponseOrigin, validateRangeRequestCapabilities, validateResponseStatus, }; diff --git a/test/unit/fetch_stream_spec.js b/test/unit/fetch_stream_spec.js index 75bda5d2caf4fa..3fbe1efc10ef41 100644 --- a/test/unit/fetch_stream_spec.js +++ b/test/unit/fetch_stream_spec.js @@ -54,7 +54,7 @@ describe("fetch_stream", function () { const fullReader = stream.getFullReader(); let isStreamingSupported, isRangeSupported; - const promise = fullReader.headersReady.then(function () { + await fullReader.headersReady.then(function () { isStreamingSupported = fullReader.isStreamingSupported; isRangeSupported = fullReader.isRangeSupported; }); @@ -71,7 +71,7 @@ describe("fetch_stream", function () { }); }; - await Promise.all([read(), promise]); + await read(); expect(len).toEqual(pdfLength); expect(isStreamingSupported).toEqual(true); @@ -90,7 +90,7 @@ describe("fetch_stream", function () { const fullReader = stream.getFullReader(); let isStreamingSupported, isRangeSupported, fullReaderCancelled; - const promise = fullReader.headersReady.then(function () { + await fullReader.headersReady.then(function () { isStreamingSupported = fullReader.isStreamingSupported; isRangeSupported = fullReader.isRangeSupported; // We shall be able to close full reader without any issue. @@ -121,7 +121,6 @@ describe("fetch_stream", function () { await Promise.all([ read(rangeReader1, result1), read(rangeReader2, result2), - promise, ]); expect(isStreamingSupported).toEqual(true); diff --git a/test/unit/network_spec.js b/test/unit/network_spec.js index e8b4b9f4c8e1e0..9a55b4771ff0eb 100644 --- a/test/unit/network_spec.js +++ b/test/unit/network_spec.js @@ -31,7 +31,7 @@ describe("network", function () { const fullReader = stream.getFullReader(); let isStreamingSupported, isRangeSupported; - const promise = fullReader.headersReady.then(function () { + await fullReader.headersReady.then(function () { isStreamingSupported = fullReader.isStreamingSupported; isRangeSupported = fullReader.isRangeSupported; }); @@ -49,7 +49,7 @@ describe("network", function () { }); }; - await Promise.all([read(), promise]); + await read(); expect(len).toEqual(pdf1Length); expect(count).toEqual(1); @@ -72,7 +72,7 @@ describe("network", function () { const fullReader = stream.getFullReader(); let isStreamingSupported, isRangeSupported, fullReaderCancelled; - const promise = fullReader.headersReady.then(function () { + await fullReader.headersReady.then(function () { isStreamingSupported = fullReader.isStreamingSupported; isRangeSupported = fullReader.isRangeSupported; // we shall be able to close the full reader without issues @@ -107,7 +107,6 @@ describe("network", function () { await Promise.all([ read(range1Reader, result1), read(range2Reader, result2), - promise, ]); expect(result1.value).toEqual(rangeSize);