Change download file name to match the installer URL

src/AppInstallerCLICore/AppInstallerCLICore.vcxproj.filters

@@ -301,9 +301,6 @@
     <ClCompile Include="ContextOrchestrator.cpp">
       <Filter>Source Files</Filter>
     </ClCompile>
-    <ClCompile Include="Workflows\DependenciesFlow.cpp">
-      <Filter>Source Files</Filter>
-    </ClCompile>
     <ClCompile Include="Commands\COMInstallCommand.cpp">
       <Filter>Commands</Filter>
     </ClCompile>
@@ -313,12 +310,15 @@
     <ClCompile Include="Workflows\SettingsFlow.cpp">
       <Filter>Workflows</Filter>
     </ClCompile>
-    <ClCompile Include="Workflows\DownloadFlow.cpp">
-      <Filter>Source Files</Filter>
-    </ClCompile>
     <ClCompile Include="Workflows\DependencyNodeProcessor.cpp">
       <Filter>Workflows</Filter>
     </ClCompile>
+    <ClCompile Include="Workflows\DependenciesFlow.cpp">
+      <Filter>Workflows</Filter>
+    </ClCompile>
+    <ClCompile Include="Workflows\DownloadFlow.cpp">
+      <Filter>Workflows</Filter>
+    </ClCompile>
   </ItemGroup>
   <ItemGroup>
     <None Include="PropertySheet.props" />

src/AppInstallerCLICore/Workflows/DownloadFlow.cpp

@@ -14,14 +14,17 @@ namespace AppInstaller::CLI::Workflow
 
     namespace
     {
-        // Get the base download path for the installer path.
-        // This path does not include the file extension, which will be added
-        // after verifying the file hash to prevent it from being ShellExecute-d
+        // Get the base download directory path for the installer.
+        // Also creates the directory as necessary.
         std::filesystem::path GetInstallerBaseDownloadPath(Execution::Context& context)
         {
             const auto& manifest = context.Get<Execution::Data::Manifest>();
+
             std::filesystem::path tempInstallerPath = Runtime::GetPathTo(Runtime::PathName::Temp);
             tempInstallerPath /= Utility::ConvertToUTF16(manifest.Id + '.' + manifest.Version);
+
+            std::filesystem::create_directories(tempInstallerPath);
+
             return tempInstallerPath;
         }
 
@@ -49,6 +52,35 @@ namespace AppInstaller::CLI::Workflow
             }
         }
 
+        // Gets a file name that should not be able to ShellExecute.
+        std::filesystem::path GetInstallerPreHashValidationFileName(Execution::Context& context)
+        {
+            return { SHA256::ConvertToString(context.Get<Execution::Data::Installer>()->Sha256) };
+        }
+
+        // Gets the file name that can be used to ShellExecute the file.
+        std::filesystem::path GetInstallerPostHashValidationFileName(Execution::Context& context)
+        {
+            // Get file name from download URI
+            std::filesystem::path filename = GetFileNameFromURI(context.Get<Execution::Data::Installer>()->Url);
+
+            // Assuming that we find a stem value in the URI, use it.
+            // This should be extremely common, but just in case fall back to the older name style.
+            if (filename.has_stem())
+            {
+                filename = filename.stem();
+            }
+            else
+            {
+                const auto& manifest = context.Get<Execution::Data::Manifest>();
+                filename = Utility::ConvertToUTF16(manifest.Id + '.' + manifest.Version);
+            }
+
+            filename += GetInstallerFileExtension(context);
+
+            return filename;
+        }
+
         // Try to remove the installer file, ignoring any errors.
         void RemoveInstallerFile(const std::filesystem::path& path)
         {
@@ -216,19 +248,20 @@ namespace AppInstaller::CLI::Workflow
 
         // Try looking for the file with and without extension.
         auto installerPath = GetInstallerBaseDownloadPath(context);
+        auto installerFilename = GetInstallerPreHashValidationFileName(context);
         SHA256::HashBuffer fileHash;
-        if (!ExistingInstallerFileHasHashMatch(installer.Sha256, installerPath, fileHash))
+        if (!ExistingInstallerFileHasHashMatch(installer.Sha256, installerPath / installerFilename, fileHash))
         {
-            installerPath += GetInstallerFileExtension(context);
-            if (!ExistingInstallerFileHasHashMatch(installer.Sha256, installerPath, fileHash))
+            installerFilename = GetInstallerPostHashValidationFileName(context);
+            if (!ExistingInstallerFileHasHashMatch(installer.Sha256, installerPath / installerFilename, fileHash))
             {
                 // No match
                 return;
             }
         }
 
         AICLI_LOG(CLI, Info, << "Existing installer file hash matches. Will use existing installer.");
-        context.Add<Execution::Data::InstallerPath>(installerPath);
+        context.Add<Execution::Data::InstallerPath>(installerPath / installerFilename);
         context.Add<Execution::Data::HashPair>(std::make_pair(installer.Sha256, fileHash));
     }
 
@@ -237,6 +270,7 @@ namespace AppInstaller::CLI::Workflow
         if (!context.Contains(Execution::Data::InstallerPath))
         {
             auto tempInstallerPath = GetInstallerBaseDownloadPath(context);
+            tempInstallerPath /= GetInstallerPreHashValidationFileName(context);
             AICLI_LOG(CLI, Info, << "Generated temp download path: " << tempInstallerPath);
             context.Add<Execution::Data::InstallerPath>(std::move(tempInstallerPath));
         }
@@ -467,16 +501,15 @@ namespace AppInstaller::CLI::Workflow
         }
 
         auto& installerPath = context.Get<Execution::Data::InstallerPath>();
-        auto installerExtension = GetInstallerFileExtension(context);
-        if (installerPath.extension() == installerExtension)
+        std::filesystem::path renamedDownloadedInstaller = installerPath;
+        renamedDownloadedInstaller.replace_filename(GetInstallerPostHashValidationFileName(context));
+
+        if (installerPath == renamedDownloadedInstaller)
         {
-            // Installer file already has expected extension.
+            // In case we are reusing an existing downloaded file
             return;
         }
 
-        std::filesystem::path renamedDownloadedInstaller(installerPath);
-        renamedDownloadedInstaller += installerExtension;
-
         RenameFile(installerPath, renamedDownloadedInstaller);
 
         installerPath.assign(renamedDownloadedInstaller);

src/AppInstallerCLITests/Strings.cpp

@@ -187,3 +187,10 @@ TEST_CASE("MakeSuitablePathPart", "[strings]")
     REQUIRE_THROWS_HR(MakeSuitablePathPart("COM1"), E_INVALIDARG);
     REQUIRE_THROWS_HR(MakeSuitablePathPart("NUL.txt"), E_INVALIDARG);
 }
+
+TEST_CASE("GetFileNameFromURI", "[strings]")
+{
+    REQUIRE(GetFileNameFromURI("https://github.com/microsoft/winget-cli/pull/1722").u8string() == "1722");
+    REQUIRE(GetFileNameFromURI("https://github.com/microsoft/winget-cli/README.md").u8string() == "README.md");
+    REQUIRE(GetFileNameFromURI("https://microsoft.com/").u8string() == "");
+}

src/AppInstallerCommonCore/AppInstallerStrings.cpp

@@ -583,4 +583,12 @@ namespace AppInstaller::Utility
 
         return result;
     }
+
+    std::filesystem::path GetFileNameFromURI(std::string_view uri)
+    {
+        winrt::Windows::Foundation::Uri winrtUri{ winrt::hstring{ ConvertToUTF16(uri) } };
+        std::filesystem::path path{ static_cast<std::wstring_view>(winrtUri.Path()) };
+
+        return path.filename();
+    }
 }

src/AppInstallerCommonCore/Public/AppInstallerStrings.h

@@ -1,7 +1,7 @@
 // Copyright (c) Microsoft Corporation.
 // Licensed under the MIT License.
 #pragma once
-
+#include <filesystem>
 #include <ostream>
 #include <string>
 #include <string_view>
@@ -142,4 +142,7 @@ namespace AppInstaller::Utility
 
     // Converts the candidate path part into one suitable for the actual file system
     std::string MakeSuitablePathPart(std::string_view candidate);
+
+    // Gets the file name part of the given URI.
+    std::filesystem::path GetFileNameFromURI(std::string_view uri);
 }