Skip to content

Commit

Permalink
Ensure that the response-origin of range requests match the full requ…
Browse files Browse the repository at this point in the history 
…est (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.
  • Loading branch information
@Snuffleupagus
Snuffleupagus committed Nov 20, 2024
1 parent c31ccc6 commit d1e27e6
Show file tree
Hide file tree
Showing 5 changed files with 65 additions and 21 deletions.
12 changes: 12 additions & 0 deletions src/display/fetch_stream.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import {
createHeaders,
createResponseStatusError,
extractFilenameFromHeader,
getResponseOrigin,
validateRangeRequestCapabilities,
validateResponseStatus,
} from "./network_utils.js";
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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);
}
Expand Down Expand Up @@ -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);
}
Expand Down
49 changes: 36 additions & 13 deletions src/display/network.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import {
createHeaders,
createResponseStatusError,
extractFilenameFromHeader,
getResponseOrigin,
validateRangeRequestCapabilities,
} from "./network_utils.js";

Expand All @@ -39,6 +40,8 @@ function getArrayBuffer(xhr) {
}

class NetworkManager {
_responseOrigin = null;

constructor({ url, httpHeaders, withCredentials }) {
this.url = url;
this.isHttp = /^https?:/i.test(url);
Expand Down Expand Up @@ -263,7 +266,6 @@ class PDFNetworkStreamFullRequestReader {
this._cachedChunks = [];
this._requests = [];
this._done = false;
this._storedError = undefined;
this._filename = null;

this.onProgress = null;
Expand All @@ -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
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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 };
Expand Down Expand Up @@ -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);
}
Expand All @@ -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) {
Expand All @@ -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;
Expand Down
11 changes: 11 additions & 0 deletions src/display/network_utils.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -116,6 +126,7 @@ export {
createHeaders,
createResponseStatusError,
extractFilenameFromHeader,
getResponseOrigin,
validateRangeRequestCapabilities,
validateResponseStatus,
};
7 changes: 3 additions & 4 deletions test/unit/fetch_stream_spec.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
});
Expand All @@ -71,7 +71,7 @@ describe("fetch_stream", function () {
});
};

await Promise.all([read(), promise]);
await read();

expect(len).toEqual(pdfLength);
expect(isStreamingSupported).toEqual(true);
Expand All @@ -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.
Expand Down Expand Up @@ -121,7 +121,6 @@ describe("fetch_stream", function () {
await Promise.all([
read(rangeReader1, result1),
read(rangeReader2, result2),
promise,
]);

expect(isStreamingSupported).toEqual(true);
Expand Down
7 changes: 3 additions & 4 deletions test/unit/network_spec.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
});
Expand All @@ -49,7 +49,7 @@ describe("network", function () {
});
};

await Promise.all([read(), promise]);
await read();

expect(len).toEqual(pdf1Length);
expect(count).toEqual(1);
Expand All @@ -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
Expand Down Expand Up @@ -107,7 +107,6 @@ describe("network", function () {
await Promise.all([
read(range1Reader, result1),
read(range2Reader, result2),
promise,
]);

expect(result1.value).toEqual(rangeSize);
Expand Down

0 comments on commit d1e27e6

Please sign in to comment.