Skip to content

Commit e1b87e7

Browse files
committed
perf(router): skip unnecessary precognition URL parsing
1 parent a3c6d11 commit e1b87e7

2 files changed

Lines changed: 42 additions & 26 deletions

File tree

storage/framework/core/router/src/stacks-router.ts

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2297,7 +2297,10 @@ export function precognitionRequest(req: EnhancedRequest): { only: string[] } |
22972297

22982298
let viaQuery = false
22992299
try {
2300-
viaQuery = new URL(req.url).searchParams.get('_validate') === '1'
2300+
// Ordinary action requests have no query to inspect. A true header also
2301+
// settles the decision without parsing the URL again.
2302+
if (!viaHeader && req.url.includes('?'))
2303+
viaQuery = new URL(req.url).searchParams.get('_validate') === '1'
23012304
}
23022305
catch {
23032306
viaQuery = false

storage/framework/core/router/tests/precognition.test.ts

Lines changed: 38 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -12,15 +12,12 @@
1212
//
1313
// The property that matters most is the negative one: a probe must never reach
1414
// handle(). A validate-only mode that still registers the user is worse than no
15-
// validate-only mode at all — and that one is asserted structurally, because
16-
// driving a real route needs a booted app, a database and an auth stack that
17-
// this package's tests do not have.
15+
// validate-only mode at all. The route tests below assert that no lifecycle
16+
// hook runs for a probe, even when the action has no validation rules.
1817

1918
import { describe, expect, it } from 'bun:test'
20-
import { readFileSync } from 'node:fs'
21-
import { join } from 'node:path'
2219
import { schema } from '@stacksjs/validation'
23-
import { precognitionRequest, precognitionSuccess, validateActionInput } from '../src/stacks-router'
20+
import { createStacksRouter, precognitionRequest, precognitionSuccess, validateActionInput } from '../src/stacks-router'
2421

2522
/** Minimal stand-in for the shape `precognitionRequest` reads. */
2623
function request(url: string, headers: Record<string, string> = {}): any {
@@ -59,10 +56,10 @@ describe('recognising a precognition request (#2226)', () => {
5956
expect(precognitionRequest(request('http://localhost/register?_validate=0'))).toBeNull()
6057
})
6158

62-
it('survives a malformed url instead of throwing', () => {
59+
it.each(['/register', '/register?_validate=1'])('survives malformed url %s instead of throwing', (url) => {
6360
// `new URL` throws on a relative url, and a throw here would take down
6461
// every request rather than just this check.
65-
expect(precognitionRequest({ url: '/register', headers: new Headers() } as any)).toBeNull()
62+
expect(precognitionRequest({ url, headers: new Headers() } as any)).toBeNull()
6663
})
6764
})
6865

@@ -174,22 +171,38 @@ describe('narrowed validation runs the real rules (#2226)', () => {
174171
})
175172

176173
describe('a precognition request never reaches the handler (#2226)', () => {
177-
const source = readFileSync(join(import.meta.dir, '../src/stacks-router.ts'), 'utf8')
178-
179-
it('returns before authorize, before and handle', () => {
180-
const at = (needle: string): number => source.indexOf(needle)
181-
const precognition = at('const precognition = precognitionRequest(req)')
182-
183-
expect(precognition).toBeGreaterThan(-1)
184-
expect(precognition).toBeLessThan(at('typeof action.authorize === \'function\''))
185-
expect(precognition).toBeLessThan(at('typeof action.before === \'function\''))
186-
expect(precognition).toBeLessThan(at('await action.handle(req)'))
187-
})
188-
189-
it('returns early even when the action declares no validations', () => {
190-
// Otherwise a probe at an action with no rules falls through and runs it —
191-
// the probe becomes the side effect, which is the exact failure this is
192-
// meant to prevent.
193-
expect(source).toContain('if (!action.validations)')
174+
it.each([
175+
{ query: '', headers: { Precognition: 'true' }, probe: true },
176+
{ query: '?_validate=1', headers: {}, probe: true },
177+
{ query: '?%5Fvalidate=%31', headers: {}, probe: true },
178+
{ query: '?_validate=0&_validate=1', headers: {}, probe: false },
179+
{ query: '?_validate=0', headers: { Precognition: 'true' }, probe: true },
180+
{ query: '#?_validate=1', headers: {}, probe: false },
181+
{ query: '', headers: {}, probe: false },
182+
])('dispatches the expected lifecycle for %j', async ({ query, headers, probe }) => {
183+
for (const withRules of [true, false]) {
184+
const calls: string[] = []
185+
const router = createStacksRouter()
186+
router.post('/precognition-dispatch', {
187+
skipCsrf: true,
188+
validations: withRules ? { name: { rule: schema.string().required() } } : undefined,
189+
authorize() { calls.push('authorize'); return true },
190+
before() { calls.push('before') },
191+
handle() { calls.push('handle'); return { ok: true } },
192+
})
193+
194+
const response = await router.handleRequest(new Request(`http://localhost/precognition-dispatch${query}`, {
195+
method: 'POST',
196+
headers: { ...headers, 'content-type': 'application/json' },
197+
body: JSON.stringify({ name: 'test' }),
198+
}))
199+
200+
expect(response.status).toBe(probe ? 204 : 200)
201+
expect(calls).toEqual(probe ? [] : ['authorize', 'before', 'handle'])
202+
if (probe)
203+
expect(response.headers.get('Precognition-Success')).toBe('true')
204+
else
205+
expect(await response.json()).toEqual({ ok: true })
206+
}
194207
})
195208
})

0 commit comments

Comments
 (0)