-
Notifications
You must be signed in to change notification settings - Fork 5.1k
[release/8.0-staging] Fix absolute path check when loading hostfxr/hostpolicy/coreclr #116776
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.
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
[release/8.0-staging] Fix absolute path check when loading hostfxr/hostpolicy/coreclr #116776
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
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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_qualified
alongside updatedis_path_rooted
logic inpal.windows.cpp
and 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_qualified
explaining its purpose and how it differs fromis_path_rooted
, to aid future maintainers.
bool is_path_fully_qualified(const string_t& path);
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm. we will take for consideration in 8.0.x
Tagging subscribers to this area: @vitek-karas, @agocke, @VSadov |
7315722
into
dotnet:release/8.0-staging
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_rooted
to match the managedPath.IsPathRooted
and adds apal::is_path_absolute
with the logic ofPath.IsPartiallyQualified
cc @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.