Repository navigation
feat: add multipart range requests #2478
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,5 @@ | ||
| --- | ||
| "webpack-dev-middleware": minor | ||
| --- | ||
|
|
||
| Added support for `multipart/byteranges` responses. A `Range` header with several separate ranges now returns a `206` with one part per range instead of the whole file. Requests with more than 200 ranges, or ranges that cover the whole file, still get a normal `200`. | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -3,6 +3,7 @@ const querystring = require("node:querystring"); | |
| const { finished } = require("node:stream"); | ||
|
|
||
| const { | ||
| createMultipartBody, | ||
| createReadStreamOrReadFile, | ||
| destroyStream, | ||
| escapeHtml, | ||
|
|
@@ -337,6 +338,8 @@ async function getFilenameFromUrl(context, url) { | |
|
|
||
| const CACHE_CONTROL_NO_CACHE_REGEXP = /(?:^|,)\s*?no-cache\s*?(?:,|$)/; | ||
|
|
||
| const MAX_MULTIPART_RANGES = 200; | ||
|
|
||
| /** @type {Record<number, string>} */ | ||
| const statuses = { | ||
| 400: "Bad Request", | ||
|
|
@@ -917,7 +920,7 @@ function wrapper(context) { | |
| /** @type {number} */ | ||
| let end; | ||
|
|
||
| /** @type {undefined | Buffer | ReadStream} */ | ||
| /** @type {undefined | Buffer | ReadStream | import("node:stream").Readable} */ | ||
| let bufferOrStream; | ||
| /** @type {number | undefined} */ | ||
| let byteLength; | ||
|
|
@@ -1060,9 +1063,45 @@ function wrapper(context) { | |
| "A malformed 'Range' header was provided. A regular response will be sent for this request.", | ||
| ); | ||
| } else if (parsedRanges.length > 1) { | ||
| context.logger.error( | ||
| "A 'Range' header with multiple ranges was provided. Multiple ranges are not supported, so a regular response will be sent for this request.", | ||
| const ranges = /** @type {import("range-parser").Ranges} */ ( | ||
| parsedRanges | ||
| ); | ||
|
|
||
| let shouldUseMultipart = true; | ||
|
|
||
| if (ranges.length > MAX_MULTIPART_RANGES) { | ||
| shouldUseMultipart = false; | ||
| } else { | ||
| let covered = 0; | ||
| for (const range of ranges) { | ||
| covered += range.end - range.start + 1; | ||
| } | ||
| if (covered >= size) { | ||
| shouldUseMultipart = false; | ||
| } | ||
| } | ||
|
|
||
| if (shouldUseMultipart) { | ||
| const contentType = context.mimeTypes.contentType(filename); | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win Use the selected response type for each part. If |
||
| const multipartResult = createMultipartBody({ | ||
| filename, | ||
| outputFileSystem: extra.outputFileSystem, | ||
| ranges, | ||
| size, | ||
| contentType, | ||
| }); | ||
| setStatusCode(res, 206); | ||
| setResponseHeader( | ||
| res, | ||
| "Content-Type", | ||
| `multipart/byteranges; boundary=${multipartResult.boundary}`, | ||
| ); | ||
| isPartialContent = true; | ||
| bufferOrStream = /** @type {any} */ ( | ||
| multipartResult.bufferOrStream | ||
| ); | ||
| byteLength = multipartResult.byteLength; | ||
| } | ||
| } | ||
|
|
||
| if (parsedRanges !== -2 && parsedRanges.length === 1) { | ||
|
|
@@ -1106,7 +1145,7 @@ function wrapper(context) { | |
| context.options.modifyResponseData( | ||
| req, | ||
| res, | ||
| bufferOrStream, | ||
| /** @type {any} */ (bufferOrStream), | ||
| /** @type {number} */ | ||
| (byteLength), | ||
| )); | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -11,6 +11,7 @@ | |
|
|
||
| const crypto = require("node:crypto"); | ||
| const path = require("node:path"); | ||
| const { Readable } = require("node:stream"); | ||
|
|
||
| /** @typedef {import("./index").IncomingMessage} IncomingMessage */ | ||
| /** @typedef {import("./index").ServerResponse} ServerResponse */ | ||
|
|
@@ -548,6 +549,100 @@ function createReadStreamOrReadFile(filename, outputFileSystem, start, end) { | |
| return { bufferOrStream, byteLength }; | ||
| } | ||
|
|
||
| /** | ||
| * @param {object} options options | ||
| * @param {string} options.filename filename | ||
| * @param {OutputFileSystem} options.outputFileSystem output file system | ||
| * @param {import("range-parser").Range[]} options.ranges ranges | ||
| * @param {number} options.size size | ||
| * @param {string | false} options.contentType content type | ||
| * @returns {{ boundary: string, byteLength: number, bufferOrStream: Readable }} result | ||
| */ | ||
| function createMultipartBody({ | ||
| filename, | ||
| outputFileSystem, | ||
| ranges, | ||
| size, | ||
| contentType, | ||
| }) { | ||
| const boundary = crypto.randomBytes(12).toString("hex"); | ||
|
|
||
| /** @type {{ start: number, end: number, head: Buffer }[]} */ | ||
| const parts = ranges.map((r) => ({ | ||
| start: r.start, | ||
| end: r.end, | ||
| head: Buffer.from( | ||
| `\r\n--${boundary}\r\n` + | ||
| (contentType ? `Content-Type: ${contentType}\r\n` : "") + | ||
| `Content-Range: bytes ${r.start}-${r.end}/${size}\r\n\r\n`, | ||
| ), | ||
| })); | ||
|
|
||
| const tail = Buffer.from(`\r\n--${boundary}--\r\n`); | ||
|
|
||
| const byteLength = | ||
| parts.reduce((n, p) => n + p.head.length + (p.end - p.start + 1), 0) + | ||
| tail.length; | ||
|
|
||
| const isFsSupportsStream = | ||
| typeof outputFileSystem.createReadStream === "function"; | ||
|
|
||
| /** @type {Buffer | undefined} */ | ||
| let cachedFullBuffer; | ||
| if (!isFsSupportsStream) { | ||
| cachedFullBuffer = /** @type {Buffer} */ ( | ||
| outputFileSystem.readFileSync(filename) | ||
| ); | ||
|
Comment on lines
+593
to
+595
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🩺 Stability & Availability | 🟠 Major | ⚡ Quick win Handle synchronous read failures before creating the response. If a file disappears after the middleware stats it, |
||
| } | ||
|
|
||
| async function* body() { | ||
| for (const p of parts) { | ||
| yield p.head; | ||
|
|
||
| if (isFsSupportsStream) { | ||
| const { bufferOrStream } = createReadStreamOrReadFile( | ||
| filename, | ||
| outputFileSystem, | ||
| p.start, | ||
| p.end, | ||
| ); | ||
|
|
||
| if ( | ||
| typeof ( | ||
| /** @type {import("fs").ReadStream} */ (bufferOrStream).pipe | ||
| ) === "function" | ||
| ) { | ||
| try { | ||
| yield* /** @type {import("fs").ReadStream} */ (bufferOrStream); | ||
| } finally { | ||
| destroyStream( | ||
| /** @type {import("fs").ReadStream} */ (bufferOrStream), | ||
| true, | ||
| ); | ||
| } | ||
| } else { | ||
| yield /** @type {Buffer} */ (bufferOrStream).subarray( | ||
| p.start, | ||
| p.end + 1, | ||
| ); | ||
| } | ||
| } else { | ||
| yield /** @type {Buffer} */ (cachedFullBuffer).subarray( | ||
| p.start, | ||
| p.end + 1, | ||
| ); | ||
| } | ||
| } | ||
| yield tail; | ||
| } | ||
|
|
||
| return { | ||
| boundary, | ||
| byteLength, | ||
| bufferOrStream: Readable.from(body()), | ||
| }; | ||
| } | ||
|
|
||
| /** | ||
| * @param {import("fs").ReadStream} stream stream | ||
| * @param {boolean} suppress do need suppress? | ||
|
|
@@ -1781,6 +1876,7 @@ module.exports = { | |
| applyCors, | ||
| clientQuery, | ||
| createMimeTypes, | ||
| createMultipartBody, | ||
| createReadStreamOrReadFile, | ||
| destroyStream, | ||
| escapeHtml, | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -1294,11 +1294,66 @@ describe.each([ | |
| expect(response.statusCode).toBe(200); | ||
| }); | ||
|
|
||
| it('should return the "200" code for the "GET" request with multiple range header which is ignored', async () => { | ||
| it('should return the "206" code for the "GET" request with multiple range header', async () => { | ||
| const response = await req | ||
| .get("/bundle.js") | ||
| .set("Range", "bytes=3000-3100,3200-3300"); | ||
|
|
||
| expect(response.statusCode).toBe(206); | ||
| expect(response.headers["content-type"]).toMatch( | ||
| /multipart\/byteranges; boundary=/, | ||
| ); | ||
| expect(response.headers["content-length"]).toBeDefined(); | ||
| }); | ||
|
|
||
| it("should return multipart 206 for two separate ranges", async () => { | ||
| const response = await req | ||
| .get("/bundle.js") | ||
| .set("Range", "bytes=0-9,20-29"); | ||
|
|
||
| expect(response.statusCode).toBe(206); | ||
| expect(response.headers["content-type"]).toMatch( | ||
| /multipart\/byteranges; boundary=/, | ||
| ); | ||
| expect(response.headers["content-length"]).toBeDefined(); | ||
| }); | ||
|
|
||
| it("should return 206 with multipart Content-Type for HEAD with multiple ranges", async () => { | ||
| const response = await req | ||
| .head("/bundle.js") | ||
| .set("Range", "bytes=0-9,20-29"); | ||
|
|
||
| expect(response.statusCode).toBe(206); | ||
| expect(response.headers["content-type"]).toMatch( | ||
| /multipart\/byteranges; boundary=/, | ||
| ); | ||
| }); | ||
|
|
||
| it("should return 206 with only valid range when one is satisfiable and one is unsatisfiable", async () => { | ||
| const response = await req | ||
| .get("/bundle.js") | ||
| .set("Range", "bytes=0-9,9999999-"); | ||
|
|
||
| expect(response.statusCode).toBe(206); | ||
| }); | ||
|
|
||
| it("should return 416 when all ranges are unsatisfiable", async () => { | ||
| const response = await req | ||
| .get("/bundle.js") | ||
| .set("Range", "bytes=9999999-10000000,19999999-20000000"); | ||
|
|
||
| expect(response.statusCode).toBe(416); | ||
| }); | ||
|
|
||
| it("should return 200 when more ranges than cap", async () => { | ||
| const ranges = []; | ||
| for (let i = 0; i < 201; i++) { | ||
| ranges.push(`bytes=${i * 2}-${i * 2 + 1}`); | ||
| } | ||
| const response = await req | ||
| .get("/bundle.js") | ||
| .set("Range", ranges.join(", ")); | ||
|
Comment on lines
+1350
to
+1355
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win Construct 201 valid, separate ranges for the cap test.
|
||
|
|
||
| expect(response.statusCode).toBe(200); | ||
| }); | ||
|
|
||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Correct the full-coverage status claim.
For a 10-byte file,
Range: bytes=0-4,5-9merges into one range and returns206, not200. The new full-coverage check only runs when multiple ranges remain. Remove or qualify the claim so the release note describes the implemented behavior. (raw.githubusercontent.com)