Skip to content

Commit

Permalink
Allow GET requests with bodies in entry worker (#677)
Browse files Browse the repository at this point in the history
Whilst prohibited by the `Request` API spec, `GET` requests are
allowed to have bodies. If `Content-Length` or `Transfer-Encoding`
are specified, `workerd` will give the request a (potentially empty)
body. Passing a bodied-GET-request through to the `new Request()`
constructor should throw, but `workerd` has special handling to allow
this if a `Request` instance is passed.

Miniflare was previously decomposing the request before passing it
back to the `new Request()` constructor, defeating this detection.
This change ensures we always pass full `Request` instances to the
`new Request()` constructor in the entry worker.

Closes cloudflare/workerd#1122
  • Loading branch information
mrbbot authored Sep 6, 2023
1 parent 6ec2eaa commit d1f1ef7
Show file tree
Hide file tree
Showing 2 changed files with 55 additions and 7 deletions.
19 changes: 12 additions & 7 deletions packages/miniflare/src/workers/core/entry.worker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -46,13 +46,18 @@ function getUserRequest(
url = new URL(path, upstreamUrl);
}

request = new Request(url, {
method: request.method,
headers: request.headers,
cf: request.cf ?? env[CoreBindings.JSON_CF_BLOB],
redirect: request.redirect,
body: request.body,
});
// Note when constructing new `Request`s from `request`, we must always pass
// `request` as is to the `new Request()` constructor. Whilst prohibited by
// the `Request` API spec, `GET` requests are allowed to have bodies. If
// `Content-Length` or `Transfer-Encoding` are specified, `workerd` will give
// the request a (potentially empty) body. Passing a bodied-GET-request
// through to the `new Request()` constructor should throw, but `workerd` has
// special handling to allow this if a `Request` instance is passed.
// See https://github.com/cloudflare/workerd/issues/1122 for more details.
request = new Request(url, request);
if (request.cf === undefined) {
request = new Request(request, { cf: env[CoreBindings.JSON_CF_BLOB] });
}
request.headers.delete(CoreHeaders.ORIGINAL_URL);
return request;
}
Expand Down
43 changes: 43 additions & 0 deletions packages/miniflare/test/index.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import http from "http";
import { AddressInfo } from "net";
import path from "path";
import { Writable } from "stream";
import { json } from "stream/consumers";
import util from "util";
import {
D1Database,
Expand Down Expand Up @@ -403,6 +404,48 @@ test("Miniflare: custom outbound service", async (t) => {
});
});

test("Miniflare: can send GET request with body", async (t) => {
// https://github.com/cloudflare/workerd/issues/1122
const mf = new Miniflare({
compatibilityDate: "2023-08-01",
modules: true,
script: `export default {
async fetch(request) {
return Response.json({
cf: request.cf,
contentLength: request.headers.get("Content-Length"),
hasBody: request.body !== null,
});
}
}`,
cf: { key: "value" },
});
t.teardown(() => mf.dispose());

// Can't use `dispatchFetch()` here as `fetch()` prohibits `GET` requests
// with bodies/`Content-Length: 0` headers
const url = await mf.ready;
function get(opts: http.RequestOptions = {}): Promise<http.IncomingMessage> {
return new Promise((resolve, reject) => {
http.get(url, opts, resolve).on("error", reject);
});
}

let res = await get();
t.deepEqual(await json(res), {
cf: { key: "value" },
contentLength: null,
hasBody: false,
});

res = await get({ headers: { "content-length": "0" } });
t.deepEqual(await json(res), {
cf: { key: "value" },
contentLength: "0",
hasBody: true,
});
});

test("Miniflare: fetch mocking", async (t) => {
const fetchMock = createFetchMock();
fetchMock.disableNetConnect();
Expand Down

0 comments on commit d1f1ef7

Please sign in to comment.