Manifest (Msix) installer validation - Try parsing package version (#…

…2692)
microsoft · Nov 11, 2022 · ae32774 · ae32774
1 parent fa660c6
commit ae32774
Show file tree

Hide file tree

Showing 6 changed files with 57 additions and 14 deletions.
diff --git a/src/AppInstallerCLITests/AppInstallerCLITests.vcxproj b/src/AppInstallerCLITests/AppInstallerCLITests.vcxproj
@@ -776,6 +776,9 @@
     <CopyFileToFolders Include="TestData\Manifest-Bad-InconsistentSignedMsixBundleInstallerFields.yaml">
       <DeploymentContent>true</DeploymentContent>
     </CopyFileToFolders>
+    <CopyFileToFolders Include="TestData\Manifest-Bad-MsixInstaller-PackageVersion.yaml">
+      <DeploymentContent>true</DeploymentContent>
+    </CopyFileToFolders>
   </ItemGroup>
   <ItemGroup>
     <ProjectReference Include="..\AppInstallerCLICore\AppInstallerCLICore.vcxproj">

diff --git a/src/AppInstallerCLITests/AppInstallerCLITests.vcxproj.filters b/src/AppInstallerCLITests/AppInstallerCLITests.vcxproj.filters
@@ -726,6 +726,9 @@
     <CopyFileToFolders Include="TestData\Manifest-Bad-MissingMsixInstallerFields.yaml">
       <Filter>TestData</Filter>
     </CopyFileToFolders>
+    <CopyFileToFolders Include="TestData\Manifest-Bad-MsixInstaller-PackageVersion.yaml">
+      <Filter>TestData</Filter>
+    </CopyFileToFolders>
     <CopyFileToFolders Include="TestData\Manifest-Bad-NoSupportedPlatforms.yaml">
       <DeploymentContent>true</DeploymentContent>
     </CopyFileToFolders>

diff --git a/src/AppInstallerCLITests/TestData/Manifest-Bad-MsixInstaller-PackageVersion.yaml b/src/AppInstallerCLITests/TestData/Manifest-Bad-MsixInstaller-PackageVersion.yaml
@@ -0,0 +1,13 @@
+PackageIdentifier: AppInstallerCliTest.BadMsixInstaller-PackageVersion
+PackageVersion: test.version # Version cannot be parsed to UINT64
+PackageLocale: es-MX
+PackageName: es-MX package name
+Publisher: es-MX publisher
+PackageFamilyName: FakeInstallerForTesting_125rzkzqaqjwj
+MinimumOSVersion: 10.0.0.0
+InstallerType: msix
+Installers: 
+    - Architecture: x64
+      InstallerUrl: Installer-Good.msix
+ManifestType: merged
+ManifestVersion: 1.0.0
diff --git a/src/AppInstallerCLITests/YamlManifest.cpp b/src/AppInstallerCLITests/YamlManifest.cpp
@@ -998,6 +998,21 @@ TEST_CASE("ReadManifestAndValidateMsixInstallers_NoSupportedPlatforms", "[Manife
     ValidateError(errors[0], ValidationError::Level::Error, ManifestError::NoSupportedPlatforms, "InstallerUrl", manifest.Installers.front().Url);
 }
 
+TEST_CASE("ReadManifestAndValidateMsixInstallers_PackageVersionNotUINT64", "[ManifestValidation]")
+{
+    Manifest manifest = YamlParser::CreateFromPath(TestDataFile("Manifest-Bad-MsixInstaller-PackageVersion.yaml"));
+
+    // Update the installer path for testing
+    REQUIRE(1 == manifest.Installers.size());
+    TestDataFile msixFile(manifest.Installers[0].Url.c_str());
+    manifest.Installers[0].Url = msixFile.GetPath().u8string();
+
+    auto errors = ValidateManifestInstallers(manifest);
+    REQUIRE(1 == errors.size());
+
+    ValidateError(errors[0], ValidationError::Level::Error, ManifestError::InstallerMsixInconsistencies, "PackageVersion", "43690.48059.52428.56797");
+}
+
 TEST_CASE("ReadManifestAndValidateMsixInstallers_MissingFields", "[ManifestValidation]")
 {
     TestDataFile testFile("Manifest-Bad-MissingMsixInstallerFields.yaml");

diff --git a/src/AppInstallerCommonCore/Manifest/MsixManifestValidation.cpp b/src/AppInstallerCommonCore/Manifest/MsixManifestValidation.cpp
@@ -9,11 +9,10 @@
 namespace AppInstaller::Manifest
 {
     std::vector<ValidationError> MsixManifestValidation::Validate(
-        const Manifest &manifest,
-        const ManifestInstaller &installer)
+        const Manifest& manifest,
+        const ManifestInstaller& installer)
     {
         std::vector<ValidationError> errors;
-        Msix::PackageVersion packageVersion(manifest.Version);
         auto msixInfo = GetMsixInfo(installer.Url);
         if (msixInfo)
         {
@@ -24,7 +23,7 @@ namespace AppInstaller::Manifest
             {
                 auto msixManifestIdentity = msixManifest.GetIdentity();
                 ValidateMsixManifestPackageFamilyName(msixManifestIdentity.GetPackageFamilyName(), installer.PackageFamilyName, errors);
-                ValidateMsixManifestPackageVersion(msixManifestIdentity.GetVersion(), packageVersion, errors);
+                ValidateMsixManifestPackageVersion(msixManifestIdentity.GetVersion(), manifest.Version, errors);
                 ValidateMsixManifestMinOSVersion(msixManifest.GetMinimumOSVersionForSupportedPlatforms(), installerMinOSVersion, installer.Url, errors);
             }
         }
@@ -147,7 +146,7 @@ namespace AppInstaller::Manifest
 
     std::optional<Msix::OSVersion> MsixManifestValidation::GetManifestInstallerMinOSVersion(
         std::string minOSVersion,
-        std::vector<ValidationError> &errors)
+        std::vector<ValidationError>& errors)
     {
         try
         {
@@ -166,7 +165,7 @@ namespace AppInstaller::Manifest
     void MsixManifestValidation::ValidateMsixManifestPackageFamilyName(
         Utility::NormalizedString msixPackageFamilyName,
         Utility::NormalizedString manifestPackageFamilyName,
-        std::vector<ValidationError> &errors)
+        std::vector<ValidationError>& errors)
     {
         if (!manifestPackageFamilyName.empty())
         {
@@ -182,11 +181,21 @@ namespace AppInstaller::Manifest
     }
 
     void MsixManifestValidation::ValidateMsixManifestPackageVersion(
-        const Msix::PackageVersion &msixPackageVersion,
-        const Msix::PackageVersion &manifestPackageVersion,
-        std::vector<ValidationError> &errors)
+        const Msix::PackageVersion& msixPackageVersion,
+        const string_t& manifestPackageVersionStr,
+        std::vector<ValidationError>& errors)
     {
-        if (msixPackageVersion != manifestPackageVersion)
+        std::optional<Msix::PackageVersion> manifestPackageVersion;
+        try
+        {
+            manifestPackageVersion = { manifestPackageVersionStr };
+        }
+        catch (...)
+        {
+            AICLI_LOG(Core, Error, << "Failed to parse package version to UINT64");
+        }
+
+        if (!manifestPackageVersion.has_value() || msixPackageVersion != manifestPackageVersion.value())
         {
             errors.emplace_back(ManifestError::InstallerMsixInconsistencies, "PackageVersion", msixPackageVersion.ToString());
         }
@@ -196,7 +205,7 @@ namespace AppInstaller::Manifest
         const std::optional<Msix::OSVersion>& msixMinOSVersion,
         const std::optional<Msix::OSVersion>& manifestMinOSVersion,
         std::string installerUrl,
-        std::vector<ValidationError> &errors)
+        std::vector<ValidationError>& errors)
     {
         if (!msixMinOSVersion.has_value())
         {

diff --git a/src/AppInstallerCommonCore/Public/winget/MsixManifestValidation.h b/src/AppInstallerCommonCore/Public/winget/MsixManifestValidation.h
@@ -16,8 +16,8 @@ namespace AppInstaller::Manifest
 
         // Validate manifest for Msix packages and Msix bundles.
         std::vector<ValidationError> Validate(
-            const Manifest &manifest,
-            const ManifestInstaller &installer);
+            const Manifest& manifest,
+            const ManifestInstaller& installer);
     private:
         std::map<std::string, std::shared_ptr<Msix::MsixInfo>> m_msixInfoCache;
         std::vector<std::filesystem::path> m_downloadedInstallers;
@@ -54,7 +54,7 @@ namespace AppInstaller::Manifest
         // Validate Msix package version.
         void ValidateMsixManifestPackageVersion(
             const Msix::PackageVersion& msixPackageVersion,
-            const Msix::PackageVersion& manifestPackageVersion,
+            const string_t& manifestPackageVersionStr,
             std::vector<ValidationError>& errors);
 
         // Validate Msix minimum OS version for supported platforms.