Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix: do not encode slashes in catch-all routes #2291

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
48 changes: 48 additions & 0 deletions packages/router/__tests__/router.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -931,6 +931,54 @@ describe('Router', () => {
})
})

it('escapes slashes in standard params', async () => {
const router = createRouter({
history: createMemoryHistory(),
routes: [{ path: '/home/:path', component: components.Home }],
})
await router.push('/home/pathparam')
await router.replace({ params: { path: 'test/this/is/escaped' } })
expect(router.currentRoute.value).toMatchObject({
fullPath: '/home/test%2Fthis%2Fis%2Fescaped',
})
})

it('keeps slashes in star params', async () => {
const router = createRouter({
history: createMemoryHistory(),
routes: [{ path: '/home/:path(.*)', component: components.Home }],
})
await router.push('/home/pathparam')
await router.replace({ params: { path: 'test/this/is/not/escaped' } })
expect(router.currentRoute.value).toMatchObject({
fullPath: '/home/test/this/is/not/escaped',
})
await router.replace({ hash: '#test' })
expect(router.currentRoute.value).toMatchObject({
fullPath: '/home/test/this/is/not/escaped#test',
})
})

it('keeps slashes in params containing slashes', async () => {
const router = createRouter({
history: createMemoryHistory(),
routes: [
{
path: '/home/:slug(.*)*/:path(test/deep/about\\.html(?!.*\\/\\).*)',
component: components.Foo,
},
],
})
await router.push('/home/slug/test/deep/about.html')
await router.replace({
params: { slug: 'another/slug' },
})
await router.replace({ hash: '#hash' })
expect(router.currentRoute.value).toMatchObject({
fullPath: '/home/another/slug/test/deep/about.html#hash',
})
})

describe('Dynamic Routing', () => {
it('resolves new added routes', async () => {
const { router } = await newRouter({ routes: [] })
Expand Down
16 changes: 15 additions & 1 deletion packages/router/src/encoding.ts
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,20 @@ export function encodePath(text: string | number): string {
return commonEncode(text).replace(HASH_RE, '%23').replace(IM_RE, '%3F')
}

/**
* Encode characters that need to be encoded on the path section of the URL as a
* path param. This function does exactly what {@link encodePath} does, but if `text` is `null` or `undefined`, returns an empty
* string instead.
*
* @param text - string to encode
* @returns encoded string
*/
export function encodePathParam(
text: string | number | null | undefined
): string {
return text == null ? '' : encodePath(text)
}

/**
* Encode characters that need to be encoded on the path section of the URL as a
* param. This function encodes everything {@link encodePath} does plus the
Expand All @@ -129,7 +143,7 @@ export function encodePath(text: string | number): string {
* @returns encoded string
*/
export function encodeParam(text: string | number | null | undefined): string {
return text == null ? '' : encodePath(text).replace(SLASH_RE, '%2F')
return encodePathParam(text).replace(SLASH_RE, '%2F')
}

/**
Expand Down
47 changes: 44 additions & 3 deletions packages/router/src/matcher/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import {
MatcherLocationRaw,
MatcherLocation,
isRouteName,
RouteParamsGeneric,
} from '../types'
import { createRouterError, ErrorTypes, MatcherError } from '../errors'
import { createRouteRecordMatcher, RouteRecordMatcher } from './pathMatcher'
Expand All @@ -17,8 +18,9 @@ import type {
import { comparePathParserScore } from './pathParserRanker'

import { warn } from '../warning'
import { assign, noop } from '../utils'
import { applyToParam, assign, noop } from '../utils'
import type { RouteRecordNameGeneric, _RouteRecordProps } from '../typed-routes'
import { encodeParam, encodePathParam } from '../encoding'

/**
* Internal RouterMatcher
Expand All @@ -40,10 +42,12 @@ export interface RouterMatcher {
*
* @param location - MatcherLocationRaw to resolve to a url
* @param currentLocation - MatcherLocation of the current location
* @param encodeParams - Whether to encode parameters or not. Defaults to `false`
*/
resolve: (
location: MatcherLocationRaw,
currentLocation: MatcherLocation
currentLocation: MatcherLocation,
encodeParams?: boolean
) => MatcherLocation
}

Expand Down Expand Up @@ -230,15 +234,48 @@ export function createRouterMatcher(
matcherMap.set(matcher.record.name, matcher)
}

function encodeParams(
matcher: RouteRecordMatcher,
params: RouteParamsGeneric | undefined
): MatcherLocation['params'] {
const newParams = {} as MatcherLocation['params']
if (params) {
for (let paramKey of Object.keys(params)) {
let matcherKey = matcher.keys.find(k => k.name == paramKey)

let keepSlash = matcherKey?.keepSlash ?? false
newParams[paramKey] = keepSlash
? applyToParam(encodePathParam, params[paramKey])
: applyToParam(encodeParam, params[paramKey])
}
}
return newParams
}

function resolve(
location: Readonly<MatcherLocationRaw>,
currentLocation: Readonly<MatcherLocation>
currentLocation: Readonly<MatcherLocation>,
doEncodeParams: boolean = false
): MatcherLocation {
let matcher: RouteRecordMatcher | undefined
let params: PathParams = {}
let path: MatcherLocation['path']
let name: MatcherLocation['name']

// Encode params
let encodeLocationsParams = (matcher: RouteRecordMatcher) => {
if (doEncodeParams) {
if ('params' in location) {
location = assign(location, {
params: encodeParams(matcher, location.params),
})
}
currentLocation = assign(currentLocation, {
params: encodeParams(matcher, currentLocation.params),
})
}
}

if ('name' in location && location.name) {
matcher = matcherMap.get(location.name)

Expand All @@ -247,6 +284,8 @@ export function createRouterMatcher(
location,
})

encodeLocationsParams(matcher)

// warn if the user is passing invalid params so they can debug it better when they get removed
if (__DEV__) {
const invalidParams: string[] = Object.keys(
Expand Down Expand Up @@ -301,6 +340,7 @@ export function createRouterMatcher(
// matcher should have a value after the loop

if (matcher) {
encodeLocationsParams(matcher)
// we know the matcher works because we tested the regexp
params = matcher.parse(path)!
name = matcher.record.name
Expand All @@ -316,6 +356,7 @@ export function createRouterMatcher(
location,
currentLocation,
})
encodeLocationsParams(matcher)
name = matcher.record.name
// since we are navigating to the same location, we don't need to pick the
// params like when `name` is provided
Expand Down
19 changes: 14 additions & 5 deletions packages/router/src/matcher/pathParserRanker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ interface PathParserParamKey {
name: string
repeatable: boolean
optional: boolean
keepSlash: boolean
}

export interface PathParser {
Expand Down Expand Up @@ -156,11 +157,6 @@ export function tokensToParser(
subSegmentScore += PathScore.Static
} else if (token.type === TokenType.Param) {
const { value, repeatable, optional, regexp } = token
keys.push({
name: value,
repeatable,
optional,
})
const re = regexp ? regexp : BASE_PARAM_PATTERN
// the user provided a custom regexp /:id(\\d+)
if (re !== BASE_PARAM_PATTERN) {
Expand All @@ -176,6 +172,19 @@ export function tokensToParser(
}
}

// Keep slash if it matches regex
// Or if a slash is litterally contained outside of lookaheads, lookbehinds and negative ranges
let keepSlash =
new RegExp(`(${re})`).test('/') ||
/\//.test(re.replace(/\(\?<?[=!].*\)|\[\^.*\]/, ''))

keys.push({
name: value,
repeatable,
optional,
keepSlash,
})

// when we repeat we must take care of the repeating leading slash
let subPattern = repeatable ? `((?:${re})(?:/(?:${re}))*)` : `(${re})`

Expand Down
14 changes: 6 additions & 8 deletions packages/router/src/router.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import {
isRouteName,
RouteLocationOptions,
MatcherLocationRaw,
RouteParamsGeneric,
} from './types'
import type {
RouteLocation,
Expand Down Expand Up @@ -40,7 +41,7 @@ import {
} from './errors'
import { applyToParams, isBrowser, assign, noop, isArray } from './utils'
import { useCallbacks } from './utils/callbacks'
import { encodeParam, decode, encodeHash } from './encoding'
import { decode, encodeHash } from './encoding'
import {
normalizeQuery,
parseQuery as originalParseQuery,
Expand Down Expand Up @@ -410,7 +411,6 @@ export function createRouter(options: RouterOptions): Router {
null,
paramValue => '' + paramValue
)
const encodeParams = applyToParams.bind(null, encodeParam)
const decodeParams: (params: RouteParams | undefined) => RouteParams =
// @ts-expect-error: intentionally avoid the type check
applyToParams.bind(null, decode)
Expand Down Expand Up @@ -529,16 +529,14 @@ export function createRouter(options: RouterOptions): Router {
delete targetParams[key]
}
}
// pass encoded values to the matcher, so it can produce encoded path and fullPath

// matcher handles param encoding by itself, just pass cleaned params
matcherLocation = assign({}, rawLocation, {
params: encodeParams(targetParams),
params: targetParams as RouteParamsGeneric,
})
// current location params are decoded, we need to encode them in case the
// matcher merges the params
currentLocation.params = encodeParams(currentLocation.params)
}

const matchedRoute = matcher.resolve(matcherLocation, currentLocation)
const matchedRoute = matcher.resolve(matcherLocation, currentLocation, true)
const hash = rawLocation.hash || ''

if (__DEV__ && hash && !hash.startsWith('#')) {
Expand Down
13 changes: 10 additions & 3 deletions packages/router/src/utils/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,15 @@ export function isESModule(obj: any): obj is { default: RouteComponent } {

export const assign = Object.assign

export function applyToParam(
fn: (v: string | number | null | undefined) => string,
param: RouteParamValueRaw | Exclude<RouteParamValueRaw, null | undefined>[]
): string | string[] {
return isArray(param)
? param.map(fn)
: fn(param as Exclude<RouteParamValueRaw, any[]>)
}

export function applyToParams(
fn: (v: string | number | null | undefined) => string,
params: RouteParamsRawGeneric | undefined
Expand All @@ -21,9 +30,7 @@ export function applyToParams(

for (const key in params) {
const value = params[key]
newParams[key] = isArray(value)
? value.map(fn)
: fn(value as Exclude<RouteParamValueRaw, any[]>)
newParams[key] = applyToParam(fn, value)
}

return newParams
Expand Down