From 1bf76000fdf08cbb1b4a86be67ea2ca36c024d11 Mon Sep 17 00:00:00 2001 From: MrBBot Date: Mon, 3 Jan 2022 19:09:02 +0000 Subject: [PATCH] Unwrap `opaqueredirect` `fetch` responses, closes #133 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 Co-Authored-By: Vinicius Zaramella --- packages/core/src/standards/http.ts | 27 +++++++++++++++++-- packages/core/test/standards/http.spec.ts | 20 ++++++++++++-- packages/http-server/src/index.ts | 3 +++ packages/http-server/test/index.spec.ts | 33 +++++++++++++++++++++++ 4 files changed, 79 insertions(+), 4 deletions(-) diff --git a/packages/core/src/standards/http.ts b/packages/core/src/standards/http.ts index d45e60993..2a6cc3eae 100644 --- a/packages/core/src/standards/http.ts +++ b/packages/core/src/standards/http.ts @@ -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; @@ -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); diff --git a/packages/core/test/standards/http.spec.ts b/packages/core/test/standards/http.spec.ts index b98ef7100..d0eb753dc 100644 --- a/packages/core/test/standards/http.spec.ts +++ b/packages/core/test/standards/http.spec.ts @@ -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); @@ -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]; @@ -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); } @@ -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)); diff --git a/packages/http-server/src/index.ts b/packages/http-server/src/index.ts index 6d9fcbba6..27ce303d8 100644 --- a/packages/http-server/src/index.ts +++ b/packages/http-server/src/index.ts @@ -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 }; } diff --git a/packages/http-server/test/index.spec.ts b/packages/http-server/test/index.spec.ts index 92037e3a1..caad77b9d 100644 --- a/packages/http-server/test/index.spec.ts +++ b/packages/http-server/test/index.spec.ts @@ -34,6 +34,7 @@ import { triggerPromise, useMiniflare, useMiniflareWithHandler, + useServer, useTmp, utf8Encode, } from "@miniflare/shared-test"; @@ -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( @@ -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((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"]); +});