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

[xinxinhe1810]

update call stripBase in utils.ts before passing to fsPathFromUrl (fix #9438)

Description

Additional context

---

What is the purpose of this pull request?

• Bug fix
• New Feature
• Documentation update
• Other

Before submitting the PR, please make sure you do the following

• Read the Contributing Guidelines.
• Read the Pull Request Guidelines and follow the PR Title Convention.
• Check that there isn't already a PR that solves the problem the same way to avoid creating a duplicate.
• Provide a description in this PR that addresses what the PR is solving, or reference the issue that it solves (e.g. fixes #123).
• Ideally, include relevant tests that fail without this PR but pass with it.

[patak-dev]

Tried it adding a /base/ to the fs-serve playground and it is working as expected. We could expand the tests, but I prefer to keep the current fs-serve using the default base.

[xinxinhe1810]

You cannot directly use the configuration from vite.config-base.js here. In vite.config-base.js, the variable __dirname is used, which returns a value based on "playground" instead of "playground-temp" that is used for testing. I guess this may be a bug. That's why I am using "serve" to start the testing server instead of relying on vite.config-base.js.

[xinxinhe1810]

The config file read in __tests__ from vite.config.js is different from the one read in playground-temp, especially when it involves expressions like __dirname.
like:

{
  build: {
    rollupOptions: {
      input: {
        main: path.resolve(__dirname, 'src/index.html'),
      },
    },
  },
  server: {
    fs: {
      strict: true,
      allow: [path.resolve(__dirname, 'src')],
    },
}

[patak-dev]

This is strange, we are using __dirname in the worker and assets playgrounds too in the configs. Maybe there is an issue here because the root is set?

[patak-dev]

You were correct, I sent a PR to fix the config variants handling here:

• test: fix playground config variants #13725

[patak-dev]

CI tests always fail. What is the reason? I haven't encountered this failure locally.

These are flaky tests. We're having issues with them since last week, it is unrelated to this PR.

[benmccann]

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)

[xinxinhe1810]

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

[sapphi-red]

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))

[xinxinhe1810]

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

[benmccann]

if we've got /base/ then we'll end up with a //. This should probably use joinUrlSegments or path.posix.join

[xinxinhe1810]

i use joinUrlSegments to fix that

[patak-dev]

IIUC, this issue can't be exploited, no? In that case, let's merge it for Vite 5 (we'll start the beta soon). @sapphi-red let me know if you think it is important to get this one in a patch though.

[bluwy]

I'm not sure if we need to hold off this change. I think it'd be nice to fix as patch though.

[sapphi-red]

I think this issue can't be exploited. But I think it'd be good to merge this in a patch.