commit c857d0ddbe2ec21098527f75ad62d3ab3074d945
parent 6e8a844541a2677ea9f3a9ee3e993073a0a764e2
Author: yao-msft <50888816+yao-msft@users.noreply.github.com>
Date: Fri, 19 Mar 2021 14:33:14 -0700
Better error message for manifest input validation errors (#812)
Diffstat:
8 files changed, 184 insertions(+), 111 deletions(-)
diff --git a/schemas/JSON/manifests/preview/manifest.0.1.0.json b/schemas/JSON/manifests/preview/manifest.0.1.0.json
@@ -33,8 +33,7 @@
},
"Url": {
"type": [ "string", "null" ],
- "format": "uri",
- "pattern": "^([Hh][Tt][Tt][Pp][Ss]?)://",
+ "pattern": "^([Hh][Tt][Tt][Pp][Ss]?)://.+$",
"maxLength": 2000
},
"Homepage": {
@@ -108,8 +107,7 @@
},
"Url": {
"type": "string",
- "format": "uri",
- "pattern": "^([Hh][Tt][Tt][Pp][Ss]?)://",
+ "pattern": "^([Hh][Tt][Tt][Pp][Ss]?)://.+$",
"maxLength": 2000,
"description": "Url is required. The path to the installer."
},
diff --git a/schemas/JSON/manifests/v1.0.0/manifest.defaultLocale.1.0.0.json b/schemas/JSON/manifests/v1.0.0/manifest.defaultLocale.1.0.0.json
@@ -5,8 +5,7 @@
"definitions": {
"Url": {
"type": [ "string", "null" ],
- "format": "uri",
- "pattern": "^([Hh][Tt][Tt][Pp][Ss]?)://",
+ "pattern": "^([Hh][Tt][Tt][Pp][Ss]?)://.+$",
"maxLength": 2000,
"description": "Optional Url type"
},
diff --git a/schemas/JSON/manifests/v1.0.0/manifest.installer.1.0.0.json b/schemas/JSON/manifests/v1.0.0/manifest.installer.1.0.0.json
@@ -304,8 +304,7 @@
},
"InstallerUrl": {
"type": "string",
- "format": "uri",
- "pattern": "^([Hh][Tt][Tt][Pp][Ss]?)://",
+ "pattern": "^([Hh][Tt][Tt][Pp][Ss]?)://.+$",
"description": "The installer Url"
},
"InstallerSha256": {
diff --git a/schemas/JSON/manifests/v1.0.0/manifest.locale.1.0.0.json b/schemas/JSON/manifests/v1.0.0/manifest.locale.1.0.0.json
@@ -5,8 +5,7 @@
"definitions": {
"Url": {
"type": [ "string", "null" ],
- "format": "uri",
- "pattern": "^([Hh][Tt][Tt][Pp][Ss]?)://",
+ "pattern": "^([Hh][Tt][Tt][Pp][Ss]?)://.+$",
"maxLength": 2000,
"description": "Optional Url type"
},
diff --git a/schemas/JSON/manifests/v1.0.0/manifest.singleton.1.0.0.json b/schemas/JSON/manifests/v1.0.0/manifest.singleton.1.0.0.json
@@ -23,8 +23,7 @@
},
"Url": {
"type": [ "string", "null" ],
- "format": "uri",
- "pattern": "^([Hh][Tt][Tt][Pp][Ss]?)://",
+ "pattern": "^([Hh][Tt][Tt][Pp][Ss]?)://.+$",
"maxLength": 2000,
"description": "Optional Url type"
},
@@ -316,8 +315,7 @@
},
"InstallerUrl": {
"type": "string",
- "format": "uri",
- "pattern": "^([Hh][Tt][Tt][Pp][Ss]?)://",
+ "pattern": "^([Hh][Tt][Tt][Pp][Ss]?)://.+$",
"description": "The installer Url"
},
"InstallerSha256": {
diff --git a/src/AppInstallerCLITests/TestData/InstallFlowTest_NonZeroExitCode.yaml b/src/AppInstallerCLITests/TestData/InstallFlowTest_NonZeroExitCode.yaml
@@ -1,5 +1,6 @@
PackageIdentifier: AppInstallerCliTest.TestInstaller
PackageVersion: 1.0.0.0
+PackageLocale: en-US
PackageName: AppInstaller Test Installer
Publisher: Microsoft Corporation
Moniker: AICLITestExe
diff --git a/src/AppInstallerCommonCore/Manifest/YamlParser.cpp b/src/AppInstallerCommonCore/Manifest/YamlParser.cpp
@@ -11,6 +11,71 @@ namespace AppInstaller::Manifest::YamlParser
{
namespace
{
+ // Basic V1 manifest required fields check for later manifest consistency check
+ void ValidateV1ManifestInput(const YamlManifestInfo& entry)
+ {
+ std::vector<ValidationError> errors;
+
+ if (!entry.Root.IsMap())
+ {
+ THROW_EXCEPTION_MSG(ManifestException(APPINSTALLER_CLI_ERROR_INVALID_MANIFEST), "The manifest does not contain a valid root. File: %S", entry.FileName.c_str());
+ }
+
+ if (!entry.Root["PackageIdentifier"])
+ {
+ errors.emplace_back(ValidationError::MessageFieldWithFile(
+ ManifestError::RequiredFieldMissing, "PackageIdentifier", entry.FileName));
+ }
+
+ if (!entry.Root["PackageVersion"])
+ {
+ errors.emplace_back(ValidationError::MessageFieldWithFile(
+ ManifestError::RequiredFieldMissing, "PackageVersion", entry.FileName));
+ }
+
+ if (!entry.Root["ManifestVersion"])
+ {
+ errors.emplace_back(ValidationError::MessageFieldWithFile(
+ ManifestError::RequiredFieldMissing, "ManifestVersion", entry.FileName));
+ }
+
+ if (!entry.Root["ManifestType"])
+ {
+ errors.emplace_back(ValidationError::MessageFieldWithFile(
+ ManifestError::InconsistentMultiFileManifestFieldValue, "ManifestType", entry.FileName));
+ }
+ else
+ {
+ auto manifestType = ConvertToManifestTypeEnum(entry.Root["ManifestType"].as<std::string>());
+
+ switch (manifestType)
+ {
+ case ManifestTypeEnum::Version:
+ if (!entry.Root["DefaultLocale"])
+ {
+ errors.emplace_back(ValidationError::MessageFieldWithFile(
+ ManifestError::RequiredFieldMissing, "DefaultLocale", entry.FileName));
+ }
+ break;
+ case ManifestTypeEnum::Singleton:
+ case ManifestTypeEnum::Locale:
+ case ManifestTypeEnum::DefaultLocale:
+ if (!entry.Root["PackageLocale"])
+ {
+ errors.emplace_back(ValidationError::MessageFieldWithFile(
+ ManifestError::RequiredFieldMissing, "PackageLocale", entry.FileName));
+ }
+ break;
+ }
+ }
+
+ if (!errors.empty())
+ {
+ ManifestException ex{ std::move(errors) };
+ THROW_EXCEPTION(ex);
+ }
+ }
+
// Input validations:
// - Determine manifest version
// - Check multi file manifest input integrity
@@ -53,107 +118,117 @@ namespace AppInstaller::Manifest::YamlParser
THROW_EXCEPTION_MSG(ManifestException(APPINSTALLER_CLI_ERROR_UNSUPPORTED_MANIFESTVERSION), "Unsupported ManifestVersion: %S", manifestVersion.ToString().c_str());
}
- // multi file manifest is only supported starting ManifestVersion 1.0.0
- if (isMultifileManifest && manifestVersion < ManifestVersionV1)
+ // Preview manifest validations
+ if (manifestVersion < ManifestVersionV1)
{
- THROW_EXCEPTION_MSG(ManifestException(APPINSTALLER_CLI_ERROR_INVALID_MANIFEST), "Preview manifest does not support multi file manifest format.");
- }
+ // multi file manifest is only supported starting ManifestVersion 1.0.0
+ if (isMultifileManifest)
+ {
+ THROW_EXCEPTION_MSG(ManifestException(APPINSTALLER_CLI_ERROR_INVALID_MANIFEST), "Preview manifest does not support multi file manifest format.");
+ }
- if (isMultifileManifest)
+ firstYamlManifest.ManifestType = ManifestTypeEnum::Preview;
+ }
+ // V1 manifest validations
+ else
{
- // Populates the PackageIdentifier and PackageVersion from first doc for later consistency check
- std::string packageId = firstYamlManifest.Root["PackageIdentifier"].as<std::string>();
- std::string packageVersion = firstYamlManifest.Root["PackageVersion"].as<std::string>();
-
- std::set<std::string> localesSet;
-
- bool isVersionManifestFound = false;
- bool isInstallerManifestFound = false;
- bool isDefaultLocaleManifestFound = false;
- std::string defaultLocaleFromVersionManifest;
- std::string defaultLocaleFromDefaultLocaleManifest;
-
- for (auto& entry : input)
+ // Check required fields used by later consistency check for better error message instead of
+ // Field Type Not Match error.
+ for (auto const& entry : input)
{
- if (!entry.Root.IsMap())
- {
- THROW_EXCEPTION_MSG(ManifestException(APPINSTALLER_CLI_ERROR_INVALID_MANIFEST), "The manifest does not contain a valid root. File: %S", entry.FileName.c_str());
- }
-
- std::string localPackageId = entry.Root["PackageIdentifier"].as<std::string>();
- if (localPackageId != packageId)
- {
- errors.emplace_back(ValidationError::MessageFieldValueWithFile(
- ManifestError::InconsistentMultiFileManifestFieldValue, "PackageIdentifier", localPackageId, entry.FileName));
- }
+ ValidateV1ManifestInput(entry);
+ }
- std::string localPackageVersion = entry.Root["PackageVersion"].as<std::string>();
- if (localPackageVersion != packageVersion)
- {
- errors.emplace_back(ValidationError::MessageFieldValueWithFile(
- ManifestError::InconsistentMultiFileManifestFieldValue, "PackageVersion", localPackageVersion, entry.FileName));
- }
+ if (isMultifileManifest)
+ {
+ // Populates the PackageIdentifier and PackageVersion from first doc for later consistency check
+ std::string packageId = firstYamlManifest.Root["PackageIdentifier"].as<std::string>();
+ std::string packageVersion = firstYamlManifest.Root["PackageVersion"].as<std::string>();
- std::string localManifestVersion = entry.Root["ManifestVersion"].as<std::string>();
- if (localManifestVersion != manifestVersionStr)
- {
- errors.emplace_back(ValidationError::MessageFieldValueWithFile(
- ManifestError::InconsistentMultiFileManifestFieldValue, "ManifestVersion", localManifestVersion, entry.FileName));
- }
+ std::set<std::string> localesSet;
- std::string manifestTypeStr = entry.Root["ManifestType"sv].as<std::string>();
- ManifestTypeEnum manifestType = ConvertToManifestTypeEnum(manifestTypeStr);
- entry.ManifestType = manifestType;
+ bool isVersionManifestFound = false;
+ bool isInstallerManifestFound = false;
+ bool isDefaultLocaleManifestFound = false;
+ std::string defaultLocaleFromVersionManifest;
+ std::string defaultLocaleFromDefaultLocaleManifest;
- switch (manifestType)
+ for (auto& entry : input)
{
- case ManifestTypeEnum::Version:
- if (isVersionManifestFound)
+ std::string localPackageId = entry.Root["PackageIdentifier"].as<std::string>();
+ if (localPackageId != packageId)
{
errors.emplace_back(ValidationError::MessageFieldValueWithFile(
- ManifestError::DuplicateMultiFileManifestType, "ManifestType", manifestTypeStr, entry.FileName));
+ ManifestError::InconsistentMultiFileManifestFieldValue, "PackageIdentifier", localPackageId, entry.FileName));
}
- else
- {
- isVersionManifestFound = true;
- defaultLocaleFromVersionManifest = entry.Root["DefaultLocale"sv].as<std::string>();
- }
- break;
- case ManifestTypeEnum::Installer:
- if (isInstallerManifestFound)
+
+ std::string localPackageVersion = entry.Root["PackageVersion"].as<std::string>();
+ if (localPackageVersion != packageVersion)
{
errors.emplace_back(ValidationError::MessageFieldValueWithFile(
- ManifestError::DuplicateMultiFileManifestType, "ManifestType", manifestTypeStr, entry.FileName));
- }
- else
- {
- isInstallerManifestFound = true;
+ ManifestError::InconsistentMultiFileManifestFieldValue, "PackageVersion", localPackageVersion, entry.FileName));
}
- break;
- case ManifestTypeEnum::DefaultLocale:
- if (isDefaultLocaleManifestFound)
+
+ std::string localManifestVersion = entry.Root["ManifestVersion"].as<std::string>();
+ if (localManifestVersion != manifestVersionStr)
{
errors.emplace_back(ValidationError::MessageFieldValueWithFile(
- ManifestError::DuplicateMultiFileManifestType, "ManifestType", manifestTypeStr, entry.FileName));
+ ManifestError::InconsistentMultiFileManifestFieldValue, "ManifestVersion", localManifestVersion, entry.FileName));
}
- else
- {
- isDefaultLocaleManifestFound = true;
- auto packageLocale = entry.Root["PackageLocale"sv].as<std::string>();
- defaultLocaleFromDefaultLocaleManifest = packageLocale;
- if (localesSet.find(packageLocale) != localesSet.end())
+ std::string manifestTypeStr = entry.Root["ManifestType"sv].as<std::string>();
+ ManifestTypeEnum manifestType = ConvertToManifestTypeEnum(manifestTypeStr);
+ entry.ManifestType = manifestType;
+
+ switch (manifestType)
+ {
+ case ManifestTypeEnum::Version:
+ if (isVersionManifestFound)
{
errors.emplace_back(ValidationError::MessageFieldValueWithFile(
- ManifestError::DuplicateMultiFileManifestLocale, "PackageLocale", packageLocale, entry.FileName));
+ ManifestError::DuplicateMultiFileManifestType, "ManifestType", manifestTypeStr, entry.FileName));
}
else
{
- localesSet.insert(packageLocale);
+ isVersionManifestFound = true;
+ defaultLocaleFromVersionManifest = entry.Root["DefaultLocale"sv].as<std::string>();
}
- }
- break;
- case ManifestTypeEnum::Locale:
+ break;
+ case ManifestTypeEnum::Installer:
+ if (isInstallerManifestFound)
+ {
+ errors.emplace_back(ValidationError::MessageFieldValueWithFile(
+ ManifestError::DuplicateMultiFileManifestType, "ManifestType", manifestTypeStr, entry.FileName));
+ }
+ else
+ {
+ isInstallerManifestFound = true;
+ }
+ break;
+ case ManifestTypeEnum::DefaultLocale:
+ if (isDefaultLocaleManifestFound)
+ {
+ errors.emplace_back(ValidationError::MessageFieldValueWithFile(
+ ManifestError::DuplicateMultiFileManifestType, "ManifestType", manifestTypeStr, entry.FileName));
+ }
+ else
+ {
+ isDefaultLocaleManifestFound = true;
+ auto packageLocale = entry.Root["PackageLocale"sv].as<std::string>();
+ defaultLocaleFromDefaultLocaleManifest = packageLocale;
+
+ if (localesSet.find(packageLocale) != localesSet.end())
+ {
+ errors.emplace_back(ValidationError::MessageFieldValueWithFile(
+ ManifestError::DuplicateMultiFileManifestLocale, "PackageLocale", packageLocale, entry.FileName));
+ }
+ else
+ {
+ localesSet.insert(packageLocale);
+ }
+ }
+ break;
+ case ManifestTypeEnum::Locale:
{
auto packageLocale = entry.Root["PackageLocale"sv].as<std::string>();
if (localesSet.find(packageLocale) != localesSet.end())
@@ -167,25 +242,23 @@ namespace AppInstaller::Manifest::YamlParser
}
}
break;
- default:
- errors.emplace_back(ValidationError::MessageFieldValueWithFile(
- ManifestError::UnsupportedMultiFileManifestType, "ManifestType", manifestTypeStr, entry.FileName));
+ default:
+ errors.emplace_back(ValidationError::MessageFieldValueWithFile(
+ ManifestError::UnsupportedMultiFileManifestType, "ManifestType", manifestTypeStr, entry.FileName));
+ }
}
- }
- if (isVersionManifestFound && isDefaultLocaleManifestFound && defaultLocaleFromDefaultLocaleManifest != defaultLocaleFromVersionManifest)
- {
- errors.emplace_back(ManifestError::InconsistentMultiFileManifestDefaultLocale);
- }
+ if (isVersionManifestFound && isDefaultLocaleManifestFound && defaultLocaleFromDefaultLocaleManifest != defaultLocaleFromVersionManifest)
+ {
+ errors.emplace_back(ManifestError::InconsistentMultiFileManifestDefaultLocale);
+ }
- if (!schemaValidationOnly && !(isVersionManifestFound && isInstallerManifestFound && isDefaultLocaleManifestFound))
- {
- errors.emplace_back(ManifestError::IncompleteMultiFileManifest);
+ if (!schemaValidationOnly && !(isVersionManifestFound && isInstallerManifestFound && isDefaultLocaleManifestFound))
+ {
+ errors.emplace_back(ManifestError::IncompleteMultiFileManifest);
+ }
}
- }
- else
- {
- if (manifestVersion >= ManifestVersionV1)
+ else
{
std::string manifestTypeStr = firstYamlManifest.Root["ManifestType"sv].as<std::string>();
ManifestTypeEnum manifestType = ConvertToManifestTypeEnum(manifestTypeStr);
@@ -201,10 +274,6 @@ namespace AppInstaller::Manifest::YamlParser
errors.emplace_back(ValidationError::MessageWithFile(ManifestError::IncompleteMultiFileManifest, firstYamlManifest.FileName));
}
}
- else
- {
- firstYamlManifest.ManifestType = ManifestTypeEnum::Preview;
- }
}
if (!errors.empty())
diff --git a/src/AppInstallerCommonCore/Public/winget/ManifestValidation.h b/src/AppInstallerCommonCore/Public/winget/ManifestValidation.h
@@ -31,7 +31,7 @@ namespace AppInstaller::Manifest
const char* const DuplicateInstallerEntry = "Duplicate installer entry found.";
const char* const InstallerTypeDoesNotSupportPackageFamilyName = "The specified installer type does not support PackageFamilyName.";
const char* const InstallerTypeDoesNotSupportProductCode = "The specified installer type does not support ProductCode.";
- const char* const IncompleteMultiFileManifest = "The multi file manifest is incomplete.";
+ const char* const IncompleteMultiFileManifest = "The multi file manifest is incomplete. A multi file manifest must contain at least version, installer and defaultLocale manifest.";
const char* const InconsistentMultiFileManifestFieldValue = "The multi file manifest has inconsistent field values.";
const char* const DuplicateMultiFileManifestType = "The multi file manifest should contain only one file with the particular ManifestType.";
const char* const DuplicateMultiFileManifestLocale = "The multi file manifest contains duplicate PackageLocale.";
@@ -85,6 +85,13 @@ namespace AppInstaller::Manifest
return error;
}
+ static ValidationError MessageFieldWithFile(std::string message, std::string field, std::string file)
+ {
+ ValidationError error{ message, field };
+ error.FileName = file;
+ return error;
+ }
+
static ValidationError MessageFieldValueWithFile(std::string message, std::string field, std::string value, std::string file)
{
ValidationError error{ message, field, value };
@@ -115,7 +122,10 @@ namespace AppInstaller::Manifest
if (m_errors.empty())
{
// Syntax error, yaml parser error is stored in FailureInfo
- m_manifestErrorMessage = Utility::ConvertToUTF8(GetFailureInfo().pszMessage);
+ if (GetFailureInfo().pszMessage)
+ {
+ m_manifestErrorMessage = Utility::ConvertToUTF8(GetFailureInfo().pszMessage);
+ }
}
else
{
