diff --git a/packages/void/src/pages/action-key.ts b/packages/void/src/pages/action-key.ts index bdb615b95..2f2088aad 100644 --- a/packages/void/src/pages/action-key.ts +++ b/packages/void/src/pages/action-key.ts @@ -11,6 +11,16 @@ export function hasActionKey(request: Request): boolean { return actionKeyRequests.has(request); } +/** The first non-empty segment of `search` as sent: `delete` in `?delete&page=2`. */ +export function actionKeySegment(search: string): string { + let start = 1; + while (search[start] === '&') { + start++; + } + const end = search.indexOf('&', start); + return search.slice(start, end === -1 ? search.length : end); +} + /** `search` without its first non-empty segment; the rest is kept as sent. */ export function searchWithoutActionKey(search: string): string { let start = 1; diff --git a/packages/void/src/runtime/routing.ts b/packages/void/src/runtime/routing.ts index 740c8bcb3..46c143bab 100644 --- a/packages/void/src/runtime/routing.ts +++ b/packages/void/src/runtime/routing.ts @@ -8,7 +8,12 @@ import { } from '@void/edge'; import type { Context, Hono, MiddlewareHandler } from 'hono'; import type { RedirectStatusCode } from 'hono/utils/http-status'; -import { hasActionKey, markActionKey, searchWithoutActionKey } from '../pages/action-key.ts'; +import { + actionKeySegment, + hasActionKey, + markActionKey, + searchWithoutActionKey, +} from '../pages/action-key.ts'; import { VoidAssetRewriteError } from './handler.ts'; import { convertReturnValue } from './response.ts'; import { ASSET_EXTENSIONS } from './routing-assets.ts'; @@ -179,10 +184,7 @@ export function createRoutingRuntime( const redirect = c.redirect.bind(c); c.redirect = (location, status) => { if (typeof location === 'string' && location.startsWith('/') && !location.startsWith('//')) { - return redirect( - mergeQueryIntoLocation(location, redirectSearch(c.req.raw, status)), - status, - ); + return redirect(redirectLocation(c.req.raw, location, status), status); } return redirect(location, status); }; @@ -252,15 +254,66 @@ export function createRoutingRuntime( }; } -function redirectSearch(request: Request, status: RedirectStatusCode | undefined): string { - const search = new URL(request.url).search; +function redirectLocation( + request: Request, + location: string, + status: RedirectStatusCode | undefined, +): string { + const url = new URL(request.url); + const search = url.search; + if (!hasActionKey(request)) { + return mergeQueryIntoLocation(location, search); + } // The key goes only when the browser follows with a GET: after a 303, or after // a 301/302 (the default) to a POST. Other redirects send the same method and // body again, and the destination needs the key to pick the action. const followsWithGet = status === 303 || ((status === undefined || status === 301 || status === 302) && request.method === 'POST'); - return hasActionKey(request) && followsWithGet ? searchWithoutActionKey(search) : search; + if (followsWithGet) { + return mergeQueryIntoLocation(location, searchWithoutActionKey(search)); + } + const hash = location.indexOf('#'); + const destinationQuery = location.indexOf('?'); + if (destinationQuery === -1 || (hash !== -1 && hash < destinationQuery)) { + return mergeQueryIntoLocation(location, search); + } + const merged = mergeQueryIntoLocation(location, searchWithoutActionKey(search)); + // On the same page, a key the destination starts with picks the action itself. + const destinationKey = actionKeySegment( + location.slice(destinationQuery, hash === -1 ? location.length : hash), + ); + if (destinationKey && new URL(location, url).pathname === url.pathname) { + return merged; + } + // Elsewhere the key goes first again, ahead of the destination parameters; a + // value the destination gives the same key wins. A bare "?" still drops the + // rest of the request query. + const mergedHash = merged.indexOf('#'); + const base = mergedHash === -1 ? merged : merged.slice(0, mergedHash); + const fragment = mergedHash === -1 ? '' : merged.slice(mergedHash); + const q = base.indexOf('?'); + const segments = q === -1 ? [] : base.slice(q + 1).split('&'); + const key = actionKeySegment(search); + const name = queryKey(key); + const hasName = (segment: string) => segment !== '' && queryKey(segment) === name; + // Only the destination's own value replaces the key. The merge then left out + // the request's values for that name and put the destination's first. + const own = location + .slice(destinationQuery + 1, hash === -1 ? location.length : hash) + .split('&') + .some(hasName) + ? segments.findIndex(hasName) + : -1; + const first = own === -1 ? key : segments.splice(own, 1)[0]; + const rest = segments.join('&'); + return ( + base.slice(0, q === -1 ? base.length : q) + '?' + first + (rest ? '&' + rest : '') + fragment + ); +} + +function queryKey(segment: string): string | undefined { + return new URLSearchParams(segment).keys().next().value; } /** A rewrite that keeps the query keeps the action key in it too. */ diff --git a/packages/void/test/unit/routing-runtime.test.ts b/packages/void/test/unit/routing-runtime.test.ts index d4d243527..743ee4db3 100644 --- a/packages/void/test/unit/routing-runtime.test.ts +++ b/packages/void/test/unit/routing-runtime.test.ts @@ -511,6 +511,14 @@ describe('typed Pages and SSR execution', () => { delete: (c: Context) => c.redirect('/named'), update: (c: Context) => c.redirect('/named', 303), move: (c: Context) => c.redirect('/named-v2', 307), + moveTab: (c: Context) => c.redirect('/named-v2?tab=details', 307), + moveHash: (c: Context) => c.redirect('/named-v2?tab=details#top', 307), + moveReset: (c: Context) => c.redirect('/named-v2?', 307), + replace: (c: Context) => c.redirect('/named-v2?tab=details', 302), + moveKey: (c: Context) => c.redirect('/named-v2?tab=1&moveKey=x', 307), + moveSame: (c: Context) => c.redirect('/named-v2?moveSame', 307), + toArchive: (c: Context) => c.redirect('/named?archive', 307), + toArchiveHere: (c: Context) => c.redirect('/named?', 307), go: (c: Context) => c.rewrite('/api/x'), goWithQuery: (c: Context) => c.rewrite('/api/x?q=1'), save: () => undefined, @@ -525,6 +533,32 @@ describe('typed Pages and SSR execution', () => { it.each([ ['POST', '/named?delete&page=2', 'middleware', '/login?page=2'], ['POST', '/named?move&page=2', 'a 307 redirect', '/named-v2?move&page=2'], + ['POST', '/named?moveTab&page=2', 'a 307 redirect', '/named-v2?moveTab&tab=details&page=2'], + [ + 'POST', + '/named?moveTab&moveTab=secondary&page=2', + 'a 307 redirect', + '/named-v2?moveTab&tab=details&moveTab=secondary&page=2', + ], + ['PUT', '/named?replace&page=2', 'a 302 redirect', '/named-v2?replace&tab=details&page=2'], + [ + 'PUT', + '/named?&replace=1&page=2', + 'a 302 redirect', + '/named-v2?replace=1&tab=details&page=2', + ], + ['POST', '/named?replace&page=2', 'a 302 redirect', '/named-v2?tab=details&page=2'], + [ + 'POST', + '/named?moveHash&page=2', + 'a 307 redirect', + '/named-v2?moveHash&tab=details&page=2#top', + ], + ['POST', '/named?moveReset&page=2', 'a 307 redirect', '/named-v2?moveReset'], + ['POST', '/named?moveKey&page=2', 'a 307 redirect', '/named-v2?moveKey=x&tab=1&page=2'], + ['POST', '/named?moveSame', 'a 307 redirect', '/named-v2?moveSame'], + ['PUT', '/named?toArchive&page=2', 'a 307 redirect', '/named?archive=&page=2'], + ['PUT', '/named?toArchiveHere&page=2', 'a 307 redirect', '/named?toArchiveHere'], ['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'], diff --git a/platform/packages/api/test/integration/email-admission.test.ts b/platform/packages/api/test/integration/email-admission.test.ts index 7de6150db..3d3fb8b97 100644 --- a/platform/packages/api/test/integration/email-admission.test.ts +++ b/platform/packages/api/test/integration/email-admission.test.ts @@ -345,6 +345,8 @@ describe('authoritative email admission', () => { payloadDigest: 'f'.repeat(64), route: receiptRoute(), }); + // It can fail before the assertion below listens; that is not an unhandled rejection. + receive.catch(() => {}); try { await waitForPostgresLockWaiter(); releaseDisable(); @@ -1185,6 +1187,8 @@ describe('authoritative email admission', () => { payloadDigest: 'd'.repeat(64), route: receiptRoute(), }); + // It can fail before the assertion below listens; that is not an unhandled rejection. + start.catch(() => {}); try { await waitForPostgresLockWaiter(); releaseSuspension();