Enhance path normalization: add handling for consecutive directory separators#13369
Conversation
…parators in Windows and Unix
There was a problem hiding this comment.
Pull request overview
This PR fixes a regression in AbsolutePath.GetCanonicalForm() where its fast path could return Windows/Unix paths containing consecutive directory separators without collapsing them, diverging from Path.GetFullPath() behavior and impacting VS deployment scenarios.
Changes:
- Extend the
GetCanonicalForm()fast-path gate to detect consecutive directory separators and fall back toPath.GetFullPath()when present. - Add unit test cases covering consecutive separators on both Windows and Unix paths.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| src/Framework/PathHelpers/AbsolutePath.cs | Adds detection of consecutive separators to ensure canonicalization matches Path.GetFullPath() semantics. |
| src/Framework.UnitTests/AbsolutePath_Tests.cs | Adds coverage for consecutive separator inputs on Windows and Unix. |
You can also share your feedback on Copilot code review. Take the survey.
|
@JanProvaznik / @AR-May - just FYI, the PR is ready for your approval. Please let me know if anything else is needed. |
|
@JanProvaznik @AR-May - friendly ping for your approval on the PR |
Hi! I placed the whole optimization under a change wave as we decided offline. With it, it should be good to go to the main. In 18.5 branch, we will be most conservative - we will remove the optimization altogether for a fix. |
Investigation from VS feedback item https://developercommunity.visualstudio.com/t/MSIX-Deployed-App-Fails-to-Find-Resource/11049946 led to finding of this issue
Context
AssignTargetPathtask in VS 18 Preview build to new code path gated behindChangeWaves.Wave18_5that usesAbsolutePath.GetCanonicalForm()instead of the previousPath.GetFullPath()for path normalization.GetCanonicalForm()doesn't normalize consecutive directory separators (\\→\), unlikePath.GetFullPath()which did. This resulted in a "flattening" of the TargetPath, breaking the a Visual Studio deployment scenario.Regression was introduced on #13069.
Changes Made
AbsolutePath.GetCanonicalForm() has a fast-path optimization that skips calling Path.GetFullPath() when the path has no relative segments (/. or .) and no mixed separators (/ on Windows). However, it didn't check for consecutive directory separators (e.g., \), so paths like ...win-x64\ were returned unchanged. The fix adds a check for consecutive separators (\ on Windows, // on Unix) so they fall through to Path.GetFullPath() which normalizes them to a single separator.
Testing
Added the appropriate unit tests. Also validated the originally reported issue in the VS feedback ticket by patching binaries in VS.