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.

Sign up for GitHub

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

Jump to bottom

fix(importAnalysis): strip url base before passing as safeModulePaths #13712

Merged
merged 10 commits into from Jul 29, 2023
Merged

fix(importAnalysis): strip url base before passing as safeModulePaths #13712

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
fa92a74
fix(node): update call stripBase in utils.ts before passing to fsPath…
xinxinhe1810 Jul 4, 2023
4fcfcf4
Merge branch 'vitejs:main' into fix/base-module-path
xinxinhe1810 Jul 4, 2023
7dd7d4c
add test for base fs-serve
xinxinhe1810 Jul 5, 2023
f9831af
update base for Absolute URL pathname
xinxinhe1810 Jul 6, 2023
1cc1ef8
update: Modify comments for better
xinxinhe1810 Jul 10, 2023
aaf7799
Merge branch 'vitejs:main' into fix/base-module-path
xinxinhe1810 Jul 14, 2023
eb81f16
chore: isFileServingAllowed is doesn't need stripBase
xinxinhe1810 Jul 24, 2023
5bb2605
Update packages/vite/src/node/plugins/importAnalysis.ts
xinxinhe1810 Jul 25, 2023
d075dff
chore: use joinUrlSegments join fetch url
xinxinhe1810 Jul 25, 2023
4556271
chore: remove serve.ts
sapphi-red Jul 25, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
6 changes: 4 additions & 2 deletions packages/vite/src/node/plugins/importAnalysis.ts
View file
Open in desktop
Expand Up @@ -556,8 +556,10 @@ export function importAnalysisPlugin(config: ResolvedConfig): Plugin {
}
}

// record as safe modules
server?.moduleGraph.safeModulesPath.add(fsPathFromUrl(url))
// record as safe modules # stripBase: https://github.com/vitejs/vite/issues/9438#issuecomment-1486662486
Copy link
Collaborator

@benmccann benmccann Jul 7, 2023 •
edited

Choose a reason for hiding this comment

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

it we're going to link to a comment it should probably be #9438 (comment), but it'd be nicer to just explain why it's necessary here rather than making someone copy paste a URL into their browser to read the explanation (same below)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I tried to fix it. Thanks for guiding me on this PR.

server?.moduleGraph.safeModulesPath.add(
fsPathFromUrl(stripBase(url, base)),
)

if (url !== specifier) {
let rewriteDone = false
Expand Down
4 changes: 3 additions & 1 deletion packages/vite/src/node/server/middlewares/static.ts
View file
Open in desktop
Expand Up @@ -19,6 +19,7 @@ import {
removeLeadingSlash,
shouldServeFile,
slash,
stripBase,
} from '../../utils'

const knownJavascriptExtensionRE = /\.[tj]sx?$/
Expand Down Expand Up @@ -194,7 +195,8 @@ export function isFileServingAllowed(
): boolean {
if (!server.config.server.fs.strict) return true

const file = fsPathFromUrl(url)
// stripBase: https://github.com/vitejs/vite/issues/9438#issuecomment-1486662486
const file = fsPathFromUrl(stripBase(url, server.config.rawBase))
Copy link
Member

Choose a reason for hiding this comment

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

This function is called from serveRawFsMiddleware/serveStaticMiddleware/transformMiddleware that runs after baseMiddleware (this middleware strips the base). So I think we shouldn't run stripBase here.

vite/packages/vite/src/node/server/index.ts

Lines 614 to 646 in 1292ad0

// base
if (config.base !== '/') {
middlewares.use(baseMiddleware(server))
}
// open in editor support
middlewares.use('/__open-in-editor', launchEditorMiddleware())
// ping request handler
// Keep the named function. The name is visible in debug logs via `DEBUG=connect:dispatcher ...`
middlewares.use(function viteHMRPingMiddleware(req, res, next) {
if (req.headers['accept'] === 'text/x-vite-ping') {
res.writeHead(204).end()
} else {
next()
}
})
// serve static files under /public
// this applies before the transform middleware so that these files are served
// as-is without transforms.
if (config.publicDir) {
middlewares.use(
servePublicMiddleware(config.publicDir, config.server.headers),
)
}
// main transform middleware
middlewares.use(transformMiddleware(server))
// serve static files
middlewares.use(serveRawFsMiddleware(server))
middlewares.use(serveStaticMiddleware(root, server))

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So that's how it is. I remove stripBase here


if (server._fsDenyGlob(file)) return false

Expand Down