Skip to content
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/9.0-staging] Fix UNC paths #111499

Merged

Conversation

github-actions[bot]
Copy link
Contributor

@github-actions github-actions bot commented Jan 16, 2025

Backport of #110033 to release/9.0-staging

/cc @agocke

Customer Impact

  • Customer reported
  • Found internally

This change causes apps which are launched from network shares ("UNC paths" aka \server-name) to have their location shown as extended UNC paths ("\?\UNC\server-name"). This is a regression both because the paths are different from what they were before, but also because some APIs will not accept UNC server paths. This manifests as a breaking change in .NET 9.

Regression

  • Yes
  • No

Regression from .NET 8 to .NET 9.

Testing

Tested manually. Unfortunately we do not have a way to test network shares in our CI system.

Risk

Low risk. The change is targeted to one API and only to paths which have extended UNC paths.

agocke and others added 4 commits January 16, 2025 15:52
If the input file was a network path then the raw path returned by
GetFinalPathByHandle may return a UNC path. If so, and if the original
path wasn't a UNC path, and the original path doesn't need normalization,
we want to use the original path.
Copy link
Contributor

Tagging subscribers to this area: @vitek-karas, @agocke, @VSadov
See info in area-owners.md if you want to be subscribed.

Copy link
Member

@jeffschwMSFT jeffschwMSFT left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm. please get a code review. we will take for consideration in 9.0.x

@jeffschwMSFT jeffschwMSFT added the Servicing-consider Issue for next servicing release review label Jan 16, 2025
@jeffschwMSFT jeffschwMSFT added this to the 9.0.x milestone Jan 16, 2025
@agocke agocke requested review from sbomer and elinor-fung January 17, 2025 22:22
@jeffschwMSFT jeffschwMSFT added Servicing-approved Approved for servicing release and removed Servicing-consider Issue for next servicing release review labels Jan 21, 2025
@jeffschwMSFT jeffschwMSFT modified the milestones: 9.0.x, 9.0.3 Jan 21, 2025
@jeffschwMSFT
Copy link
Member

@agocke can you take a look at the CI failures? we can merge when ready.

@snakefoot
Copy link

snakefoot commented Feb 2, 2025

This will just remove the Long UNC-prefix, when not needed. Should one not expect to see the network-drive-letter, instead of the network-path behind the network-drive-letter?

Ex. Z:\rwells\very\special\place (NET8) instead of \\actual-hostname\actual-sharename\rwells\very\special\place (NET9) ?

@agocke
Copy link
Member

agocke commented Feb 3, 2025

/ba-g timeout is unrelated and hitting multiple PRs.

@agocke agocke merged commit 6091bce into release/9.0-staging Feb 3, 2025
149 of 151 checks passed
@agocke agocke deleted the backport/pr-110033-to-release/9.0-staging branch February 3, 2025 03:45
@agocke
Copy link
Member

agocke commented Feb 3, 2025

@snakefoot It sounds like your scenario is slightly different. What is the input and output in your case?

@snakefoot
Copy link

snakefoot commented Feb 4, 2025

I'm just the NLog-project-janitor, and users are having issues when running their NET9-application from a network-drive:

Where System.AppContext.BaseDirectory in NET9 suddenly returns the Long UNC Network-path, instead of the mounted network-drive.

Ex. Z:\rwells\very\special\place (NET8) instead of \\actual-hostname\actual-sharename\rwells\very\special\place (NET9) ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-Host Servicing-approved Approved for servicing release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants