From 1e6bc4d0c33af61316becb681feae3cce35facb5 Mon Sep 17 00:00:00 2001 From: Blake Embrey Date: Tue, 8 Mar 2022 22:03:08 -0800 Subject: [PATCH] Resolve redirect URLs repeatedly --- src/index.spec.ts | 73 +++++++++++++++++++++++++++++++++++++++++++++++ src/index.ts | 47 +++++++++++++++--------------- 2 files changed, 96 insertions(+), 24 deletions(-) diff --git a/src/index.spec.ts b/src/index.spec.ts index 64e9b08..4ea42c1 100644 --- a/src/index.spec.ts +++ b/src/index.spec.ts @@ -28,6 +28,40 @@ describe("popsicle redirects", () => { expect(spy).toHaveBeenCalledTimes(2); }); + it("should resolve based on request url", async () => { + const spy = jest.fn(async (req: Request) => { + if (spy.mock.calls.length === 1) { + return new Response(null, { + status: 302, + headers: { + Location: "http://foobar.com", + }, + }); + } + + if (spy.mock.calls.length === 2) { + return new Response(null, { + status: 302, + headers: { + Location: "/test", + }, + }); + } + + expect(req.url).toEqual("http://foobar.com/test"); + return new Response(null, { status: 200 }); + }); + + const transport = redirects(spy); + + const res = await transport(new Request("http://example.com"), async () => { + throw new TypeError("Unexpected response"); + }); + + expect(res.status).toEqual(200); + expect(spy).toHaveBeenCalledTimes(3); + }); + describe("secure headers", () => { const headers = { cookie: "example_cookie", @@ -93,5 +127,44 @@ describe("popsicle redirects", () => { expect(res.status).toEqual(200); expect(spy).toHaveBeenCalledTimes(2); }); + + it("should discard cookies when off the original host", async () => { + const spy = jest.fn(async (req: Request) => { + if (spy.mock.calls.length === 1) { + return new Response(null, { + status: 302, + headers: { + Location: "https://example.com", + }, + }); + } + + if (spy.mock.calls.length === 2) { + return new Response(null, { + status: 302, + headers: { + Location: "/test", + }, + }); + } + + expect(req.url).toEqual("https://example.com/test"); + expect(req.headers.get("Cookie")).toBe(null); + expect(req.headers.get("Authorization")).toBe(null); + return new Response(null, { status: 200 }); + }); + + const transport = redirects(spy); + + const res = await transport( + new Request("http://example.com", { headers }), + async () => { + throw new TypeError("Unexpected response"); + } + ); + + expect(res.status).toEqual(200); + expect(spy).toHaveBeenCalledTimes(3); + }); }); }); diff --git a/src/index.ts b/src/index.ts index 468c835..086886b 100644 --- a/src/index.ts +++ b/src/index.ts @@ -43,28 +43,26 @@ export class MaxRedirectsError extends Error { /** * Create a new request object and tidy up any loose ends to avoid leaking info. */ -function safeRedirect( - request: CommonRequest, - location: string, - method: string -) { - const originalUrl = new URL(request.url); - const newUrl = new URL(location, originalUrl); - - const url = newUrl.toString(); - request.signal.emit("redirect", url); - - const newRequest = request.clone(); - newRequest.url = url; - newRequest.method = method; - - // Delete cookie header when leaving the original URL. - if (originalUrl.origin !== newUrl.origin) { - newRequest.headers.delete("cookie"); - newRequest.headers.delete("authorization"); - } +function safeRedirect(initReq: CommonRequest) { + const originalUrl = new URL(initReq.url); + + return (req: CommonRequest, location: string, method: string) => { + const newUrl = new URL(location, req.url); + + req.signal.emit("redirect", newUrl.toString()); + + const newRequest = initReq.clone(); + newRequest.url = newUrl.toString(); + newRequest.method = method; - return newRequest; + // Delete cookie header when leaving the original URL. + if (newUrl.origin !== originalUrl.origin) { + newRequest.headers.delete("cookie"); + newRequest.headers.delete("authorization"); + } + + return newRequest; + }; } /** @@ -87,6 +85,7 @@ export function redirects( confirmRedirect: ConfirmRedirect = () => false ): (req: T, next: () => Promise) => Promise { return async function (initReq, done) { + const safeClone = safeRedirect(initReq); let req = initReq.clone(); let redirectCount = 0; @@ -102,7 +101,7 @@ export function redirects( if (redirect === REDIRECT_TYPE.FOLLOW_WITH_GET) { const method = initReq.method.toUpperCase() === "HEAD" ? "HEAD" : "GET"; - req = safeRedirect(initReq, location, method); + req = safeClone(req, location, method); req.$rawBody = null; // Override internal raw body. req.headers.set("Content-Length", "0"); @@ -114,14 +113,14 @@ export function redirects( // Following HTTP spec by automatically redirecting with GET/HEAD. if (method.toUpperCase() === "GET" || method.toUpperCase() === "HEAD") { - req = safeRedirect(initReq, location, method); + req = safeClone(req, location, method); continue; } // Allow the user to confirm redirect according to HTTP spec. if (confirmRedirect(req, res)) { - req = safeRedirect(initReq, location, method); + req = safeClone(req, location, method); continue; }