commit 7775eba19bacd2dd78dba1c6773e55b141fde982
parent cd28b895761a3d2dbc541f1d0aa0eb3a4ccf16ca
Author: JohnMcPMS <johnmcp@microsoft.com>
Date: Thu, 18 Nov 2021 16:51:58 -0800
Change download file name to match the installer URL (#1722)
* Change download file name to match the installer URL
* Use installer hash as pre-hash temp file name
Diffstat:
5 files changed, 71 insertions(+), 20 deletions(-)
diff --git a/src/AppInstallerCLICore/AppInstallerCLICore.vcxproj.filters b/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" />
diff --git a/src/AppInstallerCLICore/Workflows/DownloadFlow.cpp b/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,11 +248,12 @@ 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;
@@ -228,7 +261,7 @@ namespace AppInstaller::CLI::Workflow
}
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);
diff --git a/src/AppInstallerCLITests/Strings.cpp b/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() == "");
+}
diff --git a/src/AppInstallerCommonCore/AppInstallerStrings.cpp b/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();
+ }
}
diff --git a/src/AppInstallerCommonCore/Public/AppInstallerStrings.h b/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);
}
