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

chore: join URL segments more safely #10590

Merged
merged 2 commits into from
Oct 27, 2022
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
10 changes: 8 additions & 2 deletions packages/vite/src/node/build.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,13 @@ import { isDepsOptimizerEnabled, resolveConfig } from './config'
import { buildReporterPlugin } from './plugins/reporter'
import { buildEsbuildPlugin } from './plugins/esbuild'
import { terserPlugin } from './plugins/terser'
import { copyDir, emptyDir, lookupFile, normalizePath } from './utils'
import {
copyDir,
emptyDir,
joinUrlSegments,
lookupFile,
normalizePath
} from './utils'
import { manifestPlugin } from './plugins/manifest'
import type { Logger } from './logger'
import { dataURIPlugin } from './plugins/dataUri'
Expand Down Expand Up @@ -1052,7 +1058,7 @@ export function toOutputFilePathInJS(
if (relative && !config.build.ssr) {
return toRelative(filename, hostId)
}
return config.base + filename
return joinUrlSegments(config.base, filename)
}

export function createToImportMetaURLBasedRelativeRuntime(
Expand Down
9 changes: 4 additions & 5 deletions packages/vite/src/node/plugins/asset.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ import {
} from '../build'
import type { Plugin } from '../plugin'
import type { ResolvedConfig } from '../config'
import { cleanUrl, getHash, normalizePath } from '../utils'
import { cleanUrl, getHash, joinUrlSegments, normalizePath } from '../utils'
import { FS_PREFIX } from '../constants'

export const assetUrlRE = /__VITE_ASSET__([a-z\d]{8})__(?:\$_(.*?)__)?/g
Expand Down Expand Up @@ -249,9 +249,8 @@ function fileToDevUrl(id: string, config: ResolvedConfig) {
// (this is special handled by the serve static middleware
rtn = path.posix.join(FS_PREFIX + id)
}
const origin = config.server?.origin ?? ''
const devBase = config.base
return origin + devBase + rtn.replace(/^\//, '')
const base = joinUrlSegments(config.server?.origin ?? '', config.base)
return joinUrlSegments(base, rtn.replace(/^\//, ''))
}

export function getAssetFilename(
Expand Down Expand Up @@ -396,7 +395,7 @@ export function publicFileToBuiltUrl(
): string {
if (config.command !== 'build') {
// We don't need relative base or renderBuiltUrl support during dev
return config.base + url.slice(1)
return joinUrlSegments(config.base, url)
}
const hash = getHash(url)
let cache = publicAssetUrlCache.get(config)
Expand Down
8 changes: 4 additions & 4 deletions packages/vite/src/node/plugins/css.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ import {
isDataUrl,
isExternalUrl,
isObject,
joinUrlSegments,
normalizePath,
parseRequest,
processSrcSet,
Expand Down Expand Up @@ -212,7 +213,7 @@ export function cssPlugin(config: ResolvedConfig): Plugin {
if (encodePublicUrlsInCSS(config)) {
return publicFileToBuiltUrl(url, config)
} else {
return config.base + url.slice(1)
return joinUrlSegments(config.base, url)
}
}
const resolved = await resolveUrl(url, importer)
Expand Down Expand Up @@ -251,7 +252,6 @@ export function cssPlugin(config: ResolvedConfig): Plugin {
// server only logic for handling CSS @import dependency hmr
const { moduleGraph } = server
const thisModule = moduleGraph.getModuleById(id)
const devBase = config.base
if (thisModule) {
// CSS modules cannot self-accept since it exports values
const isSelfAccepting =
Expand All @@ -260,6 +260,7 @@ export function cssPlugin(config: ResolvedConfig): Plugin {
// record deps in the module graph so edits to @import css can trigger
// main import to hot update
const depModules = new Set<string | ModuleNode>()
const devBase = config.base
for (const file of deps) {
depModules.add(
isCSSRequest(file)
Expand Down Expand Up @@ -389,10 +390,9 @@ export function cssPostPlugin(config: ResolvedConfig): Plugin {
}

const cssContent = await getContentWithSourcemap(css)
const devBase = config.base
const code = [
`import { updateStyle as __vite__updateStyle, removeStyle as __vite__removeStyle } from ${JSON.stringify(
path.posix.join(devBase, CLIENT_PUBLIC_PATH)
path.posix.join(config.base, CLIENT_PUBLIC_PATH)
)}`,
`const __vite__id = ${JSON.stringify(id)}`,
`const __vite__css = ${JSON.stringify(cssContent)}`,
Expand Down
7 changes: 3 additions & 4 deletions packages/vite/src/node/server/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -547,8 +547,7 @@ export async function createServer(
}

// base
const devBase = config.base
if (devBase !== '/') {
if (config.base !== '/') {
middlewares.use(baseMiddleware(server))
}

Expand Down Expand Up @@ -652,7 +651,6 @@ async function startServer(

const protocol = options.https ? 'https' : 'http'
const info = server.config.logger.info
const devBase = server.config.base

const serverPort = await httpServerStart(httpServer, {
port,
Expand Down Expand Up @@ -681,7 +679,8 @@ async function startServer(
}

if (options.open && !isRestart) {
const path = typeof options.open === 'string' ? options.open : devBase
const path =
typeof options.open === 'string' ? options.open : server.config.base
openBrowser(
path.startsWith('http')
? path
Expand Down
9 changes: 5 additions & 4 deletions packages/vite/src/node/server/middlewares/base.ts
Original file line number Diff line number Diff line change
@@ -1,12 +1,13 @@
import type { Connect } from 'dep-types/connect'
import type { ViteDevServer } from '..'
import { joinUrlSegments } from '../../utils'

// this middleware is only active when (config.base !== '/')

export function baseMiddleware({
config
}: ViteDevServer): Connect.NextHandleFunction {
const devBase = config.base
const devBase = config.base.endsWith('/') ? config.base : config.base + '/'

// Keep the named function. The name is visible in debug logs via `DEBUG=connect:dispatcher ...`
return function viteBaseMiddleware(req, res, next) {
Expand All @@ -29,18 +30,18 @@ export function baseMiddleware({
if (path === '/' || path === '/index.html') {
// redirect root visit to based url with search and hash
res.writeHead(302, {
Location: devBase + (parsed.search || '') + (parsed.hash || '')
Location: config.base + (parsed.search || '') + (parsed.hash || '')
})
res.end()
return
} else if (req.headers.accept?.includes('text/html')) {
// non-based page visit
const redirectPath = devBase + url.slice(1)
const redirectPath = joinUrlSegments(config.base, url)
res.writeHead(404, {
'Content-Type': 'text/html'
})
res.end(
`The server is configured with a public base URL of ${devBase} - ` +
`The server is configured with a public base URL of ${config.base} - ` +
`did you mean to visit <a href="${redirectPath}">${redirectPath}</a> instead?`
)
return
Expand Down
6 changes: 4 additions & 2 deletions packages/vite/src/node/server/middlewares/indexHtml.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import {
ensureWatchedFile,
fsPathFromId,
injectQuery,
joinUrlSegments,
normalizePath,
processSrcSetSync,
wrapId
Expand Down Expand Up @@ -93,7 +94,8 @@ const processNodeUrl = (
const devBase = config.base
if (startsWithSingleSlashRE.test(url)) {
// prefix with base (dev only, base is never relative)
overwriteAttrValue(s, sourceCodeLocation, devBase + url.slice(1))
const fullUrl = joinUrlSegments(devBase, url)
overwriteAttrValue(s, sourceCodeLocation, fullUrl)
} else if (
url.startsWith('.') &&
originalUrl &&
Expand Down Expand Up @@ -132,7 +134,7 @@ const devHtmlHook: IndexHtmlTransformHook = async (
const trailingSlash = htmlPath.endsWith('/')
if (!trailingSlash && fs.existsSync(filename)) {
proxyModulePath = htmlPath
proxyModuleUrl = base + htmlPath.slice(1)
proxyModuleUrl = joinUrlSegments(base, htmlPath)
} else {
// There are users of vite.transformIndexHtml calling it with url '/'
// for SSR integrations #7993, filename is root for this case
Expand Down
10 changes: 5 additions & 5 deletions packages/vite/src/node/ssr/ssrManifestPlugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import type { OutputChunk } from 'rollup'
import type { ResolvedConfig } from '..'
import type { Plugin } from '../plugin'
import { preloadMethod } from '../plugins/importAnalysisBuild'
import { normalizePath } from '../utils'
import { joinUrlSegments, normalizePath } from '../utils'

export function ssrManifestPlugin(config: ResolvedConfig): Plugin {
// module id => preload assets mapping
Expand All @@ -23,15 +23,15 @@ export function ssrManifestPlugin(config: ResolvedConfig): Plugin {
const mappedChunks =
ssrManifest[normalizedId] ?? (ssrManifest[normalizedId] = [])
if (!chunk.isEntry) {
mappedChunks.push(base + chunk.fileName)
mappedChunks.push(joinUrlSegments(base, chunk.fileName))
// <link> tags for entry chunks are already generated in static HTML,
// so we only need to record info for non-entry chunks.
chunk.viteMetadata.importedCss.forEach((file) => {
mappedChunks.push(base + file)
mappedChunks.push(joinUrlSegments(base, file))
})
}
chunk.viteMetadata.importedAssets.forEach((file) => {
mappedChunks.push(base + file)
mappedChunks.push(joinUrlSegments(base, file))
})
}
if (chunk.code.includes(preloadMethod)) {
Expand Down Expand Up @@ -59,7 +59,7 @@ export function ssrManifestPlugin(config: ResolvedConfig): Plugin {
const chunk = bundle[filename] as OutputChunk | undefined
if (chunk) {
chunk.viteMetadata.importedCss.forEach((file) => {
deps.push(join(base, file)) // TODO:base
deps.push(joinUrlSegments(base, file)) // TODO:base
})
chunk.imports.forEach(addDeps)
}
Expand Down
13 changes: 13 additions & 0 deletions packages/vite/src/node/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1194,3 +1194,16 @@ export const isNonDriveRelativeAbsolutePath = (p: string): boolean => {
if (!isWindows) return p.startsWith('/')
return windowsDrivePathPrefixRE.test(p)
}

export function joinUrlSegments(a: string, b: string): string {
if (!a || !b) {
return a + b
benmccann marked this conversation as resolved.
Show resolved Hide resolved
}
if (a.endsWith('/')) {
a = a.substring(0, a.length - 1)
}
if (!b.startsWith('/')) {
b = '/' + b
}
return a + b
}