Skip to content

Commit 20aac85

Browse files
schiller-manuelschiller-manuel
authored
fix: compare rewritten url to decide if server needs to redirect (#6072)
1 parent eccfd26 commit 20aac85

File tree

10 files changed

+76
-52
lines changed

10 files changed

+76
-52
lines changed

‎e2e/react-start/custom-basepath/tests/navigation.spec.ts‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -61,8 +61,16 @@ test('server-side redirect', async ({ page, baseURL }) => {
6161
expect(page.url()).toBe(`${baseURL}/posts/1`)
6262

6363
// do not follow redirects since we want to test the Location header
64+
// first go to the route WITHOUT the base path, this will just add the base path
6465
await page.request
6566
.get('/redirect/throw-it', { maxRedirects: 0 })
67+
.then((res) => {
68+
const headers = new Headers(res.headers())
69+
expect(headers.get('location')).toBe('/custom/basepath/redirect/throw-it')
70+
})
71+
// now go to the route WITH the base path, this will redirect to the final destination
72+
await page.request
73+
.get('/custom/basepath/redirect/throw-it', { maxRedirects: 0 })
6674
.then((res) => {
6775
const headers = new Headers(res.headers())
6876
expect(headers.get('location')).toBe('/custom/basepath/posts/1')

‎e2e/solid-start/custom-basepath/tests/navigation.spec.ts‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -61,8 +61,16 @@ test('server-side redirect', async ({ page, baseURL }) => {
6161
expect(page.url()).toBe(`${baseURL}/posts/1`)
6262

6363
// do not follow redirects since we want to test the Location header
64+
// first go to the route WITHOUT the base path, this will just add the base path
6465
await page.request
6566
.get('/redirect/throw-it', { maxRedirects: 0 })
67+
.then((res) => {
68+
const headers = new Headers(res.headers())
69+
expect(headers.get('location')).toBe('/custom/basepath/redirect/throw-it')
70+
})
71+
// now go to the route WITH the base path, this will redirect to the final destination
72+
await page.request
73+
.get('/custom/basepath/redirect/throw-it', { maxRedirects: 0 })
6674
.then((res) => {
6775
const headers = new Headers(res.headers())
6876
expect(headers.get('location')).toBe('/custom/basepath/posts/1')

‎packages/react-router/src/link.tsx‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -127,7 +127,9 @@ export function useLinkProps<
127127
if (disabled) {
128128
return undefined
129129
}
130-
let href = next.maskedLocation ? next.maskedLocation.url : next.url
130+
let href = next.maskedLocation
131+
? next.maskedLocation.url.href
132+
: next.url.href
131133

132134
let external = false
133135
if (router.origin) {

‎packages/react-router/tests/redirect.test.tsx‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -368,7 +368,7 @@ describe('redirect', () => {
368368
__TSR_key: currentRedirect.options._fromLocation!.state.__TSR_key,
369369
key: currentRedirect.options._fromLocation!.state.key,
370370
},
371-
url: 'http://localhost/',
371+
url: new URL('http://localhost/'),
372372
},
373373
href: '/about',
374374
to: '/about',

‎packages/react-router/tests/router.test.tsx‎

Lines changed: 21 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -350,7 +350,7 @@ describe('encoding: path params', () => {
350350

351351
await act(() => router.load())
352352

353-
expect(router.state.location.url.endsWith('/posts/tanner')).toBe(true)
353+
expect(router.state.location.url.href.endsWith('/posts/tanner')).toBe(true)
354354
expect(router.state.location.href).toBe('/posts/tanner')
355355
expect(router.state.location.pathname).toBe('/posts/tanner')
356356
})
@@ -362,7 +362,9 @@ describe('encoding: path params', () => {
362362

363363
await act(() => router.load())
364364

365-
expect(router.state.location.url.endsWith('/posts/%F0%9F%9A%80')).toBe(true)
365+
expect(router.state.location.url.href.endsWith('/posts/%F0%9F%9A%80')).toBe(
366+
true,
367+
)
366368
expect(router.state.location.href).toBe('/posts/%F0%9F%9A%80')
367369
expect(router.state.location.pathname).toBe('/posts/🚀')
368370
})
@@ -381,7 +383,9 @@ describe('encoding: path params', () => {
381383
}),
382384
)
383385

384-
expect(router.state.location.url.endsWith('/posts/100%2525')).toBe(true)
386+
expect(router.state.location.url.href.endsWith('/posts/100%2525')).toBe(
387+
true,
388+
)
385389
expect(router.state.location.href).toBe('/posts/100%2525')
386390
expect(router.state.location.pathname).toBe('/posts/100%2525')
387391
})
@@ -406,7 +410,9 @@ describe('encoding: path params', () => {
406410
)
407411

408412
expect(
409-
router.state.location.url.endsWith(`/posts/${encodedValue}jane%25`),
413+
router.state.location.url.href.endsWith(
414+
`/posts/${encodedValue}jane%25`,
415+
),
410416
).toBe(true)
411417
expect(router.state.location.href).toBe(`/posts/${encodedValue}jane%25`)
412418
expect(router.state.location.pathname).toBe(
@@ -441,7 +447,7 @@ describe('encoding: path params', () => {
441447
)
442448

443449
expect(
444-
router.state.location.url.endsWith(`/posts/${character}jane%25`),
450+
router.state.location.url.href.endsWith(`/posts/${character}jane%25`),
445451
).toBe(true)
446452
expect(router.state.location.href).toBe(`/posts/${character}jane%25`)
447453
expect(router.state.location.pathname).toBe(`/posts/${character}jane%25`)
@@ -455,7 +461,9 @@ describe('encoding: path params', () => {
455461

456462
await act(() => router.load())
457463

458-
expect(router.state.location.url.endsWith('/posts/%F0%9F%9A%80')).toBe(true)
464+
expect(router.state.location.url.href.endsWith('/posts/%F0%9F%9A%80')).toBe(
465+
true,
466+
)
459467
expect(router.state.location.href).toBe('/posts/%F0%9F%9A%80')
460468
expect(router.state.location.pathname).toBe('/posts/🚀')
461469
})
@@ -472,7 +480,7 @@ describe('encoding: path params', () => {
472480
await act(() => router.load())
473481

474482
expect(
475-
router.state.location.url.endsWith(
483+
router.state.location.url.href.endsWith(
476484
'/posts/framework%2Freact%2Fguide%2Ffile-based-routing%20tanstack',
477485
),
478486
).toBe(true)
@@ -619,7 +627,7 @@ describe('encoding/decoding: wildcard routes/params', () => {
619627

620628
await router.load()
621629

622-
expect(router.state.location.url.endsWith('/tanner')).toBe(true)
630+
expect(router.state.location.url.href.endsWith('/tanner')).toBe(true)
623631
expect(router.state.location.href).toBe('/tanner')
624632
expect(router.state.location.pathname).toBe('/tanner')
625633
})
@@ -631,7 +639,7 @@ describe('encoding/decoding: wildcard routes/params', () => {
631639

632640
await router.load()
633641

634-
expect(router.state.location.url.endsWith('/%F0%9F%9A%80')).toBe(true)
642+
expect(router.state.location.url.href.endsWith('/%F0%9F%9A%80')).toBe(true)
635643
expect(router.state.location.href).toBe('/%F0%9F%9A%80')
636644
expect(router.state.location.pathname).toBe('/🚀')
637645
})
@@ -649,7 +657,7 @@ describe('encoding/decoding: wildcard routes/params', () => {
649657
await router.load()
650658

651659
expect(
652-
router.state.location.url.endsWith(`/100${encodedValue}100`),
660+
router.state.location.url.href.endsWith(`/100${encodedValue}100`),
653661
).toBe(true)
654662
expect(router.state.location.href).toBe(`/100${encodedValue}100`)
655663
expect(router.state.location.pathname).toBe(`/100${encodedValue}100`)
@@ -664,7 +672,7 @@ describe('encoding/decoding: wildcard routes/params', () => {
664672

665673
await router.load()
666674

667-
expect(router.state.location.url.endsWith('/%F0%9F%9A%80')).toBe(true)
675+
expect(router.state.location.url.href.endsWith('/%F0%9F%9A%80')).toBe(true)
668676
expect(router.state.location.href).toBe('/%F0%9F%9A%80')
669677
expect(router.state.location.pathname).toBe('/🚀')
670678
})
@@ -681,7 +689,7 @@ describe('encoding/decoding: wildcard routes/params', () => {
681689
await router.load()
682690

683691
expect(
684-
router.state.location.url.endsWith(
692+
router.state.location.url.href.endsWith(
685693
'/framework%2Freact%2Fguide%2Ffile-based-routing%20tanstack',
686694
),
687695
).toBe(true)
@@ -703,7 +711,7 @@ describe('encoding/decoding: wildcard routes/params', () => {
703711
await router.load()
704712

705713
expect(
706-
router.state.location.url.endsWith(
714+
router.state.location.url.href.endsWith(
707715
'/framework/react/guide/file-based-routing%20tanstack',
708716
),
709717
).toBe(true)

‎packages/router-core/src/location.ts‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -38,14 +38,14 @@ export interface ParsedLocation<TSearchObj extends AnySchema = {}> {
3838
unmaskOnReload?: boolean
3939
/**
4040
* @private
41-
* @description The public href of the location, including the origin before any rewrites.
41+
* @description The public href of the location.
4242
* If a rewrite is applied, the `href` property will be the rewritten URL.
4343
*/
4444
publicHref: string
4545
/**
4646
* @private
47-
* @description The full URL of the location, including the origin.
47+
* @description The full URL of the location.
4848
* @private
4949
*/
50-
url: string
50+
url: URL
5151
}

‎packages/router-core/src/router.ts‎

Lines changed: 27 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -1176,16 +1176,14 @@ export class RouterCore<
11761176

11771177
const fullPath = url.href.replace(url.origin, '')
11781178

1179-
const { pathname, hash } = url
1180-
11811179
return {
11821180
href: fullPath,
11831181
publicHref: href,
1184-
url: url.href,
1185-
pathname: decodePath(pathname),
1182+
url: url,
1183+
pathname: decodePath(url.pathname),
11861184
searchStr,
11871185
search: replaceEqualDeep(previousLocation?.search, parsedSearch) as any,
1188-
hash: hash.split('#').reverse()[0] ?? '',
1186+
hash: url.hash.split('#').reverse()[0] ?? '',
11891187
state: replaceEqualDeep(previousLocation?.state, state),
11901188
}
11911189
}
@@ -1765,7 +1763,7 @@ export class RouterCore<
17651763
publicHref:
17661764
rewrittenUrl.pathname + rewrittenUrl.search + rewrittenUrl.hash,
17671765
href: fullPath,
1768-
url: rewrittenUrl.href,
1766+
url: rewrittenUrl,
17691767
pathname: nextPathname,
17701768
search: nextSearch,
17711769
searchStr,
@@ -1878,8 +1876,16 @@ export class RouterCore<
18781876
if (isSameUrl && isSameState()) {
18791877
this.load()
18801878
} else {
1881-
// eslint-disable-next-line prefer-const
1882-
let { maskedLocation, hashScrollIntoView, ...nextHistory } = next
1879+
let {
1880+
// eslint-disable-next-line prefer-const
1881+
maskedLocation,
1882+
// eslint-disable-next-line prefer-const
1883+
hashScrollIntoView,
1884+
// don't pass url into history since it is a URL instance that cannot be serialized
1885+
// eslint-disable-next-line prefer-const
1886+
url: _url,
1887+
...nextHistory
1888+
} = next
18831889

18841890
if (maskedLocation) {
18851891
nextHistory = {
@@ -2000,7 +2006,7 @@ export class RouterCore<
20002006
if (reloadDocument) {
20012007
if (!href) {
20022008
const location = this.buildLocation({ to, ...rest } as any)
2003-
href = location.url
2009+
href = location.url.href
20042010
}
20052011

20062012
// Check blockers for external URLs unless ignoreBlocker is true
@@ -2056,24 +2062,11 @@ export class RouterCore<
20562062
_includeValidateSearch: true,
20572063
})
20582064

2059-
// Normalize URLs for comparison to handle encoding differences
2060-
// Browser history always stores encoded URLs while buildLocation may produce decoded URLs
2061-
const normalizeUrl = (url: string) => {
2062-
try {
2063-
return encodeURI(decodeURI(url))
2064-
} catch {
2065-
return url
2066-
}
2067-
}
2068-
20692065
if (
2070-
trimPath(normalizeUrl(this.latestLocation.href)) !==
2071-
trimPath(normalizeUrl(nextLocation.href))
2066+
this.latestLocation.publicHref !== nextLocation.publicHref ||
2067+
nextLocation.url.origin !== this.origin
20722068
) {
2073-
let href = nextLocation.url
2074-
if (this.origin && href.startsWith(this.origin)) {
2075-
href = href.replace(this.origin, '') || '/'
2076-
}
2069+
const href = this.getParsedLocationHref(nextLocation)
20772070

20782071
throw redirect({ href })
20792072
}
@@ -2395,13 +2388,18 @@ export class RouterCore<
23952388
return this.load({ sync: opts?.sync })
23962389
}
23972390

2391+
getParsedLocationHref = (location: ParsedLocation) => {
2392+
let href = location.url.href
2393+
if (this.origin && location.url.origin === this.origin) {
2394+
href = href.replace(this.origin, '') || '/'
2395+
}
2396+
return href
2397+
}
2398+
23982399
resolveRedirect = (redirect: AnyRedirect): AnyRedirect => {
23992400
if (!redirect.options.href) {
24002401
const location = this.buildLocation(redirect.options)
2401-
let href = location.url
2402-
if (this.origin && href.startsWith(this.origin)) {
2403-
href = href.replace(this.origin, '') || '/'
2404-
}
2402+
const href = this.getParsedLocationHref(location)
24052403
redirect.options.href = location.href
24062404
redirect.headers.set('Location', href)
24072405
}

‎packages/solid-router/src/link.tsx‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -139,9 +139,9 @@ export function useLinkProps<
139139
let href
140140
const maskedLocation = next().maskedLocation
141141
if (maskedLocation) {
142-
href = maskedLocation.url
142+
href = maskedLocation.url.href
143143
} else {
144-
href = next().url
144+
href = next().url.href
145145
}
146146
let external = false
147147
if (router.origin) {

‎packages/solid-router/tests/redirect.test.tsx‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -352,7 +352,7 @@ describe('redirect', () => {
352352
expect(currentRedirect.options).toEqual({
353353
_fromLocation: {
354354
publicHref: '/',
355-
url: 'http://localhost/',
355+
url: new URL('http://localhost/'),
356356
hash: '',
357357
href: '/',
358358
pathname: '/',

‎packages/vue-router/src/link.tsx‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -431,9 +431,9 @@ export function useLinkProps<
431431

432432
let hrefValue: string
433433
if (maskedLocation) {
434-
hrefValue = maskedLocation.url
434+
hrefValue = maskedLocation.url.href
435435
} else {
436-
hrefValue = nextLocation?.url
436+
hrefValue = nextLocation?.url.href
437437
}
438438

439439
// Handle origin stripping like Solid does

0 commit comments

Comments
 (0)