commit 242fa158611d326341600f7c1cd8f54320b400c3 parent 508ad2bc29c5d5ce037912b486cc953d300de513 Author: yao-msft <50888816+yao-msft@users.noreply.github.com> Date: Mon, 29 Jun 2020 15:00:03 -0700 Fix unicode path issues (#466) Use utf16 when creating paths that may contain extended characters. Updated tests to include extended character cases. Diffstat:
13 files changed, 42 insertions(+), 15 deletions(-)
diff --git a/src/AppInstallerCLICore/Commands/HashCommand.cpp b/src/AppInstallerCLICore/Commands/HashCommand.cpp @@ -40,7 +40,7 @@ namespace AppInstaller::CLI [](Execution::Context& context) { auto inputFile = context.Args.GetArg(Execution::Args::Type::HashFile); - std::ifstream inStream{ inputFile, std::ifstream::binary }; + std::ifstream inStream{ Utility::ConvertToUTF16(inputFile), std::ifstream::binary }; context.Reporter.Info() << "Sha256: "_liv << Utility::LocIndString{ Utility::SHA256::ConvertToString(Utility::SHA256::ComputeHash(inStream)) } << std::endl; diff --git a/src/AppInstallerCLICore/Commands/ValidateCommand.cpp b/src/AppInstallerCLICore/Commands/ValidateCommand.cpp @@ -37,7 +37,7 @@ namespace AppInstaller::CLI Workflow::VerifyFile(Execution::Args::Type::ValidateManifest) << [](Execution::Context& context) { - auto inputFile = context.Args.GetArg(Execution::Args::Type::ValidateManifest); + auto inputFile = Utility::ConvertToUTF16(context.Args.GetArg(Execution::Args::Type::ValidateManifest)); try { diff --git a/src/AppInstallerCLICore/Workflows/InstallFlow.cpp b/src/AppInstallerCLICore/Workflows/InstallFlow.cpp @@ -79,7 +79,7 @@ namespace AppInstaller::CLI::Workflow const auto& installer = context.Get<Execution::Data::Installer>().value(); std::filesystem::path tempInstallerPath = Runtime::GetPathTo(Runtime::PathName::Temp); - tempInstallerPath /= manifest.Id + '.' + manifest.Version; + tempInstallerPath /= Utility::ConvertToUTF16(manifest.Id + '.' + manifest.Version); AICLI_LOG(CLI, Info, << "Generated temp download path: " << tempInstallerPath); diff --git a/src/AppInstallerCLICore/Workflows/ShellExecuteInstallerHandler.cpp b/src/AppInstallerCLICore/Workflows/ShellExecuteInstallerHandler.cpp @@ -16,16 +16,16 @@ namespace AppInstaller::CLI::Workflow { AICLI_LOG(CLI, Info, << "Starting installer. Path: " << filePath); - SHELLEXECUTEINFOA execInfo = { 0 }; - execInfo.cbSize = sizeof(SHELLEXECUTEINFO); + SHELLEXECUTEINFOW execInfo = { 0 }; + execInfo.cbSize = sizeof(execInfo); execInfo.fMask = SEE_MASK_NOCLOSEPROCESS; - std::string filePathUTF8Str = filePath.u8string(); - execInfo.lpFile = filePathUTF8Str.c_str(); - execInfo.lpParameters = args.c_str(); + execInfo.lpFile = filePath.c_str(); + std::wstring argsUtf16 = Utility::ConvertToUTF16(args); + execInfo.lpParameters = argsUtf16.c_str(); // Some installers force UI. Setting to SW_HIDE will hide installer UI and installation will hang forever. // Verified setting to SW_SHOW does not hurt silent mode since no UI will be shown. execInfo.nShow = SW_SHOW; - if (!ShellExecuteExA(&execInfo) || !execInfo.hProcess) + if (!ShellExecuteExW(&execInfo) || !execInfo.hProcess) { return GetLastError(); } @@ -134,7 +134,7 @@ namespace AppInstaller::CLI::Workflow if (Utility::FindAndReplace(installerArgs, std::string(ARG_TOKEN_LOGPATH), logPath)) { - context.Add<Execution::Data::LogPath>(logPath); + context.Add<Execution::Data::LogPath>(Utility::ConvertToUTF16(logPath)); } // Populate <InstallPath> with value from command line. diff --git a/src/AppInstallerCLICore/Workflows/WorkflowBase.cpp b/src/AppInstallerCLICore/Workflows/WorkflowBase.cpp @@ -249,7 +249,7 @@ namespace AppInstaller::CLI::Workflow void VerifyFile::operator()(Execution::Context& context) const { - std::filesystem::path path = context.Args.GetArg(m_arg); + std::filesystem::path path = Utility::ConvertToUTF16(context.Args.GetArg(m_arg)); if (!std::filesystem::exists(path)) { @@ -270,7 +270,7 @@ namespace AppInstaller::CLI::Workflow VerifyFile(Execution::Args::Type::Manifest) << [](Execution::Context& context) { - Manifest::Manifest manifest = Manifest::Manifest::CreateFromPath(context.Args.GetArg(Execution::Args::Type::Manifest)); + Manifest::Manifest manifest = Manifest::Manifest::CreateFromPath(Utility::ConvertToUTF16(context.Args.GetArg(Execution::Args::Type::Manifest))); Logging::Telemetry().LogManifestFields(manifest.Id, manifest.Name, manifest.Version, true); context.Add<Execution::Data::Manifest>(std::move(manifest)); }; diff --git a/src/AppInstallerCLIE2ETests/Constants.cs b/src/AppInstallerCLIE2ETests/Constants.cs @@ -24,7 +24,7 @@ namespace AppInstallerCLIE2ETests public const string AICLIPackageName = "WinGetDevCLI"; public const string AICLIAppId = "WinGetDev"; - public const string TestPackage = "TestPackage.msix"; + public const string TestPackage = "TëstPackage.msix"; public class ErrorCode { diff --git a/src/AppInstallerCLIE2ETests/InstallCommand.cs b/src/AppInstallerCLIE2ETests/InstallCommand.cs @@ -9,6 +9,7 @@ namespace AppInstallerCLIE2ETests public class InstallCommand { // Todo: this should point to a loopback address. Disabling the install tests until we have loopback support done in our e2e tests. + // Todo: add unicode test cases after install tests are enabled. private const string InstallTestSourceUrl = @"https://github.com/microsoft/appinstaller-cli/raw/master/src/AppInstallerCLIE2ETests/TestData"; private const string InstallTestSourceName = @"InstallTestSource"; diff --git a/src/AppInstallerCLIE2ETests/TestData/Manifests/TëstExeInstaller.yaml b/src/AppInstallerCLIE2ETests/TestData/Manifests/TëstExeInstaller.yaml @@ -0,0 +1,19 @@ +Id: AppInstallerTest.TëstExeInstaller +Name: TestExeInstaller +Version: 1.0.0.0 +Publisher: AppInstallerTest +License: Test +Installers: + - Arch: x86 + Url: https://github.com/microsoft/appinstaller-cli/raw/master/src/AppInstallerCLIE2ETests/TestData/AppInstallerTestExeInstaller.exe + Sha256: 7c616e2fe3853b555ddea4cb50bd307da1dc47898a5fbc3a8646a470107802ab + InstallerType: exe + Switches: + Custom: /execustom + SilentWithProgress: /exeswp + Silent: /exesilent + Interactive: /exeinteractive + Language: /exeenus + Log: /exelog <LOGPATH> + InstallLocation: /InstallDir <INSTALLPATH> +ManifestVersion: 0.1.0 diff --git a/src/AppInstallerCLIE2ETests/TestData/TestPackage.msix b/src/AppInstallerCLIE2ETests/TestData/TëstPackage.msix Binary files differ. diff --git a/src/AppInstallerCLIE2ETests/ValidateCommand.cs b/src/AppInstallerCLIE2ETests/ValidateCommand.cs @@ -15,6 +15,11 @@ namespace AppInstallerCLIE2ETests Assert.AreEqual(Constants.ErrorCode.S_OK, result.ExitCode); Assert.True(result.StdOut.Contains("Manifest validation succeeded.")); + // Validate a good yaml with extended character + result = TestCommon.RunAICLICommand("validate", TestCommon.GetTestDataFile("Manifests\\TëstExeInstaller.yaml")); + Assert.AreEqual(Constants.ErrorCode.S_OK, result.ExitCode); + Assert.True(result.StdOut.Contains("Manifest validation succeeded.")); + // Validate invalid yaml result = TestCommon.RunAICLICommand("validate", TestCommon.GetTestDataFile("Manifests\\TestInvalidManifest.yaml")); Assert.AreEqual(Constants.ErrorCode.S_OK, result.ExitCode); diff --git a/src/AppInstallerCommonCore/MsixInfo.cpp b/src/AppInstallerCommonCore/MsixInfo.cpp @@ -293,7 +293,7 @@ namespace AppInstaller::Msix } else { - std::filesystem::path path(uriStr); + std::filesystem::path path(Utility::ConvertToUTF16(uriStr)); THROW_IF_FAILED(SHCreateStreamOnFileEx(path.c_str(), STGM_READ | STGM_SHARE_DENY_WRITE | STGM_FAILIFTHERE, 0, FALSE, nullptr, &m_stream)); } diff --git a/src/AppInstallerRepositoryCore/Manifest/Manifest.cpp b/src/AppInstallerRepositoryCore/Manifest/Manifest.cpp @@ -216,7 +216,8 @@ namespace AppInstaller::Manifest try { - YAML::Node rootNode = YAML::LoadFile(inputFile.u8string()); + std::ifstream inputStream(inputFile); + YAML::Node rootNode = YAML::Load(inputStream); errors = manifest.PopulateManifestFields(rootNode, fullValidation); } catch (const ManifestException&) diff --git a/src/AppInstallerRepositoryCore/pch.h b/src/AppInstallerRepositoryCore/pch.h @@ -33,6 +33,7 @@ #include <algorithm> #include <chrono> #include <filesystem> +#include <fstream> #include <initializer_list> #include <iomanip> #include <optional>