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

feat: fs-serve import graph awareness #3784

Merged
merged 11 commits into from
Jun 25, 2021
Merged
Show file tree
Hide file tree
Changes from 5 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
26 changes: 26 additions & 0 deletions packages/playground/fs-serve/__tests__/fs-serve.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
import { isBuild } from '../../testUtils'

const json = require('../safe.json')
const stringified = JSON.stringify(json)

if (!isBuild) {
test('default import', async () => {
expect(await page.textContent('.full')).toBe(stringified)
})

test('named import', async () => {
expect(await page.textContent('.named')).toBe(json.msg)
})

test('safe fetch', async () => {
expect(await page.textContent('.safe-fetch')).toBe(stringified)
})

test('unsafe fetch', async () => {
expect(await page.textContent('.unsafe-fetch')).toBe('')
})
} else {
test('dummy test to make jest happy', async () => {
// Your test suite must contain at least one test.
})
}
5 changes: 5 additions & 0 deletions packages/playground/fs-serve/entry.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
import { msg } from './nested/foo'

export const fullmsg = msg + 'bar'

document.querySelector('.nested-entry').textContent = fullmsg
1 change: 1 addition & 0 deletions packages/playground/fs-serve/nested/foo.js
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
export const msg = 'foo'
11 changes: 11 additions & 0 deletions packages/playground/fs-serve/package.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
{
"name": "test-fs-serve",
"private": true,
"version": "0.0.0",
"scripts": {
"dev": "vite root",
"build": "vite build root",
"debug": "node --inspect-brk ../../vite/bin/vite",
"serve": "vite preview"
}
}
39 changes: 39 additions & 0 deletions packages/playground/fs-serve/root/index.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
<h2>Normal Import</h2>
<pre class="full"></pre>
<pre class="named"></pre>

<h2>Safe Fetch</h2>
<pre class="safe-fetch"></pre>

<h2>Unsafe Fetch</h2>
<pre class="unsafe-fetch"></pre>

<h2>Nested Entry</h2>
<pre class="nested-entry"></pre>

<script type="module">
import '../entry'
import json, { msg } from '../safe.json'

text('.full', JSON.stringify(json))
text('.named', msg)

// imported before, should be treated as safe
fetch('/@fs/' + ROOT + '/safe.json')
.then((r) => r.json())
.then((data) => {
text('.safe-fetch', JSON.stringify(data))
})

// not imported before, outside of root, treated as unsafe
fetch('/@fs/' + ROOT + '/unsafe.json')
.catch((e) => console.error(e))
.then((r) => r.json())
.then((data) => {
text('.unsafe-fetch', JSON.stringify(data))
})

function text(sel, text) {
document.querySelector(sel).textContent = text
}
</script>
19 changes: 19 additions & 0 deletions packages/playground/fs-serve/root/vite.config.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
const path = require('path')

/**
* @type {import('vite').UserConfig}
*/
module.exports = {
server: {
fsServe: {
root: __dirname,
strict: true
},
hmr: {
overlay: false
}
},
define: {
ROOT: JSON.stringify(path.dirname(__dirname).replace(/\\/g, '/'))
}
}
3 changes: 3 additions & 0 deletions packages/playground/fs-serve/safe.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
{
"msg": "safe"
}
3 changes: 3 additions & 0 deletions packages/playground/fs-serve/unsafe.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
{
"msg": "unsafe"
}
5 changes: 5 additions & 0 deletions packages/vite/src/node/plugins/importAnalysis.ts
Original file line number Diff line number Diff line change
Expand Up @@ -361,6 +361,11 @@ export function importAnalysisPlugin(config: ResolvedConfig): Plugin {
)
let url = normalizedUrl

// record as safe modules
server?.moduleGraph.safeModulesPath.add(
cleanUrl(url).slice(4 /* '/@fs'.length */)
)

// rewrite
if (url !== specifier) {
// for optimized cjs deps, support named imports by rewriting named
Expand Down
19 changes: 16 additions & 3 deletions packages/vite/src/node/server/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ import {
import { timeMiddleware } from './middlewares/time'
import { ModuleGraph, ModuleNode } from './moduleGraph'
import { Connect } from 'types/connect'
import { createDebugger, normalizePath } from '../utils'
import { createDebugger, ensureLeadingSlash, normalizePath } from '../utils'
import { errorMiddleware, prepareError } from './middlewares/error'
import { handleHMRUpdate, HmrOptions, handleFileAddUnlink } from './hmr'
import { openBrowser } from './openBrowser'
Expand All @@ -53,6 +53,7 @@ import { createMissingImporterRegisterFn } from '../optimizer/registerMissing'
import { printServerUrls } from '../logger'
import { resolveHostname } from '../utils'
import { searchForWorkspaceRoot } from './searchRoot'
import { CLIENT_DIR } from '../constants'

export interface ServerOptions {
host?: string | boolean
Expand Down Expand Up @@ -151,6 +152,8 @@ export interface FileSystemServeOptions {
* @expiremental
*/
root?: string

dirs?: string[]
}

/**
Expand Down Expand Up @@ -484,7 +487,7 @@ export async function createServer(
middlewares.use(transformMiddleware(server))

// serve static files
middlewares.use(serveRawFsMiddleware(config))
middlewares.use(serveRawFsMiddleware(server))
middlewares.use(serveStaticMiddleware(root, config))

// spa fallback
Expand Down Expand Up @@ -703,11 +706,21 @@ export function resolveServerOptions(
const fsServeRoot = normalizePath(
path.resolve(root, server.fsServe?.root || searchForWorkspaceRoot(root))
)
let fsServeDirs = (server.fsServe?.dirs || []).map((i) =>
normalizePath(path.resolve(root, i))
)

fsServeDirs.push(normalizePath(CLIENT_DIR))
fsServeDirs.push(fsServeRoot)

fsServeDirs = fsServeDirs.map(ensureLeadingSlash)

// TODO: make strict by default
const fsServeStrict = server.fsServe?.strict ?? false
server.fsServe = {
root: fsServeRoot,
strict: fsServeStrict
strict: fsServeStrict,
dirs: fsServeDirs
}
return server as ResolvedServerOptions
}
84 changes: 42 additions & 42 deletions packages/vite/src/node/server/middlewares/static.ts
Original file line number Diff line number Diff line change
@@ -1,11 +1,16 @@
import path from 'path'
import sirv, { Options } from 'sirv'
import { Connect } from 'types/connect'
import { FileSystemServeOptions } from '..'
import { normalizePath, ResolvedConfig } from '../..'
import { normalizePath, ResolvedConfig, ViteDevServer } from '../..'
import { FS_PREFIX } from '../../constants'
import { Logger } from '../../logger'
import { cleanUrl, fsPathFromId, isImportRequest, isWindows } from '../../utils'
import {
cleanUrl,
ensureLeadingSlash,
fsPathFromId,
isImportRequest,
isWindows,
slash
} from '../../utils'
import { AccessRestrictedError } from './error'

const sirvOptions: Options = {
Expand Down Expand Up @@ -77,7 +82,7 @@ export function serveStaticMiddleware(
}

export function serveRawFsMiddleware(
config: ResolvedConfig
server: ViteDevServer
): Connect.NextHandleFunction {
const serveFromRoot = sirv('/', sirvOptions)

Expand All @@ -90,12 +95,7 @@ export function serveRawFsMiddleware(
// searching based from fs root.
if (url.startsWith(FS_PREFIX)) {
// restrict files outside of `fsServe.root`
ensureServingAccess(
path.resolve(fsPathFromId(url)),
config.server.fsServe,
config.logger
)

ensureServingAccess(slash(path.resolve(fsPathFromId(url))), server)
url = url.slice(FS_PREFIX.length)
if (isWindows) url = url.replace(/^[A-Z]:/i, '')

Expand All @@ -107,40 +107,40 @@ export function serveRawFsMiddleware(
}
}

export function isFileAccessAllowed(
export function isFileServingAllowed(
url: string,
{ root, strict }: Required<FileSystemServeOptions>
server: ViteDevServer
): boolean {
return !strict || normalizePath(url).startsWith(root + path.posix.sep)
if (!server.config.server.fsServe.strict) return true
antfu marked this conversation as resolved.
Show resolved Hide resolved

const file = ensureLeadingSlash(normalizePath(cleanUrl(url)))

if (server.moduleGraph.safeModulesPath.has(file)) {
return true
}

return server.config.server.fsServe.dirs.some((i) => file.startsWith(i + '/'))
}

export function ensureServingAccess(
url: string,
serveOptions: Required<FileSystemServeOptions>,
logger: Logger
): void {
const { strict, root } = serveOptions
// TODO: early return, should remove once we polished the restriction logic
if (!strict) return

if (!isFileAccessAllowed(url, serveOptions)) {
const normalizedUrl = normalizePath(url)
if (strict) {
throw new AccessRestrictedError(
`The request url "${normalizedUrl}" is outside of vite dev server root "${root}".
For security concerns, accessing files outside of workspace root is restricted since Vite v2.3.x.
Refer to docs https://vitejs.dev/config/#server-fsserve-root for configurations and more details.`,
normalizedUrl,
root
)
} else {
// TODO: warn for potential unrestricted access
logger.warnOnce(
`For security concerns, accessing files outside of workspace root will ` +
`be restricted by default in the future version of Vite. ` +
`Refer to [] for more`
)
logger.warnOnce(`Unrestricted file system access to "${normalizedUrl}"`)
}
export function ensureServingAccess(url: string, server: ViteDevServer): void {
if (isFileServingAllowed(url, server)) return

const { strict, root } = server.config.server.fsServe

if (strict) {
throw new AccessRestrictedError(
`The request url "${url}" is outside of vite dev server root "${root}".
For security concerns, accessing files outside of workspace root is restricted since Vite v2.3.x.
Refer to docs https://vitejs.dev/config/#server-fsserve-root for configurations and more details.`,
url,
root
)
} else {
server.config.logger.warnOnce(`Unrestricted file system access to "${url}"`)
server.config.logger.warnOnce(
`For security concerns, accessing files outside of workspace root will ` +
`be restricted by default in the future version of Vite. ` +
`Refer to [] for more`
)
}
}
1 change: 1 addition & 0 deletions packages/vite/src/node/server/moduleGraph.ts
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ export class ModuleGraph {
idToModuleMap = new Map<string, ModuleNode>()
// a single file may corresponds to multiple modules with different queries
fileToModulesMap = new Map<string, Set<ModuleNode>>()
safeModulesPath = new Set<string>()
container: PluginContainer

constructor(container: PluginContainer) {
Expand Down
8 changes: 5 additions & 3 deletions packages/vite/src/node/server/transformRequest.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ import {
import { checkPublicFile } from '../plugins/asset'
import { ssrTransform } from '../ssr/ssrTransform'
import { injectSourcesContent } from './sourcemap'
import { isFileAccessAllowed } from './middlewares/static'
import { isFileServingAllowed } from './middlewares/static'

const debugLoad = createDebugger('vite:load')
const debugTransform = createDebugger('vite:transform')
Expand All @@ -37,9 +37,11 @@ export interface TransformOptions {

export async function transformRequest(
url: string,
{ config, pluginContainer, moduleGraph, watcher }: ViteDevServer,
server: ViteDevServer,
options: TransformOptions = {}
): Promise<TransformResult | null> {
const { config, pluginContainer, moduleGraph, watcher } = server

url = removeTimestampQuery(url)
const { root, logger } = config
const prettyUrl = isDebug ? prettifyUrl(url, root) : ''
Expand Down Expand Up @@ -75,7 +77,7 @@ export async function transformRequest(
// as string
// only try the fallback if access is allowed, skip for out of root url
// like /service-worker.js or /api/users
if (options.ssr || isFileAccessAllowed(file, config.server.fsServe)) {
if (options.ssr || isFileServingAllowed(file, server)) {
try {
code = await fs.readFile(file, 'utf-8')
isDebug && debugLoad(`${timeFrom(loadStart)} [fs] ${prettyUrl}`)
Expand Down
4 changes: 4 additions & 0 deletions packages/vite/src/node/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -361,6 +361,10 @@ export function copyDir(srcDir: string, destDir: string): void {
}
}

export function ensureLeadingSlash(path: string) {
antfu marked this conversation as resolved.
Show resolved Hide resolved
return !path.startsWith('/') ? '/' + path : path
}

export function ensureWatchedFile(
watcher: FSWatcher,
file: string | null,
Expand Down
13 changes: 9 additions & 4 deletions scripts/jestPerTestSetup.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ declare global {

let server: ViteDevServer | http.Server
let tempDir: string
let rootDir: string
let err: Error

const logs = ((global as any).browserLogs = [])
Expand Down Expand Up @@ -60,16 +61,20 @@ beforeAll(async () => {
}
})

// when `root` dir is present, use it as vite's root
let testCustomRoot = resolve(tempDir, 'root')
rootDir = fs.existsSync(testCustomRoot) ? testCustomRoot : tempDir

const testCustomServe = resolve(dirname(testPath), 'serve.js')
if (fs.existsSync(testCustomServe)) {
// test has custom server configuration.
const { serve } = require(testCustomServe)
server = await serve(tempDir, isBuildTest)
server = await serve(rootDir, isBuildTest)
return
}

const options: UserConfig = {
root: tempDir,
root: rootDir,
logLevel: 'silent',
server: {
watch: {
Expand Down Expand Up @@ -128,7 +133,7 @@ afterAll(async () => {

function startStaticServer(): Promise<string> {
// check if the test project has base config
const configFile = resolve(tempDir, 'vite.config.js')
const configFile = resolve(rootDir, 'vite.config.js')
let config: UserConfig
try {
config = require(configFile)
Expand All @@ -142,7 +147,7 @@ function startStaticServer(): Promise<string> {
}

// start static file server
const serve = sirv(resolve(tempDir, 'dist'))
const serve = sirv(resolve(rootDir, 'dist'))
const httpServer = (server = http.createServer((req, res) => {
if (req.url === '/ping') {
res.statusCode = 200
Expand Down