Skip to content

Commit

Permalink
fix(middleware/combine): prevent c.req.routeIndex from being changed (
Browse files Browse the repository at this point in the history
#3663)

* test(middleware/combine): add test for every middleware

Co-authored-by: Paweł Dąbrowski <[email protected]>

* refactor(compose): Loosen `compose` parameter types

The current implementation of `compose` does not use some elements of the received middleware,
so they do not have to be passed on.

* fix(middleware/combine): prevent `c.req.routeIndex` from being changed

---------

Co-authored-by: Paweł Dąbrowski <[email protected]>
  • Loading branch information
usualoma and paolostyle authored Nov 13, 2024
1 parent 7b30835 commit 4349735
Show file tree
Hide file tree
Showing 3 changed files with 32 additions and 14 deletions.
3 changes: 1 addition & 2 deletions src/compose.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
import { Context } from './context'
import type { ParamIndexMap, Params } from './router'
import type { Env, ErrorHandler, NotFoundHandler } from './types'

/**
Expand Down Expand Up @@ -31,7 +30,7 @@ interface ComposeContext {
* @returns {(context: C, next?: Function) => Promise<C>} - A composed middleware function.
*/
export const compose = <C extends ComposeContext, E extends Env = Env>(
middleware: [[Function, unknown], ParamIndexMap | Params][],
middleware: [[Function, unknown], unknown][] | [[Function]][],
onError?: ErrorHandler<E>,
onNotFound?: NotFoundHandler<E>
): ((context: C, next?: Function) => Promise<C>) => {
Expand Down
16 changes: 16 additions & 0 deletions src/middleware/combine/index.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -166,6 +166,22 @@ describe('every', () => {
expect(await res.text()).toBe('Hello Middleware 1')
expect(middleware2).not.toBeCalled()
})

it('Should pass the path params to middlewares', async () => {
const app = new Hono()
app.use('*', nextMiddleware)
const paramMiddleware: MiddlewareHandler = async (c) => {
return c.json(c.req.param(), 200)
}

app.use('/:id', every(paramMiddleware))
app.get('/:id', (c) => {
return c.text('Hello World')
})

const res = await app.request('http://localhost/123')
expect(await res.json()).toEqual({ id: '123' })
})
})

describe('except', () => {
Expand Down
27 changes: 15 additions & 12 deletions src/middleware/combine/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -89,19 +89,22 @@ export const some = (...middleware: (MiddlewareHandler | Condition)[]): Middlewa
* ```
*/
export const every = (...middleware: (MiddlewareHandler | Condition)[]): MiddlewareHandler => {
const wrappedMiddleware = middleware.map((m) => async (c: Context, next: Next) => {
const res = await m(c, next)
if (res === false) {
throw new Error('Unmet condition')
}
return res
})

const handler = async (c: Context, next: Next) =>
compose<Context>(wrappedMiddleware.map((m) => [[m, undefined], c.req.param()]))(c, next)

return async function every(c, next) {
await handler(c, next)
const currentRouteIndex = c.req.routeIndex
await compose<Context>(
middleware.map((m) => [
[
async (c: Context, next: Next) => {
c.req.routeIndex = currentRouteIndex // should be unchanged in this context
const res = await m(c, next)
if (res === false) {
throw new Error('Unmet condition')
}
return res
},
],
])
)(c, next)
}
}

Expand Down

0 comments on commit 4349735

Please sign in to comment.