Skip to content

Commit

Permalink
fix: normalize-asset-prefix adding leading slash when URL `assetPre…
Browse files Browse the repository at this point in the history
…fix` is provided (#68518)

### Why?

URL validation of `normalizeAssetPrefix` was wrong as it checks if is
starting with `://`:


https:/vercel/next.js/blob/37df9ab243690187725a236cbf2debbcaeb33cfa/packages/next/src/shared/lib/normalized-asset-prefix.ts#L4-L7

This resulted the value to be `/https://example.com` instead of
`example.com`.

x-ref:
#68518 (comment)

### How?

As `normalizeAssetPrefix` function was added for `getSocketUrl`, it
removes the protocol of the URL.
But for reusability, this could be a friction to other callers.
Therefore we validate the URL and let the callers handle the URL (e.g.
protocol).
  • Loading branch information
devjiwonchoi committed Sep 12, 2024
1 parent 5999a6d commit 725c6da
Show file tree
Hide file tree
Showing 3 changed files with 63 additions and 16 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -8,19 +8,19 @@ function getSocketProtocol(assetPrefix: string): string {
protocol = new URL(assetPrefix).protocol
} catch {}

return protocol === 'http:' ? 'ws' : 'wss'
return protocol === 'http:' ? 'ws:' : 'wss:'
}

export function getSocketUrl(assetPrefix: string | undefined): string {
const { hostname, port } = window.location
const protocol = getSocketProtocol(assetPrefix || '')
const prefix = normalizedAssetPrefix(assetPrefix)
const protocol = getSocketProtocol(assetPrefix || '')

// if original assetPrefix is a full URL with protocol
// we just update to use the correct `ws` protocol
if (assetPrefix?.replace(/^\/+/, '').includes('://')) {
return `${protocol}://${prefix}`
if (URL.canParse(prefix)) {
// since normalized asset prefix is ensured to be a URL format,
// we can safely replace the protocol
return prefix.replace(/^http/, 'ws')
}

return `${protocol}://${hostname}:${port}${prefix}`
const { hostname, port } = window.location
return `${protocol}//${hostname}${port ? `:${port}` : ''}${prefix}`
}
44 changes: 44 additions & 0 deletions packages/next/src/shared/lib/normalized-asset-prefix.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
import { normalizedAssetPrefix } from './normalized-asset-prefix'

describe('normalizedAssetPrefix', () => {
it('should return an empty string when assetPrefix is nullish', () => {
expect(normalizedAssetPrefix(undefined)).toBe('')
})

it('should return an empty string when assetPrefix is an empty string', () => {
expect(normalizedAssetPrefix('')).toBe('')
})

it('should return an empty string when assetPrefix is a single slash', () => {
expect(normalizedAssetPrefix('/')).toBe('')
})

// we expect an empty string because it could be an unnecessary trailing slash
it('should remove leading slash(es) when assetPrefix has more than one', () => {
expect(normalizedAssetPrefix('///path/to/asset')).toBe('/path/to/asset')
})

it('should not remove the leading slash when assetPrefix has only one', () => {
expect(normalizedAssetPrefix('/path/to/asset')).toBe('/path/to/asset')
})

it('should add a leading slash when assetPrefix is missing one', () => {
expect(normalizedAssetPrefix('path/to/asset')).toBe('/path/to/asset')
})

it('should remove all trailing slash(es) when assetPrefix has one', () => {
expect(normalizedAssetPrefix('/path/to/asset///')).toBe('/path/to/asset')
})

it('should return the URL when assetPrefix is a URL', () => {
expect(normalizedAssetPrefix('https://example.com/path/to/asset')).toBe(
'https://example.com/path/to/asset'
)
})

it('should not leave a trailing slash when assetPrefix is a URL with no pathname', () => {
expect(normalizedAssetPrefix('https://example.com')).toBe(
'https://example.com'
)
})
})
19 changes: 11 additions & 8 deletions packages/next/src/shared/lib/normalized-asset-prefix.ts
Original file line number Diff line number Diff line change
@@ -1,16 +1,19 @@
export function normalizedAssetPrefix(assetPrefix: string | undefined): string {
const escapedAssetPrefix = assetPrefix?.replace(/^\/+/, '') || false
// remove all leading slashes and trailing slashes
const escapedAssetPrefix = assetPrefix?.replace(/^\/+|\/+$/g, '') || false

// assetPrefix as a url
if (escapedAssetPrefix && escapedAssetPrefix.startsWith('://')) {
return escapedAssetPrefix.split('://', 2)[1]
}

// assetPrefix is set to `undefined` or '/'
// if an assetPrefix was '/', we return empty string
// because it could be an unnecessary trailing slash
if (!escapedAssetPrefix) {
return ''
}

// assetPrefix is a common path but escaped so let's add one leading slash
if (URL.canParse(escapedAssetPrefix)) {
const url = new URL(escapedAssetPrefix).toString()
return url.endsWith('/') ? url.slice(0, -1) : url
}

// assuming assetPrefix here is a pathname-style,
// restore the leading slash
return `/${escapedAssetPrefix}`
}

0 comments on commit 725c6da

Please sign in to comment.