From e3508c30541018e6384ce8b373baf17d3dc52673 Mon Sep 17 00:00:00 2001 From: Marvin Hagemeister Date: Tue, 12 Mar 2024 17:36:34 +0100 Subject: [PATCH] feat: add `ctx.redirect()` helper (#2358) This PR adds a `ctx.redirect(path, status)` helper that aims to reduce the verbosity of having to manually create a `Response` obejct for a mere redirect. ```ts // Before new Response(null, { status: 307, headers: { Location: "/foo/bar" }, }) // After ctx.redirect("/foo/bar", 307) ``` If the second parameter is not passed, it will default to `302`. --- src/server/context.ts | 33 ++++++++++++++--- src/server/types.ts | 3 +- tests/fixture/fresh.gen.ts | 2 + tests/fixture/routes/redirect.tsx | 10 +++++ tests/fixture_base_path/routes/api/rewrite.ts | 10 +---- tests/main_test.ts | 37 +++++++++++++++++++ tests/server_components_test.ts | 1 + www/routes/docs/_middleware.ts | 6 +-- 8 files changed, 82 insertions(+), 20 deletions(-) create mode 100644 tests/fixture/routes/redirect.tsx diff --git a/src/server/context.ts b/src/server/context.ts index 72d1b2b4507..ddc69de4f44 100644 --- a/src/server/context.ts +++ b/src/server/context.ts @@ -114,6 +114,31 @@ export async function getServerContext(state: InternalFreshState) { ); } +function redirectTo(pathOrUrl: string = "/", status = 302): Response { + let location = pathOrUrl; + + // Disallow protocol relative URLs + if (pathOrUrl !== "/" && pathOrUrl.startsWith("/")) { + let idx = pathOrUrl.indexOf("?"); + if (idx === -1) { + idx = pathOrUrl.indexOf("#"); + } + + const pathname = idx > -1 ? pathOrUrl.slice(0, idx) : pathOrUrl; + const search = idx > -1 ? pathOrUrl.slice(idx) : ""; + + // Remove double slashes to prevent open redirect vulnerability. + location = `${pathname.replaceAll(/\/+/g, "/")}${search}`; + } + + return new Response(null, { + status, + headers: { + location, + }, + }); +} + export class ServerContext { #renderFn: RenderFunction; #plugins: Plugin[]; @@ -283,6 +308,7 @@ export class ServerContext { ctx.data = data; return await renderNotFound(req, ctx); }, + redirect: redirectTo, route: "", get pattern() { return ctx.route; @@ -609,12 +635,7 @@ export class ServerContext { if (key !== null && BUILD_ID !== key) { url.searchParams.delete(ASSET_CACHE_BUST_KEY); const location = url.pathname + url.search; - return new Response(null, { - status: 307, - headers: { - location, - }, - }); + return redirectTo(location, 307); } const headers = new Headers({ "content-type": contentType, diff --git a/src/server/types.ts b/src/server/types.ts index 492eaf87299..a284fee6156 100644 --- a/src/server/types.ts +++ b/src/server/types.ts @@ -157,7 +157,7 @@ export type PageProps> = Omit< S, T >, - "render" | "next" | "renderNotFound" + "render" | "next" | "renderNotFound" | "redirect" >; export interface StaticFile { @@ -206,6 +206,7 @@ export interface FreshContext< ) => Response | Promise; Component: ComponentType; next: () => Promise; + redirect: (path: string, statusCode?: number) => Response; } /** * Context passed to async route components. diff --git a/tests/fixture/fresh.gen.ts b/tests/fixture/fresh.gen.ts index e60cd48d357..c624dbf5789 100644 --- a/tests/fixture/fresh.gen.ts +++ b/tests/fixture/fresh.gen.ts @@ -65,6 +65,7 @@ import * as $not_found from "./routes/not_found.ts"; import * as $params from "./routes/params.tsx"; import * as $preact_boolean_attrs from "./routes/preact/boolean_attrs.tsx"; import * as $props_id_ from "./routes/props/[id].tsx"; +import * as $redirect from "./routes/redirect.tsx"; import * as $route_groups_islands_index from "./routes/route-groups-islands/index.tsx"; import * as $route_groups_bar_baz_layout from "./routes/route-groups/(bar)/(baz)/_layout.tsx"; import * as $route_groups_bar_baz_baz from "./routes/route-groups/(bar)/(baz)/baz.tsx"; @@ -186,6 +187,7 @@ const manifest = { "./routes/params.tsx": $params, "./routes/preact/boolean_attrs.tsx": $preact_boolean_attrs, "./routes/props/[id].tsx": $props_id_, + "./routes/redirect.tsx": $redirect, "./routes/route-groups-islands/index.tsx": $route_groups_islands_index, "./routes/route-groups/(bar)/(baz)/_layout.tsx": $route_groups_bar_baz_layout, diff --git a/tests/fixture/routes/redirect.tsx b/tests/fixture/routes/redirect.tsx new file mode 100644 index 00000000000..22ad353ae4d --- /dev/null +++ b/tests/fixture/routes/redirect.tsx @@ -0,0 +1,10 @@ +import { FreshContext } from "$fresh/server.ts"; + +export const handler = { + GET(_req: Request, ctx: FreshContext) { + const rawStatus = ctx.url.searchParams.get("status"); + const status = rawStatus !== null ? Number(rawStatus) : undefined; + const location = ctx.url.searchParams.get("path") ?? "/"; + return ctx.redirect(location, status); + }, +}; diff --git a/tests/fixture_base_path/routes/api/rewrite.ts b/tests/fixture_base_path/routes/api/rewrite.ts index 7873dc9f8fe..7f0fb402537 100644 --- a/tests/fixture_base_path/routes/api/rewrite.ts +++ b/tests/fixture_base_path/routes/api/rewrite.ts @@ -1,13 +1,7 @@ import { Handlers } from "$fresh/server.ts"; export const handler: Handlers = { - GET(req) { - const url = new URL(req.url); - return new Response(null, { - status: 302, - headers: { - "Location": url.origin, - }, - }); + GET(_req, ctx) { + return ctx.redirect(ctx.url.origin, 302); }, }; diff --git a/tests/main_test.ts b/tests/main_test.ts index 259b8669859..454c5a397da 100644 --- a/tests/main_test.ts +++ b/tests/main_test.ts @@ -273,6 +273,42 @@ Deno.test("no open redirect when passing double slashes", async () => { assertEquals(resp.headers.get("location"), "/evil.com"); }); +Deno.test("ctx.redirect() - relative urls", async () => { + let resp = await handler( + new Request("https://fresh.deno.dev/redirect?path=//evil.com/"), + ); + assertEquals(resp.status, 302); + assertEquals(resp.headers.get("location"), "/evil.com/"); + + resp = await handler( + new Request( + "https://fresh.deno.dev/redirect?path=//evil.com//foo&status=307", + ), + ); + assertEquals(resp.status, 307); + assertEquals(resp.headers.get("location"), "/evil.com/foo"); +}); + +Deno.test("ctx.redirect() - absolute urls", async () => { + const resp = await handler( + new Request("https://fresh.deno.dev/redirect?path=https://example.com/"), + ); + assertEquals(resp.status, 302); + assertEquals(resp.headers.get("location"), "https://example.com/"); +}); + +Deno.test("ctx.redirect() - with search and hash", async () => { + const resp = await handler( + new Request( + `https://fresh.deno.dev/redirect?path=${ + encodeURIComponent("/foo/bar?baz=123#foo") + }`, + ), + ); + assertEquals(resp.status, 302); + assertEquals(resp.headers.get("location"), "/foo/bar?baz=123#foo"); +}); + Deno.test("/failure", async () => { const resp = await handler(new Request("https://fresh.deno.dev/failure")); assert(resp); @@ -1169,6 +1205,7 @@ Deno.test("Expose config in ctx", async () => { next: "Function", render: "AsyncFunction", renderNotFound: "AsyncFunction", + redirect: "Function", localAddr: "", pattern: "/ctx_config", data: "", diff --git a/tests/server_components_test.ts b/tests/server_components_test.ts index a32cac6539a..ef8422ae2f2 100644 --- a/tests/server_components_test.ts +++ b/tests/server_components_test.ts @@ -117,6 +117,7 @@ Deno.test("passes context to server component", async () => { params: { id: "foo", }, + redirect: "Function", state: {}, isPartial: false, }, diff --git a/www/routes/docs/_middleware.ts b/www/routes/docs/_middleware.ts index ecbca4b1470..c6c48a55f67 100644 --- a/www/routes/docs/_middleware.ts +++ b/www/routes/docs/_middleware.ts @@ -12,11 +12,7 @@ export async function handler( // Redirect from old doc URLs to new ones const redirect = REDIRECTS[ctx.url.pathname]; if (redirect) { - const url = new URL(redirect, ctx.url.origin); - return new Response("", { - status: 307, - headers: { location: url.href }, - }); + return ctx.redirect(redirect, 307); } return await ctx.next();