diff --git a/packages/event-handler/src/http/Router.ts b/packages/event-handler/src/http/Router.ts index 9ef1f1e685..3e4acd2c92 100644 --- a/packages/event-handler/src/http/Router.ts +++ b/packages/event-handler/src/http/Router.ts @@ -81,6 +81,7 @@ import { isBinaryResult, isExtendedAPIGatewayProxyResult, resolvePrefixedPath, + stripTrailingSlashes, } from './utils.js'; class Router { @@ -342,7 +343,10 @@ class Router { try { const method = req.method as HttpMethod; - const path = new URL(req.url).pathname as Path; + const rawPath = new URL(req.url).pathname; + const path = ( + rawPath === '/' ? rawPath : stripTrailingSlashes(rawPath) + ) as Path; const route = this.routeRegistry.resolve(method, path); diff --git a/packages/event-handler/src/http/utils.ts b/packages/event-handler/src/http/utils.ts index 7935df3c33..550fa345c9 100644 --- a/packages/event-handler/src/http/utils.ts +++ b/packages/event-handler/src/http/utils.ts @@ -322,27 +322,41 @@ export const composeMiddleware = (middleware: Middleware[]): Middleware => { }; }; +// Linear-time trailing-slash strip. Avoids `replace(/\/+$/, '')` to keep +// behavior linear on attacker-controlled request paths. +export const stripTrailingSlashes = (input: string): string => { + let end = input.length; + while (end > 0 && input[end - 1] === '/') end--; + return end === input.length ? input : input.slice(0, end); +}; + /** * Resolves a prefixed path by combining the provided path and prefix. * - * The function returns a RegExp if any of the path or prefix is a RegExp. - * Otherwise, it returns a `/${string}` type value. + * Trailing slashes on the prefix are stripped before joining, so a prefix of + * `/api` and `/api/` produce the same result. When the resulting path would + * end with a redundant `/` (e.g. path `/` under any prefix), the trailing + * slash is collapsed so the route id is canonical (`/api`, not `/api/`). + * Incoming request paths are normalized the same way at lookup time, so a + * route registered with path `/` under prefix `/api` matches both `/api` and + * `/api/`. + * + * Returns a `RegExp` if either argument is a `RegExp`; otherwise a + * `/${string}` typed value. * * @param path - The path to resolve - * @param prefix - The prefix to prepend to the path + * @param prefix - The prefix to prepend to the path; trailing slashes are ignored */ export const resolvePrefixedPath = (path: Path, prefix?: Path): Path => { if (!prefix) return path; - if (isRegExp(prefix)) { - if (isRegExp(path)) { - return new RegExp(`${getPathString(prefix)}/${getPathString(path)}`); - } - return new RegExp(`${getPathString(prefix)}${path}`); - } - if (isRegExp(path)) { - return new RegExp(`${prefix}/${getPathString(path)}`); + const prefixStr = stripTrailingSlashes(getPathString(prefix)); + if (isRegExp(prefix) || isRegExp(path)) { + const pathStr = getPathString(path); + const sep = pathStr.startsWith('/') ? '' : '/'; + return new RegExp(`${prefixStr}${sep}${pathStr}`); } - return `${prefix}${path}`.replace(/\/$/, '') as Path; + const joined = `${prefixStr}${path}`; + return (joined.endsWith('/') ? joined.slice(0, -1) : joined) as Path; }; export const HttpResponseStream = diff --git a/packages/event-handler/tests/unit/http/Router/basic-routing.test.ts b/packages/event-handler/tests/unit/http/Router/basic-routing.test.ts index eda21a26ea..199e64b5cd 100644 --- a/packages/event-handler/tests/unit/http/Router/basic-routing.test.ts +++ b/packages/event-handler/tests/unit/http/Router/basic-routing.test.ts @@ -218,6 +218,39 @@ describe.each([ expect(JSON.parse(getResult.body ?? '{}').actualPath).toBe('/todos/1'); }); + it('matches a root route registered under a prefix for both /prefix and /prefix/', async () => { + // Prepare + const app = new Router({ prefix: '/api' }); + app.get('/', () => ({ root: true })); + + // Act + const noSlash = await app.resolve(createEvent('/api', 'GET'), context); + const trailingSlash = await app.resolve( + createEvent('/api/', 'GET'), + context + ); + + // Assess + expect(noSlash.statusCode).toBe(200); + expect(JSON.parse(noSlash.body ?? '{}')).toEqual({ root: true }); + expect(trailingSlash.statusCode).toBe(200); + expect(JSON.parse(trailingSlash.body ?? '{}')).toEqual({ root: true }); + }); + + it('routes correctly when prefix accidentally ends with a slash', async () => { + // Prepare: prefix ends with `/` — the registered route id should not + // contain `//` and incoming requests should still match. + const app = new Router({ prefix: '/api/' }); + app.get('/users', () => ({ users: [] })); + + // Act + const result = await app.resolve(createEvent('/api/users', 'GET'), context); + + // Assess + expect(result.statusCode).toBe(200); + expect(JSON.parse(result.body ?? '{}')).toEqual({ users: [] }); + }); + it('routes to the included router when using split routers', async () => { // Prepare const todoRouter = new Router({ logger: console }); diff --git a/packages/event-handler/tests/unit/http/utils.test.ts b/packages/event-handler/tests/unit/http/utils.test.ts index b4d3cd9ed7..0f2f551806 100644 --- a/packages/event-handler/tests/unit/http/utils.test.ts +++ b/packages/event-handler/tests/unit/http/utils.test.ts @@ -855,6 +855,8 @@ describe('Path Utilities', () => { it.each([ { path: '/test', prefix: '/prefix', expected: '/prefix/test' }, { path: '/', prefix: '/prefix', expected: '/prefix' }, + { path: '/users', prefix: '/api/', expected: '/api/users' }, + { path: '/', prefix: '/api/', expected: '/api' }, { path: '/test', expected: '/test' }, { path: /.+/, prefix: '/prefix', expected: /\/prefix\/.+/ }, { path: '/test', prefix: /\/prefix/, expected: /\/prefix\/test/ },