Skip to content

Commit

Permalink
fix: support Base URLs with trailing slash (fix #1258)
Browse files Browse the repository at this point in the history
  • Loading branch information
brillout committed Nov 15, 2023
1 parent 0a02be6 commit 701e2e5
Show file tree
Hide file tree
Showing 3 changed files with 43 additions and 22 deletions.
4 changes: 2 additions & 2 deletions vike/node/runtime/renderPage.ts
Original file line number Diff line number Diff line change
Expand Up @@ -470,10 +470,10 @@ function skipRequest(urlOriginal: string): boolean {
)
}
function normalizeUrl(pageContextInit: { urlOriginal: string }, httpRequestId: number) {
const { trailingSlash, disableUrlNormalization } = getGlobalContext()
const { trailingSlash, disableUrlNormalization, baseServer } = getGlobalContext()
if (disableUrlNormalization) return null
const { urlOriginal } = pageContextInit
const urlNormalized = normalizeUrlPathname(urlOriginal, trailingSlash)
const urlNormalized = normalizeUrlPathname(urlOriginal, trailingSlash, baseServer)
if (!urlNormalized) return null
logRuntimeInfo?.(
`URL normalized from ${pc.cyan(urlOriginal)} to ${pc.cyan(urlNormalized)} (https://vike.dev/url-normalization)`,
Expand Down
38 changes: 25 additions & 13 deletions vike/utils/parseUrl-extras.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,20 +3,32 @@ import { expect, describe, it } from 'vitest'

describe('normalizeUrlPathname()', () => {
it('works', () => {
expect(normalizeUrlPathname('/bla/', false)).toBe('/bla')
expect(normalizeUrlPathname('/bla', false)).toBe(null)
expect(normalizeUrlPathname('/bla///', false)).toBe('/bla')
expect(normalizeUrlPathname('/bla///foo//', false)).toBe('/bla/foo')
expect(normalizeUrlPathname('/', false)).toBe(null)
expect(normalizeUrlPathname('//', false)).toBe('/')
expect(normalizeUrlPathname('/////', false)).toBe('/')
expect(normalizeUrlPathname('https://example.org/p/', false)).toBe('https://example.org/p')
expect(normalizeUrlPathname('/p/?foo=bar#bla', false)).toBe('/p?foo=bar#bla')
expect(normalizeUrlPathname('////?foo=bar#bla', false)).toBe('/?foo=bar#bla')
expect(normalizeUrlPathname('https://example.org/some-url/?foo=bar#bla', false)).toBe(
'https://example.org/some-url?foo=bar#bla'
)
expect(n('/bla/')).toBe('/bla')
expect(n('/bla')).toBe(null)
expect(n('/bla///')).toBe('/bla')
expect(n('/bla///foo//')).toBe('/bla/foo')
expect(n('/')).toBe(null)
expect(n('//')).toBe('/')
expect(n('/////')).toBe('/')
expect(n('https://example.org/p/')).toBe('https://example.org/p')
expect(n('/p/?foo=bar#bla')).toBe('/p?foo=bar#bla')
expect(n('////?foo=bar#bla')).toBe('/?foo=bar#bla')
expect(n('https://example.org/some-url/?foo=bar#bla')).toBe('https://example.org/some-url?foo=bar#bla')
})
it('Base URL with trailing slash', () => {
// Adding a trailing slash is needed because of Vite, see comment in source code of normalizeUrlPathname()
expect(normalizeUrlPathname('/foo', false, '/foo/')).toBe('/foo/')
expect(normalizeUrlPathname('/foo', true, '/foo/')).toBe('/foo/')
expect(normalizeUrlPathname('/foo/', false, '/foo/')).toBe(null)
expect(normalizeUrlPathname('/foo/', true, '/foo/')).toBe(null)
// It's fine if URL has trailing slash but not the Base URL
expect(normalizeUrlPathname('/foo/', true, '/foo')).toBe(null)
// Works as usual
expect(normalizeUrlPathname('/foo/', false, '/foo')).toBe('/foo')
})
function n(urlOriginal: string) {
return normalizeUrlPathname(urlOriginal, false, '/')
}
})

describe('removeUrlOrigin() / addUrlOrigin()', () => {
Expand Down
23 changes: 16 additions & 7 deletions vike/utils/parseUrl-extras.ts
Original file line number Diff line number Diff line change
Expand Up @@ -62,21 +62,30 @@ function isBaseAssets(base: string): boolean {
return base.startsWith('/') || base.startsWith('http://') || base.startsWith('https://')
}

function normalizeUrlPathname(urlOriginal: string, trailingSlash: boolean): string | null {
function normalizeUrlPathname(urlOriginal: string, trailingSlash: boolean, baseServer: string): string | null {
const urlNormalized = modifyUrlPathname(urlOriginal, (urlPathname) => {
assert(urlPathname.startsWith('/'))
let urlPathnameNormalized = '/' + urlPathname.split('/').filter(Boolean).join('/')
if (urlPathnameNormalized !== '/') {
assert(!urlPathnameNormalized.endsWith('/'))
if (trailingSlash) {
urlPathnameNormalized = urlPathnameNormalized + '/'
}
let urlPathnameNormalized = normalize(urlPathname)
if (urlPathnameNormalized === '/') {
return urlPathnameNormalized
}
// If the Base URL has a trailing slash, then Vite (as of [email protected]) expects the root URL to also have a trailing slash, see https://github.com/vikejs/vike/issues/1258#issuecomment-1812226260
if (baseServer.endsWith('/') && baseServer !== '/' && normalize(baseServer) === urlPathnameNormalized) {
trailingSlash = true
}
assert(!urlPathnameNormalized.endsWith('/'))
if (trailingSlash) {
urlPathnameNormalized = urlPathnameNormalized + '/'
}
return urlPathnameNormalized
})
if (urlNormalized === urlOriginal) return null
return urlNormalized
}
function normalize(urlPathname: string): string {
assert(urlPathname.startsWith('/'))
return '/' + urlPathname.split('/').filter(Boolean).join('/')
}

function modifyUrlPathname(url: string, modifier: (urlPathname: string) => string | null): string {
const { origin, pathnameOriginal, searchOriginal, hashOriginal } = parseUrl(url, '/')
Expand Down

0 comments on commit 701e2e5

Please sign in to comment.