Fix renaming issue with invalid file character url (#2708)

microsoft · Nov 19, 2022 · 1520f75 · 1520f75
1 parent 1661096
commit 1520f75
Show file tree

Hide file tree

Showing 6 changed files with 67 additions and 6 deletions.
diff --git a/src/AppInstallerCLICore/Workflows/DownloadFlow.cpp b/src/AppInstallerCLICore/Workflows/DownloadFlow.cpp
@@ -81,6 +81,9 @@ namespace AppInstaller::CLI::Workflow
 
             filename += installerExtension;
 
+            // Make file name suitable for file system path
+            filename = Utility::ConvertToUTF16(Utility::MakeSuitablePathPart(filename.u8string()));
+
             return filename;
         }
 

diff --git a/src/AppInstallerCLITests/AppInstallerCLITests.vcxproj b/src/AppInstallerCLITests/AppInstallerCLITests.vcxproj
@@ -264,6 +264,9 @@
     <CopyFileToFolders Include="TestData\InstallFlowTest_Exe.yaml">
       <DeploymentContent>true</DeploymentContent>
     </CopyFileToFolders>
+    <CopyFileToFolders Include="TestData\InstallFlowTest_InvalidFileCharacterUrl.yaml">
+      <DeploymentContent>true</DeploymentContent>
+    </CopyFileToFolders>
     <CopyFileToFolders Include="TestData\InstallFlowTest_Msix_DownloadFlow.yaml">
       <DeploymentContent>true</DeploymentContent>
     </CopyFileToFolders>

diff --git a/src/AppInstallerCLITests/AppInstallerCLITests.vcxproj.filters b/src/AppInstallerCLITests/AppInstallerCLITests.vcxproj.filters
@@ -525,6 +525,9 @@
     <CopyFileToFolders Include="TestData\InstallFlowTest_Exe.yaml">
       <Filter>TestData</Filter>
     </CopyFileToFolders>
+    <CopyFileToFolders Include="TestData\InstallFlowTest_InvalidFileCharacterUrl.yaml">
+      <Filter>TestData</Filter>
+    </CopyFileToFolders>
     <CopyFileToFolders Include="TestData\InstallFlowTest_LicenseAgreement.yaml">
       <Filter>TestData</Filter>
     </CopyFileToFolders>

diff --git a/src/AppInstallerCLITests/TestData/InstallFlowTest_InvalidFileCharacterUrl.yaml b/src/AppInstallerCLITests/TestData/InstallFlowTest_InvalidFileCharacterUrl.yaml
@@ -0,0 +1,18 @@
+Id: AppInstallerCliTest.InvalidFileCharacterUrlTest
+Version: 1.0.0.0
+Name: AppInstaller Test Exe Installer
+Publisher: Microsoft Corporation
+AppMoniker: AICLITestExe
+License: Test
+ProductCode: AppInstallerCliTest.TestExeInstaller
+Installers: 
+    - Arch: x64
+      Url: https://EncodedUrlTest/te*st.exe
+      InstallerType: exe
+      Sha256: 65DB2F2AC2686C7F2FD69D4A4C6683B888DC55BFA20A0E32CA9F838B51689A3B
+      Switches:
+        Custom: /invalidFileCharacterUrl
+        SilentWithProgress: /silentwithprogress
+        Silent: /silence
+        Update: /update
+ManifestVersion: 0.1.0
diff --git a/src/AppInstallerCLITests/WorkFlow.cpp b/src/AppInstallerCLITests/WorkFlow.cpp
@@ -930,6 +930,38 @@ TEST_CASE("InstallFlow_RenameFromEncodedUrl", "[InstallFlow][workflow]")
     REQUIRE(installResultStr.find("/encodedUrl") != std::string::npos);
 }
 
+TEST_CASE("InstallFlow_RenameFromInvalidFileCharacterUrl", "[InstallFlow][workflow]")
+{
+    TestCommon::TempFile installResultPath("TestExeInstalled.txt");
+
+    std::ostringstream installOutput;
+    TestContext context{ installOutput, std::cin };
+    auto previousThreadGlobals = context.SetForCurrentThread();
+    OverrideForCheckExistingInstaller(context);
+    context.Override({ DownloadInstallerFile, [](TestContext& context)
+    {
+        context.Add<Data::HashPair>({ {}, {} });
+        auto installerPath = std::filesystem::temp_directory_path();
+        installerPath /= "InvalidFileCharacterUrlTest.exe";
+        std::filesystem::copy(TestDataFile("AppInstallerTestExeInstaller.exe"), installerPath, std::filesystem::copy_options::overwrite_existing);
+        context.Add<Data::InstallerPath>(installerPath);
+    } });
+    OverrideForUpdateInstallerMotw(context);
+    context.Args.AddArg(Execution::Args::Type::Manifest, TestDataFile("InstallFlowTest_InvalidFileCharacterUrl.yaml").GetPath().u8string());
+
+    InstallCommand install({});
+    install.Execute(context);
+    INFO(installOutput.str());
+
+    // Verify Installer is called and parameters are passed in.
+    REQUIRE(std::filesystem::exists(installResultPath.GetPath()));
+    std::ifstream installResultFile(installResultPath.GetPath());
+    REQUIRE(installResultFile.is_open());
+    std::string installResultStr;
+    std::getline(installResultFile, installResultStr);
+    REQUIRE(installResultStr.find("/invalidFileCharacterUrl") != std::string::npos);
+}
+
 TEST_CASE("InstallFlowNonZeroExitCode", "[InstallFlow][workflow]")
 {
     TestCommon::TempFile installResultPath("TestExeInstalled.txt");

diff --git a/src/AppInstallerCommonCore/Downloader.cpp b/src/AppInstallerCommonCore/Downloader.cpp
@@ -32,17 +32,19 @@ namespace AppInstaller::Utility
 
         AICLI_LOG(Core, Info, << "WinINet downloading from url: " << url);
 
-        wil::unique_hinternet session(InternetOpenA(
-            Runtime::GetDefaultUserAgent().get().c_str(),
+        auto agentWide = Utility::ConvertToUTF16(Runtime::GetDefaultUserAgent().get());
+        wil::unique_hinternet session(InternetOpen(
+            agentWide.c_str(),
             INTERNET_OPEN_TYPE_PRECONFIG,
             NULL,
             NULL,
             0));
         THROW_LAST_ERROR_IF_NULL_MSG(session, "InternetOpen() failed.");
 
-        wil::unique_hinternet urlFile(InternetOpenUrlA(
+        auto urlWide = Utility::ConvertToUTF16(url);
+        wil::unique_hinternet urlFile(InternetOpenUrl(
             session.get(),
-            url.c_str(),
+            urlWide.c_str(),
             NULL,
             0,
             INTERNET_FLAG_IGNORE_REDIRECT_TO_HTTPS, // This allows http->https redirection
@@ -53,7 +55,7 @@ namespace AppInstaller::Utility
         DWORD requestStatus = 0;
         DWORD cbRequestStatus = sizeof(requestStatus);
 
-        THROW_LAST_ERROR_IF_MSG(!HttpQueryInfoA(urlFile.get(),
+        THROW_LAST_ERROR_IF_MSG(!HttpQueryInfo(urlFile.get(),
             HTTP_QUERY_STATUS_CODE | HTTP_QUERY_FLAG_NUMBER,
             &requestStatus,
             &cbRequestStatus,
@@ -71,7 +73,7 @@ namespace AppInstaller::Utility
         LONGLONG contentLength = 0;
         DWORD cbContentLength = sizeof(contentLength);
 
-        HttpQueryInfoA(
+        HttpQueryInfo(
             urlFile.get(),
             HTTP_QUERY_CONTENT_LENGTH | HTTP_QUERY_FLAG_NUMBER64,
             &contentLength,