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 all commits
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(config.decodedBase, 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 @@ -1226,7 +1226,7 @@ export function toOutputFilePathInJS(
if (relative && !config.build.ssr) {
return toRelative(filename, hostId)
}
return joinUrlSegments(config.base, filename)
return joinUrlSegments(config.decodedBase, filename)
}

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

Expand Down
7 changes: 6 additions & 1 deletion packages/vite/src/node/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -365,6 +365,8 @@ export type ResolvedConfig = Readonly<
root: string
base: string
/** @internal */
decodedBase: string
/** @internal */
rawBase: string
publicDir: string
cacheDir: string
Expand Down Expand Up @@ -763,14 +765,17 @@ export async function resolveConfig(
rollupOptions: config.worker?.rollupOptions || {},
}

const base = withTrailingSlash(resolvedBase)

resolved = {
configFile: configFile ? normalizePath(configFile) : undefined,
configFileDependencies: configFileDependencies.map((name) =>
normalizePath(path.resolve(name)),
),
inlineConfig,
root: resolvedRoot,
base: withTrailingSlash(resolvedBase),
base,
decodedBase: decodeURI(base),
rawBase: resolvedBase,
resolve: resolveOptions,
publicDir: resolvedPublicDir,
Expand Down
4 changes: 2 additions & 2 deletions packages/vite/src/node/plugins/asset.ts
Original file line number Diff line number Diff line change
Expand Up @@ -281,7 +281,7 @@ 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 ?? '', config.decodedBase)
return joinUrlSegments(base, removeLeadingSlash(rtn))
}

Expand All @@ -306,7 +306,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(config.decodedBase, url)
}
const hash = getHash(url)
let cache = publicAssetUrlCache.get(config)
Expand Down
30 changes: 19 additions & 11 deletions packages/vite/src/node/server/middlewares/indexHtml.ts
Original file line number Diff line number Diff line change
Expand Up @@ -167,7 +167,13 @@ const processNodeUrl = (
)
}
if (preTransformUrl) {
preTransformRequest(server, preTransformUrl, config.base)
try {
preTransformUrl = decodeURI(preTransformUrl)
} catch (err) {
// Malformed uri. Skip pre-transform.
return url
}
preTransformRequest(server, preTransformUrl, config.decodedBase)
}
}
return url
Expand All @@ -184,6 +190,7 @@ const devHtmlHook: IndexHtmlTransformHook = async (
) => {
const { config, moduleGraph, watcher } = server!
const base = config.base || '/'
const decodedBase = config.decodedBase || '/'

let proxyModulePath: string
let proxyModuleUrl: string
Expand All @@ -202,7 +209,7 @@ const devHtmlHook: IndexHtmlTransformHook = async (
proxyModulePath = `\0${validPath}`
proxyModuleUrl = wrapId(proxyModulePath)
}
proxyModuleUrl = joinUrlSegments(base, proxyModuleUrl)
proxyModuleUrl = joinUrlSegments(decodedBase, proxyModuleUrl)

const s = new MagicString(html)
let inlineModuleIndex = -1
Expand Down Expand Up @@ -252,7 +259,7 @@ const devHtmlHook: IndexHtmlTransformHook = async (
node.sourceCodeLocation!.endOffset,
`<script type="module" src="${modulePath}"></script>`,
)
preTransformRequest(server!, modulePath, base)
preTransformRequest(server!, modulePath, decodedBase)
}

await traverseHtml(html, filename, (node) => {
Expand Down Expand Up @@ -447,15 +454,16 @@ export function indexHtmlMiddleware(
}
}

function preTransformRequest(server: ViteDevServer, url: string, base: string) {
// NOTE: We usually don't prefix `url` and `base` with `decoded`, but in this file particularly
// we're dealing with mixed encoded/decoded paths often, so we make this explicit for now.
function preTransformRequest(
server: ViteDevServer,
decodedUrl: string,
decodedBase: string,
) {
if (!server.config.server.preTransformRequests) return

// transform all url as non-ssr as html includes client-side assets only
try {
url = unwrapId(stripBase(decodeURI(url), base))
} catch {
// ignore
return
}
server.warmupRequest(url)
decodedUrl = unwrapId(stripBase(decodedUrl, decodedBase))
server.warmupRequest(decodedUrl)
}
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
Loading