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 a missing/invalid "Content-Range" header is handled in PDFNetworkStream (issue 19075) #19114

Merged
merged 2 commits into from
Dec 6, 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
30 changes: 17 additions & 13 deletions src/display/network.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
* limitations under the License.
*/

import { assert, stringToBytes } from "../shared/util.js";
import { assert, stringToBytes, warn } from "../shared/util.js";
import {
createHeaders,
createResponseStatusError,
Expand Down Expand Up @@ -85,11 +85,10 @@ class NetworkManager {
}
xhr.responseType = "arraybuffer";

if (args.onError) {
xhr.onerror = function (evt) {
args.onError(xhr.status);
};
}
assert(args.onError, "Expected `onError` callback to be provided.");
xhr.onerror = () => {
args.onError(xhr.status);
};
xhr.onreadystatechange = this.onStateChange.bind(this, xhrId);
xhr.onprogress = this.onProgress.bind(this, xhrId);

Expand Down Expand Up @@ -137,7 +136,7 @@ class NetworkManager {

// Success status == 0 can be on ftp, file and other protocols.
if (xhr.status === 0 && this.isHttp) {
pendingRequest.onError?.(xhr.status);
pendingRequest.onError(xhr.status);
return;
}
const xhrStatus = xhr.status || OK_RESPONSE;
Expand All @@ -153,25 +152,30 @@ class NetworkManager {
!ok_response_on_range_request &&
xhrStatus !== pendingRequest.expectedStatus
) {
pendingRequest.onError?.(xhr.status);
pendingRequest.onError(xhr.status);
return;
}

const chunk = getArrayBuffer(xhr);
if (xhrStatus === PARTIAL_CONTENT_RESPONSE) {
const rangeHeader = xhr.getResponseHeader("Content-Range");
const matches = /bytes (\d+)-(\d+)\/(\d+)/.exec(rangeHeader);
pendingRequest.onDone({
begin: parseInt(matches[1], 10),
chunk,
});
if (matches) {
pendingRequest.onDone({
begin: parseInt(matches[1], 10),
chunk,
});
} else {
warn(`Missing or invalid "Content-Range" header.`);
pendingRequest.onError(0);
}
} else if (chunk) {
pendingRequest.onDone({
begin: 0,
chunk,
});
} else {
pendingRequest.onError?.(xhr.status);
pendingRequest.onError(xhr.status);
}
}

Expand Down
43 changes: 42 additions & 1 deletion test/unit/network_spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,10 @@
* limitations under the License.
*/

import { AbortException } from "../../src/shared/util.js";
import {
AbortException,
UnexpectedResponseException,
} from "../../src/shared/util.js";
import { PDFNetworkStream } from "../../src/display/network.js";
import { testCrossOriginRedirects } from "./common_pdfstream_tests.js";
import { TestPdfsServer } from "./test_utils.js";
Expand Down Expand Up @@ -118,6 +121,44 @@ describe("network", function () {
expect(fullReaderCancelled).toEqual(true);
});

it(`handle reading ranges with missing/invalid "Content-Range" header`, async function () {
async function readRanges(mode) {
const rangeSize = 32768;
const stream = new PDFNetworkStream({
url: `${pdf1}?test-network-break-ranges=${mode}`,
length: pdf1Length,
rangeChunkSize: rangeSize,
disableStream: true,
disableRange: false,
});

const fullReader = stream.getFullReader();

await fullReader.headersReady;
// Ensure that range requests are supported.
expect(fullReader.isRangeSupported).toEqual(true);
// We shall be able to close the full reader without issues.
fullReader.cancel(new AbortException("Don't need fullReader."));

const rangeReader = stream.getRangeReader(
pdf1Length - rangeSize,
pdf1Length
);

try {
await rangeReader.read();

// Shouldn't get here.
expect(false).toEqual(true);
} catch (ex) {
expect(ex instanceof UnexpectedResponseException).toEqual(true);
expect(ex.status).toEqual(0);
}
}

await Promise.all([readRanges("missing"), readRanges("invalid")]);
});

describe("Redirects", function () {
beforeAll(async function () {
await TestPdfsServer.ensureStarted();
Expand Down
13 changes: 12 additions & 1 deletion test/webserver.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -177,6 +177,7 @@ class WebServer {
this.#serveFileRange(
response,
localURL,
url.searchParams,
fileSize,
start,
isNaN(end) ? fileSize : end + 1
Expand Down Expand Up @@ -307,7 +308,7 @@ class WebServer {
stream.pipe(response);
}

#serveFileRange(response, fileURL, fileSize, start, end) {
#serveFileRange(response, fileURL, searchParams, fileSize, start, end) {
if (end > fileSize || start > end) {
response.writeHead(416);
response.end();
Expand All @@ -330,6 +331,16 @@ class WebServer {
"Content-Range",
`bytes ${start}-${end - 1}/${fileSize}`
);

// Support test in `test/unit/network_spec.js`.
switch (searchParams.get("test-network-break-ranges")) {
Snuffleupagus marked this conversation as resolved.
Show resolved Hide resolved
case "missing":
response.removeHeader("Content-Range");
break;
case "invalid":
response.setHeader("Content-Range", "bytes abc-def/qwerty");
break;
}
response.writeHead(206);
stream.pipe(response);
}
Expand Down
Loading