Add hash mismatch telemetry details (microsoft#4857)

## Issue
From looking at our hash mismatch data, there were two main modes:

1. Lots of actual hashes that are the result of hashing a zero-byte file
2. Other hashes with small numbers of cases, including some non-one
counts. This suggests at least some cases with consistent results even
if they are not what we expected.

My suspicion is that there are some cases where a successful HTTP
response is made, but the content is actually a webpage or similar. This
would happen with captive portals or potentially firewalls.

## Change
Collect the total size and content type of downloads and add them to our
existing hash mismatch telemetry events.
Retry downloading when we get a zero-byte file.
Report a different error when a zero-byte file is the final result of
downloading (also requires the hash to not match, so you can still have
a zero-byte installer for what that is worth...).

## Validation
Updates the downloader tests for new fields.
Adds a new pair of tests for zero-byte file downloads.

.github/actions/spelling/expect.txt

@@ -126,6 +126,8 @@ decompressor
 dedupe
 deigh
 deleteifnotneeded
+deliveryoptimization
+deliveryoptimizationerrors
 DENYWR
 desktopappinstaller
 devhome
@@ -338,6 +340,7 @@ msft
 msftrubengu
 MSIHASH
 MSIXHASH
+MSIXSTRM
 msstore
 MSZIP
 mszyml
@@ -405,6 +408,7 @@ PACL
 PARAMETERMAP
 pathparts
 Patil
+pbstr
 pcb
 PCCERT
 PCs

doc/windows/package-manager/winget/returnCodes.md

@@ -145,6 +145,7 @@ ms.localizationpriority: medium
 | 0x8A150083 | -1978335101 | APPINSTALLER_CLI_ERROR_LICENSING_API_FAILED | Failed to retrieve Microsoft Store package license. |
 | 0x8A150084 | -1978335100 | APPINSTALLER_CLI_ERROR_SFSCLIENT_PACKAGE_NOT_SUPPORTED | The Microsoft Store package does not support download command. |
 | 0x8A150085 | -1978335099 | APPINSTALLER_CLI_ERROR_LICENSING_API_FAILED_FORBIDDEN | Failed to retrieve Microsoft Store package license. The Microsoft Entra Id account does not have required privilege. |
+| 0x8A150086 | -1978335098 | APPINSTALLER_CLI_ERROR_INSTALLER_ZERO_BYTE_FILE | Downloaded zero byte installer; ensure that your network connection is working properly. |
 
 ## Install errors.
 

src/AppInstallerCLICore/ExecutionContextData.h

@@ -1,6 +1,7 @@
 // Copyright (c) Microsoft Corporation.
 // Licensed under the MIT License.
 #pragma once
+#include <AppInstallerDownloader.h>
 #include <winget/RepositorySource.h>
 #include <winget/Manifest.h>
 #include <winget/ARPCorrelation.h>
@@ -34,7 +35,7 @@ namespace AppInstaller::CLI::Execution
         Manifest,
         PackageVersion,
         Installer,
-        HashPair,
+        DownloadHashInfo,
         InstallerPath,
         LogPath,
         InstallerArgs,
@@ -128,9 +129,9 @@ namespace AppInstaller::CLI::Execution
         };
 
         template <>
-        struct DataMapping<Data::HashPair>
+        struct DataMapping<Data::DownloadHashInfo>
         {
-            using value_t = std::pair<std::vector<uint8_t>, std::vector<uint8_t>>;
+            using value_t = std::pair<std::vector<uint8_t>, Utility::DownloadResult>;
         };
 
         template <>

src/AppInstallerCLICore/Resources.h

@@ -308,6 +308,7 @@ namespace AppInstaller::CLI::Resource
         WINGET_DEFINE_RESOURCE_STRINGID(InstallersAbortTerminal);
         WINGET_DEFINE_RESOURCE_STRINGID(InstallersRequireInstallLocation);
         WINGET_DEFINE_RESOURCE_STRINGID(InstallerTypeArgumentDescription);
+        WINGET_DEFINE_RESOURCE_STRINGID(InstallerZeroByteFile);
         WINGET_DEFINE_RESOURCE_STRINGID(InstallFlowInstallSuccess);
         WINGET_DEFINE_RESOURCE_STRINGID(InstallFlowRegistrationDeferred);
         WINGET_DEFINE_RESOURCE_STRINGID(InstallFlowReturnCodeAlreadyInstalled);

src/AppInstallerCLICore/Workflows/DownloadFlow.cpp

@@ -172,15 +172,15 @@ namespace AppInstaller::CLI::Workflow
         // Checks the file hash for an existing installer file.
         // Returns true if the file exists and its hash matches, false otherwise.
         // If the hash does not match, deletes the file.
-        bool ExistingInstallerFileHasHashMatch(const SHA256::HashBuffer& expectedHash, const std::filesystem::path& filePath, SHA256::HashBuffer& fileHash)
+        bool ExistingInstallerFileHasHashMatch(const SHA256::HashBuffer& expectedHash, const std::filesystem::path& filePath, SHA256::HashDetails& fileHashDetails)
         {
             if (std::filesystem::exists(filePath))
             {
                 AICLI_LOG(CLI, Info, << "Found existing installer file at '" << filePath << "'. Verifying file hash.");
                 std::ifstream inStream{ filePath, std::ifstream::binary };
-                fileHash = SHA256::ComputeHash(inStream);
+                fileHashDetails = SHA256::ComputeHashDetails(inStream);
 
-                if (SHA256::AreEqual(expectedHash, fileHash))
+                if (SHA256::AreEqual(expectedHash, fileHashDetails.Hash))
                 {
                     return true;
                 }
@@ -280,11 +280,11 @@ 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 / installerFilename, fileHash))
+        SHA256::HashDetails fileHashDetails;
+        if (!ExistingInstallerFileHasHashMatch(installer.Sha256, installerPath / installerFilename, fileHashDetails))
         {
             installerFilename = GetInstallerPostHashValidationFileName(context);
-            if (!ExistingInstallerFileHasHashMatch(installer.Sha256, installerPath / installerFilename, fileHash))
+            if (!ExistingInstallerFileHasHashMatch(installer.Sha256, installerPath / installerFilename, fileHashDetails))
             {
                 // No match
                 return;
@@ -293,7 +293,8 @@ namespace AppInstaller::CLI::Workflow
 
         AICLI_LOG(CLI, Info, << "Existing installer file hash matches. Will use existing installer.");
         context.Add<Execution::Data::InstallerPath>(installerPath / installerFilename);
-        context.Add<Execution::Data::HashPair>(std::make_pair(installer.Sha256, fileHash));
+        context.Add<Execution::Data::DownloadHashInfo>(std::make_pair(installer.Sha256,
+            DownloadResult{ std::move(fileHashDetails.Hash), fileHashDetails.SizeInBytes }));
     }
 
     void GetInstallerDownloadPath(Execution::Context& context)
@@ -325,7 +326,7 @@ namespace AppInstaller::CLI::Workflow
 
         context.Reporter.Info() << Resource::String::Downloading << ' ' << Execution::UrlEmphasis << installer.Url << std::endl;
 
-        std::optional<std::vector<BYTE>> hash;
+        DownloadResult downloadResult;
 
         constexpr int MaxRetryCount = 2;
         constexpr std::chrono::seconds maximumWaitTimeAllowed = 60s;
@@ -334,15 +335,22 @@ namespace AppInstaller::CLI::Workflow
             bool success = false;
             try
             {
-                hash = context.Reporter.ExecuteWithProgress(std::bind(Utility::Download,
+                downloadResult = context.Reporter.ExecuteWithProgress(std::bind(Utility::Download,
                     installer.Url,
                     installerPath,
                     Utility::DownloadType::Installer,
                     std::placeholders::_1,
-                    true,
                     downloadInfo));
 
-                success = true;
+                if (downloadResult.SizeInBytes == 0)
+                {
+                    AICLI_LOG(CLI, Info, << "Got zero byte file; retrying download after a short wait...");
+                    std::this_thread::sleep_for(5s);
+                }
+                else
+                {
+                    success = true;
+                }
             }
             catch (const ServiceUnavailableException& sue)
             {
@@ -388,13 +396,13 @@ namespace AppInstaller::CLI::Workflow
             }
         }
 
-        if (!hash)
+        if (downloadResult.Sha256Hash.empty())
         {
             context.Reporter.Info() << Resource::String::Cancelled << std::endl;
             AICLI_TERMINATE_CONTEXT(E_ABORT);
         }
 
-        context.Add<Execution::Data::HashPair>(std::make_pair(installer.Sha256, hash.value()));
+        context.Add<Execution::Data::DownloadHashInfo>(std::make_pair(installer.Sha256, downloadResult));
     }
 
     void GetMsixSignatureHash(Execution::Context& context)
@@ -408,9 +416,14 @@ namespace AppInstaller::CLI::Workflow
 
             // Signature hash is only used for streaming installs, which don't use proxy
             Msix::MsixInfo msixInfo(installer.Url);
-            auto signatureHash = msixInfo.GetSignatureHash();
 
-            context.Add<Execution::Data::HashPair>(std::make_pair(installer.SignatureSha256, signatureHash));
+            DownloadResult hashInfo{ msixInfo.GetSignatureHash() };
+            // Value is ASCII for MSIXSTRM
+            // A sentinel value to indicate that this is a streaming hash rather than a download.
+            // The primary purpose is to prevent us from falling into the code path for zero byte files.
+            hashInfo.SizeInBytes = 0x4D5349585354524D;
+
+            context.Add<Execution::Data::DownloadHashInfo>(std::make_pair(installer.SignatureSha256, hashInfo));
             context.Add<Execution::Data::MsixDigests>({ std::make_pair(installer.Url, msixInfo.GetDigest()) });
         }
         catch (...)
@@ -427,17 +440,23 @@ namespace AppInstaller::CLI::Workflow
 
     void VerifyInstallerHash(Execution::Context& context)
     {
-        const auto& hashPair = context.Get<Execution::Data::HashPair>();
+        const auto& [expectedHash, downloadResult] = context.Get<Execution::Data::DownloadHashInfo>();
 
         if (!std::equal(
-            hashPair.first.begin(),
-            hashPair.first.end(),
-            hashPair.second.begin()))
+            expectedHash.begin(),
+            expectedHash.end(),
+            downloadResult.Sha256Hash.begin()))
         {
             bool overrideHashMismatch = context.Args.Contains(Execution::Args::Type::HashOverride);
 
             const auto& manifest = context.Get<Execution::Data::Manifest>();
-            Logging::Telemetry().LogInstallerHashMismatch(manifest.Id, manifest.Version, manifest.Channel, hashPair.first, hashPair.second, overrideHashMismatch);
+            Logging::Telemetry().LogInstallerHashMismatch(manifest.Id, manifest.Version, manifest.Channel, expectedHash, downloadResult.Sha256Hash, overrideHashMismatch, downloadResult.SizeInBytes, downloadResult.ContentType);
+
+            if (downloadResult.SizeInBytes == 0)
+            {
+                context.Reporter.Error() << Resource::String::InstallerZeroByteFile << std::endl;
+                AICLI_TERMINATE_CONTEXT(APPINSTALLER_CLI_ERROR_INSTALLER_ZERO_BYTE_FILE);
+            }
 
             // If running as admin, do not allow the user to override the hash failure.
             if (Runtime::IsRunningAsAdmin())
@@ -526,8 +545,9 @@ namespace AppInstaller::CLI::Workflow
             // Get the hash from the installer file
             const auto& installerPath = context.Get<Execution::Data::InstallerPath>();
             std::ifstream inStream{ installerPath, std::ifstream::binary };
-            auto existingFileHash = SHA256::ComputeHash(inStream);
-            context.Add<Execution::Data::HashPair>(std::make_pair(installer.Sha256, existingFileHash));
+            auto existingFileHashDetails = SHA256::ComputeHashDetails(inStream);
+            context.Add<Execution::Data::DownloadHashInfo>(std::make_pair(installer.Sha256,
+                DownloadResult{ existingFileHashDetails.Hash, existingFileHashDetails.SizeInBytes }));
         }
         else if (installer.EffectiveInstallerType() == InstallerTypeEnum::MSStore)
         {

src/AppInstallerCLICore/Workflows/MSStoreInstallerHandler.cpp

@@ -101,8 +101,8 @@ namespace AppInstaller::CLI::Workflow
                 }
 
                 // Verify hash
-                const auto& hashPair = subContext.Get<Execution::Data::HashPair>();
-                if (std::equal(hashPair.first.begin(), hashPair.first.end(), hashPair.second.begin()))
+                const auto& hashPair = subContext.Get<Execution::Data::DownloadHashInfo>();
+                if (std::equal(hashPair.first.begin(), hashPair.first.end(), hashPair.second.Sha256Hash.begin()))
                 {
                     AICLI_LOG(CLI, Info, << "Microsoft Store package hash verified");
                     subContext.Reporter.Info() << Resource::String::MSStoreDownloadPackageHashVerified << std::endl;

src/AppInstallerCLIE2ETests/AppInstallerCLIE2ETests.csproj

@@ -60,6 +60,7 @@
   <ItemGroup>
     <None Remove="TestData\Configuration\ShowDetails_TestRepo_0_3.yml" />
     <None Remove="TestData\Configuration\WithParameters_0_3.yml" />
+    <None Remove="TestData\empty" />
     <None Remove="TestData\Manifests\TestUpgradeAddsDependency.1.0.yaml" />
     <None Remove="TestData\Manifests\TestUpgradeAddsDependency.2.0.yaml" />
     <None Remove="TestData\Manifests\TestUpgradeAddsDependencyDependent.1.0.yaml" />

src/AppInstallerCLIE2ETests/Constants.cs

@@ -270,6 +270,8 @@ public class ErrorCode
             public const int ERROR_REPAIR_NOT_SUPPORTED = unchecked((int)0x8A15007C);
             public const int ERROR_ADMIN_CONTEXT_REPAIR_PROHIBITED = unchecked((int)0x8A15007D);
 
+            public const int ERROR_INSTALLER_ZERO_BYTE_FILE = unchecked((int)0x8A150086);
+
             public const int ERROR_INSTALL_PACKAGE_IN_USE = unchecked((int)0x8A150101);
             public const int ERROR_INSTALL_INSTALL_IN_PROGRESS = unchecked((int)0x8A150102);
             public const int ERROR_INSTALL_FILE_IN_USE = unchecked((int)0x8A150103);