winget-cli

commit c6d76bd1b7945349abab7d037d610bb7390b8f5c
parent e623cedc71a43af39313c3ada301fc309ce51acf
Author: Ryan <69221034+ryfu-msft@users.noreply.github.com>
Date:   Fri, 27 Sep 2024 11:22:06 -0400

Fix issue with unicode characters in symlink target path (#4834)


Diffstat:
M
src/AppInstallerCLICore/PortableInstaller.cpp
 | 
17
++++++++++-------
M
src/AppInstallerCLITests/PortableInstaller.cpp
 | 
40
++++++++++++++++++++++++++++++++++++++--

2 files changed, 48 insertions(+), 9 deletions(-)
diff --git a/src/AppInstallerCLICore/PortableInstaller.cpp b/src/AppInstallerCLICore/PortableInstaller.cpp
@@ -72,9 +72,10 @@ namespace AppInstaller::CLI::Portable
         }
         else if (fileType == PortableFileType::Symlink)
         {
-            if (Filesystem::SymlinkExists(filePath) && !Filesystem::VerifySymlink(filePath, entry.SymlinkTarget))
+            std::filesystem::path symlinkTargetPath{ AppInstaller::Utility::ConvertToUTF16(entry.SymlinkTarget) };
+            if (Filesystem::SymlinkExists(filePath) && !Filesystem::VerifySymlink(filePath, symlinkTargetPath))
             {
-                AICLI_LOG(CLI, Warning, << "Symlink target does not match ARP Entry. Expected: " << entry.SymlinkTarget << " Actual: " << std::filesystem::read_symlink(filePath));
+                AICLI_LOG(CLI, Warning, << "Symlink target does not match ARP Entry. Expected: " << symlinkTargetPath << " Actual: " << std::filesystem::read_symlink(filePath));
                 return false;
             }
         }
@@ -121,11 +122,13 @@ namespace AppInstaller::CLI::Portable
         }
         else if (entry.FileType == PortableFileType::Symlink)
         {
+            std::filesystem::path symlinkTargetPath{ Utility::ConvertToUTF16(entry.SymlinkTarget) };
+
             if (BinariesDependOnPath && !InstallDirectoryAddedToPath)
             {
                 // Scenario indicated by 'ArchiveBinariesDependOnPath' manifest entry.
                 // Skip symlink creation for portables dependent on binaries that require the install directory to be added to PATH.
-                std::filesystem::path installDirectory = std::filesystem::path(entry.SymlinkTarget).parent_path();
+                std::filesystem::path installDirectory = symlinkTargetPath.parent_path();
                 AddToPathVariable(installDirectory);
                 AICLI_LOG(Core, Info, << "Install directory added to PATH: " << installDirectory);
                 CommitToARPEntry(PortableValueName::InstallDirectoryAddedToPath, InstallDirectoryAddedToPath = true);
@@ -150,15 +153,15 @@ namespace AppInstaller::CLI::Portable
                     m_stream << Resource::String::OverwritingExistingFileAtMessage(Utility::LocIndView{ filePath.u8string() }) << std::endl;
                 }
 
-                if (Filesystem::CreateSymlink(entry.SymlinkTarget, filePath))
+                if (Filesystem::CreateSymlink(symlinkTargetPath, filePath))
                 {
-                    AICLI_LOG(Core, Info, << "Symlink created at: " << filePath);
+                    AICLI_LOG(Core, Info, << "Symlink created at: " << filePath << " with target path: " << symlinkTargetPath);
                 }
                 else
                 {
                     // If symlink creation fails, resort to adding the package directory to PATH.
                     AICLI_LOG(Core, Info, << "Failed to create symlink at: " << filePath);
-                    AddToPathVariable(std::filesystem::path(entry.SymlinkTarget).parent_path());
+                    AddToPathVariable(symlinkTargetPath.parent_path());
                     CommitToARPEntry(PortableValueName::InstallDirectoryAddedToPath, InstallDirectoryAddedToPath = true);
                 }
             }
@@ -186,7 +189,7 @@ namespace AppInstaller::CLI::Portable
             else if (InstallDirectoryAddedToPath)
             {
                 // If symlink doesn't exist, check if install directory was added to PATH directly and remove.
-                RemoveFromPathVariable(std::filesystem::path(entry.SymlinkTarget).parent_path());
+                RemoveFromPathVariable(std::filesystem::path(Utility::ConvertToUTF16(entry.SymlinkTarget)).parent_path());
             }
         }
         else if (fileType == PortableFileType::Symlink && Filesystem::SymlinkExists(filePath))
diff --git a/src/AppInstallerCLITests/PortableInstaller.cpp b/src/AppInstallerCLITests/PortableInstaller.cpp
@@ -171,4 +171,41 @@ TEST_CASE("PortableInstaller_InstallToIndex_ExistingInstallRoot", "[PortableInst
     REQUIRE_FALSE(AppInstaller::Filesystem::SymlinkExists(symlinkPath));
     REQUIRE_FALSE(AppInstaller::Filesystem::SymlinkExists(symlinkPath2));
     REQUIRE_FALSE(std::filesystem::exists(directoryPath));
-}-
\ No newline at end of file
+}
+
+TEST_CASE("PortableInstaller_UnicodeSymlinkPath", "[PortableInstaller]")
+{
+    TempDirectory tempDirectory = TestCommon::TempDirectory("TempDirectory", false);
+
+    // Modify install location path to include unicode characters.
+    std::filesystem::path testInstallLocation = tempDirectory.GetPath() / std::filesystem::path{ ConvertToUTF16("романтический") };
+
+    std::vector<PortableFileEntry> desiredTestState;
+
+    TestCommon::TempFile testPortable("testPortable.txt");
+    std::ofstream file(testPortable, std::ofstream::out);
+    file.close();
+
+    std::filesystem::path targetPath = testInstallLocation / "testPortable.txt";
+    std::filesystem::path symlinkPath = tempDirectory.GetPath() / "testSymlink.exe";
+
+    desiredTestState.emplace_back(std::move(PortableFileEntry::CreateFileEntry(testPortable.GetPath(), targetPath, {})));
+    desiredTestState.emplace_back(std::move(PortableFileEntry::CreateSymlinkEntry(symlinkPath, targetPath)));
+
+    PortableInstaller portableInstaller = PortableInstaller(ScopeEnum::User, Architecture::X64, "testProductCode");
+    portableInstaller.TargetInstallLocation = testInstallLocation;
+    portableInstaller.SetDesiredState(desiredTestState);
+    REQUIRE(portableInstaller.VerifyExpectedState());
+
+    portableInstaller.Install();
+
+    PortableInstaller portableInstaller2 = PortableInstaller(ScopeEnum::User, Architecture::X64, "testProductCode");
+    REQUIRE(portableInstaller2.ARPEntryExists());
+    REQUIRE(std::filesystem::exists(portableInstaller2.PortableTargetFullPath));
+    REQUIRE(AppInstaller::Filesystem::SymlinkExists(portableInstaller2.PortableSymlinkFullPath));
+
+    portableInstaller2.Uninstall();
+    REQUIRE_FALSE(std::filesystem::exists(portableInstaller2.PortableTargetFullPath));
+    REQUIRE_FALSE(AppInstaller::Filesystem::SymlinkExists(portableInstaller2.PortableSymlinkFullPath));
+    REQUIRE_FALSE(std::filesystem::exists(portableInstaller2.InstallLocation));
+}