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: handle encoded base paths #17577

Merged
merged 7 commits into from
Aug 1, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
14 changes: 13 additions & 1 deletion packages/plugin-legacy/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ function toOutputFilePathInHtml(
if (relative && !config.build.ssr) {
return toRelative(filename, hostId)
} else {
return config.base + filename
return joinUrlSegments(decodeURI(config.base), filename)
}
}
function getBaseInHTML(urlRelativePath: string, config: ResolvedConfig) {
Expand All @@ -96,6 +96,18 @@ function getBaseInHTML(urlRelativePath: string, config: ResolvedConfig) {
)
: config.base
}
function joinUrlSegments(a: string, b: string): string {
if (!a || !b) {
return a || b || ''
}
if (a[a.length - 1] === '/') {
a = a.substring(0, a.length - 1)
}
if (b[0] !== '/') {
b = '/' + b
}
return a + b
}

function toAssetPathFromHtml(
filename: string,
Expand Down
4 changes: 2 additions & 2 deletions packages/vite/src/node/build.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1223,7 +1223,7 @@ export function toOutputFilePathInJS(
if (relative && !config.build.ssr) {
return toRelative(filename, hostId)
}
return joinUrlSegments(config.base, filename)
return joinUrlSegments(decodeURI(config.base), filename)
}

export function createToImportMetaURLBasedRelativeRuntime(
Expand Down Expand Up @@ -1272,7 +1272,7 @@ export function toOutputFilePathWithoutRuntime(
if (relative && !config.build.ssr) {
return toRelative(filename, hostId)
} else {
return joinUrlSegments(config.base, filename)
return joinUrlSegments(decodeURI(config.base), filename)
}
}

Expand Down
7 changes: 5 additions & 2 deletions packages/vite/src/node/plugins/asset.ts
Original file line number Diff line number Diff line change
Expand Up @@ -291,7 +291,10 @@ function fileToDevUrl(id: string, config: ResolvedConfig) {
// (this is special handled by the serve static middleware
rtn = path.posix.join(FS_PREFIX, id)
}
const base = joinUrlSegments(config.server?.origin ?? '', config.base)
const base = joinUrlSegments(
config.server?.origin ?? '',
decodeURI(config.base),
)
return joinUrlSegments(base, removeLeadingSlash(rtn))
}

Expand All @@ -316,7 +319,7 @@ export function publicFileToBuiltUrl(
): string {
if (config.command !== 'build') {
// We don't need relative base or renderBuiltUrl support during dev
return joinUrlSegments(config.base, url)
return joinUrlSegments(decodeURI(config.base), url)
}
const hash = getHash(url)
let cache = publicAssetUrlCache.get(config)
Expand Down
229 changes: 229 additions & 0 deletions playground/assets/__tests__/encoded-base/assets-encoded-base.spec.ts
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This file is largely a copy of the other test files.

Original file line number Diff line number Diff line change
@@ -0,0 +1,229 @@
import { beforeAll, describe, expect, test } from 'vitest'
import {
browserLogs,
findAssetFile,
getBg,
getColor,
isBuild,
page,
} from '~utils'

const urlAssetMatch = isBuild
? /\/foo%20bar\/other-assets\/asset-[-\w]{8}\.png/
: '/nested/asset.png'

const iconMatch = '/icon.png'

const absoluteIconMatch = isBuild
? /\/foo%20bar\/.*\/icon-[-\w]{8}\.png/
: '/nested/icon.png'

const absolutePublicIconMatch = isBuild ? /\/foo%20bar\/icon\.png/ : '/icon.png'

test('should have no 404s', () => {
browserLogs.forEach((msg) => {
expect(msg).not.toMatch('404')
})
})

describe('raw references from /public', () => {
test('load raw js from /public', async () => {
expect(await page.textContent('.raw-js')).toMatch('[success]')
})

test('load raw css from /public', async () => {
expect(await getColor('.raw-css')).toBe('red')
})
})

test('import-expression from simple script', async () => {
expect(await page.textContent('.import-expression')).toMatch(
'[success][success]',
)
})

describe('asset imports from js', () => {
test('relative', async () => {
expect(await page.textContent('.asset-import-relative')).toMatch(
urlAssetMatch,
)
})

test('absolute', async () => {
expect(await page.textContent('.asset-import-absolute')).toMatch(
urlAssetMatch,
)
})

test('from /public', async () => {
expect(await page.textContent('.public-import')).toMatch(
absolutePublicIconMatch,
)
})
})

describe('css url() references', () => {
test('fonts', async () => {
expect(
await page.evaluate(() => document.fonts.check('700 32px Inter')),
).toBe(true)
})

test('relative', async () => {
const bg = await getBg('.css-url-relative')
expect(bg).toMatch(urlAssetMatch)
})

test('image-set relative', async () => {
const imageSet = await getBg('.css-image-set-relative')
imageSet.split(', ').forEach((s) => {
expect(s).toMatch(urlAssetMatch)
})
})

test('image-set without the url() call', async () => {
const imageSet = await getBg('.css-image-set-without-url-call')
imageSet.split(', ').forEach((s) => {
expect(s).toMatch(urlAssetMatch)
})
})

test('image-set with var', async () => {
const imageSet = await getBg('.css-image-set-with-var')
imageSet.split(', ').forEach((s) => {
expect(s).toMatch(urlAssetMatch)
})
})

test('image-set with mix', async () => {
const imageSet = await getBg('.css-image-set-mix-url-var')
imageSet.split(', ').forEach((s) => {
expect(s).toMatch(urlAssetMatch)
})
})

test('relative in @import', async () => {
expect(await getBg('.css-url-relative-at-imported')).toMatch(urlAssetMatch)
})

test('absolute', async () => {
expect(await getBg('.css-url-absolute')).toMatch(urlAssetMatch)
})

test('from /public', async () => {
expect(await getBg('.css-url-public')).toMatch(iconMatch)
})

test('multiple urls on the same line', async () => {
const bg = await getBg('.css-url-same-line')
expect(bg).toMatch(urlAssetMatch)
expect(bg).toMatch(iconMatch)
})

test('aliased', async () => {
const bg = await getBg('.css-url-aliased')
expect(bg).toMatch(urlAssetMatch)
})
})

describe.runIf(isBuild)('index.css URLs', () => {
let css: string
beforeAll(() => {
css = findAssetFile(/index.*\.css$/, 'encoded-base', 'other-assets')
})

test('use base URL for asset URL', () => {
expect(css).toMatch(urlAssetMatch)
})

test('preserve postfix query/hash', () => {
expect(css).toMatch('woff2?#iefix')
})
})

describe('image', () => {
test('srcset', async () => {
const img = await page.$('.img-src-set')
const srcset = await img.getAttribute('srcset')
srcset.split(', ').forEach((s) => {
expect(s).toMatch(
isBuild
? /\/foo%20bar\/other-assets\/asset-[-\w]{8}\.png \dx/
: /\/foo%20bar\/nested\/asset\.png \dx/,
)
})
})
})

describe('svg fragments', () => {
// 404 is checked already, so here we just ensure the urls end with #fragment
test('img url', async () => {
const img = await page.$('.svg-frag-img')
expect(await img.getAttribute('src')).toMatch(/svg#icon-clock-view$/)
})

test('via css url()', async () => {
const bg = await page.evaluate(
() => getComputedStyle(document.querySelector('.icon')).backgroundImage,
)
expect(bg).toMatch(/svg#icon-clock-view"\)$/)
})

test('from js import', async () => {
const img = await page.$('.svg-frag-import')
expect(await img.getAttribute('src')).toMatch(/svg#icon-heart-view$/)
})
})

test('?raw import', async () => {
expect(await page.textContent('.raw')).toMatch('SVG')
})

test('?url import', async () => {
expect(await page.textContent('.url')).toMatch(
isBuild ? /\/foo%20bar\/other-assets\/foo-[-\w]{8}\.js/ : '/foo.js',
)
})

test('?url import on css', async () => {
const txt = await page.textContent('.url-css')
expect(txt).toMatch(
isBuild
? /\/foo%20bar\/other-assets\/icons-[-\w]{8}\.css/
: '/css/icons.css',
)
})

test('new URL(..., import.meta.url)', async () => {
expect(await page.textContent('.import-meta-url')).toMatch(urlAssetMatch)
})

test('new URL(`${dynamic}`, import.meta.url)', async () => {
const dynamic1 = await page.textContent('.dynamic-import-meta-url-1')
expect(dynamic1).toMatch(absoluteIconMatch)
const dynamic2 = await page.textContent('.dynamic-import-meta-url-2')
expect(dynamic2).toMatch(urlAssetMatch)
})

test('new URL(`non-existent`, import.meta.url)', async () => {
expect(await page.textContent('.non-existent-import-meta-url')).toMatch(
'/non-existent',
)
})

test('inline style test', async () => {
expect(await getBg('.inline-style')).toMatch(urlAssetMatch)
expect(await getBg('.style-url-assets')).toMatch(urlAssetMatch)
})

test('html import word boundary', async () => {
expect(await page.textContent('.obj-import-express')).toMatch(
'ignore object import prop',
)
expect(await page.textContent('.string-import-express')).toMatch('no load')
})

test('relative path in html asset', async () => {
expect(await page.textContent('.relative-js')).toMatch('hello')
expect(await getColor('.relative-css')).toMatch('red')
})
3 changes: 3 additions & 0 deletions playground/assets/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,9 @@
"dev": "vite",
"build": "vite build",
"preview": "vite preview",
"dev:encoded-base": "vite --config ./vite.config-encoded-base.js dev",
"build:encoded-base": "vite --config ./vite.config-encoded-base.js build",
"preview:encoded-base": "vite --config ./vite.config-encoded-base.js preview",
"dev:relative-base": "vite --config ./vite.config-relative-base.js dev",
"build:relative-base": "vite --config ./vite.config-relative-base.js build",
"preview:relative-base": "vite --config ./vite.config-relative-base.js preview",
Expand Down
34 changes: 34 additions & 0 deletions playground/assets/vite.config-encoded-base.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
import { defineConfig } from 'vite'
import baseConfig from './vite.config.js'

/** see `ports` variable in test-utils.ts */
const port = 9524

export default defineConfig({
...baseConfig,
// Vite should auto-encode this as `/foo%20bar/` internally
base: '/foo bar/',
server: {
port,
strictPort: true,
},
build: {
...baseConfig.build,
outDir: 'dist/encoded-base',
watch: null,
minify: false,
assetsInlineLimit: 0,
rollupOptions: {
output: {
entryFileNames: 'entries/[name].js',
chunkFileNames: 'chunks/[name]-[hash].js',
assetFileNames: 'other-assets/[name]-[hash][extname]',
},
},
},
preview: {
port,
strictPort: true,
},
cacheDir: 'node_modules/.vite-encoded-base',
})
2 changes: 1 addition & 1 deletion playground/assets/vite.config-url-base.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import { defineConfig } from 'vite'
import baseConfig from './vite.config.js'

/** see `ports` variable in test-utils.ts */
const port = 9524
const port = 9525

export default defineConfig({
...baseConfig,
Expand Down
3 changes: 2 additions & 1 deletion playground/test-utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,8 @@ export const ports = {
lib: 9521,
'optimize-missing-deps': 9522,
'legacy/client-and-ssr': 9523,
'assets/url-base': 9524, // not imported but used in `assets/vite.config-url-base.js`
'assets/encoded-base': 9554, // not imported but used in `assets/vite.config-encoded-base.js`
'assets/url-base': 9525, // not imported but used in `assets/vite.config-url-base.js`
ssr: 9600,
'ssr-deps': 9601,
'ssr-html': 9602,
Expand Down