Skip to content

Commit b0d28c4

Browse files
authored
fix(reg-exp-router): support line terminators in route paths for v5 (#5547)
* fix(reg-exp-router): match line terminators in RegExpRouter wildcards * fix(reg-exp-router): keep line terminators in dynamic route paths
1 parent 0f5ad7b commit b0d28c4

5 files changed

Lines changed: 63 additions & 16 deletions

File tree

‎src/router/common.case.test.ts‎

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,8 @@ const getSuiteHierarchy = (suite?: RunnerTestSuite) => {
1111
return res
1212
}
1313

14+
const lineTerminators = ['\n', '\r', '\u2028', '\u2029']
15+
1416
export const runTest = ({
1517
skip = [],
1618
newRouter,
@@ -237,6 +239,11 @@ export const runTest = ({
237239
expect(res.length).toBe(0)
238240
})
239241

242+
it.each(lineTerminators)('Regexp dot does not match %j', (char) => {
243+
router.add('GET', '/:id{.+}', 'pattern')
244+
expect(match('GET', `/a${char}b`)).toEqual([])
245+
})
246+
240247
it('Parameter with {.*} regexp', () => {
241248
router.add('GET', '/files/:name{.*}', 'file')
242249
let res = match('GET', '/files')
@@ -454,6 +461,10 @@ export const runTest = ({
454461
expect(res[2].handler).toEqual('/x')
455462
expect(res[3].handler).toEqual('/x/*')
456463
})
464+
465+
it.each(lineTerminators)('Matches a line terminator %j', (char) => {
466+
expect(match('GET', `/a${char}b`).map(({ handler }) => handler)).toEqual(['/*', '*'])
467+
})
457468
})
458469

459470
describe('Suffix wildcard', () => {
@@ -516,6 +527,10 @@ export const runTest = ({
516527
const res = match('GET', '/pathfoo')
517528
expect(res.length).toBe(0)
518529
})
530+
531+
it.each(lineTerminators)('Matches a line terminator %j', (char) => {
532+
expect(match('GET', `/path/a${char}b`)).toEqual([{ handler: 'path', params: {} }])
533+
})
519534
})
520535

521536
describe('Trailing wildcard after a label', () => {
@@ -956,6 +971,16 @@ export const runTest = ({
956971
expect(res[1].handler).toEqual('get () name')
957972
expect(res[1].params['name']).toEqual('hono')
958973
})
974+
975+
it.each(lineTerminators)('line terminator %j', (char) => {
976+
router.add('ALL', `/a${char}b/*`, 'middleware')
977+
router.add('GET', `/a${char}b/:name`, 'get name')
978+
expect(match('GET', `/a${char}b/hono`)).toEqual([
979+
{ handler: 'middleware', params: {} },
980+
{ handler: 'get name', params: { name: 'hono' } },
981+
])
982+
expect(match('GET', '/ab/hono')).toEqual([])
983+
})
959984
})
960985

961986
describe('REST API', () => {

‎src/router/reg-exp-router/node.ts‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,10 @@
11
import { createNullObject } from '../utils'
22

33
export const LABEL_REG_EXP_STR = '[^/]+'
4-
export const ONLY_WILDCARD_REG_EXP_STR = '.*'
4+
// Match line terminators without changing the flags of user-defined parameter patterns.
5+
export const ONLY_WILDCARD_REG_EXP_STR = '[^]*'
56
const PREFIXED_WILDCARD_REG_EXP_STR = '[^/]*'
6-
export const TAIL_WILDCARD_REG_EXP_STR = '(?:|/.*)'
7+
export const TAIL_WILDCARD_REG_EXP_STR = '(?:|/[^]*)'
78
export const PATH_ERROR = Symbol()
89

910
export type ParamAssocArray = [string, number][]
@@ -28,7 +29,7 @@ function compareKey(a: string, b: string): number {
2829
return 1
2930
}
3031

31-
// wildcard: the only wildcard (.*) precedes the tail wildcard
32+
// wildcard: the only wildcard precedes the tail wildcard
3233
if (a === ONLY_WILDCARD_REG_EXP_STR || a === TAIL_WILDCARD_REG_EXP_STR) {
3334
return b === TAIL_WILDCARD_REG_EXP_STR ? -1 : 1
3435
} else if (b === ONLY_WILDCARD_REG_EXP_STR || b === TAIL_WILDCARD_REG_EXP_STR) {
@@ -74,7 +75,7 @@ export class Node {
7475
: ['', '', PREFIXED_WILDCARD_REG_EXP_STR] // 'x*' may take nothing
7576
: null
7677
: token === '/*'
77-
? ['', '', TAIL_WILDCARD_REG_EXP_STR] // '/path/to/*' is /\/path\/to(?:|/.*)$
78+
? ['', '', TAIL_WILDCARD_REG_EXP_STR] // '/path/to/*' also matches '/path/to'
7879
: token.match(/^\:([^\{\}]+)(?:\{(.+)\})?$/)
7980

8081
let nextNode: Node

‎src/router/reg-exp-router/prepared-router.test.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -122,7 +122,7 @@ describe('buildInitParams() and serializeInitParams()', () => {
122122
})
123123
expect(params).toEqual([
124124
{
125-
[METHOD_NAME_ALL]: [/^.*$()/, [0, []], {}],
125+
[METHOD_NAME_ALL]: [/^[^]*$()/, [0, []], {}],
126126
},
127127
{},
128128
])
@@ -136,7 +136,7 @@ describe('buildInitParams() and serializeInitParams()', () => {
136136
expect(params).toEqual([
137137
{
138138
[METHOD_NAME_ALL]: [
139-
/^(?:\/hello\/([^/]+)(?:$()|\/posts\/([^/]+)$())|.*$())/,
139+
/^(?:\/hello\/([^/]+)(?:$()|\/posts\/([^/]+)$())|[^]*$())/,
140140
[0, 0, [], 0, [], []],
141141
{
142142
'/hello': [[], []],

‎src/router/reg-exp-router/router.test.ts‎

Lines changed: 30 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import type { ParamIndexMap, ParamStash } from '../../router'
22
import { UnsupportedPathError } from '../../router'
33
import { runTest } from '../common.case.test'
4+
import { ONLY_WILDCARD_REG_EXP_STR, TAIL_WILDCARD_REG_EXP_STR } from './node'
45
import { RegExpRouter } from './router'
56

67
describe('RegExpRouter', () => {
@@ -176,15 +177,21 @@ describe('RegExpRouter', () => {
176177
})
177178

178179
describe('Pattern that matches an empty string', () => {
179-
it.each(['/api/:id{[0-9]?}', '/user/:id{[0-9]*}', '/:id{(?:a|)}', '/:id{a{0,3}}'])(
180-
'%s',
181-
(path) => {
182-
const router = new RegExpRouter<string>()
183-
expect(() => {
184-
router.add('GET', path, 'ok')
185-
}).toThrowError(UnsupportedPathError)
186-
}
187-
)
180+
it.each([
181+
'/api/:id{[0-9]?}',
182+
'/user/:id{[0-9]*}',
183+
'/:id{(?:a|)}',
184+
'/:id{a{0,3}}',
185+
`/:id{${ONLY_WILDCARD_REG_EXP_STR}}`,
186+
`/:id{(?:${ONLY_WILDCARD_REG_EXP_STR})}`,
187+
`/:id{${TAIL_WILDCARD_REG_EXP_STR}}`,
188+
`/:id{(?:${TAIL_WILDCARD_REG_EXP_STR})}`,
189+
])('%s', (path) => {
190+
const router = new RegExpRouter<string>()
191+
expect(() => {
192+
router.add('GET', path, 'ok')
193+
}).toThrowError(UnsupportedPathError)
194+
})
188195

189196
it('Should still accept a pattern that requires at least one character', () => {
190197
const router = new RegExpRouter<string>()
@@ -199,6 +206,20 @@ describe('RegExpRouter', () => {
199206
})
200207
})
201208

209+
it.each([
210+
['[^]+', 'a\nb'],
211+
['(?:/[^]+)', '/a\nb'],
212+
])('Should extract parameters when the pattern is %s', (pattern, value) => {
213+
const router = new RegExpRouter<string>()
214+
router.add('GET', `/:value{${pattern}}/foo/:id`, 'handler')
215+
216+
const [res, stash] = router.match('GET', `/${value}/foo/end`)
217+
const params = res[0][1] as ParamIndexMap
218+
expect(res[0][0]).toBe('handler')
219+
expect((stash as ParamStash)[params.value]).toBe(value)
220+
expect((stash as ParamStash)[params.id]).toBe('end')
221+
})
222+
202223
describe('Wildcard after label', () => {
203224
it('Should be able to add a tail wildcard after a label', () => {
204225
const router = new RegExpRouter<string>()

‎src/router/reg-exp-router/trie.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@ export class Trie {
4141
* - /* wildcard
4242
* - character
4343
*/
44-
const tokens = markedPath.match(/(?::[^\/]+)|(?:\/\*$)|./g) || []
44+
const tokens = markedPath.match(/(?::[^\/]+)|(?:\/\*$)|./gs) || []
4545
for (let i = groups.length - 1; i >= 0; i--) {
4646
const [mark] = groups[i]
4747
for (let j = tokens.length - 1; j >= 0; j--) {

0 commit comments

Comments
 (0)