[release/8.0-staging] Fix absolute path check when loading hostfxr/hostpolicy/coreclr#116776
Merged
elinor-fung merged 1 commit intodotnet:release/8.0-stagingfrom Jun 25, 2025
Conversation
On Windows, we were incorrectly only checking for <letter>: and incorrectly determining that UNC and device paths were not absolute. This updates pal::is_path_rooted to match the managed Path.IsPathRooted and adds a pal::is_path_absolute with the logic of Path.IsPartiallyQualified
Contributor
There was a problem hiding this comment.
Pull Request Overview
This PR refines how hostfxr/hostpolicy/coreclr resolver checks for absolute paths on Windows by replacing is_path_rooted with a new is_path_fully_qualified, and adds tests for UNC and device path scenarios.
- Introduces
pal::is_path_fully_qualifiedalongside updatedis_path_rootedlogic inpal.windows.cppand a matching stub inpal.unix.cpp. - Updates various resolver components (
coreclr_resolver,deps_resolver,fxr_resolver,hostpolicy_resolver, apphost, corehost) to use the new qualification check. - Adds Windows-specific tests for UNC (
\\?\) and device (\\.\) paths, and an error trace on failure.
Reviewed Changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| src/native/corehost/hostpolicy/standalone/coreclr_resolver.cpp | Use fully qualified path check instead of rooted check |
| src/native/corehost/hostpolicy/deps_resolver.cpp | Update assertion to use fully qualified path check |
| src/native/corehost/hostmisc/pal.windows.cpp | Implement is_path_fully_qualified and refine rooted logic |
| src/native/corehost/hostmisc/pal.unix.cpp | Stub is_path_fully_qualified to Unix-rooted logic |
| src/native/corehost/hostmisc/pal.h | Declare is_path_fully_qualified |
| src/native/corehost/fxr_resolver.h | Switch to fully qualified check for hostfxr path |
| src/native/corehost/fxr/standalone/hostpolicy_resolver.cpp | Switch to fully qualified check for hostpolicy path |
| src/native/corehost/corehost.cpp | Add error trace when fxr resolution fails |
| src/native/corehost/apphost/standalone/hostfxr_resolver.cpp | Switch to fully qualified check for apphost fxr path |
| src/installer/tests/HostActivation.Tests/StandaloneAppActivation.cs | Add tests for UNC and device EXE paths |
| src/installer/tests/HostActivation.Tests/PortableAppActivation.cs | Add tests for device paths in DotNetRoot setting |
Comments suppressed due to low confidence (1)
src/native/corehost/hostmisc/pal.h:331
- Add a brief comment above
is_path_fully_qualifiedexplaining its purpose and how it differs fromis_path_rooted, to aid future maintainers.
bool is_path_fully_qualified(const string_t& path);
AaronRobinsonMSFT
approved these changes
Jun 18, 2025
jeffschwMSFT
approved these changes
Jun 18, 2025
Member
jeffschwMSFT
left a comment
There was a problem hiding this comment.
lgtm. we will take for consideration in 8.0.x
Contributor
|
Tagging subscribers to this area: @vitek-karas, @agocke, @VSadov |
7315722
into
dotnet:release/8.0-staging
188 of 200 checks passed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Backport of #116597 to release/8.0-staging
On Windows, we were incorrectly only checking for
<letter>:and incorrectly determining that UNC and device paths were not absolute. This updatespal::is_path_rootedto match the managedPath.IsPathRootedand adds apal::is_path_absolutewith the logic ofPath.IsPartiallyQualifiedcc @dotnet/appmodel @AaronRobinsonMSFT
Customer Impact
Customers are unable to run self-contained apps from or point framework-dependent apps to use a .NET runtime from a UNC share or device path.
Issue: #116521
Regression
Regressed in last servicing release.
b33d4e3
Testing
Manual testing for UNC share and device path. Automated test added for device paths.
Risk
Medium. The fix has to be in a path that every app will exercise. The updated logic is a mirror of existing logic in managed APIs.