From 75cbea1e68c7cf66f2b753757f4cd1175c59ec63 Mon Sep 17 00:00:00 2001 From: Jonny Burger Date: Wed, 11 Aug 2021 11:50:31 +0200 Subject: [PATCH] feat: don't read full file if range header is present --- src/middleware.js | 37 ++++++++++--------- src/utils/handleRangeHeaders.js | 20 ++++------ test/middleware.test.js | 26 +++++++++++-- .../handleRangeHeaders.test.js.snap.webpack4 | 4 +- .../handleRangeHeaders.test.js.snap.webpack5 | 4 +- test/utils/handleRangeHeaders.test.js | 18 ++++----- 6 files changed, 64 insertions(+), 45 deletions(-) diff --git a/src/middleware.js b/src/middleware.js index 8c63c1a23..5193ea8ec 100644 --- a/src/middleware.js +++ b/src/middleware.js @@ -48,7 +48,7 @@ export default function wrapper(context) { headers = headers(req, res, context); } - let content; + let fileSize; if (!filename) { await goNext(); @@ -56,7 +56,7 @@ export default function wrapper(context) { } try { - content = context.outputFileSystem.readFileSync(filename); + fileSize = context.outputFileSystem.lstatSync(filename).size; } catch (_ignoreError) { await goNext(); return; @@ -100,21 +100,24 @@ export default function wrapper(context) { } // Buffer - content = handleRangeHeaders(context, content, req, res); - - // Express API - if (res.send) { - res.send(content); - } - // Node.js API - else { - res.setHeader("Content-Length", content.length); - - if (req.method === "HEAD") { - res.end(); - } else { - res.end(content); - } + const ranges = handleRangeHeaders(context, fileSize, req, res); + const stream = context.outputFileSystem.createReadStream( + filename, + ranges + ? { + start: ranges.start, + end: ranges.end, + } + : {} + ); + + const responseSize = ranges ? 1 + (ranges.end - ranges.start) : fileSize; + res.setHeader("Content-Length", responseSize); + + if (req.method === "HEAD") { + res.end(); + } else { + stream.pipe(res); } } }; diff --git a/src/utils/handleRangeHeaders.js b/src/utils/handleRangeHeaders.js index 06514a437..f8eb471da 100644 --- a/src/utils/handleRangeHeaders.js +++ b/src/utils/handleRangeHeaders.js @@ -1,6 +1,6 @@ import parseRange from "range-parser"; -export default function handleRangeHeaders(context, content, req, res) { +export default function handleRangeHeaders(context, size, req, res) { // assumes express API. For other servers, need to add logic to access // alternative header APIs if (res.set) { @@ -21,20 +21,20 @@ export default function handleRangeHeaders(context, content, req, res) { } if (range) { - const ranges = parseRange(content.length, range); + const ranges = parseRange(size, range); // unsatisfiable if (ranges === -1) { // Express API if (res.set) { - res.set("Content-Range", `bytes */${content.length}`); + res.set("Content-Range", `bytes */${size}`); res.status(416); } // Node.js API else { // eslint-disable-next-line no-param-reassign res.statusCode = 416; - res.setHeader("Content-Range", `bytes */${content.length}`); + res.setHeader("Content-Range", `bytes */${size}`); } } else if (ranges === -2) { // malformed header treated as regular response @@ -47,16 +47,13 @@ export default function handleRangeHeaders(context, content, req, res) { "A Range header with multiple ranges was provided. Multiple ranges are not supported, so a regular response will be sent for this request." ); } else { - // valid range header - const { length } = content; - // Express API if (res.set) { // Content-Range res.status(206); res.set( "Content-Range", - `bytes ${ranges[0].start}-${ranges[0].end}/${length}` + `bytes ${ranges[0].start}-${ranges[0].end}/${size}` ); } // Node.js API @@ -66,14 +63,13 @@ export default function handleRangeHeaders(context, content, req, res) { res.statusCode = 206; res.setHeader( "Content-Range", - `bytes ${ranges[0].start}-${ranges[0].end}/${length}` + `bytes ${ranges[0].start}-${ranges[0].end}/${size}` ); } - // eslint-disable-next-line no-param-reassign - content = content.slice(ranges[0].start, ranges[0].end + 1); + return ranges[0]; } } - return content; + return null; } diff --git a/test/middleware.test.js b/test/middleware.test.js index 024b6de34..861aa3a68 100644 --- a/test/middleware.test.js +++ b/test/middleware.test.js @@ -231,23 +231,43 @@ describe.each([ }); it('should return the "206" code for the "GET" request with the valid range header', (done) => { + const fileData = instance.context.outputFileSystem.readFileSync( + path.resolve(outputPath, "bundle.js"), + "utf8" + ); request(app) .get("/bundle.js") .set("Range", "bytes=3000-3500") .expect("Content-Length", "501") .expect("Content-Range", `bytes 3000-3500/${codeLength}`) - .expect(206, done); + .expect(206) + .then((response) => { + expect(response.text).toBe(fileData.substr(3000, 501)); + expect(response.text.length).toBe(501); + done(); + }); }); it('should return the "200" code for the "GET" request with malformed range header which is ignored', (done) => { - request(app).get("/bundle.js").set("Range", "abc").expect(200, done); + request(app) + .get("/bundle.js") + .set("Range", "abc") + .expect(200) + .then((response) => { + expect(response.text.length).toBe(codeLength); + done(); + }); }); it('should return the "200" code for the "GET" request with multiple range header which is ignored', (done) => { request(app) .get("/bundle.js") .set("Range", "bytes=3000-3100,3200-3300") - .expect(200, done); + .expect(200) + .then((response) => { + expect(response.text.length).toBe(codeLength); + done(); + }); }); it('should return the "404" code for the "GET" request with to the non-public path', (done) => { diff --git a/test/utils/__snapshots__/handleRangeHeaders.test.js.snap.webpack4 b/test/utils/__snapshots__/handleRangeHeaders.test.js.snap.webpack4 index c92ff14b0..5fdef9f2c 100644 --- a/test/utils/__snapshots__/handleRangeHeaders.test.js.snap.webpack4 +++ b/test/utils/__snapshots__/handleRangeHeaders.test.js.snap.webpack4 @@ -17,7 +17,7 @@ Array [ ] `; -exports[`handleRangeHeaders should handle multiple ranges 1`] = ` +exports[`handleRangeHeaders should handle multiple ranges by sending a regular response 1`] = ` Array [ Array [ "A Range header with multiple ranges was provided. Multiple ranges are not supported, so a regular response will be sent for this request.", @@ -25,7 +25,7 @@ Array [ ] `; -exports[`handleRangeHeaders should handle multiple ranges 2`] = ` +exports[`handleRangeHeaders should handle multiple ranges by sending a regular response 2`] = ` Array [ Array [ "Accept-Ranges", diff --git a/test/utils/__snapshots__/handleRangeHeaders.test.js.snap.webpack5 b/test/utils/__snapshots__/handleRangeHeaders.test.js.snap.webpack5 index c92ff14b0..5fdef9f2c 100644 --- a/test/utils/__snapshots__/handleRangeHeaders.test.js.snap.webpack5 +++ b/test/utils/__snapshots__/handleRangeHeaders.test.js.snap.webpack5 @@ -17,7 +17,7 @@ Array [ ] `; -exports[`handleRangeHeaders should handle multiple ranges 1`] = ` +exports[`handleRangeHeaders should handle multiple ranges by sending a regular response 1`] = ` Array [ Array [ "A Range header with multiple ranges was provided. Multiple ranges are not supported, so a regular response will be sent for this request.", @@ -25,7 +25,7 @@ Array [ ] `; -exports[`handleRangeHeaders should handle multiple ranges 2`] = ` +exports[`handleRangeHeaders should handle multiple ranges by sending a regular response 2`] = ` Array [ Array [ "Accept-Ranges", diff --git a/test/utils/handleRangeHeaders.test.js b/test/utils/handleRangeHeaders.test.js index 8aa3dcdb0..6904e5193 100644 --- a/test/utils/handleRangeHeaders.test.js +++ b/test/utils/handleRangeHeaders.test.js @@ -29,8 +29,8 @@ describe("handleRangeHeaders", () => { }, }; - const contentRes = handleRangeHeaders(context, content, req, res); - expect(contentRes).toEqual("bcde"); + const ranges = handleRangeHeaders(context, content.length, req, res); + expect(ranges).toEqual({ start: 1, end: 4 }); expect(res.statusCode).toEqual(206); expect(res.set.mock.calls).toMatchSnapshot(); }); @@ -53,8 +53,8 @@ describe("handleRangeHeaders", () => { }, }; - const contentRes = handleRangeHeaders(context, content, req, res); - expect(contentRes).toEqual("abcdef"); + const ranges = handleRangeHeaders(context, content, req, res); + expect(ranges).toEqual(null); expect(context.logger.error.mock.calls).toMatchSnapshot(); expect(res.statusCode).toBeUndefined(); expect(res.set.mock.calls).toMatchSnapshot(); @@ -78,13 +78,13 @@ describe("handleRangeHeaders", () => { }, }; - const contentRes = handleRangeHeaders(context, content, req, res); - expect(contentRes).toEqual("abcdef"); + const ranges = handleRangeHeaders(context, content.length, req, res); + expect(ranges).toEqual(null); expect(res.statusCode).toEqual(416); expect(res.set.mock.calls).toMatchSnapshot(); }); - it("should handle multiple ranges", () => { + it("should handle multiple ranges by sending a regular response", () => { const content = "abcdef"; const req = { headers: { @@ -102,8 +102,8 @@ describe("handleRangeHeaders", () => { }, }; - const contentRes = handleRangeHeaders(context, content, req, res); - expect(contentRes).toEqual("abcdef"); + const ranges = handleRangeHeaders(context, content.length, req, res); + expect(ranges).toEqual(null); expect(context.logger.error.mock.calls).toMatchSnapshot(); expect(res.statusCode).toBeUndefined(); expect(res.set.mock.calls).toMatchSnapshot();