fix: normalize url in ensureServingAccess

[cawa-93]

Description

Fix #3346

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 Commit 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.

[lovasoa]

I think the problem here is with the approach of using string manipulation to prevent path traversal. The way path are resolved depends on the file system and its mount options, and trying to solve that with a String.startsWith is always going to be problematic.

It is already causing a lot of issues:

• Serving index.html from different directory in your project is problematic currently #3354
• .map file that is in fsServe.root is defined as one that is outside #3346
• monorepo setups break when updating to vite 2.3.x #3347
• I keep getting [vite] Internal server error: The request url "/path" is outside of vite dev server root "/path". when using vitepress vuejs/vitepress#297

[lovasoa]

This doesn't fix the issue for e.g. case-insensitive file systems.

[patak-dev]

This doesn't fix the issue for e.g. case-insensitive file systems.

Please search the issues for case-insensitive fs, there is no easy way to support them and be generic with other systems in Vite. This is not related to this PR in particular.

[lovasoa]

@patak-js What I am saying is that this PR is trying to solve the wrong problem. There is one common cause to all the issues I mentioned above, the problem is the general approach of trying to "normalize" the path, and then testing it with String.startsWith.

[cawa-93]

@lovasoa

@patak-js What I am saying is that this PR is trying to solve the wrong problem. There is one common cause to all the issues I mentioned above, the problem is the general approach of trying to "normalize" the path, and then testing it with String.startsWith.

This PR is not intended to correct logic (which requires more time and effort). This PR is a quick hot fix for that can't wait long.

You can solve a right problem later.

[lovasoa]

Yes, I see where you are coming from. The boggy logic itself was also introduced by a quick hot fix that couldn't wait long. I find vite amazing, but piling hot fixes is dangerous on the long run...

[patak-dev]

Yes, I see where you are coming from. The boggy logic itself was also introduced by a quick hot fix for that couldn't wait long. I find vite amazing, but piling hot fixes is dangerous on the long run...

This is fixing a bug in Windows, we are using normalizePath across the board in the code-base and was missing. That being said, we all agree that further PRs are needed to improve fsServe (for example, better handling of symlinks)

[jods4]

Looking forward to this being released!