diff --git a/src/index.spec.ts b/src/index.spec.ts index 9e8b7a4..64e9b08 100644 --- a/src/index.spec.ts +++ b/src/index.spec.ts @@ -3,32 +3,95 @@ import { Request, Response } from "servie/dist/node"; import { redirects } from "./index"; describe("popsicle redirects", () => { - const req = new Request("http://example.com/"); - const ok = new Response(null, { status: 200 }); - - const redirect = new Response(null, { - status: 302, - headers: { - Location: "/test", - }, - }); - it("should follow 302 redirect", async () => { - let i = 0; - const spy = jest.fn(async (req: Request) => { - if (i++ === 0) return redirect.clone(); + if (spy.mock.calls.length === 1) { + return new Response(null, { + status: 302, + headers: { + Location: "/test", + }, + }); + } + expect(req.url).toEqual("http://example.com/test"); - return ok.clone(); + return new Response(null, { status: 200 }); }); const transport = redirects(spy); - const res = await transport(req.clone(), async () => { + const res = await transport(new Request("http://example.com"), async () => { throw new TypeError("Unexpected response"); }); expect(res.status).toEqual(200); expect(spy).toHaveBeenCalledTimes(2); }); + + describe("secure headers", () => { + const headers = { + cookie: "example_cookie", + authorization: "example_authorization", + }; + + it("should maintain cookies when staying with original host", async () => { + const spy = jest.fn(async (req: Request) => { + if (spy.mock.calls.length === 1) { + return new Response(null, { + status: 302, + headers: { + Location: "/test", + }, + }); + } + + expect(req.url).toEqual("http://example.com/test"); + expect(req.headers.get("Cookie")).toBe(headers.cookie); + expect(req.headers.get("Authorization")).toBe(headers.authorization); + 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(2); + }); + + it("should discard cookies when leaving 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", + }, + }); + } + + expect(req.url).toEqual("https://example.com/"); + 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(2); + }); + }); }); diff --git a/src/index.ts b/src/index.ts index 6bbaa9e..2931810 100644 --- a/src/index.ts +++ b/src/index.ts @@ -1,4 +1,4 @@ -import { resolve } from "url"; +import { URL } from "url"; import { CommonRequest, CommonResponse } from "servie/dist/common"; /** @@ -6,7 +6,7 @@ import { CommonRequest, CommonResponse } from "servie/dist/common"; */ declare module "servie/dist/signal" { export interface SignalEvents { - redirect: [string]; + redirect: [URL]; } } @@ -40,6 +40,32 @@ 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); + + request.signal.emit("redirect", newUrl); + + const newRequest = request.clone(); + newRequest.url = newUrl.toString(); + newRequest.method = method; + + // Delete cookie header when leaving the original URL. + if (originalUrl.origin !== newUrl.origin) { + newRequest.headers.delete("cookie"); + newRequest.headers.delete("authorization"); + } + + return newRequest; +} + /** * Redirect confirmation function. */ @@ -66,24 +92,17 @@ export function redirects( while (redirectCount++ < maxRedirects) { const res = await fn(req as T, done); const redirect = REDIRECT_STATUS[res.status]; + const location = res.headers.get("Location"); - if (redirect === undefined || !res.headers.has("Location")) return res; - - const newUrl = resolve(req.url, res.headers.get("Location")!); // tslint:disable-line + if (redirect === undefined || !location) return res; await res.destroy(); // Ignore the result of the response on redirect. - req.signal.emit("redirect", newUrl); - if (redirect === REDIRECT_TYPE.FOLLOW_WITH_GET) { const method = initReq.method.toUpperCase() === "HEAD" ? "HEAD" : "GET"; - req = initReq.clone(); - req.url = newUrl; - req.method = method; + req = safeRedirect(initReq, location, method); req.$rawBody = null; // Override internal raw body. - - // No body will be sent with this redirect. req.headers.set("Content-Length", "0"); continue; @@ -94,18 +113,14 @@ export function redirects( // Following HTTP spec by automatically redirecting with GET/HEAD. if (method.toUpperCase() === "GET" || method.toUpperCase() === "HEAD") { - req = initReq.clone(); - req.url = newUrl; - req.method = method; + req = safeRedirect(initReq, location, method); continue; } // Allow the user to confirm redirect according to HTTP spec. if (confirmRedirect(req, res)) { - req = initReq.clone(); - req.url = newUrl; - req.method = method; + req = safeRedirect(initReq, location, method); continue; }