-
Notifications
You must be signed in to change notification settings - Fork 27.7k
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
fix:
normalize-asset-prefix
adding leading slash when URL `assetPre…
…fix` is provided (#68518) ### Why? URL validation of `normalizeAssetPrefix` was wrong as it checks if is starting with `://`: https://github.com/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
1 parent
f17f570
commit a5e2a02
Showing
3 changed files
with
63 additions
and
16 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
44 changes: 44 additions & 0 deletions
44
packages/next/src/shared/lib/normalized-asset-prefix.test.ts
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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' | ||
) | ||
}) | ||
}) |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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}` | ||
} |