From ee2885de725b3792190d56d9640f2e00f34fba35 Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Wed, 4 Nov 2026 10:13:08 +0000 Subject: [PATCH 1/2] fix(runtime): send c.redirect() to the given location Handler, loader, and middleware redirects no longer copy the request query onto an internal path. routing.redirects still merge that query. A named action still places its key and keeps the page query. Co-authored-by: sayhola --- packages/void/src/runtime/routing.ts | 5 +- packages/void/test/unit/compile.test.ts | 4 +- .../void/test/unit/routing-runtime.test.ts | 120 ++++++++++++++++-- 3 files changed, 114 insertions(+), 15 deletions(-) diff --git a/packages/void/src/runtime/routing.ts b/packages/void/src/runtime/routing.ts index 154eed712..581b67e4f 100644 --- a/packages/void/src/runtime/routing.ts +++ b/packages/void/src/runtime/routing.ts @@ -208,7 +208,10 @@ export function createRoutingRuntime( const redirect = context.redirect.bind(context); c.redirect = (destination, status) => { const location = toUrlString(destination); - if (location.startsWith('/') && !location.startsWith('//')) { + // A named action still places its key and keeps the page query. Every + // other code redirect sends the location it was given. routing.redirects + // merge the request query before they call this. + if (location.startsWith('/') && !location.startsWith('//') && hasActionKey(c.req.raw)) { return redirect(redirectLocation(c.req.raw, location, status), status); } return redirect(location, status); diff --git a/packages/void/test/unit/compile.test.ts b/packages/void/test/unit/compile.test.ts index 5d56eb189..3d7ae18e2 100644 --- a/packages/void/test/unit/compile.test.ts +++ b/packages/void/test/unit/compile.test.ts @@ -292,7 +292,9 @@ function installVoidWrappers(app: Hono): void { describe('c.redirect runtime wrapper', () => { it.each([ - ['/new', '/new?x=1&y=2'], + ['/new', '/new'], + ['/new?tab=1', '/new?tab=1'], + ['https://example.com/next', 'https://example.com/next'], // A protocol-relative location is cross-origin, so the request query must not leak to it. ['//partner.example.com/landing', '//partner.example.com/landing'], ])('redirects to %s with Location %s', async (destination, location) => { diff --git a/packages/void/test/unit/routing-runtime.test.ts b/packages/void/test/unit/routing-runtime.test.ts index 383432c9a..d11affb2c 100644 --- a/packages/void/test/unit/routing-runtime.test.ts +++ b/packages/void/test/unit/routing-runtime.test.ts @@ -32,6 +32,7 @@ import { createRoutingRuntime, type RoutingContext, type RoutingHeaderRule, + type RoutingRuntime, } from '../../src/runtime/routing.ts'; import type { StandardSchemaV1 } from '../../src/standard-schema.ts'; @@ -168,6 +169,107 @@ describe('typed HTTP execution', () => { expect(response.status).toBe(301); expect(response.headers.get('location')).toBe('/evil.com'); }); + describe('code redirects and routing.redirects', () => { + function redirectApp(setup: (app: Hono, runtime: RoutingRuntime) => void): Hono { + const app = new Hono(); + const runtime = createRoutingRuntime(app, { nodeTarget: true }); + app.use(runtime.contextMiddleware); + setup(app, runtime); + return app; + } + + it.each([ + ['/sign-in?redirectTo=/notes', '/notes', '/notes'], + ['/notes?x=1', '/sign-in?redirectTo=%2Fnotes', '/sign-in?redirectTo=%2Fnotes'], + ['/notes?x=1&y=2', '/notes#saved', '/notes#saved'], + ['/old?x=1&y=2', 'https://example.com/next?ok=1', 'https://example.com/next?ok=1'], + ['/old?x=1&y=2', '//partner.example.com/landing', '//partner.example.com/landing'], + ])('c.redirect from %s to %s sends %s', async (requestPath, destination, location) => { + const app = redirectApp((app) => { + app.get('/*', (c) => c.redirect(destination)); + }); + const response = await app.fetch(new Request('https://app.test' + requestPath)); + expect(response.status).toBe(302); + expect(response.headers.get('location')).toBe(location); + }); + + it('does not carry redirectTo out of a page loader', async () => { + const app = redirectApp((app) => { + app.get( + '/sign-in', + createPageGetHandler({ + componentId: 'sign-in', + render: async () => '', + assets: async () => ({ css: '', body: '', preloads: '' }), + headConfig: undefined, + renderOptions: { renderMode: 'server' }, + load: async () => ({ loader: (c: Context) => c.redirect('/notes') }), + }), + ); + }); + const response = await app.fetch(new Request('https://app.test/sign-in?redirectTo=/notes')); + expect(response.status).toBe(302); + expect(response.headers.get('location')).toBe('/notes'); + }); + + it('does not carry a single action query into c.redirect()', async () => { + const app = redirectApp((app) => { + app.post( + '/single', + createPageMutationHandler({ + componentId: 'page', + render: async () => '', + assets: async () => ({ css: '', body: '', preloads: '' }), + headConfig: undefined, + renderOptions: { renderMode: 'server' }, + load: async () => ({ action: (c: Context) => c.redirect('/done') }), + }), + ); + }); + const response = await app.fetch( + new Request('https://app.test/single?page=2', { method: 'POST' }), + ); + expect(response.status).toBe(302); + expect(response.headers.get('location')).toBe('/done'); + }); + + it('does not leak a request token through middleware', async () => { + const app = redirectApp((app) => { + app.use('/any', async (c) => c.redirect('/banned')); + app.get('/any', (c) => c.text('ok')); + }); + const response = await app.fetch(new Request('https://app.test/any?token=abc')); + expect(response.status).toBe(302); + expect(response.headers.get('location')).toBe('/banned'); + }); + + it('does not append the request query to a routeUrl redirect', async () => { + const destination = routeUrl('/typed-rewrite/:id', { + params: { id: 'a/b' }, + query: { tab: 'activity' }, + hash: 'details', + }); + const app = redirectApp((app) => { + app.get('/redirect', (c) => c.redirect(destination)); + }); + const response = await app.fetch(new Request('https://app.test/redirect?extra=1')); + expect(response.headers.get('location')).toBe('/typed-rewrite/a%2Fb?tab=activity#details'); + }); + + it.each([ + ['/old?redirectTo=/notes', '/new', '/new?redirectTo=/notes'], + ['/old?token=abc&keep=1', '/new?ref=1', '/new?ref=1&token=abc&keep=1'], + ['/old?token=abc', '/new?', '/new'], + ['/old?utm=x', '/new#faq', '/new?utm=x#faq'], + ])('a routing redirect from %s to %s sends %s', async (requestPath, destination, location) => { + const app = redirectApp((app, runtime) => { + app.use(runtime.routingMiddleware([{ source: '/old', destination, status: 302 }])); + }); + const response = await app.fetch(new Request('https://app.test' + requestPath)); + expect(response.status).toBe(302); + expect(response.headers.get('location')).toBe(location); + }); + }); it.each([true, false])('gates dynamic rewrite tracing on dev=%s', async (dev) => { const app = new Hono(); const runtime = createRoutingRuntime(app, { nodeTarget: true, dev }); @@ -687,7 +789,6 @@ describe('typed Pages and SSR execution', () => { ['/named?&delete&page=2', '/named?page=2'], ['/named?delete&delete=x&page=2', '/named?delete=x&page=2'], ['/named?delete&q=a%20b&flag', '/named?q=a%20b&flag'], - ['/single?page=2', '/done?page=2'], ])('drops only the named action key when c.redirect() carries %s', async (path, location) => { const app = new Hono(); const runtime = createRoutingRuntime(app, { nodeTarget: true }); @@ -701,13 +802,6 @@ describe('typed Pages and SSR execution', () => { namedActions: true, }), ); - app.post( - '/single', - createPageMutationHandler({ - ...pageOptions, - load: async () => ({ action: redirectTo('/done') }), - }), - ); const response = await app.fetch(new Request('https://app.test' + path, { method: 'POST' })); expect(response.headers.get('location')).toBe(location); }); @@ -783,7 +877,7 @@ describe('typed Pages and SSR execution', () => { ['PUT', '/named?delete&page=2', 'a 302 redirect', '/named?delete&page=2'], ['PUT', '/named?update&page=2', 'a 303 redirect', '/named?page=2'], ['POST', '/named?go&page=2', 'a rewrite', '/done?page=2'], - ['POST', '/named?goWithQuery&page=2', 'a rewrite with its own query', '/done?q=1'], + ['POST', '/named?goWithQuery&page=2', 'a rewrite with its own query', '/done'], ])('%s %s through %s redirects to %s', async (method, path, through, location) => { const response = await createNamedActionApp().fetch( new Request('https://app.test' + path, { @@ -807,14 +901,14 @@ describe('typed Pages and SSR execution', () => { expect(response.headers.get('location')).toBe('/maintenance?page=2'); }); - it('leaves the query alone when another route answers a catch-all page pattern', async () => { + it('sends the given location when another route answers a catch-all page pattern', async () => { const response = await createNamedActionApp('/*').fetch( new Request('https://app.test/api/x?return=/a&page=2', { method: 'POST' }), ); - expect(response.headers.get('location')).toBe('/done?return=/a&page=2'); + expect(response.headers.get('location')).toBe('/done'); }); - it('leaves the query alone when a tagged Void route answers before a catch-all page', async () => { + it('sends the given location when a tagged Void route answers before a catch-all page', async () => { const app = new Hono(); const runtime = createRoutingRuntime(app, { nodeTarget: true }); app.use(runtime.contextMiddleware); @@ -831,7 +925,7 @@ describe('typed Pages and SSR execution', () => { headers: { 'x-anon': '1' }, }), ); - expect(response.headers.get('location')).toBe('/login?code=1&state=2'); + expect(response.headers.get('location')).toBe('/login'); }); it('leaves the key out of the page URL when the action renders the page again', async () => { From 6d0e5cb32b65e083afe92261e684960b62e2f968 Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Wed, 4 Nov 2026 10:13:08 +0000 Subject: [PATCH 2/2] docs: state that c.redirect() keeps its location Config redirects still merge the request query into the destination. Co-authored-by: sayhola --- docs/guide/edge/redirects.md | 1 + docs/reference/api/rewrites.md | 4 +++- 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/docs/guide/edge/redirects.md b/docs/guide/edge/redirects.md index 9a8e14b4f..8d1307392 100644 --- a/docs/guide/edge/redirects.md +++ b/docs/guide/edge/redirects.md @@ -23,6 +23,7 @@ Define URL redirects in [`void.config.ts`](../../reference/config) using the `ro - **Destinations** can be strings (default `302`) or objects with `to` and optional `status`. Supported statuses: `301`, `302`, `303`, `307`, `308`. - `:splat` in the destination is replaced with the portion of the path matched by `*` in the source pattern. - Destination query strings are merged into the `Location` header (the destination's parameters take precedence on per-key conflict). To drop the incoming query, write an explicit reset like `?`. +- `c.redirect()` in a handler, loader, or middleware sends that location unchanged and does not append the request query. A named action still drops its action key when the browser follows with GET, and keeps the page's other parameters. See [`c.redirect()`](../../reference/api/rewrites.md#c-rewrite-destination). - When multiple rules match, the **first match wins**. Put specific patterns before catch-all patterns. - Redirects are evaluated **before** the request reaches the worker, so they short-circuit static asset serving, ISR, and SSR. diff --git a/docs/reference/api/rewrites.md b/docs/reference/api/rewrites.md index 7cfe37aeb..82a4e6664 100644 --- a/docs/reference/api/rewrites.md +++ b/docs/reference/api/rewrites.md @@ -30,7 +30,9 @@ import { routeUrl } from 'void/routes'; return c.redirect(routeUrl('/users/:id', { params: { id: 42 } })); ``` -`c.redirect()` merges request query parameters into internal destinations, with destination values taking precedence. Redirecting a [named action](../../guide/pages-routing/actions-and-forms.md#named-actions) to a GET page drops the action key: `POST /users/42?delete&tab=profile` redirects to `/users/42?tab=profile`. Redirects that preserve the request method keep the action key: `307` and `308`, or `301` and `302` after a `PUT`, `PATCH`, or `DELETE`. +`c.redirect()` sends the location it is given. The request query is not added to an internal path, a protocol-relative URL, or an absolute URL, so `c.redirect('/notes')` from `/sign-in?redirectTo=/notes` goes to `/notes`. [`routing.redirects`](../../guide/edge/redirects.md) still merge the request query into the destination. + +A [named action](../../guide/pages-routing/actions-and-forms.md#named-actions) is the exception. Redirecting it to a GET page drops the action key and keeps the page's other query parameters: `POST /users/42?delete&tab=profile` with `c.redirect('/users/42')` goes to `/users/42?tab=profile`. Redirects that preserve the request method keep the action key: `307` and `308`, or `301` and `302` after a `PUT`, `PATCH`, or `DELETE`. ```ts import { defineHandler } from 'void';