Skip to content

Commit a5ce745

Browse files
JonnyBurgerJonnyBurger
committed
feat: use streaming
fix length mismatch should not close before fix content length remove headers add res.send back again Update middleware.js make all tests but 1 pass clarify that multiple ranges are not supported size -> fileSize and resolve warning clean up contentRes -> ranges minimize diff fix content length bug add assertion for response length add more assertions validate the correct content Update handleRangeHeaders.test.js.snap.webpack4 fix: return on error
1 parent d50e308 commit a5ce745

File tree

6 files changed

+69
-44
lines changed

6 files changed

+69
-44
lines changed

‎src/middleware.js

Lines changed: 25 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -48,15 +48,16 @@ export default function wrapper(context) {
4848
headers = headers(req, res, context);
4949
}
5050

51-
let content;
51+
let stream;
52+
let fileSize;
5253

5354
if (!filename) {
5455
await goNext();
5556
return;
5657
}
5758

5859
try {
59-
content = context.outputFileSystem.readFileSync(filename);
60+
fileSize = context.outputFileSystem.lstatSync(filename).size;
6061
} catch (_ignoreError) {
6162
await goNext();
6263
return;
@@ -100,21 +101,29 @@ export default function wrapper(context) {
100101
}
101102

102103
// Buffer
103-
content = handleRangeHeaders(context, content, req, res);
104-
105-
// Express API
106-
if (res.send) {
107-
res.send(content);
104+
const ranges = handleRangeHeaders(context, fileSize, req, res);
105+
try {
106+
stream = context.outputFileSystem.createReadStream(
107+
filename,
108+
ranges
109+
? {
110+
start: ranges.start,
111+
end: ranges.end,
112+
}
113+
: {}
114+
);
115+
} catch (_ignoreError) {
116+
await goNext();
117+
return;
108118
}
109-
// Node.js API
110-
else {
111-
res.setHeader("Content-Length", content.length);
112-
113-
if (req.method === "HEAD") {
114-
res.end();
115-
} else {
116-
res.end(content);
117-
}
119+
120+
const responseSize = ranges ? 1 + (ranges.end - ranges.start) : fileSize;
121+
res.setHeader("Content-Length", responseSize);
122+
123+
if (req.method === "HEAD") {
124+
res.end();
125+
} else {
126+
stream.pipe(res);
118127
}
119128
}
120129
};

‎src/utils/handleRangeHeaders.js

Lines changed: 8 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import parseRange from "range-parser";
22

3-
export default function handleRangeHeaders(context, content, req, res) {
3+
export default function handleRangeHeaders(context, size, req, res) {
44
// assumes express API. For other servers, need to add logic to access
55
// alternative header APIs
66
if (res.set) {
@@ -21,20 +21,20 @@ export default function handleRangeHeaders(context, content, req, res) {
2121
}
2222

2323
if (range) {
24-
const ranges = parseRange(content.length, range);
24+
const ranges = parseRange(size, range);
2525

2626
// unsatisfiable
2727
if (ranges === -1) {
2828
// Express API
2929
if (res.set) {
30-
res.set("Content-Range", `bytes */${content.length}`);
30+
res.set("Content-Range", `bytes */${size}`);
3131
res.status(416);
3232
}
3333
// Node.js API
3434
else {
3535
// eslint-disable-next-line no-param-reassign
3636
res.statusCode = 416;
37-
res.setHeader("Content-Range", `bytes */${content.length}`);
37+
res.setHeader("Content-Range", `bytes */${size}`);
3838
}
3939
} else if (ranges === -2) {
4040
// malformed header treated as regular response
@@ -47,16 +47,13 @@ export default function handleRangeHeaders(context, content, req, res) {
4747
"A Range header with multiple ranges was provided. Multiple ranges are not supported, so a regular response will be sent for this request."
4848
);
4949
} else {
50-
// valid range header
51-
const { length } = content;
52-
5350
// Express API
5451
if (res.set) {
5552
// Content-Range
5653
res.status(206);
5754
res.set(
5855
"Content-Range",
59-
`bytes ${ranges[0].start}-${ranges[0].end}/${length}`
56+
`bytes ${ranges[0].start}-${ranges[0].end}/${size}`
6057
);
6158
}
6259
// Node.js API
@@ -66,14 +63,13 @@ export default function handleRangeHeaders(context, content, req, res) {
6663
res.statusCode = 206;
6764
res.setHeader(
6865
"Content-Range",
69-
`bytes ${ranges[0].start}-${ranges[0].end}/${length}`
66+
`bytes ${ranges[0].start}-${ranges[0].end}/${size}`
7067
);
7168
}
7269

73-
// eslint-disable-next-line no-param-reassign
74-
content = content.slice(ranges[0].start, ranges[0].end + 1);
70+
return ranges[0];
7571
}
7672
}
7773

78-
return content;
74+
return null;
7975
}

‎test/middleware.test.js

Lines changed: 23 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -231,23 +231,43 @@ describe.each([
231231
});
232232

233233
it('should return the "206" code for the "GET" request with the valid range header', (done) => {
234+
const fileData = instance.context.outputFileSystem.readFileSync(
235+
path.resolve(outputPath, "bundle.js"),
236+
"utf8"
237+
);
234238
request(app)
235239
.get("/bundle.js")
236240
.set("Range", "bytes=3000-3500")
237241
.expect("Content-Length", "501")
238242
.expect("Content-Range", `bytes 3000-3500/${codeLength}`)
239-
.expect(206, done);
243+
.expect(206)
244+
.then((response) => {
245+
expect(response.text).toBe(fileData.substr(3000, 501));
246+
expect(response.text.length).toBe(501);
247+
done();
248+
});
240249
});
241250

242251
it('should return the "200" code for the "GET" request with malformed range header which is ignored', (done) => {
243-
request(app).get("/bundle.js").set("Range", "abc").expect(200, done);
252+
request(app)
253+
.get("/bundle.js")
254+
.set("Range", "abc")
255+
.expect(200)
256+
.then((response) => {
257+
expect(response.text.length).toBe(codeLength);
258+
done();
259+
});
244260
});
245261

246262
it('should return the "200" code for the "GET" request with multiple range header which is ignored', (done) => {
247263
request(app)
248264
.get("/bundle.js")
249265
.set("Range", "bytes=3000-3100,3200-3300")
250-
.expect(200, done);
266+
.expect(200)
267+
.then((response) => {
268+
expect(response.text.length).toBe(codeLength);
269+
done();
270+
});
251271
});
252272

253273
it('should return the "404" code for the "GET" request with to the non-public path', (done) => {

‎test/utils/__snapshots__/handleRangeHeaders.test.js.snap.webpack4

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,15 +17,15 @@ Array [
1717
]
1818
`;
1919

20-
exports[`handleRangeHeaders should handle multiple ranges 1`] = `
20+
exports[`handleRangeHeaders should handle multiple ranges by sending a regular response 1`] = `
2121
Array [
2222
Array [
2323
"A Range header with multiple ranges was provided. Multiple ranges are not supported, so a regular response will be sent for this request.",
2424
],
2525
]
2626
`;
2727

28-
exports[`handleRangeHeaders should handle multiple ranges 2`] = `
28+
exports[`handleRangeHeaders should handle multiple ranges by sending a regular response 2`] = `
2929
Array [
3030
Array [
3131
"Accept-Ranges",

‎test/utils/__snapshots__/handleRangeHeaders.test.js.snap.webpack5

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,15 +17,15 @@ Array [
1717
]
1818
`;
1919

20-
exports[`handleRangeHeaders should handle multiple ranges 1`] = `
20+
exports[`handleRangeHeaders should handle multiple ranges by sending a regular response 1`] = `
2121
Array [
2222
Array [
2323
"A Range header with multiple ranges was provided. Multiple ranges are not supported, so a regular response will be sent for this request.",
2424
],
2525
]
2626
`;
2727

28-
exports[`handleRangeHeaders should handle multiple ranges 2`] = `
28+
exports[`handleRangeHeaders should handle multiple ranges by sending a regular response 2`] = `
2929
Array [
3030
Array [
3131
"Accept-Ranges",

‎test/utils/handleRangeHeaders.test.js

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -29,8 +29,8 @@ describe("handleRangeHeaders", () => {
2929
},
3030
};
3131

32-
const contentRes = handleRangeHeaders(context, content, req, res);
33-
expect(contentRes).toEqual("bcde");
32+
const ranges = handleRangeHeaders(context, content.length, req, res);
33+
expect(ranges).toEqual({ start: 1, end: 4 });
3434
expect(res.statusCode).toEqual(206);
3535
expect(res.set.mock.calls).toMatchSnapshot();
3636
});
@@ -53,8 +53,8 @@ describe("handleRangeHeaders", () => {
5353
},
5454
};
5555

56-
const contentRes = handleRangeHeaders(context, content, req, res);
57-
expect(contentRes).toEqual("abcdef");
56+
const ranges = handleRangeHeaders(context, content, req, res);
57+
expect(ranges).toEqual(null);
5858
expect(context.logger.error.mock.calls).toMatchSnapshot();
5959
expect(res.statusCode).toBeUndefined();
6060
expect(res.set.mock.calls).toMatchSnapshot();
@@ -78,13 +78,13 @@ describe("handleRangeHeaders", () => {
7878
},
7979
};
8080

81-
const contentRes = handleRangeHeaders(context, content, req, res);
82-
expect(contentRes).toEqual("abcdef");
81+
const ranges = handleRangeHeaders(context, content.length, req, res);
82+
expect(ranges).toEqual(null);
8383
expect(res.statusCode).toEqual(416);
8484
expect(res.set.mock.calls).toMatchSnapshot();
8585
});
8686

87-
it("should handle multiple ranges", () => {
87+
it("should handle multiple ranges by sending a regular response", () => {
8888
const content = "abcdef";
8989
const req = {
9090
headers: {
@@ -102,8 +102,8 @@ describe("handleRangeHeaders", () => {
102102
},
103103
};
104104

105-
const contentRes = handleRangeHeaders(context, content, req, res);
106-
expect(contentRes).toEqual("abcdef");
105+
const ranges = handleRangeHeaders(context, content.length, req, res);
106+
expect(ranges).toEqual(null);
107107
expect(context.logger.error.mock.calls).toMatchSnapshot();
108108
expect(res.statusCode).toBeUndefined();
109109
expect(res.set.mock.calls).toMatchSnapshot();

0 commit comments

Comments
 (0)