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

Fix issue with unicode characters in symlink target path #4834

Merged
merged 3 commits into from
Sep 27, 2024

Conversation

ryfu-msft
Copy link
Contributor

@ryfu-msft ryfu-msft commented Sep 26, 2024

Resolves #3312

Fixes an issue when creating a symlink with a target path that includes unicode characters by converting the symlink target path to a UTF16 string prior to creation.

Added a test that includes unicode characters in the symlink target path and verified that install and uninstall are both supported.

Microsoft Reviewers: Open in CodeFlow

@ryfu-msft ryfu-msft requested a review from a team as a code owner September 26, 2024 22:21
@microsoft-github-policy-service microsoft-github-policy-service bot added the Issue-Bug It either shouldn't be doing this or needs an investigation. label Sep 26, 2024
@yao-msft
Copy link
Contributor

There are a few more places to be updated, where we directly create std::filesystem::path from SymlinkTarget

@ryfu-msft ryfu-msft merged commit c6d76bd into microsoft:master Sep 27, 2024
9 checks passed
@ryfu-msft ryfu-msft deleted the fixSymlinkIssue branch September 27, 2024 15:22
@MagicAndre1981
Copy link

when can we get a new preview to test this?

@denelon
Copy link
Contributor

denelon commented Oct 1, 2024

We're getting ready to cut a release candidate for WinGet 1.9 and we'll start cutting WinGet 1.10 preview builds shortly after. ETA is early next week.

@MagicAndre1981
Copy link

@denelon the fix works fine in latest 1.9 stable versions, but I think the yt-dlp issue is similar. Can this be also fixed? All path strings in code should be checked for such unicode issues.

@denelon
Copy link
Contributor

denelon commented Nov 26, 2024

If:

is resolved, then can you make a new issue calling out where you're seeing unicode so we can track that issue separately?

That way we can close 161349.

@MagicAndre1981
Copy link

then can you make a new issue calling out where you're seeing unicode so we can track that issue separately?

I found that there is already an old issue about unicode issues, so there is no need for creating a new one.

@denelon
Copy link
Contributor

denelon commented Nov 27, 2024

@MagicAndre1981 thanks. I added it to the WinGet project to make sure we get that one fixed sooner rather than later.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Issue-Bug It either shouldn't be doing this or needs an investigation.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Broken symlinks created for non-latin character usernames
4 participants