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(vite): precisely check if files are in dirs #14241

Merged
merged 6 commits into from
Sep 3, 2023
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
9 changes: 7 additions & 2 deletions packages/vite/src/node/build.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ import {
joinUrlSegments,
normalizePath,
requireResolveFromRootWithFallback,
withTrailingSlash,
} from './utils'
import { manifestPlugin } from './plugins/manifest'
import type { Logger } from './logger'
Expand Down Expand Up @@ -712,7 +713,7 @@ function prepareOutDir(
for (const outDir of nonDuplicateDirs) {
if (
fs.existsSync(outDir) &&
!normalizePath(outDir).startsWith(config.root + '/')
!normalizePath(outDir).startsWith(withTrailingSlash(config.root))
) {
// warn if outDir is outside of root
config.logger.warn(
Expand Down Expand Up @@ -1238,5 +1239,9 @@ export const toOutputFilePathInHtml = toOutputFilePathWithoutRuntime
function areSeparateFolders(a: string, b: string) {
const na = normalizePath(a)
const nb = normalizePath(b)
return na !== nb && !na.startsWith(nb + '/') && !nb.startsWith(na + '/')
return (
na !== nb &&
!na.startsWith(withTrailingSlash(nb)) &&
!nb.startsWith(withTrailingSlash(na))
)
}
5 changes: 3 additions & 2 deletions packages/vite/src/node/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ import {
mergeConfig,
normalizeAlias,
normalizePath,
withTrailingSlash,
} from './utils'
import {
createPluginHookUtils,
Expand Down Expand Up @@ -680,7 +681,7 @@ export async function resolveConfig(
),
inlineConfig,
root: resolvedRoot,
base: resolvedBase.endsWith('/') ? resolvedBase : resolvedBase + '/',
base: withTrailingSlash(resolvedBase),
rawBase: resolvedBase,
resolve: resolveOptions,
publicDir: resolvedPublicDir,
Expand Down Expand Up @@ -856,7 +857,7 @@ assetFileNames isn't equal for every build.rollupOptions.output. A single patter
) {
resolved.logger.warn(
colors.yellow(`
(!) Experimental legacy.buildSsrCjsExternalHeuristics and ssr.format: 'cjs' are going to be removed in Vite 5.
(!) Experimental legacy.buildSsrCjsExternalHeuristics and ssr.format: 'cjs' are going to be removed in Vite 5.
Find more information and give feedback at https://github.com/vitejs/vite/discussions/13816.
`),
)
Expand Down
9 changes: 7 additions & 2 deletions packages/vite/src/node/plugins/asset.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import {
joinUrlSegments,
normalizePath,
removeLeadingSlash,
withTrailingSlash,
} from '../utils'
import { FS_PREFIX } from '../constants'

Expand Down Expand Up @@ -239,7 +240,11 @@ export function checkPublicFile(
return
}
const publicFile = path.join(publicDir, cleanUrl(url))
if (!publicFile.startsWith(publicDir)) {
if (
!normalizePath(publicFile).startsWith(
withTrailingSlash(normalizePath(publicDir)),
)
) {
// can happen if URL starts with '../'
return
}
Expand Down Expand Up @@ -267,7 +272,7 @@ function fileToDevUrl(id: string, config: ResolvedConfig) {
if (checkPublicFile(id, config)) {
// in public dir during dev, keep the url as-is
rtn = id
} else if (id.startsWith(config.root)) {
} else if (id.startsWith(withTrailingSlash(config.root))) {
// in project root, infer short public path
rtn = '/' + path.posix.relative(config.root, id)
} else {
Expand Down
5 changes: 3 additions & 2 deletions packages/vite/src/node/plugins/importAnalysis.ts
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ import {
timeFrom,
transformStableResult,
unwrapId,
withTrailingSlash,
wrapId,
} from '../utils'
import { getDepOptimizationConfig } from '../config'
Expand Down Expand Up @@ -335,7 +336,7 @@ export function importAnalysisPlugin(config: ResolvedConfig): Plugin {

// normalize all imports into resolved URLs
// e.g. `import 'foo'` -> `import '/@fs/.../node_modules/foo/index.js'`
if (resolved.id.startsWith(root + '/')) {
if (resolved.id.startsWith(withTrailingSlash(root))) {
// in root: infer short absolute path from root
url = resolved.id.slice(root.length)
} else if (
Expand Down Expand Up @@ -672,7 +673,7 @@ export function importAnalysisPlugin(config: ResolvedConfig): Plugin {
config.logger.error(e.message, { error: e })
})
}
} else if (!importer.startsWith(clientDir)) {
} else if (!importer.startsWith(withTrailingSlash(clientDir))) {
if (!isInNodeModules(importer)) {
// check @vite-ignore which suppresses dynamic import warning
const hasViteIgnore = hasViteIgnoreRE.test(
Expand Down
3 changes: 2 additions & 1 deletion packages/vite/src/node/plugins/importAnalysisBuild.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import {
isInNodeModules,
moduleListContains,
numberToPos,
withTrailingSlash,
} from '../utils'
import type { Plugin } from '../plugin'
import { getDepOptimizationConfig } from '../config'
Expand Down Expand Up @@ -271,7 +272,7 @@ export function buildImportAnalysisPlugin(config: ResolvedConfig): Plugin {

// normalize all imports into resolved URLs
// e.g. `import 'foo'` -> `import '/@fs/.../node_modules/foo/index.js'`
if (resolved.id.startsWith(root + '/')) {
if (resolved.id.startsWith(withTrailingSlash(root))) {
// in root: infer short absolute path from root
url = resolved.id.slice(root.length)
} else {
Expand Down
3 changes: 2 additions & 1 deletion packages/vite/src/node/plugins/preAlias.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import {
isInNodeModules,
isOptimizable,
moduleListContains,
withTrailingSlash,
} from '../utils'
import { getDepsOptimizer } from '../optimizer'
import { tryOptimizedResolve } from './resolve'
Expand Down Expand Up @@ -114,7 +115,7 @@ function matches(pattern: string | RegExp, importee: string) {
if (importee === pattern) {
return true
}
return importee.startsWith(pattern + '/')
return importee.startsWith(withTrailingSlash(pattern))
}

function getAliasPatterns(
Expand Down
12 changes: 9 additions & 3 deletions packages/vite/src/node/plugins/reporter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,12 @@ import { promisify } from 'node:util'
import colors from 'picocolors'
import type { Plugin } from 'rollup'
import type { ResolvedConfig } from '../config'
import { isDefined, isInNodeModules, normalizePath } from '../utils'
import {
isDefined,
isInNodeModules,
normalizePath,
withTrailingSlash,
} from '../utils'
import { LogLevels } from '../logger'

const groups = [
Expand Down Expand Up @@ -243,9 +248,10 @@ export function buildReporterPlugin(config: ResolvedConfig): Plugin {
group.name === 'JS' && entry.size / 1000 > chunkLimit
if (isLarge) hasLargeChunks = true
const sizeColor = isLarge ? colors.yellow : colors.dim
let log = colors.dim(relativeOutDir + '/')
let log = colors.dim(withTrailingSlash(relativeOutDir))
log +=
!config.build.lib && entry.name.startsWith(assetsDir)
!config.build.lib &&
entry.name.startsWith(withTrailingSlash(assetsDir))
? colors.dim(assetsDir) +
group.color(
entry.name
Expand Down
9 changes: 7 additions & 2 deletions packages/vite/src/node/plugins/resolve.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ import {
safeRealpathSync,
slash,
tryStatSync,
withTrailingSlash,
} from '../utils'
import { optimizedDepInfoFromFile, optimizedDepInfoFromId } from '../optimizer'
import type { DepsOptimizer } from '../optimizer'
Expand Down Expand Up @@ -228,7 +229,11 @@ export function resolvePlugin(resolveOptions: InternalResolveOptions): Plugin {

// URL
// /foo -> /fs-root/foo
if (asSrc && id[0] === '/' && (rootInRoot || !id.startsWith(root))) {
if (
asSrc &&
id[0] === '/' &&
(rootInRoot || !id.startsWith(withTrailingSlash(root)))
) {
const fsPath = path.resolve(root, id.slice(1))
if ((res = tryFsResolve(fsPath, options))) {
debug?.(`[url] ${colors.cyan(id)} -> ${colors.dim(res)}`)
Expand Down Expand Up @@ -939,7 +944,7 @@ export async function tryOptimizedResolve(
}

// match by src to correctly identify if id belongs to nested dependency
if (optimizedData.src.startsWith(idPkgDir)) {
if (optimizedData.src.startsWith(withTrailingSlash(idPkgDir))) {
return depsOptimizer.getOptimizedDepId(optimizedData)
}
}
Expand Down
14 changes: 11 additions & 3 deletions packages/vite/src/node/server/hmr.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,13 @@ import colors from 'picocolors'
import type { Update } from 'types/hmrPayload'
import type { RollupError } from 'rollup'
import { CLIENT_DIR } from '../constants'
import { createDebugger, normalizePath, unique, wrapId } from '../utils'
import {
createDebugger,
normalizePath,
unique,
withTrailingSlash,
wrapId,
} from '../utils'
import type { ViteDevServer } from '..'
import { isCSSRequest } from '../plugins/css'
import { getAffectedGlobModules } from '../plugins/importMetaGlob'
Expand Down Expand Up @@ -38,7 +44,9 @@ export interface HmrContext {
}

export function getShortName(file: string, root: string): string {
return file.startsWith(root + '/') ? path.posix.relative(root, file) : file
return file.startsWith(withTrailingSlash(root))
? path.posix.relative(root, file)
: file
}

export async function handleHMRUpdate(
Expand Down Expand Up @@ -81,7 +89,7 @@ export async function handleHMRUpdate(
debugHmr?.(`[file change] ${colors.dim(shortFile)}`)

// (dev only) the client itself cannot be hot updated.
if (file.startsWith(normalizedClientDir)) {
if (file.startsWith(withTrailingSlash(normalizedClientDir))) {
ws.send({
type: 'full-reload',
path: '*',
Expand Down
4 changes: 2 additions & 2 deletions packages/vite/src/node/server/middlewares/base.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import type { Connect } from 'dep-types/connect'
import type { ViteDevServer } from '..'
import { joinUrlSegments, stripBase } from '../../utils'
import { joinUrlSegments, stripBase, withTrailingSlash } from '../../utils'

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

Expand Down Expand Up @@ -36,7 +36,7 @@ export function baseMiddleware({
} else if (req.headers.accept?.includes('text/html')) {
// non-based page visit
const redirectPath =
url + '/' !== base ? joinUrlSegments(base, url) : base
withTrailingSlash(url) !== base ? joinUrlSegments(base, url) : base
res.writeHead(404, {
'Content-Type': 'text/html',
})
Expand Down
5 changes: 3 additions & 2 deletions packages/vite/src/node/server/middlewares/static.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import {
removeLeadingSlash,
shouldServeFile,
slash,
withTrailingSlash,
} from '../../utils'

const knownJavascriptExtensionRE = /\.[tj]sx?$/
Expand Down Expand Up @@ -118,7 +119,7 @@ export function serveStaticMiddleware(
}
if (redirectedPathname) {
// dir is pre-normalized to posix style
if (redirectedPathname.startsWith(dir)) {
if (redirectedPathname.startsWith(withTrailingSlash(dir))) {
redirectedPathname = redirectedPathname.slice(dir.length)
}
}
Expand All @@ -129,7 +130,7 @@ export function serveStaticMiddleware(
resolvedPathname[resolvedPathname.length - 1] === '/' &&
fileUrl[fileUrl.length - 1] !== '/'
) {
fileUrl = fileUrl + '/'
fileUrl = withTrailingSlash(fileUrl)
}
if (!ensureServingAccess(fileUrl, server, res, next)) {
return
Expand Down
5 changes: 3 additions & 2 deletions packages/vite/src/node/server/middlewares/transform.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import {
removeImportQuery,
removeTimestampQuery,
unwrapId,
withTrailingSlash,
} from '../../utils'
import { send } from '../send'
import { ERR_LOAD_URL, transformRequest } from '../transformRequest'
Expand Down Expand Up @@ -129,10 +130,10 @@ export function transformMiddleware(
// check if public dir is inside root dir
const publicDir = normalizePath(server.config.publicDir)
const rootDir = normalizePath(server.config.root)
if (publicDir.startsWith(rootDir)) {
if (publicDir.startsWith(withTrailingSlash(rootDir))) {
const publicPath = `${publicDir.slice(rootDir.length)}/`
// warn explicit public paths
if (url.startsWith(publicPath)) {
if (url.startsWith(withTrailingSlash(publicPath))) {
let warning: string

if (isImportRequest(url)) {
Expand Down
6 changes: 5 additions & 1 deletion packages/vite/src/node/ssr/ssrExternal.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import {
isInNodeModules,
lookupFile,
normalizePath,
withTrailingSlash,
} from '../utils'
import type { Logger, ResolvedConfig } from '..'
import { resolvePackageData } from '../packages'
Expand Down Expand Up @@ -340,7 +341,10 @@ export function cjsShouldExternalizeForSSR(
}
// deep imports, check ext before externalizing - only externalize
// extension-less imports and explicit .js imports
if (id.startsWith(e + '/') && (!path.extname(id) || id.endsWith('.js'))) {
if (
id.startsWith(withTrailingSlash(e)) &&
(!path.extname(id) || id.endsWith('.js'))
) {
return true
}
})
Expand Down
19 changes: 13 additions & 6 deletions packages/vite/src/node/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,9 @@ export function moduleListContains(
moduleList: string[] | undefined,
id: string,
): boolean | undefined {
return moduleList?.some((m) => m === id || id.startsWith(m + '/'))
return moduleList?.some(
(m) => m === id || id.startsWith(withTrailingSlash(m)),
)
}

export function isOptimizable(
Expand Down Expand Up @@ -224,6 +226,13 @@ export function fsPathFromUrl(url: string): string {
return fsPathFromId(cleanUrl(url))
}

export function withTrailingSlash(path: string): string {
if (path[path.length - 1] !== '/') {
return `${path}/`
}
return path
}

/**
* Check if dir is a parent of file
*
Expand All @@ -234,9 +243,7 @@ export function fsPathFromUrl(url: string): string {
* @returns true if dir is a parent of file
*/
export function isParentDirectory(dir: string, file: string): boolean {
if (dir[dir.length - 1] !== '/') {
dir = `${dir}/`
}
dir = withTrailingSlash(dir)
return (
file.startsWith(dir) ||
(isCaseInsensitiveFS && file.toLowerCase().startsWith(dir.toLowerCase()))
Expand Down Expand Up @@ -647,7 +654,7 @@ export function ensureWatchedFile(
if (
file &&
// only need to watch if out of root
!file.startsWith(root + '/') &&
!file.startsWith(withTrailingSlash(root)) &&
// some rollup plugins use null bytes for private resolved Ids
!file.includes('\0') &&
fs.existsSync(file)
Expand Down Expand Up @@ -1225,7 +1232,7 @@ export function stripBase(path: string, base: string): string {
if (path === base) {
return '/'
}
const devBase = base[base.length - 1] === '/' ? base : base + '/'
const devBase = withTrailingSlash(base)
return path.startsWith(devBase) ? path.slice(devBase.length - 1) : path
}

Expand Down