Skip to content

Commit

Permalink
Use actual body length for Content-Length header, closes #148
Browse files Browse the repository at this point in the history
Instead of always using the `Content-Length` header set by the user
in a `Response`, use the known body length (e.g. for strings/arrays)
  • Loading branch information
mrbbot committed Jan 14, 2022
1 parent 2ff03e2 commit c010add
Show file tree
Hide file tree
Showing 5 changed files with 58 additions and 4 deletions.
13 changes: 13 additions & 0 deletions packages/core/src/standards/http.ts
Original file line number Diff line number Diff line change
Expand Up @@ -630,6 +630,19 @@ export function _getURLList(res: BaseResponse): URL[] | undefined {
return res[fetchSymbols.kState]?.urlList;
}

/** @internal */
export function _getBodyLength(
res: Response | BaseResponse
): number | undefined {
// Extract the actual body length of the Response body. Cloudflare will return
// this for the Content-Length header instead of the user specified value
// if its set. When the body is a stream, it's the user's responsibility to
// set the Content-Length header if they want to.
if (res instanceof Response) res = res[_kInner];
// @ts-expect-error symbol properties are not included in type definitions
return res[fetchSymbols.kState]?.body?.length ?? undefined; // (normalise nullish to undefined)
}

/** @internal */
export const _kLoopHeader = "MF-Loop";

Expand Down
1 change: 1 addition & 0 deletions packages/core/src/standards/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ export {
Response,
withWaitUntil,
_getURLList,
_getBodyLength,
_kLoopHeader,
fetch,
_urlFromRequestInput,
Expand Down
14 changes: 14 additions & 0 deletions packages/core/test/standards/http.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import {
IncomingRequestCfProperties,
Request,
Response,
_getBodyLength,
_getURLList,
_isByteStream,
createCompatFetch,
Expand Down Expand Up @@ -853,6 +854,19 @@ test("_getURLList: extracts URL list from Response", async (t) => {
]);
});

test("_getBodyLength: extracts actual Response Content-Length", async (t) => {
let res = new Response("body", { headers: { "Content-Length": "100" } });
t.is(_getBodyLength(res), 4);
res = new Response(new Uint8Array([1, 2, 3]));
t.is(_getBodyLength(res), 3);
res = new Response(null);
t.is(_getBodyLength(res), undefined);
res = new Response(new ReadableStream(), {
headers: { "Content-Length": "50" },
});
t.is(_getBodyLength(res), undefined);
});

test("fetch: can fetch from existing Request", async (t) => {
const upstream = (await useServer(t, (req, res) => res.end("upstream"))).http;
const req = new Request(upstream);
Expand Down
16 changes: 12 additions & 4 deletions packages/http-server/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import {
MiniflareCore,
Request,
Response,
_getBodyLength,
logResponse,
} from "@miniflare/core";
import { prefixError, randomHex } from "@miniflare/shared";
Expand Down Expand Up @@ -196,6 +197,15 @@ export function createRequestListener<Plugins extends HTTPPluginSignatures>(
}
}

// Use body's actual length instead of the Content-Length header if set,
// see https://github.com/cloudflare/miniflare/issues/148. We also might
// need to adjust this later for live reloading so hold onto it.
const contentLengthHeader = response.headers.get("Content-Length");
const contentLength =
_getBodyLength(response) ??
(contentLengthHeader === null ? null : parseInt(contentLengthHeader));
if (contentLength !== null) headers["content-length"] = contentLength;

// If a Content-Encoding is set, and the user hasn't encoded the body,
// we're responsible for doing so.
const encoders: Transform[] = [];
Expand Down Expand Up @@ -239,12 +249,10 @@ export function createRequestListener<Plugins extends HTTPPluginSignatures>(

// If Content-Length is specified, and we're live-reloading, we'll
// need to adjust it to make room for the live reload script
const contentLength = response.headers.get("content-length");
if (liveReloadEnabled && contentLength !== null) {
const length = parseInt(contentLength);
if (!isNaN(length)) {
if (!isNaN(contentLength)) {
// Append length of live reload script
headers["content-length"] = length + liveReloadScriptLength;
headers["content-length"] = contentLength + liveReloadScriptLength;
}
}

Expand Down
18 changes: 18 additions & 0 deletions packages/http-server/test/index.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -304,6 +304,24 @@ test("createRequestListener: handles http headers in response", async (t) => {
});
t.is(status, 404);
});
test("createRequestListener: uses body length instead of Content-Length header", async (t) => {
// https://github.com/cloudflare/miniflare/issues/148
const mf = useMiniflareWithHandler(
{ HTTPPlugin, BindingsPlugin },
{ globals: { t } },
(globals) => {
const res = new globals.Response("body", {
headers: { "Content-Length": "50" },
});
globals.t.is(res.headers.get("Content-Length"), "50");
return res;
}
);
const port = await listen(t, http.createServer(createRequestListener(mf)));
const [body, headers] = await request(port);
t.is(body, "body");
t.is(headers["content-length"], "4");
});
test("createRequestListener: handles scheduled event trigger over http", async (t) => {
const events: ScheduledEvent[] = [];
const mf = useMiniflare(
Expand Down

0 comments on commit c010add

Please sign in to comment.