Skip to content

Commit

Permalink
Unwrap opaqueredirect fetch responses, closes #133
Browse files Browse the repository at this point in the history
Also sets the `redirect` mode of incoming requests to `manual`:
https://developers.cloudflare.com/workers/runtime-apis/request#requestinit

This ensures redirects are proxied to the end user, meaning cookies
set in responses are stored in the browser.

Co-Authored-By: Henrique Sarmento <hnrqer@gmail.com>
Co-Authored-By: Vinicius Zaramella <vinicius.zaramella@gmail.com>
  • Loading branch information
3 people committed Jan 7, 2022
1 parent be3013a commit 1bf7600
Show file tree
Hide file tree
Showing 4 changed files with 79 additions and 4 deletions.
27 changes: 25 additions & 2 deletions packages/core/src/standards/http.ts
Original file line number Diff line number Diff line change
Expand Up @@ -571,7 +571,9 @@ export class Response<
return this[_kInner].statusText;
}
get type(): ResponseType {
return this[_kInner].type;
throw new Error(
"Failed to get the 'type' property on 'Response': the property is not implemented."
);
}
get url(): string {
return this[_kInner].url;
Expand Down Expand Up @@ -647,7 +649,28 @@ export async function fetch(
}

// Convert the response to our hybrid Response
const res = new Response(baseRes.body, baseRes);
let res: Response;
if (baseRes.type === "opaqueredirect") {
// Unpack opaque responses. This restriction isn't needed server-side,
// and Cloudflare doesn't support Response types anyway.
// @ts-expect-error symbol properties are not included type definitions
const internalResponse = baseRes[fetchSymbols.kState].internalResponse;
const headersList = internalResponse.headersList;
assert(headersList.length % 2 === 0);
const headers = new Headers();
for (let i = 0; i < headersList.length; i += 2) {
headers.append(headersList[i], headersList[i + 1]);
}
// Cloudflare returns a body here, but undici aborts the stream so
// unfortunately it's unusable :(
res = new Response(null, {
status: internalResponse.status,
statusText: internalResponse.statusText,
headers,
});
} else {
res = new Response(baseRes.body, baseRes);
}

await waitForOpenInputGate();
return withInputGating(res);
Expand Down
20 changes: 18 additions & 2 deletions packages/core/test/standards/http.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -608,7 +608,6 @@ test("Response: constructing from BaseResponse doesn't create new BaseResponse u
t.is(res.status, base.status);
t.is(res.ok, base.ok);
t.is(res.statusText, base.statusText);
t.is(res.type, base.type);
t.is(res.url, base.url);
t.is(res.redirected, base.redirected);

Expand Down Expand Up @@ -800,6 +799,14 @@ test("Response: can use byob reader when cloning", async (t) => {
t.is(await byobReadFirstChunk(clone.body), "body");
t.is(await byobReadFirstChunk(res.body), "body");
});
test("Response: type throws with unimplemented error", async (t) => {
const res = new Response();
t.throws(() => res.type, {
instanceOf: Error,
message:
"Failed to get the 'type' property on 'Response': the property is not implemented.",
});
});

test("withWaitUntil: adds wait until to (Base)Response", async (t) => {
const waitUntil = [1];
Expand All @@ -820,7 +827,7 @@ function redirectingServerListener(
const { searchParams } = new URL(req.url ?? "", "http://localhost");
const n = parseInt(searchParams.get("n") ?? "0");
if (n > 0) {
res.writeHead(302, { Location: `/?n=${n - 1}` });
res.writeHead(302, { Location: `/?n=${n - 1}`, "Set-Cookie": `n=${n}` });
} else {
res.writeHead(200);
}
Expand Down Expand Up @@ -889,6 +896,15 @@ test("fetch: removes Host and CF-Connecting-IP headers from Request", async (t)
"x-real-ip": "127.0.0.1",
});
});
test('fetch: returns full Response for "manual" redirect', async (t) => {
const upstream = (await useServer(t, redirectingServerListener)).http;
const url = new URL("/?n=3", upstream);
const res = await fetch(url, { redirect: "manual" });
t.is(res.status, 302);
t.is(res.statusText, "Found");
t.is(res.headers.get("Location"), `/?n=2`);
t.is(res.headers.get("Set-Cookie"), "n=3");
});
test("fetch: waits for input gate to open before returning", async (t) => {
const upstream = (await useServer(t, (req, res) => res.end("upstream"))).http;
await waitsForInputGate(t, () => fetch(upstream));
Expand Down
3 changes: 3 additions & 0 deletions packages/http-server/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -130,6 +130,9 @@ export async function convertNodeRequest(
headers,
body,
cf: meta?.cf,
// Incoming requests always have their redirect mode set to manual:
// https://developers.cloudflare.com/workers/runtime-apis/request#requestinit
redirect: "manual",
});
return { request, url };
}
Expand Down
33 changes: 33 additions & 0 deletions packages/http-server/test/index.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ import {
triggerPromise,
useMiniflare,
useMiniflareWithHandler,
useServer,
useTmp,
utf8Encode,
} from "@miniflare/shared-test";
Expand Down Expand Up @@ -220,6 +221,10 @@ test("convertNodeRequest: includes cf object on request", async (t) => {
t.not(req.cf, cf);
t.deepEqual(req.cf, cf);
});
test('convertNodeRequest: defaults to "manual" redirect mode', async (t) => {
const [req] = await buildConvertNodeRequest(t);
t.is(req.redirect, "manual");
});

test("createRequestListener: gets cf object from custom provider", async (t) => {
const mf = useMiniflareWithHandler(
Expand Down Expand Up @@ -705,3 +710,31 @@ test("createServer: handles https requests", async (t) => {
const [body] = await request(port, "", {}, true);
t.is(body, `body:https://localhost:${port}/`);
});
test("createServer: proxies redirect responses", async (t) => {
// https://github.com/cloudflare/miniflare/issues/133
const upstream = await useServer(t, async (req, res) => {
const { pathname } = new URL(req.url ?? "", "http://localhost");
if (pathname === "/redirect") {
t.is(await text(req), "body");
res.writeHead(302, { Location: `/`, "Set-Cookie": `key=value` });
} else {
t.fail();
}
res.end();
});
const mf = useMiniflareWithHandler(
{ HTTPPlugin, WebSocketPlugin },
{ upstream: upstream.http.toString() },
(globals, req) => globals.fetch(req)
);
const port = await listen(t, await createServer(mf));

const res = await new Promise<http.IncomingMessage>((resolve) =>
http
.request({ port, method: "POST", path: "/redirect" }, resolve)
.end("body")
);
t.is(res.statusCode, 302);
t.is(res.headers.location, `/`);
t.deepEqual(res.headers["set-cookie"], ["key=value"]);
});

0 comments on commit 1bf7600

Please sign in to comment.