Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ensure that the response-origin of range requests match the full request (issue 12744) #19028

Merged
merged 1 commit into from
Nov 24, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions src/display/fetch_stream.js
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
25 changes: 24 additions & 1 deletion src/display/network.js
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 @@ -273,6 +276,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 @@ -370,6 +377,8 @@ class PDFNetworkStreamFullRequestReader {
}

async read() {
await this._headersCapability.promise;

if (this._storedError) {
throw this._storedError;
}
Expand Down Expand Up @@ -405,6 +414,7 @@ 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),
Expand All @@ -420,6 +430,19 @@ class PDFNetworkStreamRangeRequestReader {
this.onClosed = null;
}

_onHeadersReceived() {
const responseOrigin = getResponseOrigin(
this._manager.getRequestXhr(this._requestId)?.responseURL
);

if (responseOrigin !== this._manager._responseOrigin) {
this._storedError = new Error(
`Expected range response-origin "${responseOrigin}" to match "${this._manager._responseOrigin}".`
);
this._onError(0);
}
}

_close() {
this.onClosed?.(this);
}
Expand All @@ -441,7 +464,7 @@ class PDFNetworkStreamRangeRequestReader {
}

_onError(status) {
this._storedError = createResponseStatusError(status, this._url);
this._storedError ??= createResponseStatusError(status, this._url);
for (const requestCapability of this._requests) {
requestCapability.reject(this._storedError);
}
Expand Down
11 changes: 11 additions & 0 deletions src/display/network_utils.js
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;
Snuffleupagus marked this conversation as resolved.
Show resolved Hide resolved
}

function validateRangeRequestCapabilities({
responseHeaders,
isHttp,
Expand Down Expand Up @@ -116,6 +126,7 @@ export {
createHeaders,
createResponseStatusError,
extractFilenameFromHeader,
getResponseOrigin,
validateRangeRequestCapabilities,
validateResponseStatus,
};
4 changes: 4 additions & 0 deletions src/interfaces.js
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,10 @@ class IPDFStream {

/**
* Gets a reader for the range of the PDF data.
*
* NOTE: Currently this method is only expected to be invoked *after*
* the `IPDFStreamReader.prototype.headersReady` promise has resolved.
*
* @param {number} begin - the start offset of the data.
* @param {number} end - the end offset of the data.
* @returns {IPDFStreamRangeReader}
Expand Down
7 changes: 3 additions & 4 deletions test/unit/fetch_stream_spec.js
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
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