From 8b9fce67b3fcd972e0c58de169aaf1dc2c55b204 Mon Sep 17 00:00:00 2001 From: Madhusudhan Gumbalapura Sudarshan Date: Sat, 11 Jan 2025 18:14:07 -0800 Subject: [PATCH 01/18] Add schema header validation for YAML manifests V.1.10.0 and above 1. Introduce functions to search, parse, and validate schema headers in YAML manifests. 2. Add/Update structs and enums to support schema header validation. 3. Add new error messages and validation options, including treating schema header validation errors as warnings. Schema Header Validation Includes: 1. Validate presence of Manifest Schema Header. 2. Ensure Schema Header format correctness. 3. Verify Schema Header URL Pattern accuracy. 4. Confirm Schema Header Manifest Type matches ManifestType Property. 5. Check Schema Header Manifest Version matches ManifestVersion Property. --- .../Manifest/ManifestSchemaValidation.cpp | 198 +++++++++++++++++- .../Manifest/ManifestValidation.cpp | 7 +- .../Manifest/YamlParser.cpp | 13 +- .../Public/winget/ManifestCommon.h | 1 + .../Public/winget/ManifestSchemaValidation.h | 8 +- .../Public/winget/ManifestValidation.h | 23 +- .../Public/winget/ManifestYamlParser.h | 5 +- 7 files changed, 247 insertions(+), 8 deletions(-) diff --git a/src/AppInstallerCommonCore/Manifest/ManifestSchemaValidation.cpp b/src/AppInstallerCommonCore/Manifest/ManifestSchemaValidation.cpp index d25182d0a0..21d55c3fe6 100644 --- a/src/AppInstallerCommonCore/Manifest/ManifestSchemaValidation.cpp +++ b/src/AppInstallerCommonCore/Manifest/ManifestSchemaValidation.cpp @@ -16,6 +16,18 @@ namespace AppInstaller::Manifest::YamlParser namespace { + constexpr std::string_view YamlLanguageServerKey = "yaml-language-server"sv; + + struct ManifestSchemaHeader + { + std::string SchemaHeaderString; + std::string ManifestType; + std::string ManifestVersion; + std::string FileName; + size_t Line; + size_t column; + }; + enum class YamlScalarType { String, @@ -24,7 +36,7 @@ namespace AppInstaller::Manifest::YamlParser }; // List of fields that use non string scalar types - const std::map ManifestFieldTypes= + const std::map ManifestFieldTypes = { { "InstallerSuccessCodes"sv, YamlScalarType::Int }, { "InstallerAbortsTerminal"sv, YamlScalarType::Bool }, @@ -98,6 +110,167 @@ namespace AppInstaller::Manifest::YamlParser return result; } + + bool SearchForManifestSchemaHeaderString(std::shared_ptr yamlInputStream, const size_t& rootNodeBeginsAtLine, ManifestSchemaHeader& schemaHeader) + { + std::string line; + size_t currentLine = 1; + schemaHeader.SchemaHeaderString.clear(); + + // Search for the schema header string in the comments before the root node. + while (currentLine < rootNodeBeginsAtLine && std::getline(*yamlInputStream, line)) + { + std::string comment = Utility::Trim(line); + + // Check if the line is a comment + if (!comment.empty() && comment[0] == '#') + { + size_t pos = comment.find(YamlLanguageServerKey); + + // Check if the comment contains the schema header string + if (pos != std::string::npos) + { + schemaHeader.SchemaHeaderString = std::move(comment); + schemaHeader.Line = currentLine; + schemaHeader.column = pos; + + return true; + } + } + + currentLine++; + } + + return false; + } + + std::vector ParseSchemaHeaderString(const ManifestSchemaHeader& manifestSchemaHeader, const ValidationError::Level& errorLevel, std::string& schemaHeaderUrlString) + { + std::vector errors; + std::string schemaHeader = manifestSchemaHeader.SchemaHeaderString; + + // Remove the leading '#' and any leading/trailing whitespaces + if (schemaHeader[0] == '#') + { + schemaHeader = schemaHeader.substr(1); // Remove the leading '#' + schemaHeader = Utility::Trim(schemaHeader); // Trim leading/trailing whitespaces + } + + // Parse the schema header string as YAML string to get the schema header URL + + try + { + auto root = YAML::Load(schemaHeader); + + if (root.IsNull() || (!root.IsNull() && !root.IsDefined())) + { + errors.emplace_back(ValidationError::MessageContextValueLineLevelWithFile(ManifestError::InvalidSchemaHeader, "", schemaHeader, manifestSchemaHeader.Line, manifestSchemaHeader.column, errorLevel, manifestSchemaHeader.FileName)); + } + else + { + schemaHeaderUrlString = root[YamlLanguageServerKey].as(); + } + } + catch (const YAML::Exception&) + { + errors.emplace_back(ValidationError::MessageContextValueLineLevelWithFile(ManifestError::InvalidSchemaHeader, "", schemaHeader, manifestSchemaHeader.Line, manifestSchemaHeader.column, errorLevel, manifestSchemaHeader.FileName)); + } + catch (const std::exception&) + { + errors.emplace_back(ValidationError::MessageContextValueLineLevelWithFile(ManifestError::InvalidSchemaHeader, "", schemaHeader, manifestSchemaHeader.Line, manifestSchemaHeader.column, errorLevel, manifestSchemaHeader.FileName)); + } + + return errors; + } + + bool ParseSchemaHeaderUrl(const std::string& schemaHeaderValue, std::string& schemaType, std::string& schemaVersion) + { + // Use regex to match the pattern of @"winget-manifest\.(?\w+)\.(?[\d\.]+)\.schema\.json$" + std::regex schemaUrlPattern(R"(^\$schema=https://aka\.ms/winget-manifest\.(\w+)\.([\d\.]+)\.schema\.json$)"); + std::smatch match; + + if (std::regex_search(schemaHeaderValue, match, schemaUrlPattern)) + { + schemaType = match[1].str(); + schemaVersion = match[2].str(); + return true; + } + + return false; + } + + std::vector ValidateManifestSchemaHeaderDetails(const ManifestSchemaHeader& schemaHeader, const ManifestTypeEnum& expectedManifestType, const ManifestVer& expectedManifestVersion, ValidationError::Level errorLevel) + { + std::vector errors; + ManifestTypeEnum actualManifestType = ConvertToManifestTypeEnum(schemaHeader.ManifestType); + ManifestVer actualHeaderVersion(schemaHeader.ManifestVersion); + + size_t actualManifestTypeIndex = schemaHeader.SchemaHeaderString.find(schemaHeader.ManifestType) + 1; + size_t actualHeaderVersionIndex = schemaHeader.SchemaHeaderString.find(schemaHeader.ManifestVersion) + 1; + + if (actualManifestType != expectedManifestType) + { + errors.emplace_back(ValidationError::MessageContextValueLineLevelWithFile(ManifestError::SchemaHeaderManifestTypeMismatch, "", schemaHeader.ManifestType, schemaHeader.Line, actualManifestTypeIndex, errorLevel, schemaHeader.FileName)); + } + + if (actualHeaderVersion != expectedManifestVersion) + { + errors.emplace_back(ValidationError::MessageContextValueLineLevelWithFile(ManifestError::SchemaHeaderManifestVersionMismatch, "", schemaHeader.ManifestVersion, schemaHeader.Line, actualHeaderVersionIndex, errorLevel, schemaHeader.FileName)); + } + + return errors; + } + + std::vector ValidateYamlManifestSchemaHeader(const YamlManifestInfo& manifestInfo, const ManifestVer& manifestVersion, const ValidationError::Level& errorLevel) + { + std::vector errors; + + if (manifestInfo.ManifestType == ManifestTypeEnum::Shadow) + { + // There's no schema for a shadow manifest. + return errors; + } + + size_t rootNodeLine = manifestInfo.Root.Mark().line; + std::string schemaHeaderString; + + ManifestSchemaHeader schemaHeader{}; + schemaHeader.FileName = manifestInfo.FileName; + + if (!SearchForManifestSchemaHeaderString(manifestInfo.InputStream, rootNodeLine, schemaHeader)) + { + errors.emplace_back(ValidationError::MessageLevelWithFile(ManifestError::SchemaHeaderNotFound, errorLevel, manifestInfo.FileName)); + return errors; + } + + std::string schemaHeaderUrlString; + auto parserErrors = ParseSchemaHeaderString(schemaHeader, errorLevel, schemaHeaderUrlString); + std::move(parserErrors.begin(), parserErrors.end(), std::inserter(errors, errors.end())); + + if (!errors.empty()) + { + return errors; + } + + std::string manifestTypeString; + std::string manifestVersionString; + + if (ParseSchemaHeaderUrl(schemaHeaderUrlString, manifestTypeString, manifestVersionString)) + { + schemaHeader.ManifestType = std::move(manifestTypeString); + schemaHeader.ManifestVersion = std::move(manifestVersionString); + + auto compareErrors = ValidateManifestSchemaHeaderDetails(schemaHeader, manifestInfo.ManifestType, manifestVersion, errorLevel); + std::move(compareErrors.begin(), compareErrors.end(), std::inserter(errors, errors.end())); + } + else + { + size_t schemaHeaderUrlIndex = schemaHeader.SchemaHeaderString.find(schemaHeaderUrlString) + 1; + errors.emplace_back(ValidationError::MessageContextValueLineLevelWithFile(ManifestError::SchemaHeaderUrlPatternMismatch, "", schemaHeaderUrlString, schemaHeader.Line, schemaHeaderUrlIndex, errorLevel, schemaHeader.FileName)); + } + + return errors; + } } Json::Value LoadSchemaDoc(const ManifestVer& manifestVersion, ManifestTypeEnum manifestType) @@ -134,7 +307,7 @@ namespace AppInstaller::Manifest::YamlParser { ManifestTypeEnum::DefaultLocale, IDX_MANIFEST_SCHEMA_V1_7_DEFAULTLOCALE }, { ManifestTypeEnum::Locale, IDX_MANIFEST_SCHEMA_V1_7_LOCALE }, }; - } + } else if (manifestVersion >= ManifestVer{ s_ManifestVersionV1_6 }) { resourceMap = { @@ -251,4 +424,25 @@ namespace AppInstaller::Manifest::YamlParser return errors; } + + std::vector ValidateYamlManifestsSchemaHeader(const std::vector& manifestList, const ManifestVer& manifestVersion, bool treatErrorAsWarning) + { + std::vector errors; + ValidationError::Level errorLevel = treatErrorAsWarning ? ValidationError::Level::Warning : ValidationError::Level::Error; + + // Read the manifest schema header and ensure it exists + for (const auto& entry : manifestList) + { + if (entry.ManifestType == ManifestTypeEnum::Shadow) + { + // There's no schema for a shadow manifest. + continue; + } + + auto schemaHeaderErrors = ValidateYamlManifestSchemaHeader(entry, manifestVersion, errorLevel); + std::move(schemaHeaderErrors.begin(), schemaHeaderErrors.end(), std::inserter(errors, errors.end())); + } + + return errors; + } } diff --git a/src/AppInstallerCommonCore/Manifest/ManifestValidation.cpp b/src/AppInstallerCommonCore/Manifest/ManifestValidation.cpp index d045a88292..6154a2579f 100644 --- a/src/AppInstallerCommonCore/Manifest/ManifestValidation.cpp +++ b/src/AppInstallerCommonCore/Manifest/ManifestValidation.cpp @@ -63,7 +63,12 @@ namespace AppInstaller::Manifest { AppInstaller::Manifest::ManifestError::ArpValidationError, "Arp Validation Error."sv }, { AppInstaller::Manifest::ManifestError::SchemaError, "Schema Error."sv }, { AppInstaller::Manifest::ManifestError::MsixSignatureHashFailed, "Failed to calculate MSIX signature hash.Please verify that the input file is a valid, signed MSIX."sv }, - { AppInstaller::Manifest::ManifestError::ShadowManifestNotAllowed, "Shadow manifest is not allowed." } + { AppInstaller::Manifest::ManifestError::ShadowManifestNotAllowed, "Shadow manifest is not allowed." }, + { AppInstaller::Manifest::ManifestError::SchemaHeaderNotFound, "Schema header not found." }, + { AppInstaller::Manifest::ManifestError::InvalidSchemaHeader , "The schema header is invalid. Please verify that the schema header is present and formatted correctly as a valid YAML node with the appropriate commented syntax."sv }, + { AppInstaller::Manifest::ManifestError::SchemaHeaderManifestTypeMismatch , "The manifest type in the schema header does not match the ManifestType property value in the manifest."sv }, + { AppInstaller::Manifest::ManifestError::SchemaHeaderManifestVersionMismatch, "The manifest version in the schema header does not match the ManifestVersion property value in the manifest."sv }, + { AppInstaller::Manifest::ManifestError::SchemaHeaderUrlPatternMismatch, "The schema header URL does not match the expected pattern."sv }, }; return ErrorIdToMessageMap; diff --git a/src/AppInstallerCommonCore/Manifest/YamlParser.cpp b/src/AppInstallerCommonCore/Manifest/YamlParser.cpp index 0223e79c0b..a6f1f2b111 100644 --- a/src/AppInstallerCommonCore/Manifest/YamlParser.cpp +++ b/src/AppInstallerCommonCore/Manifest/YamlParser.cpp @@ -479,6 +479,14 @@ namespace AppInstaller::Manifest::YamlParser { errors = ValidateManifest(manifest); std::move(errors.begin(), errors.end(), std::inserter(resultErrors, resultErrors.end())); + + // Validate the schema header for manifest version 1.10 and above + if (manifestVersion >= ManifestVer{ s_ManifestVersionV1_10 }) + { + // Validate the schema header. + errors = ValidateYamlManifestsSchemaHeader(input, manifestVersion, validateOption.SchemaHeaderValidationAsWarning); + std::move(errors.begin(), errors.end(), std::inserter(resultErrors, resultErrors.end())); + } } if (validateOption.InstallerValidation) @@ -521,6 +529,7 @@ namespace AppInstaller::Manifest::YamlParser YamlManifestInfo doc; doc.Root = YAML::Load(file.path()); doc.FileName = file.path().filename().u8string(); + doc.InputStream = std::make_shared(file.path(), std::ios_base::in | std::ios_base::binary); docList.emplace_back(std::move(doc)); } } @@ -529,6 +538,7 @@ namespace AppInstaller::Manifest::YamlParser YamlManifestInfo doc; doc.Root = YAML::Load(inputPath, doc.StreamSha256); doc.FileName = inputPath.filename().u8string(); + doc.InputStream = std::make_shared(inputPath, std::ios_base::in | std::ios_base::binary); docList.emplace_back(std::move(doc)); } } @@ -551,6 +561,7 @@ namespace AppInstaller::Manifest::YamlParser { YamlManifestInfo doc; doc.Root = YAML::Load(input); + doc.InputStream = std::make_shared(input); docList.emplace_back(std::move(doc)); } catch (const std::exception& e) @@ -595,4 +606,4 @@ namespace AppInstaller::Manifest::YamlParser return manifest; } -} \ No newline at end of file +} diff --git a/src/AppInstallerCommonCore/Public/winget/ManifestCommon.h b/src/AppInstallerCommonCore/Public/winget/ManifestCommon.h index dc6b83bc2b..2e06654839 100644 --- a/src/AppInstallerCommonCore/Public/winget/ManifestCommon.h +++ b/src/AppInstallerCommonCore/Public/winget/ManifestCommon.h @@ -63,6 +63,7 @@ namespace AppInstaller::Manifest bool FullValidation = false; bool ThrowOnWarning = false; bool AllowShadowManifest = false; + bool SchemaHeaderValidationAsWarning = false; }; // ManifestVer is inherited from Utility::Version and is a more restricted version. diff --git a/src/AppInstallerCommonCore/Public/winget/ManifestSchemaValidation.h b/src/AppInstallerCommonCore/Public/winget/ManifestSchemaValidation.h index 7e6d2a68c6..a170caf4bd 100644 --- a/src/AppInstallerCommonCore/Public/winget/ManifestSchemaValidation.h +++ b/src/AppInstallerCommonCore/Public/winget/ManifestSchemaValidation.h @@ -18,4 +18,10 @@ namespace AppInstaller::Manifest::YamlParser std::vector ValidateAgainstSchema( const std::vector& manifestList, const ManifestVer& manifestVersion); -} \ No newline at end of file + + // Validate the schema header of a list of manifests + std::vector ValidateYamlManifestsSchemaHeader( + const std::vector& manifestList, + const ManifestVer& manifestVersion, + bool treatErrorAsWarning = true); +} diff --git a/src/AppInstallerCommonCore/Public/winget/ManifestValidation.h b/src/AppInstallerCommonCore/Public/winget/ManifestValidation.h index 6ecc54b619..e1c958230e 100644 --- a/src/AppInstallerCommonCore/Public/winget/ManifestValidation.h +++ b/src/AppInstallerCommonCore/Public/winget/ManifestValidation.h @@ -65,7 +65,12 @@ namespace AppInstaller::Manifest WINGET_DEFINE_RESOURCE_STRINGID(ScopeNotSupported); WINGET_DEFINE_RESOURCE_STRINGID(ShadowManifestNotAllowed); WINGET_DEFINE_RESOURCE_STRINGID(SingleManifestPackageHasDependencies); - WINGET_DEFINE_RESOURCE_STRINGID(UnsupportedMultiFileManifestType); + WINGET_DEFINE_RESOURCE_STRINGID(UnsupportedMultiFileManifestType); + WINGET_DEFINE_RESOURCE_STRINGID(SchemaHeaderNotFound); + WINGET_DEFINE_RESOURCE_STRINGID(InvalidSchemaHeader); + WINGET_DEFINE_RESOURCE_STRINGID(SchemaHeaderManifestTypeMismatch); + WINGET_DEFINE_RESOURCE_STRINGID(SchemaHeaderManifestVersionMismatch); + WINGET_DEFINE_RESOURCE_STRINGID(SchemaHeaderUrlPatternMismatch); } struct ValidationError @@ -134,6 +139,20 @@ namespace AppInstaller::Manifest error.FileName = file; return error; } + + static ValidationError MessageLevelWithFile(AppInstaller::StringResource::StringId message, Level level, std::string file) + { + ValidationError error{ message, level }; + error.FileName = file; + return error; + } + + static ValidationError MessageContextValueLineLevelWithFile(AppInstaller::StringResource::StringId message, std::string context, std::string value, size_t line, size_t column , Level level , std::string file) + { + ValidationError error{ message, context, value, line, column, level }; + error.FileName = file; + return error; + } }; struct ManifestException : public wil::ResultException @@ -232,4 +251,4 @@ namespace AppInstaller::Manifest std::vector ValidateManifest(const Manifest& manifest, bool fullValidation = true); std::vector ValidateManifestLocalization(const ManifestLocalization& localization, bool treatErrorAsWarning = false); std::vector ValidateManifestInstallers(const Manifest& manifest, bool treatErrorAsWarning = false); -} \ No newline at end of file +} diff --git a/src/AppInstallerCommonCore/Public/winget/ManifestYamlParser.h b/src/AppInstallerCommonCore/Public/winget/ManifestYamlParser.h index 0334fa7622..6eab96ff80 100644 --- a/src/AppInstallerCommonCore/Public/winget/ManifestYamlParser.h +++ b/src/AppInstallerCommonCore/Public/winget/ManifestYamlParser.h @@ -18,6 +18,9 @@ namespace AppInstaller::Manifest::YamlParser // File name of the manifest file if applicable for error reporting std::string FileName; + // Input stream for reading/parsing the Raw YAML content. + std::shared_ptr InputStream; + // The SHA256 hash of the stream Utility::SHA256::HashBuffer StreamSha256; @@ -38,4 +41,4 @@ namespace AppInstaller::Manifest::YamlParser std::vector& input, ManifestValidateOption validateOption = {}, const std::filesystem::path& mergedManifestPath = {}); -} \ No newline at end of file +} From 4db758377995db1559a0e7b71ecc2631276de74d Mon Sep 17 00:00:00 2001 From: Madhusudhan Gumbalapura Sudarshan Date: Sat, 11 Jan 2025 18:19:58 -0800 Subject: [PATCH 02/18] Treat schema header validation errors as warnings for 'winget validate --manifest <>' command 1. Introduced a new option `SchemaHeaderValidationAsWarning` to the `validateOption` object and set it to `true`. 2. This change treats schema header validation issues as warnings instead of errors, making the validation process more lenient. [NOTE:] 1. SchemaHeaderValidation errors should be treated as warnings for the winget CLI validate command. 2. SchemaHeaderValidation errors should be treated as errors for wingetsvc community manifests. --- src/AppInstallerCLICore/Commands/ValidateCommand.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/src/AppInstallerCLICore/Commands/ValidateCommand.cpp b/src/AppInstallerCLICore/Commands/ValidateCommand.cpp index b569fb9239..ad23874be6 100644 --- a/src/AppInstallerCLICore/Commands/ValidateCommand.cpp +++ b/src/AppInstallerCLICore/Commands/ValidateCommand.cpp @@ -46,6 +46,7 @@ namespace AppInstaller::CLI { ManifestValidateOption validateOption; validateOption.FullValidation = true; + validateOption.SchemaHeaderValidationAsWarning = true; validateOption.ThrowOnWarning = !(context.Args.Contains(Execution::Args::Type::IgnoreWarnings)); auto manifest = YamlParser::CreateFromPath(inputFile, validateOption); From c60d2673d283e37925e0f50ea6f71041f0c916b3 Mon Sep 17 00:00:00 2001 From: Madhusudhan Gumbalapura Sudarshan Date: Sat, 11 Jan 2025 18:42:50 -0800 Subject: [PATCH 03/18] Add tests for YAML Manifest schema header validations 1. Added new test data files to validate various schema header errors in YAML manifests, including a. invalid headers, b. type mismatches, c. version mismatches, d. missing headers, and e. URL pattern mismatches. 2. Updated project files to include these test data files. 3. Added a new test case in `YamlManifest.cpp` to validate the schema headers of the new test data files and existing v.1.10 schema test files. --- .../AppInstallerCLITests.vcxproj | 17 ++++++++++++++- .../AppInstallerCLITests.vcxproj.filters | 15 +++++++++++++ ...ManifestV1_10-Bad-SchemaHeaderInvalid.yaml | 16 ++++++++++++++ ...-Bad-SchemaHeaderManifestTypeMismatch.yaml | 16 ++++++++++++++ ...d-SchemaHeaderManifestVersionMismatch.yaml | 16 ++++++++++++++ ...anifestV1_10-Bad-SchemaHeaderNotFound.yaml | 15 +++++++++++++ ...10-Bad-SchemaHeaderURLPatternMismatch.yaml | 16 ++++++++++++++ .../TestData/ManifestV1_10-Singleton.yaml | 1 + ...ManifestV1_10-MultiFile-DefaultLocale.yaml | 1 + .../ManifestV1_10-MultiFile-Installer.yaml | 1 + .../ManifestV1_10-MultiFile-Locale.yaml | 1 + .../ManifestV1_10-MultiFile-Version.yaml | 1 + src/AppInstallerCLITests/YamlManifest.cpp | 21 +++++++++++++++++++ 13 files changed, 136 insertions(+), 1 deletion(-) create mode 100644 src/AppInstallerCLITests/TestData/ManifestV1_10-Bad-SchemaHeaderInvalid.yaml create mode 100644 src/AppInstallerCLITests/TestData/ManifestV1_10-Bad-SchemaHeaderManifestTypeMismatch.yaml create mode 100644 src/AppInstallerCLITests/TestData/ManifestV1_10-Bad-SchemaHeaderManifestVersionMismatch.yaml create mode 100644 src/AppInstallerCLITests/TestData/ManifestV1_10-Bad-SchemaHeaderNotFound.yaml create mode 100644 src/AppInstallerCLITests/TestData/ManifestV1_10-Bad-SchemaHeaderURLPatternMismatch.yaml diff --git a/src/AppInstallerCLITests/AppInstallerCLITests.vcxproj b/src/AppInstallerCLITests/AppInstallerCLITests.vcxproj index 5bf26983b7..538c2f6367 100644 --- a/src/AppInstallerCLITests/AppInstallerCLITests.vcxproj +++ b/src/AppInstallerCLITests/AppInstallerCLITests.vcxproj @@ -1033,6 +1033,21 @@ true + + true + + + true + + + true + + + true + + + true + @@ -1076,4 +1091,4 @@ - + \ No newline at end of file diff --git a/src/AppInstallerCLITests/AppInstallerCLITests.vcxproj.filters b/src/AppInstallerCLITests/AppInstallerCLITests.vcxproj.filters index 5895890272..5da81fec15 100644 --- a/src/AppInstallerCLITests/AppInstallerCLITests.vcxproj.filters +++ b/src/AppInstallerCLITests/AppInstallerCLITests.vcxproj.filters @@ -1044,5 +1044,20 @@ TestData + + TestData + + + TestData + + + TestData + + + TestData + + + TestData + \ No newline at end of file diff --git a/src/AppInstallerCLITests/TestData/ManifestV1_10-Bad-SchemaHeaderInvalid.yaml b/src/AppInstallerCLITests/TestData/ManifestV1_10-Bad-SchemaHeaderInvalid.yaml new file mode 100644 index 0000000000..305aed7d8d --- /dev/null +++ b/src/AppInstallerCLITests/TestData/ManifestV1_10-Bad-SchemaHeaderInvalid.yaml @@ -0,0 +1,16 @@ +# yaml-language-server= $schema=https://aka.ms/winget-manifest.singleton.1.10.0.schema.json +PackageIdentifier: AppInstallerCliTest.SchemaHeaderInvalid +PackageVersion: 1.0.0.0 +PackageLocale: en-US +PackageName: AppInstaller Test Schema Header Invalid +Publisher: Microsoft Corporation +License: Test +ShortDescription: This manifest has an invalid schema header + +Installers: + - Architecture: x86 + InstallerUrl: https://ThisIsNotUsed + InstallerType: msi + InstallerSha256: 65DB2F2AC2686C7F2FD69D4A4C6683B888DC55BFA20A0E32CA9F838B51689A3B +ManifestType: singleton +ManifestVersion: 1.10.0 diff --git a/src/AppInstallerCLITests/TestData/ManifestV1_10-Bad-SchemaHeaderManifestTypeMismatch.yaml b/src/AppInstallerCLITests/TestData/ManifestV1_10-Bad-SchemaHeaderManifestTypeMismatch.yaml new file mode 100644 index 0000000000..ce370d7dd3 --- /dev/null +++ b/src/AppInstallerCLITests/TestData/ManifestV1_10-Bad-SchemaHeaderManifestTypeMismatch.yaml @@ -0,0 +1,16 @@ +# yaml-language-server: $schema=https://aka.ms/winget-manifest.installer.1.10.0.schema.json +PackageIdentifier: AppInstallerCliTest.SchemaHeaderManifestTypeMismatch +PackageVersion: 1.0.0.0 +PackageLocale: en-US +PackageName: AppInstaller Test Schema Header ManifestType Mismatch +Publisher: Microsoft Corporation +License: Test +ShortDescription: This manifest has a mismatched ManisfestType in the schema header + +Installers: + - Architecture: x86 + InstallerUrl: https://ThisIsNotUsed + InstallerType: msi + InstallerSha256: 65DB2F2AC2686C7F2FD69D4A4C6683B888DC55BFA20A0E32CA9F838B51689A3B +ManifestType: singleton +ManifestVersion: 1.10.0 diff --git a/src/AppInstallerCLITests/TestData/ManifestV1_10-Bad-SchemaHeaderManifestVersionMismatch.yaml b/src/AppInstallerCLITests/TestData/ManifestV1_10-Bad-SchemaHeaderManifestVersionMismatch.yaml new file mode 100644 index 0000000000..630bb21182 --- /dev/null +++ b/src/AppInstallerCLITests/TestData/ManifestV1_10-Bad-SchemaHeaderManifestVersionMismatch.yaml @@ -0,0 +1,16 @@ +# yaml-language-server: $schema=https://aka.ms/winget-manifest.singleton.1.9.0.schema.json +PackageIdentifier: AppInstallerCliTest.SchemaHeaderVersionMismatch +PackageVersion: 1.0.0.0 +PackageLocale: en-US +PackageName: AppInstaller Test Schema Header ManifestVersion Mismatch +Publisher: Microsoft Corporation +License: Test +ShortDescription: This manifest has a mismatched ManisfestVersion in the schema header + +Installers: + - Architecture: x86 + InstallerUrl: https://ThisIsNotUsed + InstallerType: msi + InstallerSha256: 65DB2F2AC2686C7F2FD69D4A4C6683B888DC55BFA20A0E32CA9F838B51689A3B +ManifestType: singleton +ManifestVersion: 1.10.0 diff --git a/src/AppInstallerCLITests/TestData/ManifestV1_10-Bad-SchemaHeaderNotFound.yaml b/src/AppInstallerCLITests/TestData/ManifestV1_10-Bad-SchemaHeaderNotFound.yaml new file mode 100644 index 0000000000..2e42d9a7b7 --- /dev/null +++ b/src/AppInstallerCLITests/TestData/ManifestV1_10-Bad-SchemaHeaderNotFound.yaml @@ -0,0 +1,15 @@ +PackageIdentifier: AppInstallerCliTest.SchemaHeaderNotFound +PackageVersion: 1.0.0.0 +PackageLocale: en-US +PackageName: AppInstaller Test Schema Header Not Found +Publisher: Microsoft Corporation +License: Test +ShortDescription: This manifest has a missing schema header + +Installers: + - Architecture: x86 + InstallerUrl: https://ThisIsNotUsed + InstallerType: msi + InstallerSha256: 65DB2F2AC2686C7F2FD69D4A4C6683B888DC55BFA20A0E32CA9F838B51689A3B +ManifestType: singleton +ManifestVersion: 1.10.0 diff --git a/src/AppInstallerCLITests/TestData/ManifestV1_10-Bad-SchemaHeaderURLPatternMismatch.yaml b/src/AppInstallerCLITests/TestData/ManifestV1_10-Bad-SchemaHeaderURLPatternMismatch.yaml new file mode 100644 index 0000000000..2f6fe41a52 --- /dev/null +++ b/src/AppInstallerCLITests/TestData/ManifestV1_10-Bad-SchemaHeaderURLPatternMismatch.yaml @@ -0,0 +1,16 @@ +# yaml-language-server: $schema=https://aka.ms/winget-manifest-invalid.singleton.1.10.0.schema.json +PackageIdentifier: AppInstallerCliTest.SchemaHeaderURLPatternMismatch +PackageVersion: 1.0.0.0 +PackageLocale: en-US +PackageName: AppInstaller Test Schema Header URL Pattern Mismatch +Publisher: Microsoft Corporation +License: Test +ShortDescription: This manifest has a mismatched schema header URL pattern + +Installers: + - Architecture: x86 + InstallerUrl: https://ThisIsNotUsed + InstallerType: msi + InstallerSha256: 65DB2F2AC2686C7F2FD69D4A4C6683B888DC55BFA20A0E32CA9F838B51689A3B +ManifestType: singleton +ManifestVersion: 1.10.0 diff --git a/src/AppInstallerCLITests/TestData/ManifestV1_10-Singleton.yaml b/src/AppInstallerCLITests/TestData/ManifestV1_10-Singleton.yaml index bf13ddc447..3b68b17acb 100644 --- a/src/AppInstallerCLITests/TestData/ManifestV1_10-Singleton.yaml +++ b/src/AppInstallerCLITests/TestData/ManifestV1_10-Singleton.yaml @@ -1,3 +1,4 @@ +# yaml-language-server: $schema=https://aka.ms/winget-manifest.singleton.1.10.0.schema.json PackageIdentifier: microsoft.msixsdk PackageVersion: 1.7.32 PackageLocale: en-US diff --git a/src/AppInstallerCLITests/TestData/MultiFileManifestV1_10/ManifestV1_10-MultiFile-DefaultLocale.yaml b/src/AppInstallerCLITests/TestData/MultiFileManifestV1_10/ManifestV1_10-MultiFile-DefaultLocale.yaml index 6dd4c32c51..3a35bbbe7a 100644 --- a/src/AppInstallerCLITests/TestData/MultiFileManifestV1_10/ManifestV1_10-MultiFile-DefaultLocale.yaml +++ b/src/AppInstallerCLITests/TestData/MultiFileManifestV1_10/ManifestV1_10-MultiFile-DefaultLocale.yaml @@ -1,3 +1,4 @@ +# yaml-language-server: $schema=https://aka.ms/winget-manifest.defaultLocale.1.10.0.schema.json PackageIdentifier: microsoft.msixsdk PackageVersion: 1.7.32 PackageLocale: en-US diff --git a/src/AppInstallerCLITests/TestData/MultiFileManifestV1_10/ManifestV1_10-MultiFile-Installer.yaml b/src/AppInstallerCLITests/TestData/MultiFileManifestV1_10/ManifestV1_10-MultiFile-Installer.yaml index b7c1958495..e241eb09a9 100644 --- a/src/AppInstallerCLITests/TestData/MultiFileManifestV1_10/ManifestV1_10-MultiFile-Installer.yaml +++ b/src/AppInstallerCLITests/TestData/MultiFileManifestV1_10/ManifestV1_10-MultiFile-Installer.yaml @@ -1,3 +1,4 @@ +# yaml-language-server: $schema=https://aka.ms/winget-manifest.installer.1.10.0.schema.json PackageIdentifier: microsoft.msixsdk PackageVersion: 1.7.32 InstallerLocale: en-US diff --git a/src/AppInstallerCLITests/TestData/MultiFileManifestV1_10/ManifestV1_10-MultiFile-Locale.yaml b/src/AppInstallerCLITests/TestData/MultiFileManifestV1_10/ManifestV1_10-MultiFile-Locale.yaml index 65e42dbd3e..76a79a934d 100644 --- a/src/AppInstallerCLITests/TestData/MultiFileManifestV1_10/ManifestV1_10-MultiFile-Locale.yaml +++ b/src/AppInstallerCLITests/TestData/MultiFileManifestV1_10/ManifestV1_10-MultiFile-Locale.yaml @@ -1,3 +1,4 @@ +# yaml-language-server: $schema=https://aka.ms/winget-manifest.locale.1.10.0.schema.json PackageIdentifier: microsoft.msixsdk PackageVersion: 1.7.32 PackageLocale: en-GB diff --git a/src/AppInstallerCLITests/TestData/MultiFileManifestV1_10/ManifestV1_10-MultiFile-Version.yaml b/src/AppInstallerCLITests/TestData/MultiFileManifestV1_10/ManifestV1_10-MultiFile-Version.yaml index b31d77874b..1ae085fdc7 100644 --- a/src/AppInstallerCLITests/TestData/MultiFileManifestV1_10/ManifestV1_10-MultiFile-Version.yaml +++ b/src/AppInstallerCLITests/TestData/MultiFileManifestV1_10/ManifestV1_10-MultiFile-Version.yaml @@ -1,3 +1,4 @@ +# yaml-language-server: $schema=https://aka.ms/winget-manifest.version.1.10.0.schema.json PackageIdentifier: microsoft.msixsdk PackageVersion: 1.7.32 DefaultLocale: en-US diff --git a/src/AppInstallerCLITests/YamlManifest.cpp b/src/AppInstallerCLITests/YamlManifest.cpp index 0bbe8243cc..7d6744d861 100644 --- a/src/AppInstallerCLITests/YamlManifest.cpp +++ b/src/AppInstallerCLITests/YamlManifest.cpp @@ -1846,6 +1846,27 @@ TEST_CASE("ManifestArpVersionRange", "[ManifestValidation]") REQUIRE(arpRangeMultiArp.GetMaxVersion().ToString() == "13.0"); } +TEST_CASE("ManifestV1_10_SchemaHeaderValidations", "[ManifestValidation]") +{ + ManifestValidateOption validateOption; + validateOption.FullValidation = true; + + // Schema header not found + REQUIRE_THROWS_MATCHES(YamlParser::CreateFromPath(TestDataFile("ManifestV1_10-Bad-SchemaHeaderNotFound.yaml"),validateOption), ManifestException, ManifestExceptionMatcher("Schema header not found")); + + // Schema header not valid + REQUIRE_THROWS_MATCHES(YamlParser::CreateFromPath(TestDataFile("ManifestV1_10-Bad-SchemaHeaderInvalid.yaml"), validateOption), ManifestException, ManifestExceptionMatcher("The schema header is invalid. Please verify that the schema header is present and formatted correctly as a valid YAML node with the appropriate commented syntax")); + + // Schema header URL does not match the expected schema URL + REQUIRE_THROWS_MATCHES(YamlParser::CreateFromPath(TestDataFile("ManifestV1_10-Bad-SchemaHeaderURLPatternMismatch.yaml"), validateOption), ManifestException, ManifestExceptionMatcher("The schema header URL does not match the expected pattern.")); + + // Schema header ManifestType does not match the expected value + REQUIRE_THROWS_MATCHES(YamlParser::CreateFromPath(TestDataFile("ManifestV1_10-Bad-SchemaHeaderManifestTypeMismatch.yaml"), validateOption), ManifestException, ManifestExceptionMatcher("The manifest type in the schema header does not match the ManifestType property value in the manifest.")); + + // Schema header version does not match the expected version + REQUIRE_THROWS_MATCHES(YamlParser::CreateFromPath(TestDataFile("ManifestV1_10-Bad-SchemaHeaderManifestVersionMismatch.yaml"), validateOption), ManifestException, ManifestExceptionMatcher("The schema header version does not match the expected version.")); +} + TEST_CASE("ShadowManifest", "[ShadowManifest]") { ManifestValidateOption validateOption; From c8e69e3083c8666800c75df2acf521caa2e5b3fc Mon Sep 17 00:00:00 2001 From: Madhusudhan Gumbalapura Sudarshan Date: Sat, 11 Jan 2025 18:45:59 -0800 Subject: [PATCH 04/18] Extend Add schema header validation tests for WinGet CLI Validation command and WinGetUtils Interop binary 1. Added new test manifests with schema headers to validate various scenarios. 2. Updated `ValidateCommand.cs` to include new test cases for extended characters and schema headers. 3. Enhanced `WinGetUtilManifest.cs` with new test cases for schema headers and added a helper method for validation. Also, made minor formatting corrections. --- .../TestGoodManifestV1_10-SchemaHeader.yaml | 16 ++++ ...ifestV1_10-SchemaHeadeVersionMismatch.yaml | 16 ++++ ...ningManifestV1_10-SchemaHeaderInvalid.yaml | 16 ++++ ...1_10-SchemaHeaderManifestTypeMismatch.yaml | 16 ++++ ...ingManifestV1_10-SchemaHeaderNotFound.yaml | 15 ++++ ...tV1_10-SchemaHeaderURLPatternMismatch.yaml | 16 ++++ .../ValidateCommand.cs | 46 +++++++++- .../WinGetUtil/WinGetUtilManifest.cs | 85 ++++++++++++++++++- 8 files changed, 221 insertions(+), 5 deletions(-) create mode 100644 src/AppInstallerCLIE2ETests/TestData/Manifests/TestGoodManifestV1_10-SchemaHeader.yaml create mode 100644 src/AppInstallerCLIE2ETests/TestData/Manifests/TestWarningManifestV1_10-SchemaHeadeVersionMismatch.yaml create mode 100644 src/AppInstallerCLIE2ETests/TestData/Manifests/TestWarningManifestV1_10-SchemaHeaderInvalid.yaml create mode 100644 src/AppInstallerCLIE2ETests/TestData/Manifests/TestWarningManifestV1_10-SchemaHeaderManifestTypeMismatch.yaml create mode 100644 src/AppInstallerCLIE2ETests/TestData/Manifests/TestWarningManifestV1_10-SchemaHeaderNotFound.yaml create mode 100644 src/AppInstallerCLIE2ETests/TestData/Manifests/TestWarningManifestV1_10-SchemaHeaderURLPatternMismatch.yaml diff --git a/src/AppInstallerCLIE2ETests/TestData/Manifests/TestGoodManifestV1_10-SchemaHeader.yaml b/src/AppInstallerCLIE2ETests/TestData/Manifests/TestGoodManifestV1_10-SchemaHeader.yaml new file mode 100644 index 0000000000..199dc578ca --- /dev/null +++ b/src/AppInstallerCLIE2ETests/TestData/Manifests/TestGoodManifestV1_10-SchemaHeader.yaml @@ -0,0 +1,16 @@ +# yaml-language-server: $schema=https://aka.ms/winget-manifest.singleton.1.10.0.schema.json +PackageIdentifier: AppInstallerCliTest.SchemaHeader +PackageVersion: 1.0.0.0 +PackageLocale: en-US +PackageName: AppInstaller Test Schema Header +Publisher: Microsoft Corporation +License: Test +ShortDescription: This manifest with schema header + +Installers: + - Architecture: x86 + InstallerUrl: https://ThisIsNotUsed + InstallerType: msi + InstallerSha256: 65DB2F2AC2686C7F2FD69D4A4C6683B888DC55BFA20A0E32CA9F838B51689A3B +ManifestType: singleton +ManifestVersion: 1.10.0 diff --git a/src/AppInstallerCLIE2ETests/TestData/Manifests/TestWarningManifestV1_10-SchemaHeadeVersionMismatch.yaml b/src/AppInstallerCLIE2ETests/TestData/Manifests/TestWarningManifestV1_10-SchemaHeadeVersionMismatch.yaml new file mode 100644 index 0000000000..630bb21182 --- /dev/null +++ b/src/AppInstallerCLIE2ETests/TestData/Manifests/TestWarningManifestV1_10-SchemaHeadeVersionMismatch.yaml @@ -0,0 +1,16 @@ +# yaml-language-server: $schema=https://aka.ms/winget-manifest.singleton.1.9.0.schema.json +PackageIdentifier: AppInstallerCliTest.SchemaHeaderVersionMismatch +PackageVersion: 1.0.0.0 +PackageLocale: en-US +PackageName: AppInstaller Test Schema Header ManifestVersion Mismatch +Publisher: Microsoft Corporation +License: Test +ShortDescription: This manifest has a mismatched ManisfestVersion in the schema header + +Installers: + - Architecture: x86 + InstallerUrl: https://ThisIsNotUsed + InstallerType: msi + InstallerSha256: 65DB2F2AC2686C7F2FD69D4A4C6683B888DC55BFA20A0E32CA9F838B51689A3B +ManifestType: singleton +ManifestVersion: 1.10.0 diff --git a/src/AppInstallerCLIE2ETests/TestData/Manifests/TestWarningManifestV1_10-SchemaHeaderInvalid.yaml b/src/AppInstallerCLIE2ETests/TestData/Manifests/TestWarningManifestV1_10-SchemaHeaderInvalid.yaml new file mode 100644 index 0000000000..305aed7d8d --- /dev/null +++ b/src/AppInstallerCLIE2ETests/TestData/Manifests/TestWarningManifestV1_10-SchemaHeaderInvalid.yaml @@ -0,0 +1,16 @@ +# yaml-language-server= $schema=https://aka.ms/winget-manifest.singleton.1.10.0.schema.json +PackageIdentifier: AppInstallerCliTest.SchemaHeaderInvalid +PackageVersion: 1.0.0.0 +PackageLocale: en-US +PackageName: AppInstaller Test Schema Header Invalid +Publisher: Microsoft Corporation +License: Test +ShortDescription: This manifest has an invalid schema header + +Installers: + - Architecture: x86 + InstallerUrl: https://ThisIsNotUsed + InstallerType: msi + InstallerSha256: 65DB2F2AC2686C7F2FD69D4A4C6683B888DC55BFA20A0E32CA9F838B51689A3B +ManifestType: singleton +ManifestVersion: 1.10.0 diff --git a/src/AppInstallerCLIE2ETests/TestData/Manifests/TestWarningManifestV1_10-SchemaHeaderManifestTypeMismatch.yaml b/src/AppInstallerCLIE2ETests/TestData/Manifests/TestWarningManifestV1_10-SchemaHeaderManifestTypeMismatch.yaml new file mode 100644 index 0000000000..ce370d7dd3 --- /dev/null +++ b/src/AppInstallerCLIE2ETests/TestData/Manifests/TestWarningManifestV1_10-SchemaHeaderManifestTypeMismatch.yaml @@ -0,0 +1,16 @@ +# yaml-language-server: $schema=https://aka.ms/winget-manifest.installer.1.10.0.schema.json +PackageIdentifier: AppInstallerCliTest.SchemaHeaderManifestTypeMismatch +PackageVersion: 1.0.0.0 +PackageLocale: en-US +PackageName: AppInstaller Test Schema Header ManifestType Mismatch +Publisher: Microsoft Corporation +License: Test +ShortDescription: This manifest has a mismatched ManisfestType in the schema header + +Installers: + - Architecture: x86 + InstallerUrl: https://ThisIsNotUsed + InstallerType: msi + InstallerSha256: 65DB2F2AC2686C7F2FD69D4A4C6683B888DC55BFA20A0E32CA9F838B51689A3B +ManifestType: singleton +ManifestVersion: 1.10.0 diff --git a/src/AppInstallerCLIE2ETests/TestData/Manifests/TestWarningManifestV1_10-SchemaHeaderNotFound.yaml b/src/AppInstallerCLIE2ETests/TestData/Manifests/TestWarningManifestV1_10-SchemaHeaderNotFound.yaml new file mode 100644 index 0000000000..2e42d9a7b7 --- /dev/null +++ b/src/AppInstallerCLIE2ETests/TestData/Manifests/TestWarningManifestV1_10-SchemaHeaderNotFound.yaml @@ -0,0 +1,15 @@ +PackageIdentifier: AppInstallerCliTest.SchemaHeaderNotFound +PackageVersion: 1.0.0.0 +PackageLocale: en-US +PackageName: AppInstaller Test Schema Header Not Found +Publisher: Microsoft Corporation +License: Test +ShortDescription: This manifest has a missing schema header + +Installers: + - Architecture: x86 + InstallerUrl: https://ThisIsNotUsed + InstallerType: msi + InstallerSha256: 65DB2F2AC2686C7F2FD69D4A4C6683B888DC55BFA20A0E32CA9F838B51689A3B +ManifestType: singleton +ManifestVersion: 1.10.0 diff --git a/src/AppInstallerCLIE2ETests/TestData/Manifests/TestWarningManifestV1_10-SchemaHeaderURLPatternMismatch.yaml b/src/AppInstallerCLIE2ETests/TestData/Manifests/TestWarningManifestV1_10-SchemaHeaderURLPatternMismatch.yaml new file mode 100644 index 0000000000..2f6fe41a52 --- /dev/null +++ b/src/AppInstallerCLIE2ETests/TestData/Manifests/TestWarningManifestV1_10-SchemaHeaderURLPatternMismatch.yaml @@ -0,0 +1,16 @@ +# yaml-language-server: $schema=https://aka.ms/winget-manifest-invalid.singleton.1.10.0.schema.json +PackageIdentifier: AppInstallerCliTest.SchemaHeaderURLPatternMismatch +PackageVersion: 1.0.0.0 +PackageLocale: en-US +PackageName: AppInstaller Test Schema Header URL Pattern Mismatch +Publisher: Microsoft Corporation +License: Test +ShortDescription: This manifest has a mismatched schema header URL pattern + +Installers: + - Architecture: x86 + InstallerUrl: https://ThisIsNotUsed + InstallerType: msi + InstallerSha256: 65DB2F2AC2686C7F2FD69D4A4C6683B888DC55BFA20A0E32CA9F838B51689A3B +ManifestType: singleton +ManifestVersion: 1.10.0 diff --git a/src/AppInstallerCLIE2ETests/ValidateCommand.cs b/src/AppInstallerCLIE2ETests/ValidateCommand.cs index 5f1f7b89b5..c3d305dbe9 100644 --- a/src/AppInstallerCLIE2ETests/ValidateCommand.cs +++ b/src/AppInstallerCLIE2ETests/ValidateCommand.cs @@ -31,7 +31,7 @@ public void ValidateManifest() [Test] public void ValidateManifestWithExtendedCharacter() { - var result = TestCommon.RunAICLICommand("validate", TestCommon.GetTestDataFile("Manifests\\TëstExeInstaller.yaml")); + var result = TestCommon.RunAICLICommand("validate", TestCommon.GetTestDataFile("Manifests\\TëstExeInstaller.yaml")); Assert.AreEqual(Constants.ErrorCode.S_OK, result.ExitCode); Assert.True(result.StdOut.Contains("Manifest validation succeeded.")); } @@ -80,5 +80,47 @@ public void ValidateManifestDoesNotExist() Assert.AreEqual(Constants.ErrorCode.ERROR_PATH_NOT_FOUND, result.ExitCode); Assert.True(result.StdOut.Contains("Path does not exist")); } + + /// + /// Test validate manifest with invalid schema and expect warnings. + /// + [Test] + public void ValidateManifestV1_10_SchemaHeaderExpectWarnings() + { + var result = TestCommon.RunAICLICommand("validate", TestCommon.GetTestDataFile("Manifests\\TestWarningManifestV1_10-SchemaHeaderNotFound.yaml")); + Assert.AreEqual(Constants.ErrorCode.ERROR_MANIFEST_VALIDATION_WARNING, result.ExitCode); + Assert.True(result.StdOut.Contains("Manifest validation succeeded with warnings.")); + Assert.True(result.StdOut.Contains("Manifest Warning: Schema header not found.")); + + result = TestCommon.RunAICLICommand("validate", TestCommon.GetTestDataFile("Manifests\\TestWarningManifestV1_10-SchemaHeaderInvalid.yaml")); + Assert.AreEqual(Constants.ErrorCode.ERROR_MANIFEST_VALIDATION_WARNING, result.ExitCode); + Assert.True(result.StdOut.Contains("Manifest validation succeeded with warnings.")); + Assert.True(result.StdOut.Contains("Manifest Warning: The schema header is invalid. Please verify that the schema header is present and formatted correctly as a valid YAML node with the appropriate commented syntax.")); + + result = TestCommon.RunAICLICommand("validate", TestCommon.GetTestDataFile("Manifests\\TestWarningManifestV1_10-SchemaHeaderURLPatternMismatch.yaml")); + Assert.AreEqual(Constants.ErrorCode.ERROR_MANIFEST_VALIDATION_WARNING, result.ExitCode); + Assert.True(result.StdOut.Contains("Manifest validation succeeded with warnings.")); + Assert.True(result.StdOut.Contains("Manifest Warning: The schema header URL does not match the expected pattern")); + + result = TestCommon.RunAICLICommand("validate", TestCommon.GetTestDataFile("Manifests\\TestWarningManifestV1_10-SchemaHeaderManifestTypeMismatch.yaml")); + Assert.AreEqual(Constants.ErrorCode.ERROR_MANIFEST_VALIDATION_WARNING, result.ExitCode); + Assert.True(result.StdOut.Contains("Manifest validation succeeded with warnings.")); + Assert.True(result.StdOut.Contains("Manifest Warning: The manifest type in the schema header does not match the ManifestType property value in the manifest.")); + + result = TestCommon.RunAICLICommand("validate", TestCommon.GetTestDataFile("Manifests\\TestWarningManifestV1_10-SchemaHeadeVersionMismatch.yaml")); + Assert.AreEqual(Constants.ErrorCode.ERROR_MANIFEST_VALIDATION_WARNING, result.ExitCode); + Assert.True(result.StdOut.Contains("Manifest validation succeeded with warnings.")); + Assert.True(result.StdOut.Contains("Manifest Warning: The manifest version in the schema header does not match the ManifestVersion property value in the manifest.")); + } + + /// + /// Test validate manifest with valid schema header. + /// + [Test] + public void ValidateManifestV1_10_SchemaHeaderExpectNoWarning() + { + var result = TestCommon.RunAICLICommand("validate", TestCommon.GetTestDataFile("Manifests\\TestGoodManifestV1_10-SchemaHeader.yaml")); + Assert.AreEqual(Constants.ErrorCode.S_OK, result.ExitCode); + } } -} \ No newline at end of file +} diff --git a/src/AppInstallerCLIE2ETests/WinGetUtil/WinGetUtilManifest.cs b/src/AppInstallerCLIE2ETests/WinGetUtil/WinGetUtilManifest.cs index 0cb5e42b0c..a8d16b3dfb 100644 --- a/src/AppInstallerCLIE2ETests/WinGetUtil/WinGetUtilManifest.cs +++ b/src/AppInstallerCLIE2ETests/WinGetUtil/WinGetUtilManifest.cs @@ -1,4 +1,4 @@ -// ----------------------------------------------------------------------------- +// ----------------------------------------------------------------------------- // // Copyright (c) Microsoft Corporation. Licensed under the MIT License. // @@ -80,6 +80,85 @@ public void WinGetUtil_ValidateManifest_Success(WinGetUtilWrapper.CreateManifest // Close manifest WinGetUtilWrapper.WinGetCloseManifest(manifestHandle); - } - } + } + + /// + /// Test validate manifest with schema header. + /// + /// Create manifest options. + [Test] + [TestCase(WinGetUtilWrapper.CreateManifestOption.NoValidation)] + [TestCase(WinGetUtilWrapper.CreateManifestOption.SchemaAndSemanticValidation)] + public void WinGetUtil_ValidateManifest_V1_10_WithSchemaHeader_Success(WinGetUtilWrapper.CreateManifestOption createManifestOption) + { + string manifestsFilePath = TestCommon.GetTestDataFile(@"Manifests\TestGoodManifestV1_10-SchemaHeader.yaml"); + + // Create manifest + WinGetUtilWrapper.WinGetCreateManifest( + manifestsFilePath, + out bool succeeded, + out IntPtr manifestHandle, + out string createFailureMessage, + string.Empty, + createManifestOption); + + Assert.True(succeeded); + Assert.AreNotEqual(IntPtr.Zero, manifestHandle); + Assert.IsNull(createFailureMessage); + + // Close manifest + WinGetUtilWrapper.WinGetCloseManifest(manifestHandle); + } + + /// + /// Test validate manifest with schema header for failure scenarios. + /// + /// Create manifest options. + [Test] + [TestCase(WinGetUtilWrapper.CreateManifestOption.SchemaAndSemanticValidation)] + public void WinGetUtil_ValidateManifest_V1_10_WithSchemaHeader_Failure(WinGetUtilWrapper.CreateManifestOption createManifestOption) + { + // Schema header not found + string manifestsFilePath = TestCommon.GetTestDataFile(@"Manifests\TestWarningManifestV1_10-SchemaHeaderNotFound.yaml"); + string expectedError = "Manifest Error: Schema header not found."; + this.ValidateSchemaHeaderFailure(manifestsFilePath, createManifestOption, expectedError); + + // Schema header invalid + manifestsFilePath = TestCommon.GetTestDataFile(@"Manifests\TestWarningManifestV1_10-SchemaHeaderInvalid.yaml"); + expectedError = "Manifest Error: The schema header is invalid. Please verify that the schema header is present and formatted correctly as a valid YAML node with the appropriate commented syntax."; + this.ValidateSchemaHeaderFailure(manifestsFilePath, createManifestOption, expectedError); + + // Schema header URL pattern mismatch + manifestsFilePath = TestCommon.GetTestDataFile(@"Manifests\TestWarningManifestV1_10-SchemaHeaderURLPatternMismatch.yaml"); + expectedError = "Manifest Error: The schema header URL does not match the expected pattern."; + this.ValidateSchemaHeaderFailure(manifestsFilePath, createManifestOption, expectedError); + + // Schema header manifest type mismatch + manifestsFilePath = TestCommon.GetTestDataFile(@"Manifests\TestWarningManifestV1_10-SchemaHeaderManifestTypeMismatch.yaml"); + expectedError = "Manifest Error: The manifest type in the schema header does not match the ManifestType property value in the manifest."; + this.ValidateSchemaHeaderFailure(manifestsFilePath, createManifestOption, expectedError); + + // Schema header version mismatch + manifestsFilePath = TestCommon.GetTestDataFile(@"Manifests\TestWarningManifestV1_10-SchemaHeadeVersionMismatch.yaml"); + expectedError = "Manifest Error: The manifest version in the schema header does not match the ManifestVersion property value in the manifest."; + this.ValidateSchemaHeaderFailure(manifestsFilePath, createManifestOption, expectedError); + } + + private void ValidateSchemaHeaderFailure(string manifestsFilePath, WinGetUtilWrapper.CreateManifestOption createManifestOption, string expectedError) + { + // Create manifest + WinGetUtilWrapper.WinGetCreateManifest( + manifestsFilePath, + out bool succeeded, + out IntPtr manifestHandle, + out string createFailureMessage, + string.Empty, + createManifestOption); + + Assert.False(succeeded); + Assert.AreEqual(IntPtr.Zero, manifestHandle); + Assert.IsNotNull(createFailureMessage); + Assert.IsTrue(createFailureMessage.Contains(expectedError)); + } + } } From 8065a7ad41656b737ce825f02cfa996906c77e31 Mon Sep 17 00:00:00 2001 From: Madhusudhan Gumbalapura Sudarshan Date: Sat, 11 Jan 2025 20:03:40 -0800 Subject: [PATCH 05/18] Update error message in ManifestV1_10 test case The error message in the test case `TEST_CASE("ManifestV1_10_SchemaHeaderValidations", "[ManifestValidation]")` has been updated to provide a clearer and more specific description. The new message clarifies the mismatch issue between the manifest version in the schema header and the `ManifestVersion` property value in the manifest. --- src/AppInstallerCLITests/YamlManifest.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/AppInstallerCLITests/YamlManifest.cpp b/src/AppInstallerCLITests/YamlManifest.cpp index 7d6744d861..4f7459a94f 100644 --- a/src/AppInstallerCLITests/YamlManifest.cpp +++ b/src/AppInstallerCLITests/YamlManifest.cpp @@ -1864,7 +1864,7 @@ TEST_CASE("ManifestV1_10_SchemaHeaderValidations", "[ManifestValidation]") REQUIRE_THROWS_MATCHES(YamlParser::CreateFromPath(TestDataFile("ManifestV1_10-Bad-SchemaHeaderManifestTypeMismatch.yaml"), validateOption), ManifestException, ManifestExceptionMatcher("The manifest type in the schema header does not match the ManifestType property value in the manifest.")); // Schema header version does not match the expected version - REQUIRE_THROWS_MATCHES(YamlParser::CreateFromPath(TestDataFile("ManifestV1_10-Bad-SchemaHeaderManifestVersionMismatch.yaml"), validateOption), ManifestException, ManifestExceptionMatcher("The schema header version does not match the expected version.")); + REQUIRE_THROWS_MATCHES(YamlParser::CreateFromPath(TestDataFile("ManifestV1_10-Bad-SchemaHeaderManifestVersionMismatch.yaml"), validateOption), ManifestException, ManifestExceptionMatcher("The manifest version in the schema header does not match the ManifestVersion property value in the manifest.")); } TEST_CASE("ShadowManifest", "[ShadowManifest]") From f594f1b8f7da3789c421194ca50d99112b11392e Mon Sep 17 00:00:00 2001 From: Madhusudhan Gumbalapura Sudarshan Date: Sat, 11 Jan 2025 23:05:27 -0800 Subject: [PATCH 06/18] Fix typo in test manifest file path in two test files The changes correct a typo in the file path for a test manifest file in two different test files. Specifically, the file name `TestWarningManifestV1_10-SchemaHeadeVersionMismatch.yaml` is corrected to `TestWarningManifestV1_10-SchemaHeaderVersionMismatch.yaml` in both `ValidateCommand.cs` and `WinGetUtilManifest.cs`. --- ...> TestWarningManifestV1_10-SchemaHeaderVersionMismatch.yaml} | 0 src/AppInstallerCLIE2ETests/ValidateCommand.cs | 2 +- src/AppInstallerCLIE2ETests/WinGetUtil/WinGetUtilManifest.cs | 2 +- 3 files changed, 2 insertions(+), 2 deletions(-) rename src/AppInstallerCLIE2ETests/TestData/Manifests/{TestWarningManifestV1_10-SchemaHeadeVersionMismatch.yaml => TestWarningManifestV1_10-SchemaHeaderVersionMismatch.yaml} (100%) diff --git a/src/AppInstallerCLIE2ETests/TestData/Manifests/TestWarningManifestV1_10-SchemaHeadeVersionMismatch.yaml b/src/AppInstallerCLIE2ETests/TestData/Manifests/TestWarningManifestV1_10-SchemaHeaderVersionMismatch.yaml similarity index 100% rename from src/AppInstallerCLIE2ETests/TestData/Manifests/TestWarningManifestV1_10-SchemaHeadeVersionMismatch.yaml rename to src/AppInstallerCLIE2ETests/TestData/Manifests/TestWarningManifestV1_10-SchemaHeaderVersionMismatch.yaml diff --git a/src/AppInstallerCLIE2ETests/ValidateCommand.cs b/src/AppInstallerCLIE2ETests/ValidateCommand.cs index c3d305dbe9..2196012824 100644 --- a/src/AppInstallerCLIE2ETests/ValidateCommand.cs +++ b/src/AppInstallerCLIE2ETests/ValidateCommand.cs @@ -107,7 +107,7 @@ public void ValidateManifestV1_10_SchemaHeaderExpectWarnings() Assert.True(result.StdOut.Contains("Manifest validation succeeded with warnings.")); Assert.True(result.StdOut.Contains("Manifest Warning: The manifest type in the schema header does not match the ManifestType property value in the manifest.")); - result = TestCommon.RunAICLICommand("validate", TestCommon.GetTestDataFile("Manifests\\TestWarningManifestV1_10-SchemaHeadeVersionMismatch.yaml")); + result = TestCommon.RunAICLICommand("validate", TestCommon.GetTestDataFile("Manifests\\TestWarningManifestV1_10-SchemaHeaderVersionMismatch.yaml")); Assert.AreEqual(Constants.ErrorCode.ERROR_MANIFEST_VALIDATION_WARNING, result.ExitCode); Assert.True(result.StdOut.Contains("Manifest validation succeeded with warnings.")); Assert.True(result.StdOut.Contains("Manifest Warning: The manifest version in the schema header does not match the ManifestVersion property value in the manifest.")); diff --git a/src/AppInstallerCLIE2ETests/WinGetUtil/WinGetUtilManifest.cs b/src/AppInstallerCLIE2ETests/WinGetUtil/WinGetUtilManifest.cs index a8d16b3dfb..4dd75b3531 100644 --- a/src/AppInstallerCLIE2ETests/WinGetUtil/WinGetUtilManifest.cs +++ b/src/AppInstallerCLIE2ETests/WinGetUtil/WinGetUtilManifest.cs @@ -139,7 +139,7 @@ public void WinGetUtil_ValidateManifest_V1_10_WithSchemaHeader_Failure(WinGetUti this.ValidateSchemaHeaderFailure(manifestsFilePath, createManifestOption, expectedError); // Schema header version mismatch - manifestsFilePath = TestCommon.GetTestDataFile(@"Manifests\TestWarningManifestV1_10-SchemaHeadeVersionMismatch.yaml"); + manifestsFilePath = TestCommon.GetTestDataFile(@"Manifests\TestWarningManifestV1_10-SchemaHeaderVersionMismatch.yaml"); expectedError = "Manifest Error: The manifest version in the schema header does not match the ManifestVersion property value in the manifest."; this.ValidateSchemaHeaderFailure(manifestsFilePath, createManifestOption, expectedError); } From 1d27243ad1f87f0d03b292548027d794e1d7f1bd Mon Sep 17 00:00:00 2001 From: Madhusudhan Gumbalapura Sudarshan Date: Sun, 12 Jan 2025 01:01:18 -0800 Subject: [PATCH 07/18] Enhance YAML manifest schema header validation - Updated regex pattern for schema URL matching, - Renamed and revised validation functions, and introduced new helper functions for improved error handling and validation logic. - These changes ensure accurate validation of schema header URLs against schema definition files. --- .../Manifest/ManifestSchemaValidation.cpp | 88 ++++++++++++++----- 1 file changed, 67 insertions(+), 21 deletions(-) diff --git a/src/AppInstallerCommonCore/Manifest/ManifestSchemaValidation.cpp b/src/AppInstallerCommonCore/Manifest/ManifestSchemaValidation.cpp index 21d55c3fe6..0db0e25f5f 100644 --- a/src/AppInstallerCommonCore/Manifest/ManifestSchemaValidation.cpp +++ b/src/AppInstallerCommonCore/Manifest/ManifestSchemaValidation.cpp @@ -186,7 +186,7 @@ namespace AppInstaller::Manifest::YamlParser bool ParseSchemaHeaderUrl(const std::string& schemaHeaderValue, std::string& schemaType, std::string& schemaVersion) { // Use regex to match the pattern of @"winget-manifest\.(?\w+)\.(?[\d\.]+)\.schema\.json$" - std::regex schemaUrlPattern(R"(^\$schema=https://aka\.ms/winget-manifest\.(\w+)\.([\d\.]+)\.schema\.json$)"); + std::regex schemaUrlPattern(R"(winget-manifest\.(\w+)\.([\d\.]+)\.schema\.json$)"); std::smatch match; if (std::regex_search(schemaHeaderValue, match, schemaUrlPattern)) @@ -221,30 +221,42 @@ namespace AppInstaller::Manifest::YamlParser return errors; } - std::vector ValidateYamlManifestSchemaHeader(const YamlManifestInfo& manifestInfo, const ManifestVer& manifestVersion, const ValidationError::Level& errorLevel) + bool IsValidSchemaHeaderUrl(const std::string& schemaHeaderUrlString, const YamlManifestInfo& manifestInfo, const ManifestVer& manifestVersion) { - std::vector errors; + // Load the schema file to compare the schema header URL with the schema ID in the schema file + Json::Value schemaFile = LoadSchemaDoc(manifestVersion, manifestInfo.ManifestType); - if (manifestInfo.ManifestType == ManifestTypeEnum::Shadow) + if (schemaFile.isMember("$id")) { - // There's no schema for a shadow manifest. - return errors; + std::string schemaId = schemaFile["$id"].asString(); + + // Prefix schema ID with "schema=" to match the schema header URL pattern and compare it with the schema header URL + schemaId = "$schema=" + schemaId; + + if (std::equal(schemaId.begin(), schemaId.end(), schemaHeaderUrlString.begin(), schemaHeaderUrlString.end(), + [](char a, char b) { return tolower(a) == tolower(b); })) + { + return true; + } } - size_t rootNodeLine = manifestInfo.Root.Mark().line; - std::string schemaHeaderString; + return false; + } - ManifestSchemaHeader schemaHeader{}; - schemaHeader.FileName = manifestInfo.FileName; + ValidationError GetSchemaHeaderUrlPatternMismatchError(const std::string& schemaHeaderUrlString,const ManifestSchemaHeader& manifestSchemaHeader, const ValidationError::Level& errorLevel) + { + size_t schemaHeaderUrlIndex = manifestSchemaHeader.SchemaHeaderString.find(schemaHeaderUrlString) + 1; - if (!SearchForManifestSchemaHeaderString(manifestInfo.InputStream, rootNodeLine, schemaHeader)) - { - errors.emplace_back(ValidationError::MessageLevelWithFile(ManifestError::SchemaHeaderNotFound, errorLevel, manifestInfo.FileName)); - return errors; - } + return ValidationError::MessageContextValueLineLevelWithFile(ManifestError::SchemaHeaderUrlPatternMismatch, "", manifestSchemaHeader.SchemaHeaderString, manifestSchemaHeader.Line, schemaHeaderUrlIndex, errorLevel, manifestSchemaHeader.FileName); + } + + std::vector ValidateSchemaHeaderUrl(const YamlManifestInfo& manifestInfo, const ManifestVer& manifestVersion, ManifestSchemaHeader& manifestSchemaHeader, const ValidationError::Level& errorLevel) + { + std::vector errors; std::string schemaHeaderUrlString; - auto parserErrors = ParseSchemaHeaderString(schemaHeader, errorLevel, schemaHeaderUrlString); + // Parse the schema header string to get the schema header URL + auto parserErrors = ParseSchemaHeaderString(manifestSchemaHeader, errorLevel, schemaHeaderUrlString); std::move(parserErrors.begin(), parserErrors.end(), std::inserter(errors, errors.end())); if (!errors.empty()) @@ -255,22 +267,56 @@ namespace AppInstaller::Manifest::YamlParser std::string manifestTypeString; std::string manifestVersionString; + // Parse the schema header URL to get the manifest type and version if (ParseSchemaHeaderUrl(schemaHeaderUrlString, manifestTypeString, manifestVersionString)) { - schemaHeader.ManifestType = std::move(manifestTypeString); - schemaHeader.ManifestVersion = std::move(manifestVersionString); + manifestSchemaHeader.ManifestType = std::move(manifestTypeString); + manifestSchemaHeader.ManifestVersion = std::move(manifestVersionString); - auto compareErrors = ValidateManifestSchemaHeaderDetails(schemaHeader, manifestInfo.ManifestType, manifestVersion, errorLevel); + auto compareErrors = ValidateManifestSchemaHeaderDetails(manifestSchemaHeader, manifestInfo.ManifestType, manifestVersion, errorLevel); std::move(compareErrors.begin(), compareErrors.end(), std::inserter(errors, errors.end())); + + // Finally, match the entire schema header URL with the schema ID in the schema file to ensure the URL domain matches the schema definition file. + if (!IsValidSchemaHeaderUrl(schemaHeaderUrlString, manifestInfo, manifestVersion)) + { + errors.emplace_back(GetSchemaHeaderUrlPatternMismatchError(schemaHeaderUrlString, manifestSchemaHeader, errorLevel)); + } } else { - size_t schemaHeaderUrlIndex = schemaHeader.SchemaHeaderString.find(schemaHeaderUrlString) + 1; - errors.emplace_back(ValidationError::MessageContextValueLineLevelWithFile(ManifestError::SchemaHeaderUrlPatternMismatch, "", schemaHeaderUrlString, schemaHeader.Line, schemaHeaderUrlIndex, errorLevel, schemaHeader.FileName)); + errors.emplace_back(GetSchemaHeaderUrlPatternMismatchError(schemaHeaderUrlString, manifestSchemaHeader, errorLevel)); } return errors; } + + std::vector ValidateYamlManifestSchemaHeader(const YamlManifestInfo& manifestInfo, const ManifestVer& manifestVersion, const ValidationError::Level& errorLevel) + { + std::vector errors; + + if (manifestInfo.ManifestType == ManifestTypeEnum::Shadow) + { + // There's no schema for a shadow manifest. + return errors; + } + + size_t rootNodeLine = manifestInfo.Root.Mark().line; + std::string schemaHeaderString; + + ManifestSchemaHeader schemaHeader{}; + schemaHeader.FileName = manifestInfo.FileName; + + if (!SearchForManifestSchemaHeaderString(manifestInfo.InputStream, rootNodeLine, schemaHeader)) + { + errors.emplace_back(ValidationError::MessageLevelWithFile(ManifestError::SchemaHeaderNotFound, errorLevel, manifestInfo.FileName)); + return errors; + } + + auto parserErrors = ValidateSchemaHeaderUrl(manifestInfo, manifestVersion, schemaHeader, errorLevel); + std::move(parserErrors.begin(), parserErrors.end(), std::inserter(errors, errors.end())); + + return errors; + } } Json::Value LoadSchemaDoc(const ManifestVer& manifestVersion, ManifestTypeEnum manifestType) From bd4fd2487e28c0dca4ca1117536e2185b4c8c5d9 Mon Sep 17 00:00:00 2001 From: Madhusudhan Gumbalapura Sudarshan Date: Sun, 12 Jan 2025 01:01:37 -0800 Subject: [PATCH 08/18] Fix casing in YAML language server schema URL comment Updated the URL in the comment for the YAML language server schema to correct the casing of "defaultLocale" to "defaultlocale". This ensures the schema URL is correctly referenced. --- .../ManifestV1_10-MultiFile-DefaultLocale.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/AppInstallerCLITests/TestData/MultiFileManifestV1_10/ManifestV1_10-MultiFile-DefaultLocale.yaml b/src/AppInstallerCLITests/TestData/MultiFileManifestV1_10/ManifestV1_10-MultiFile-DefaultLocale.yaml index 3a35bbbe7a..50c2dc3044 100644 --- a/src/AppInstallerCLITests/TestData/MultiFileManifestV1_10/ManifestV1_10-MultiFile-DefaultLocale.yaml +++ b/src/AppInstallerCLITests/TestData/MultiFileManifestV1_10/ManifestV1_10-MultiFile-DefaultLocale.yaml @@ -1,4 +1,4 @@ -# yaml-language-server: $schema=https://aka.ms/winget-manifest.defaultLocale.1.10.0.schema.json +# yaml-language-server: $schema=https://aka.ms/winget-manifest.defaultlocale.1.10.0.schema.json PackageIdentifier: microsoft.msixsdk PackageVersion: 1.7.32 PackageLocale: en-US From 4b18e9e6c43cc9c4090b3d8c6f41d8d71e7af6bd Mon Sep 17 00:00:00 2001 From: Madhusudhan Gumbalapura Sudarshan Date: Sun, 12 Jan 2025 11:30:01 -0800 Subject: [PATCH 09/18] Fix casing in YAML language server schema URL to address ValidateV1_10GoodManifestAndVerifyContents test failure Corrected the URL in the comment for the YAML language server schema by changing "defaultlocale" to "defaultLocale" to ensure proper casing. --- .../ManifestV1_10-MultiFile-DefaultLocale.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/AppInstallerCLITests/TestData/MultiFileManifestV1_10/ManifestV1_10-MultiFile-DefaultLocale.yaml b/src/AppInstallerCLITests/TestData/MultiFileManifestV1_10/ManifestV1_10-MultiFile-DefaultLocale.yaml index 50c2dc3044..3a35bbbe7a 100644 --- a/src/AppInstallerCLITests/TestData/MultiFileManifestV1_10/ManifestV1_10-MultiFile-DefaultLocale.yaml +++ b/src/AppInstallerCLITests/TestData/MultiFileManifestV1_10/ManifestV1_10-MultiFile-DefaultLocale.yaml @@ -1,4 +1,4 @@ -# yaml-language-server: $schema=https://aka.ms/winget-manifest.defaultlocale.1.10.0.schema.json +# yaml-language-server: $schema=https://aka.ms/winget-manifest.defaultLocale.1.10.0.schema.json PackageIdentifier: microsoft.msixsdk PackageVersion: 1.7.32 PackageLocale: en-US From 097ee920fd095c15bd031d2aea13eeb1b9d6ae15 Mon Sep 17 00:00:00 2001 From: Madhusudhan Gumbalapura Sudarshan Date: Mon, 13 Jan 2025 17:13:02 -0800 Subject: [PATCH 10/18] Addressing PR review comments: Simplify schema header validation error messages 1. Updated error messages in multiple files to simplify instructions for verifying schema headers. 2. Removed detailed YAML node verification instructions. 3. Modified function signatures in ManifestSchemaValidation.cpp to remove const qualifiers. --- src/AppInstallerCLIE2ETests/ValidateCommand.cs | 2 +- .../WinGetUtil/WinGetUtilManifest.cs | 2 +- src/AppInstallerCLITests/YamlManifest.cpp | 2 +- .../Manifest/ManifestSchemaValidation.cpp | 10 ++-------- .../Manifest/ManifestValidation.cpp | 2 +- 5 files changed, 6 insertions(+), 12 deletions(-) diff --git a/src/AppInstallerCLIE2ETests/ValidateCommand.cs b/src/AppInstallerCLIE2ETests/ValidateCommand.cs index 2196012824..4eb6f61623 100644 --- a/src/AppInstallerCLIE2ETests/ValidateCommand.cs +++ b/src/AppInstallerCLIE2ETests/ValidateCommand.cs @@ -95,7 +95,7 @@ public void ValidateManifestV1_10_SchemaHeaderExpectWarnings() result = TestCommon.RunAICLICommand("validate", TestCommon.GetTestDataFile("Manifests\\TestWarningManifestV1_10-SchemaHeaderInvalid.yaml")); Assert.AreEqual(Constants.ErrorCode.ERROR_MANIFEST_VALIDATION_WARNING, result.ExitCode); Assert.True(result.StdOut.Contains("Manifest validation succeeded with warnings.")); - Assert.True(result.StdOut.Contains("Manifest Warning: The schema header is invalid. Please verify that the schema header is present and formatted correctly as a valid YAML node with the appropriate commented syntax.")); + Assert.True(result.StdOut.Contains("Manifest Warning: The schema header is invalid. Please verify that the schema header is present and formatted correctly.")); result = TestCommon.RunAICLICommand("validate", TestCommon.GetTestDataFile("Manifests\\TestWarningManifestV1_10-SchemaHeaderURLPatternMismatch.yaml")); Assert.AreEqual(Constants.ErrorCode.ERROR_MANIFEST_VALIDATION_WARNING, result.ExitCode); diff --git a/src/AppInstallerCLIE2ETests/WinGetUtil/WinGetUtilManifest.cs b/src/AppInstallerCLIE2ETests/WinGetUtil/WinGetUtilManifest.cs index 4dd75b3531..968eea9602 100644 --- a/src/AppInstallerCLIE2ETests/WinGetUtil/WinGetUtilManifest.cs +++ b/src/AppInstallerCLIE2ETests/WinGetUtil/WinGetUtilManifest.cs @@ -125,7 +125,7 @@ public void WinGetUtil_ValidateManifest_V1_10_WithSchemaHeader_Failure(WinGetUti // Schema header invalid manifestsFilePath = TestCommon.GetTestDataFile(@"Manifests\TestWarningManifestV1_10-SchemaHeaderInvalid.yaml"); - expectedError = "Manifest Error: The schema header is invalid. Please verify that the schema header is present and formatted correctly as a valid YAML node with the appropriate commented syntax."; + expectedError = "Manifest Error: The schema header is invalid. Please verify that the schema header is present and formatted correctly."; this.ValidateSchemaHeaderFailure(manifestsFilePath, createManifestOption, expectedError); // Schema header URL pattern mismatch diff --git a/src/AppInstallerCLITests/YamlManifest.cpp b/src/AppInstallerCLITests/YamlManifest.cpp index 4f7459a94f..4e536fc6b7 100644 --- a/src/AppInstallerCLITests/YamlManifest.cpp +++ b/src/AppInstallerCLITests/YamlManifest.cpp @@ -1855,7 +1855,7 @@ TEST_CASE("ManifestV1_10_SchemaHeaderValidations", "[ManifestValidation]") REQUIRE_THROWS_MATCHES(YamlParser::CreateFromPath(TestDataFile("ManifestV1_10-Bad-SchemaHeaderNotFound.yaml"),validateOption), ManifestException, ManifestExceptionMatcher("Schema header not found")); // Schema header not valid - REQUIRE_THROWS_MATCHES(YamlParser::CreateFromPath(TestDataFile("ManifestV1_10-Bad-SchemaHeaderInvalid.yaml"), validateOption), ManifestException, ManifestExceptionMatcher("The schema header is invalid. Please verify that the schema header is present and formatted correctly as a valid YAML node with the appropriate commented syntax")); + REQUIRE_THROWS_MATCHES(YamlParser::CreateFromPath(TestDataFile("ManifestV1_10-Bad-SchemaHeaderInvalid.yaml"), validateOption), ManifestException, ManifestExceptionMatcher("The schema header is invalid. Please verify that the schema header is present and formatted correctly.")); // Schema header URL does not match the expected schema URL REQUIRE_THROWS_MATCHES(YamlParser::CreateFromPath(TestDataFile("ManifestV1_10-Bad-SchemaHeaderURLPatternMismatch.yaml"), validateOption), ManifestException, ManifestExceptionMatcher("The schema header URL does not match the expected pattern.")); diff --git a/src/AppInstallerCommonCore/Manifest/ManifestSchemaValidation.cpp b/src/AppInstallerCommonCore/Manifest/ManifestSchemaValidation.cpp index 0db0e25f5f..78db8b27e3 100644 --- a/src/AppInstallerCommonCore/Manifest/ManifestSchemaValidation.cpp +++ b/src/AppInstallerCommonCore/Manifest/ManifestSchemaValidation.cpp @@ -111,7 +111,7 @@ namespace AppInstaller::Manifest::YamlParser return result; } - bool SearchForManifestSchemaHeaderString(std::shared_ptr yamlInputStream, const size_t& rootNodeBeginsAtLine, ManifestSchemaHeader& schemaHeader) + bool SearchForManifestSchemaHeaderString(std::shared_ptr yamlInputStream, size_t rootNodeBeginsAtLine, ManifestSchemaHeader& schemaHeader) { std::string line; size_t currentLine = 1; @@ -290,16 +290,10 @@ namespace AppInstaller::Manifest::YamlParser return errors; } - std::vector ValidateYamlManifestSchemaHeader(const YamlManifestInfo& manifestInfo, const ManifestVer& manifestVersion, const ValidationError::Level& errorLevel) + std::vector ValidateYamlManifestSchemaHeader(const YamlManifestInfo& manifestInfo, const ManifestVer& manifestVersion, ValidationError::Level errorLevel) { std::vector errors; - if (manifestInfo.ManifestType == ManifestTypeEnum::Shadow) - { - // There's no schema for a shadow manifest. - return errors; - } - size_t rootNodeLine = manifestInfo.Root.Mark().line; std::string schemaHeaderString; diff --git a/src/AppInstallerCommonCore/Manifest/ManifestValidation.cpp b/src/AppInstallerCommonCore/Manifest/ManifestValidation.cpp index 6154a2579f..cb42ea9c94 100644 --- a/src/AppInstallerCommonCore/Manifest/ManifestValidation.cpp +++ b/src/AppInstallerCommonCore/Manifest/ManifestValidation.cpp @@ -65,7 +65,7 @@ namespace AppInstaller::Manifest { AppInstaller::Manifest::ManifestError::MsixSignatureHashFailed, "Failed to calculate MSIX signature hash.Please verify that the input file is a valid, signed MSIX."sv }, { AppInstaller::Manifest::ManifestError::ShadowManifestNotAllowed, "Shadow manifest is not allowed." }, { AppInstaller::Manifest::ManifestError::SchemaHeaderNotFound, "Schema header not found." }, - { AppInstaller::Manifest::ManifestError::InvalidSchemaHeader , "The schema header is invalid. Please verify that the schema header is present and formatted correctly as a valid YAML node with the appropriate commented syntax."sv }, + { AppInstaller::Manifest::ManifestError::InvalidSchemaHeader , "The schema header is invalid. Please verify that the schema header is present and formatted correctly."sv }, { AppInstaller::Manifest::ManifestError::SchemaHeaderManifestTypeMismatch , "The manifest type in the schema header does not match the ManifestType property value in the manifest."sv }, { AppInstaller::Manifest::ManifestError::SchemaHeaderManifestVersionMismatch, "The manifest version in the schema header does not match the ManifestVersion property value in the manifest."sv }, { AppInstaller::Manifest::ManifestError::SchemaHeaderUrlPatternMismatch, "The schema header URL does not match the expected pattern."sv }, From 7d9c4f2e73fbe0c68cb554141e0af87997aafe7f Mon Sep 17 00:00:00 2001 From: Madhusudhan Gumbalapura Sudarshan Date: Tue, 14 Jan 2025 13:24:12 -0800 Subject: [PATCH 11/18] Fix formatting issues in ValidateCommand.cs --- src/AppInstallerCLIE2ETests/ValidateCommand.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/AppInstallerCLIE2ETests/ValidateCommand.cs b/src/AppInstallerCLIE2ETests/ValidateCommand.cs index 4eb6f61623..2251764630 100644 --- a/src/AppInstallerCLIE2ETests/ValidateCommand.cs +++ b/src/AppInstallerCLIE2ETests/ValidateCommand.cs @@ -79,7 +79,7 @@ public void ValidateManifestDoesNotExist() var result = TestCommon.RunAICLICommand("validate", TestCommon.GetTestDataFile("Manifests\\DoesNotExist")); Assert.AreEqual(Constants.ErrorCode.ERROR_PATH_NOT_FOUND, result.ExitCode); Assert.True(result.StdOut.Contains("Path does not exist")); - } + } /// /// Test validate manifest with invalid schema and expect warnings. @@ -123,4 +123,4 @@ public void ValidateManifestV1_10_SchemaHeaderExpectNoWarning() Assert.AreEqual(Constants.ErrorCode.S_OK, result.ExitCode); } } -} +} \ No newline at end of file From ea009789f6400e928401c7b0dcfd58922a8a7cb2 Mon Sep 17 00:00:00 2001 From: Madhusudhan Gumbalapura Sudarshan Date: Wed, 15 Jan 2025 10:40:47 -0800 Subject: [PATCH 12/18] [PRFeedback:] Refactor YAML schema header handling - Simplified schema header handling by directly associating schema headers with YAML documents. - Removed unnecessary input streams and search functions, resulting in cleaner and more maintainable code. - Updated relevant structures and methods across multiple files to support the new approach. --- .../WinGetUtil/WinGetUtilManifest.cs | 12 ++--- .../Manifest/ManifestSchemaValidation.cpp | 40 ++------------ .../Manifest/YamlParser.cpp | 31 ++++++----- .../Public/winget/ManifestYamlParser.h | 5 +- .../Public/winget/Yaml.h | 29 ++++++++++ src/AppInstallerSharedLib/Yaml.cpp | 54 +++++++++++++++++++ src/AppInstallerSharedLib/YamlWrapper.cpp | 52 ++++++++++++++++++ src/AppInstallerSharedLib/YamlWrapper.h | 12 +++++ 8 files changed, 176 insertions(+), 59 deletions(-) diff --git a/src/AppInstallerCLIE2ETests/WinGetUtil/WinGetUtilManifest.cs b/src/AppInstallerCLIE2ETests/WinGetUtil/WinGetUtilManifest.cs index 968eea9602..ab0d76a223 100644 --- a/src/AppInstallerCLIE2ETests/WinGetUtil/WinGetUtilManifest.cs +++ b/src/AppInstallerCLIE2ETests/WinGetUtil/WinGetUtilManifest.cs @@ -121,30 +121,30 @@ public void WinGetUtil_ValidateManifest_V1_10_WithSchemaHeader_Failure(WinGetUti // Schema header not found string manifestsFilePath = TestCommon.GetTestDataFile(@"Manifests\TestWarningManifestV1_10-SchemaHeaderNotFound.yaml"); string expectedError = "Manifest Error: Schema header not found."; - this.ValidateSchemaHeaderFailure(manifestsFilePath, createManifestOption, expectedError); + ValidateSchemaHeaderFailure(manifestsFilePath, createManifestOption, expectedError); // Schema header invalid manifestsFilePath = TestCommon.GetTestDataFile(@"Manifests\TestWarningManifestV1_10-SchemaHeaderInvalid.yaml"); expectedError = "Manifest Error: The schema header is invalid. Please verify that the schema header is present and formatted correctly."; - this.ValidateSchemaHeaderFailure(manifestsFilePath, createManifestOption, expectedError); + ValidateSchemaHeaderFailure(manifestsFilePath, createManifestOption, expectedError); // Schema header URL pattern mismatch manifestsFilePath = TestCommon.GetTestDataFile(@"Manifests\TestWarningManifestV1_10-SchemaHeaderURLPatternMismatch.yaml"); expectedError = "Manifest Error: The schema header URL does not match the expected pattern."; - this.ValidateSchemaHeaderFailure(manifestsFilePath, createManifestOption, expectedError); + ValidateSchemaHeaderFailure(manifestsFilePath, createManifestOption, expectedError); // Schema header manifest type mismatch manifestsFilePath = TestCommon.GetTestDataFile(@"Manifests\TestWarningManifestV1_10-SchemaHeaderManifestTypeMismatch.yaml"); expectedError = "Manifest Error: The manifest type in the schema header does not match the ManifestType property value in the manifest."; - this.ValidateSchemaHeaderFailure(manifestsFilePath, createManifestOption, expectedError); + ValidateSchemaHeaderFailure(manifestsFilePath, createManifestOption, expectedError); // Schema header version mismatch manifestsFilePath = TestCommon.GetTestDataFile(@"Manifests\TestWarningManifestV1_10-SchemaHeaderVersionMismatch.yaml"); expectedError = "Manifest Error: The manifest version in the schema header does not match the ManifestVersion property value in the manifest."; - this.ValidateSchemaHeaderFailure(manifestsFilePath, createManifestOption, expectedError); + ValidateSchemaHeaderFailure(manifestsFilePath, createManifestOption, expectedError); } - private void ValidateSchemaHeaderFailure(string manifestsFilePath, WinGetUtilWrapper.CreateManifestOption createManifestOption, string expectedError) + private static void ValidateSchemaHeaderFailure(string manifestsFilePath, WinGetUtilWrapper.CreateManifestOption createManifestOption, string expectedError) { // Create manifest WinGetUtilWrapper.WinGetCreateManifest( diff --git a/src/AppInstallerCommonCore/Manifest/ManifestSchemaValidation.cpp b/src/AppInstallerCommonCore/Manifest/ManifestSchemaValidation.cpp index 78db8b27e3..47b8f1684e 100644 --- a/src/AppInstallerCommonCore/Manifest/ManifestSchemaValidation.cpp +++ b/src/AppInstallerCommonCore/Manifest/ManifestSchemaValidation.cpp @@ -111,39 +111,6 @@ namespace AppInstaller::Manifest::YamlParser return result; } - bool SearchForManifestSchemaHeaderString(std::shared_ptr yamlInputStream, size_t rootNodeBeginsAtLine, ManifestSchemaHeader& schemaHeader) - { - std::string line; - size_t currentLine = 1; - schemaHeader.SchemaHeaderString.clear(); - - // Search for the schema header string in the comments before the root node. - while (currentLine < rootNodeBeginsAtLine && std::getline(*yamlInputStream, line)) - { - std::string comment = Utility::Trim(line); - - // Check if the line is a comment - if (!comment.empty() && comment[0] == '#') - { - size_t pos = comment.find(YamlLanguageServerKey); - - // Check if the comment contains the schema header string - if (pos != std::string::npos) - { - schemaHeader.SchemaHeaderString = std::move(comment); - schemaHeader.Line = currentLine; - schemaHeader.column = pos; - - return true; - } - } - - currentLine++; - } - - return false; - } - std::vector ParseSchemaHeaderString(const ManifestSchemaHeader& manifestSchemaHeader, const ValidationError::Level& errorLevel, std::string& schemaHeaderUrlString) { std::vector errors; @@ -293,14 +260,15 @@ namespace AppInstaller::Manifest::YamlParser std::vector ValidateYamlManifestSchemaHeader(const YamlManifestInfo& manifestInfo, const ManifestVer& manifestVersion, ValidationError::Level errorLevel) { std::vector errors; - - size_t rootNodeLine = manifestInfo.Root.Mark().line; std::string schemaHeaderString; ManifestSchemaHeader schemaHeader{}; schemaHeader.FileName = manifestInfo.FileName; + schemaHeader.SchemaHeaderString = manifestInfo.SchemaHeader.SchemaHeaderString; + schemaHeader.Line = manifestInfo.SchemaHeader.Mark.line; + schemaHeader.column = manifestInfo.SchemaHeader.Mark.column; - if (!SearchForManifestSchemaHeaderString(manifestInfo.InputStream, rootNodeLine, schemaHeader)) + if (schemaHeader.SchemaHeaderString.empty()) { errors.emplace_back(ValidationError::MessageLevelWithFile(ManifestError::SchemaHeaderNotFound, errorLevel, manifestInfo.FileName)); return errors; diff --git a/src/AppInstallerCommonCore/Manifest/YamlParser.cpp b/src/AppInstallerCommonCore/Manifest/YamlParser.cpp index a6f1f2b111..328d5bc031 100644 --- a/src/AppInstallerCommonCore/Manifest/YamlParser.cpp +++ b/src/AppInstallerCommonCore/Manifest/YamlParser.cpp @@ -526,20 +526,22 @@ namespace AppInstaller::Manifest::YamlParser { THROW_HR_IF_MSG(HRESULT_FROM_WIN32(ERROR_DIRECTORY_NOT_SUPPORTED), std::filesystem::is_directory(file.path()), "Subdirectory not supported in manifest path"); - YamlManifestInfo doc; - doc.Root = YAML::Load(file.path()); - doc.FileName = file.path().filename().u8string(); - doc.InputStream = std::make_shared(file.path(), std::ios_base::in | std::ios_base::binary); - docList.emplace_back(std::move(doc)); + YamlManifestInfo manifestInfo; + YAML::DocumentRootWithSchema doc = YAML::LoadDocument(file.path()); + manifestInfo.Root = doc.GetRoot(); + manifestInfo.SchemaHeader = doc.GetSchemaHeader(); + manifestInfo.FileName = file.path().filename().u8string(); + docList.emplace_back(std::move(manifestInfo)); } } else { - YamlManifestInfo doc; - doc.Root = YAML::Load(inputPath, doc.StreamSha256); - doc.FileName = inputPath.filename().u8string(); - doc.InputStream = std::make_shared(inputPath, std::ios_base::in | std::ios_base::binary); - docList.emplace_back(std::move(doc)); + YamlManifestInfo manifestInfo; + YAML::DocumentRootWithSchema doc = YAML::LoadDocument(inputPath, manifestInfo.StreamSha256); + manifestInfo.Root = doc.GetRoot(); + manifestInfo.SchemaHeader = doc.GetSchemaHeader(); + manifestInfo.FileName = inputPath.filename().u8string(); + docList.emplace_back(std::move(manifestInfo)); } } catch (const std::exception& e) @@ -559,10 +561,11 @@ namespace AppInstaller::Manifest::YamlParser try { - YamlManifestInfo doc; - doc.Root = YAML::Load(input); - doc.InputStream = std::make_shared(input); - docList.emplace_back(std::move(doc)); + YamlManifestInfo manifestInfo; + YAML::DocumentRootWithSchema doc = YAML::LoadDocument(input); + manifestInfo.Root = doc.GetRoot(); + manifestInfo.SchemaHeader = doc.GetSchemaHeader(); + docList.emplace_back(std::move(manifestInfo)); } catch (const std::exception& e) { diff --git a/src/AppInstallerCommonCore/Public/winget/ManifestYamlParser.h b/src/AppInstallerCommonCore/Public/winget/ManifestYamlParser.h index 6eab96ff80..7062054ab9 100644 --- a/src/AppInstallerCommonCore/Public/winget/ManifestYamlParser.h +++ b/src/AppInstallerCommonCore/Public/winget/ManifestYamlParser.h @@ -5,7 +5,6 @@ #include #include #include - #include namespace AppInstaller::Manifest::YamlParser @@ -18,8 +17,8 @@ namespace AppInstaller::Manifest::YamlParser // File name of the manifest file if applicable for error reporting std::string FileName; - // Input stream for reading/parsing the Raw YAML content. - std::shared_ptr InputStream; + // Schema header string found in the manifest file + YAML::DocumentSchemaHeader SchemaHeader; // The SHA256 hash of the stream Utility::SHA256::HashBuffer StreamSha256; diff --git a/src/AppInstallerSharedLib/Public/winget/Yaml.h b/src/AppInstallerSharedLib/Public/winget/Yaml.h index 59e582f123..17556efe57 100644 --- a/src/AppInstallerSharedLib/Public/winget/Yaml.h +++ b/src/AppInstallerSharedLib/Public/winget/Yaml.h @@ -237,12 +237,41 @@ namespace AppInstaller::YAML Folded, }; + // A schema header for a document. + struct DocumentSchemaHeader + { + DocumentSchemaHeader() = default; + DocumentSchemaHeader(std::string schemaHeaderString, const Mark& mark) : SchemaHeaderString(std::move(schemaHeaderString)), Mark(mark) {} + + std::string SchemaHeaderString; + Mark Mark; + }; + + struct DocumentRootWithSchema + { + DocumentRootWithSchema() = default; + DocumentRootWithSchema(Node root, DocumentSchemaHeader schemaHeader) : m_root(std::move(root)), m_schemaHeader(std::move(schemaHeader)) {} + + Node GetRoot() const { return m_root; } + const DocumentSchemaHeader& GetSchemaHeader() const { return m_schemaHeader; } + + private: + Node m_root; + DocumentSchemaHeader m_schemaHeader; + }; + // Forward declaration to allow pImpl in this Emitter. namespace Wrapper { struct Document; } + // Loads from the input; returns the root node of the first document. + DocumentRootWithSchema LoadDocument(std::string_view input); + DocumentRootWithSchema LoadDocument(const std::string& input); + DocumentRootWithSchema LoadDocument(const std::filesystem::path& input); + DocumentRootWithSchema LoadDocument(const std::filesystem::path& input, Utility::SHA256::HashBuffer& hashOut); + // A YAML emitter. struct Emitter { diff --git a/src/AppInstallerSharedLib/Yaml.cpp b/src/AppInstallerSharedLib/Yaml.cpp index a263cf7253..e5c883bd6b 100644 --- a/src/AppInstallerSharedLib/Yaml.cpp +++ b/src/AppInstallerSharedLib/Yaml.cpp @@ -601,6 +601,60 @@ namespace AppInstaller::YAML return Load(input, &hashOut); } + DocumentRootWithSchema LoadDocument(std::string_view input) + { + Wrapper::Parser parser(input); + Wrapper::Document document = parser.Load(); + + if (document.HasRoot()) + { + return { document.GetRoot(), document.GetSchemaHeader() }; + } + else + { + // Return an empty root and schema header. + return {}; + } + } + + DocumentRootWithSchema LoadDocument(const std::string& input) + { + return LoadDocument(static_cast(input)); + } + + DocumentRootWithSchema LoadDocument(std::istream& input, Utility::SHA256::HashBuffer* hashOut) + { + Wrapper::Parser parser(input, hashOut); + Wrapper::Document document = parser.Load(); + + if (document.HasRoot()) + { + return { document.GetRoot(), document.GetSchemaHeader() }; + } + else + { + // Return an empty root and schema header. + return {}; + } + } + + DocumentRootWithSchema LoadDocument(const std::filesystem::path& input, Utility::SHA256::HashBuffer* hashOut) + { + std::ifstream stream(input, std::ios_base::in | std::ios_base::binary); + THROW_LAST_ERROR_IF(stream.fail()); + return LoadDocument(stream, hashOut); + } + + DocumentRootWithSchema LoadDocument(const std::filesystem::path& input) + { + return LoadDocument(input, nullptr); + } + + DocumentRootWithSchema LoadDocument(const std::filesystem::path& input, Utility::SHA256::HashBuffer& hashOut) + { + return LoadDocument(input, &hashOut); + } + Emitter::Emitter() : m_document(std::make_unique(true)) { diff --git a/src/AppInstallerSharedLib/YamlWrapper.cpp b/src/AppInstallerSharedLib/YamlWrapper.cpp index a9664b0e0b..b661914f66 100644 --- a/src/AppInstallerSharedLib/YamlWrapper.cpp +++ b/src/AppInstallerSharedLib/YamlWrapper.cpp @@ -258,6 +258,21 @@ namespace AppInstaller::YAML::Wrapper } } + void Document::AddSchemaHeader(const DocumentSchemaHeader& schemHeader) + { + m_schemaHeader = schemHeader; + } + + bool Document::HasSchemaHeader() + { + return !m_schemaHeader.SchemaHeaderString.empty(); + } + + DocumentSchemaHeader Document::GetSchemaHeader() + { + return m_schemaHeader; + } + yaml_node_t* Document::GetNode(yaml_node_item_t index) { yaml_node_t* result = yaml_document_get_node(&m_document, index); @@ -319,6 +334,12 @@ namespace AppInstaller::YAML::Wrapper } } + DocumentSchemaHeader schemaHeader; + if (result.HasRoot() && FindManifestSchemaHeaderString(result.GetRoot().Mark().line, schemaHeader)) + { + result.AddSchemaHeader(schemaHeader); + } + return result; } @@ -387,6 +408,37 @@ namespace AppInstaller::YAML::Wrapper yaml_parser_set_encoding(&m_parser, YAML_UTF8_ENCODING); } + bool Parser::FindManifestSchemaHeaderString(size_t rootNodeLine, DocumentSchemaHeader& schemaHeader) + { + std::istringstream input(m_input); + std::string line; + size_t currentLine = 1; + + // Search for the schema header string in the comments before the root node. + while (currentLine < rootNodeLine && std::getline(input, line)) + { + std::string comment = Utility::Trim(line); + size_t pos = line.find(YamlLanguageServerKey); + + // Check if the line is a comment + if (!comment.empty() && comment[0] == '#') + { + // Check if the comment contains the schema header string + if (pos != std::string::npos) + { + schemaHeader.SchemaHeaderString = std::move(comment); + schemaHeader.Mark = { currentLine, pos }; + + return true; + } + } + + currentLine++; + } + + return false; + } + Event::~Event() { if (m_token) diff --git a/src/AppInstallerSharedLib/YamlWrapper.h b/src/AppInstallerSharedLib/YamlWrapper.h index d87141f56e..48fd903085 100644 --- a/src/AppInstallerSharedLib/YamlWrapper.h +++ b/src/AppInstallerSharedLib/YamlWrapper.h @@ -55,12 +55,22 @@ namespace AppInstaller::YAML::Wrapper // Adds a pair to the mapping. void AppendMappingPair(int mapping, int key, int value); + // Adds a schema header to the document. + void AddSchemaHeader(const DocumentSchemaHeader& schemaHeader); + + // Determines whether the document has a schema header. + bool HasSchemaHeader(); + + // Gets the schema header string. + DocumentSchemaHeader GetSchemaHeader(); + private: // Gets the node referenced by the index. yaml_node_t* GetNode(yaml_node_item_t index); DestructionToken m_token; yaml_document_t m_document; + DocumentSchemaHeader m_schemaHeader = {}; }; // A libyaml yaml_parser_t. @@ -86,10 +96,12 @@ namespace AppInstaller::YAML::Wrapper private: // Determines the type of encoding in use, transforming the input as necessary. void PrepareInput(); + bool FindManifestSchemaHeaderString(size_t rootNodeLine, DocumentSchemaHeader& schemaHeader); DestructionToken m_token; yaml_parser_t m_parser; std::string m_input; + static constexpr std::string_view YamlLanguageServerKey = "yaml-language-server"; }; // A libyaml yaml_event_t. From 8607fbbbcd9cdfd710df7bb493d052b5704e2b92 Mon Sep 17 00:00:00 2001 From: Madhusudhan Gumbalapura Sudarshan Date: Wed, 15 Jan 2025 10:47:01 -0800 Subject: [PATCH 13/18] Fix typo in AddSchemaHeader parameter name - Renamed the parameter `schemHeader` to `schemaHeader` in the `AddSchemaHeader` method of the `Document` class in `YamlWrapper.cpp`. --- src/AppInstallerSharedLib/YamlWrapper.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/AppInstallerSharedLib/YamlWrapper.cpp b/src/AppInstallerSharedLib/YamlWrapper.cpp index b661914f66..b6d1f854b7 100644 --- a/src/AppInstallerSharedLib/YamlWrapper.cpp +++ b/src/AppInstallerSharedLib/YamlWrapper.cpp @@ -258,9 +258,9 @@ namespace AppInstaller::YAML::Wrapper } } - void Document::AddSchemaHeader(const DocumentSchemaHeader& schemHeader) + void Document::AddSchemaHeader(const DocumentSchemaHeader& schemaHeader) { - m_schemaHeader = schemHeader; + m_schemaHeader = schemaHeader; } bool Document::HasSchemaHeader() From 91f1fb286c37584eaac76dd7ed873b6ef085c821 Mon Sep 17 00:00:00 2001 From: Madhusudhan Gumbalapura Sudarshan Date: Wed, 15 Jan 2025 11:30:33 -0800 Subject: [PATCH 14/18] Fix visual studio editor introduced encoding character issue with the test MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Replaced the extended character 'ë' in the file name `TëstExeInstaller.yaml` with a placeholder character '�', resulting in the new file name `T�stExeInstaller.yaml`. This change addresses potential encoding issues or compatibility concerns with systems that may not support the original extended character. --- src/AppInstallerCLIE2ETests/ValidateCommand.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/AppInstallerCLIE2ETests/ValidateCommand.cs b/src/AppInstallerCLIE2ETests/ValidateCommand.cs index 2251764630..9f24b6cda2 100644 --- a/src/AppInstallerCLIE2ETests/ValidateCommand.cs +++ b/src/AppInstallerCLIE2ETests/ValidateCommand.cs @@ -31,7 +31,7 @@ public void ValidateManifest() [Test] public void ValidateManifestWithExtendedCharacter() { - var result = TestCommon.RunAICLICommand("validate", TestCommon.GetTestDataFile("Manifests\\TëstExeInstaller.yaml")); + var result = TestCommon.RunAICLICommand("validate", TestCommon.GetTestDataFile("Manifests\\T�stExeInstaller.yaml")); Assert.AreEqual(Constants.ErrorCode.S_OK, result.ExitCode); Assert.True(result.StdOut.Contains("Manifest validation succeeded.")); } From ad62417717b79a76d30f5e2926c581f4360fa023 Mon Sep 17 00:00:00 2001 From: Madhusudhan Gumbalapura Sudarshan Date: Wed, 15 Jan 2025 11:38:25 -0800 Subject: [PATCH 15/18] Revert chagnes ValidateCommand.cs and replacing it with orginal file from master branch to address the encoding issue introduced by editor --- .../ValidateCommand.cs | 44 +------------------ 1 file changed, 1 insertion(+), 43 deletions(-) diff --git a/src/AppInstallerCLIE2ETests/ValidateCommand.cs b/src/AppInstallerCLIE2ETests/ValidateCommand.cs index 9f24b6cda2..5f1f7b89b5 100644 --- a/src/AppInstallerCLIE2ETests/ValidateCommand.cs +++ b/src/AppInstallerCLIE2ETests/ValidateCommand.cs @@ -31,7 +31,7 @@ public void ValidateManifest() [Test] public void ValidateManifestWithExtendedCharacter() { - var result = TestCommon.RunAICLICommand("validate", TestCommon.GetTestDataFile("Manifests\\T�stExeInstaller.yaml")); + var result = TestCommon.RunAICLICommand("validate", TestCommon.GetTestDataFile("Manifests\\TëstExeInstaller.yaml")); Assert.AreEqual(Constants.ErrorCode.S_OK, result.ExitCode); Assert.True(result.StdOut.Contains("Manifest validation succeeded.")); } @@ -79,48 +79,6 @@ public void ValidateManifestDoesNotExist() var result = TestCommon.RunAICLICommand("validate", TestCommon.GetTestDataFile("Manifests\\DoesNotExist")); Assert.AreEqual(Constants.ErrorCode.ERROR_PATH_NOT_FOUND, result.ExitCode); Assert.True(result.StdOut.Contains("Path does not exist")); - } - - /// - /// Test validate manifest with invalid schema and expect warnings. - /// - [Test] - public void ValidateManifestV1_10_SchemaHeaderExpectWarnings() - { - var result = TestCommon.RunAICLICommand("validate", TestCommon.GetTestDataFile("Manifests\\TestWarningManifestV1_10-SchemaHeaderNotFound.yaml")); - Assert.AreEqual(Constants.ErrorCode.ERROR_MANIFEST_VALIDATION_WARNING, result.ExitCode); - Assert.True(result.StdOut.Contains("Manifest validation succeeded with warnings.")); - Assert.True(result.StdOut.Contains("Manifest Warning: Schema header not found.")); - - result = TestCommon.RunAICLICommand("validate", TestCommon.GetTestDataFile("Manifests\\TestWarningManifestV1_10-SchemaHeaderInvalid.yaml")); - Assert.AreEqual(Constants.ErrorCode.ERROR_MANIFEST_VALIDATION_WARNING, result.ExitCode); - Assert.True(result.StdOut.Contains("Manifest validation succeeded with warnings.")); - Assert.True(result.StdOut.Contains("Manifest Warning: The schema header is invalid. Please verify that the schema header is present and formatted correctly.")); - - result = TestCommon.RunAICLICommand("validate", TestCommon.GetTestDataFile("Manifests\\TestWarningManifestV1_10-SchemaHeaderURLPatternMismatch.yaml")); - Assert.AreEqual(Constants.ErrorCode.ERROR_MANIFEST_VALIDATION_WARNING, result.ExitCode); - Assert.True(result.StdOut.Contains("Manifest validation succeeded with warnings.")); - Assert.True(result.StdOut.Contains("Manifest Warning: The schema header URL does not match the expected pattern")); - - result = TestCommon.RunAICLICommand("validate", TestCommon.GetTestDataFile("Manifests\\TestWarningManifestV1_10-SchemaHeaderManifestTypeMismatch.yaml")); - Assert.AreEqual(Constants.ErrorCode.ERROR_MANIFEST_VALIDATION_WARNING, result.ExitCode); - Assert.True(result.StdOut.Contains("Manifest validation succeeded with warnings.")); - Assert.True(result.StdOut.Contains("Manifest Warning: The manifest type in the schema header does not match the ManifestType property value in the manifest.")); - - result = TestCommon.RunAICLICommand("validate", TestCommon.GetTestDataFile("Manifests\\TestWarningManifestV1_10-SchemaHeaderVersionMismatch.yaml")); - Assert.AreEqual(Constants.ErrorCode.ERROR_MANIFEST_VALIDATION_WARNING, result.ExitCode); - Assert.True(result.StdOut.Contains("Manifest validation succeeded with warnings.")); - Assert.True(result.StdOut.Contains("Manifest Warning: The manifest version in the schema header does not match the ManifestVersion property value in the manifest.")); - } - - /// - /// Test validate manifest with valid schema header. - /// - [Test] - public void ValidateManifestV1_10_SchemaHeaderExpectNoWarning() - { - var result = TestCommon.RunAICLICommand("validate", TestCommon.GetTestDataFile("Manifests\\TestGoodManifestV1_10-SchemaHeader.yaml")); - Assert.AreEqual(Constants.ErrorCode.S_OK, result.ExitCode); } } } \ No newline at end of file From 001e6103de2a6b85023273591639e8ae311411f7 Mon Sep 17 00:00:00 2001 From: Madhusudhan Gumbalapura Sudarshan Date: Wed, 15 Jan 2025 11:56:47 -0800 Subject: [PATCH 16/18] Bring back validatecommand schema header tests. --- .../ValidateCommand.cs | 42 +++++++++++++++++++ 1 file changed, 42 insertions(+) diff --git a/src/AppInstallerCLIE2ETests/ValidateCommand.cs b/src/AppInstallerCLIE2ETests/ValidateCommand.cs index 5f1f7b89b5..4893568dbc 100644 --- a/src/AppInstallerCLIE2ETests/ValidateCommand.cs +++ b/src/AppInstallerCLIE2ETests/ValidateCommand.cs @@ -80,5 +80,47 @@ public void ValidateManifestDoesNotExist() Assert.AreEqual(Constants.ErrorCode.ERROR_PATH_NOT_FOUND, result.ExitCode); Assert.True(result.StdOut.Contains("Path does not exist")); } + + /// + /// Test validate manifest with invalid schema and expect warnings. + /// + [Test] + public void ValidateManifestV1_10_SchemaHeaderExpectWarnings() + { + var result = TestCommon.RunAICLICommand("validate", TestCommon.GetTestDataFile("Manifests\\TestWarningManifestV1_10-SchemaHeaderNotFound.yaml")); + Assert.AreEqual(Constants.ErrorCode.ERROR_MANIFEST_VALIDATION_WARNING, result.ExitCode); + Assert.True(result.StdOut.Contains("Manifest validation succeeded with warnings.")); + Assert.True(result.StdOut.Contains("Manifest Warning: Schema header not found.")); + + result = TestCommon.RunAICLICommand("validate", TestCommon.GetTestDataFile("Manifests\\TestWarningManifestV1_10-SchemaHeaderInvalid.yaml")); + Assert.AreEqual(Constants.ErrorCode.ERROR_MANIFEST_VALIDATION_WARNING, result.ExitCode); + Assert.True(result.StdOut.Contains("Manifest validation succeeded with warnings.")); + Assert.True(result.StdOut.Contains("Manifest Warning: The schema header is invalid. Please verify that the schema header is present and formatted correctly.")); + + result = TestCommon.RunAICLICommand("validate", TestCommon.GetTestDataFile("Manifests\\TestWarningManifestV1_10-SchemaHeaderURLPatternMismatch.yaml")); + Assert.AreEqual(Constants.ErrorCode.ERROR_MANIFEST_VALIDATION_WARNING, result.ExitCode); + Assert.True(result.StdOut.Contains("Manifest validation succeeded with warnings.")); + Assert.True(result.StdOut.Contains("Manifest Warning: The schema header URL does not match the expected pattern")); + + result = TestCommon.RunAICLICommand("validate", TestCommon.GetTestDataFile("Manifests\\TestWarningManifestV1_10-SchemaHeaderManifestTypeMismatch.yaml")); + Assert.AreEqual(Constants.ErrorCode.ERROR_MANIFEST_VALIDATION_WARNING, result.ExitCode); + Assert.True(result.StdOut.Contains("Manifest validation succeeded with warnings.")); + Assert.True(result.StdOut.Contains("Manifest Warning: The manifest type in the schema header does not match the ManifestType property value in the manifest.")); + + result = TestCommon.RunAICLICommand("validate", TestCommon.GetTestDataFile("Manifests\\TestWarningManifestV1_10-SchemaHeaderVersionMismatch.yaml")); + Assert.AreEqual(Constants.ErrorCode.ERROR_MANIFEST_VALIDATION_WARNING, result.ExitCode); + Assert.True(result.StdOut.Contains("Manifest validation succeeded with warnings.")); + Assert.True(result.StdOut.Contains("Manifest Warning: The manifest version in the schema header does not match the ManifestVersion property value in the manifest.")); + } + + /// + /// Test validate manifest with valid schema header. + /// + [Test] + public void ValidateManifestV1_10_SchemaHeaderExpectNoWarning() + { + var result = TestCommon.RunAICLICommand("validate", TestCommon.GetTestDataFile("Manifests\\TestGoodManifestV1_10-SchemaHeader.yaml")); + Assert.AreEqual(Constants.ErrorCode.S_OK, result.ExitCode); + } } } \ No newline at end of file From 812f54705f05bd0ec8c62046b7445fe72c6c7248 Mon Sep 17 00:00:00 2001 From: Madhusudhan Gumbalapura Sudarshan Date: Fri, 17 Jan 2025 00:02:52 -0800 Subject: [PATCH 17/18] PRFeedback: Refactor YAML schema handling and improve maintainability - Refactored and renamed components within `AppInstaller::Manifest::YamlParser` and `AppInstaller::YAML` namespaces for better clarity. - Removed `ManifestSchemaHeader` struct, transferring its responsibilities to `YamlManifestInfo` with a new `DocumentSchemaHeader` member. - Updated `DocumentSchemaHeader` to rename `SchemaHeaderString` to `SchemaHeader` and added a static constant `YamlLanguageServerKey`. - Revised `ParseSchemaHeaderString` to use `YamlManifestInfo`. - Split `ValidateManifestSchemaHeaderDetails` into `ValidateSchemaHeaderType` and `ValidateSchemaHeaderVersion`. - Updated `LoadDocument` functions to return `Document` instead of `DocumentRootWithSchema`. - Simplified `Document` struct by removing schema header methods and including `DocumentSchemaHeader` directly. - Added `ExtractSchemaHeaderFromYaml` function. - Updated `YamlParser.cpp` and `YamlWrapper.cpp` to reflect these changes. - Removed schema header methods from `YamlWrapper.h`. - Utilized `Utility::CaseInsensitiveEquals` for case-insensitive comparison of schema header URLs. --- .../Manifest/ManifestSchemaValidation.cpp | 82 ++++++++----------- .../Manifest/YamlParser.cpp | 12 +-- .../Public/winget/ManifestYamlParser.h | 2 +- .../Public/winget/Yaml.h | 22 ++--- src/AppInstallerSharedLib/Yaml.cpp | 51 ++++++++++-- src/AppInstallerSharedLib/YamlWrapper.cpp | 52 ------------ src/AppInstallerSharedLib/YamlWrapper.h | 15 +--- 7 files changed, 99 insertions(+), 137 deletions(-) diff --git a/src/AppInstallerCommonCore/Manifest/ManifestSchemaValidation.cpp b/src/AppInstallerCommonCore/Manifest/ManifestSchemaValidation.cpp index 47b8f1684e..a7078dbbf4 100644 --- a/src/AppInstallerCommonCore/Manifest/ManifestSchemaValidation.cpp +++ b/src/AppInstallerCommonCore/Manifest/ManifestSchemaValidation.cpp @@ -16,18 +16,6 @@ namespace AppInstaller::Manifest::YamlParser namespace { - constexpr std::string_view YamlLanguageServerKey = "yaml-language-server"sv; - - struct ManifestSchemaHeader - { - std::string SchemaHeaderString; - std::string ManifestType; - std::string ManifestVersion; - std::string FileName; - size_t Line; - size_t column; - }; - enum class YamlScalarType { String, @@ -111,10 +99,10 @@ namespace AppInstaller::Manifest::YamlParser return result; } - std::vector ParseSchemaHeaderString(const ManifestSchemaHeader& manifestSchemaHeader, const ValidationError::Level& errorLevel, std::string& schemaHeaderUrlString) + std::vector ParseSchemaHeaderString(const YamlManifestInfo& manifestInfo, const ValidationError::Level& errorLevel, std::string& schemaHeaderUrlString) { std::vector errors; - std::string schemaHeader = manifestSchemaHeader.SchemaHeaderString; + std::string schemaHeader = manifestInfo.DocumentSchemaHeader.SchemaHeader; // Remove the leading '#' and any leading/trailing whitespaces if (schemaHeader[0] == '#') @@ -124,27 +112,26 @@ namespace AppInstaller::Manifest::YamlParser } // Parse the schema header string as YAML string to get the schema header URL - try { auto root = YAML::Load(schemaHeader); if (root.IsNull() || (!root.IsNull() && !root.IsDefined())) { - errors.emplace_back(ValidationError::MessageContextValueLineLevelWithFile(ManifestError::InvalidSchemaHeader, "", schemaHeader, manifestSchemaHeader.Line, manifestSchemaHeader.column, errorLevel, manifestSchemaHeader.FileName)); + errors.emplace_back(ValidationError::MessageContextValueLineLevelWithFile(ManifestError::InvalidSchemaHeader, "", schemaHeader, manifestInfo.DocumentSchemaHeader.Mark.line, manifestInfo.DocumentSchemaHeader.Mark.column, errorLevel, manifestInfo.FileName)); } else { - schemaHeaderUrlString = root[YamlLanguageServerKey].as(); + schemaHeaderUrlString = root[YAML::DocumentSchemaHeader::YamlLanguageServerKey].as(); } } catch (const YAML::Exception&) { - errors.emplace_back(ValidationError::MessageContextValueLineLevelWithFile(ManifestError::InvalidSchemaHeader, "", schemaHeader, manifestSchemaHeader.Line, manifestSchemaHeader.column, errorLevel, manifestSchemaHeader.FileName)); + errors.emplace_back(ValidationError::MessageContextValueLineLevelWithFile(ManifestError::InvalidSchemaHeader, "", schemaHeader, manifestInfo.DocumentSchemaHeader.Mark.line, manifestInfo.DocumentSchemaHeader.Mark.column, errorLevel, manifestInfo.FileName)); } catch (const std::exception&) { - errors.emplace_back(ValidationError::MessageContextValueLineLevelWithFile(ManifestError::InvalidSchemaHeader, "", schemaHeader, manifestSchemaHeader.Line, manifestSchemaHeader.column, errorLevel, manifestSchemaHeader.FileName)); + errors.emplace_back(ValidationError::MessageContextValueLineLevelWithFile(ManifestError::InvalidSchemaHeader, "", schemaHeader, manifestInfo.DocumentSchemaHeader.Mark.line, manifestInfo.DocumentSchemaHeader.Mark.column, errorLevel, manifestInfo.FileName)); } return errors; @@ -166,23 +153,29 @@ namespace AppInstaller::Manifest::YamlParser return false; } - std::vector ValidateManifestSchemaHeaderDetails(const ManifestSchemaHeader& schemaHeader, const ManifestTypeEnum& expectedManifestType, const ManifestVer& expectedManifestVersion, ValidationError::Level errorLevel) + std::vector ValidateSchemaHeaderType(const std::string& headerManifestType, const ManifestTypeEnum& expectedManifestType, const YamlManifestInfo& manifestInfo, ValidationError::Level errorLevel) { std::vector errors; - ManifestTypeEnum actualManifestType = ConvertToManifestTypeEnum(schemaHeader.ManifestType); - ManifestVer actualHeaderVersion(schemaHeader.ManifestVersion); - - size_t actualManifestTypeIndex = schemaHeader.SchemaHeaderString.find(schemaHeader.ManifestType) + 1; - size_t actualHeaderVersionIndex = schemaHeader.SchemaHeaderString.find(schemaHeader.ManifestVersion) + 1; + ManifestTypeEnum actualManifestType = ConvertToManifestTypeEnum(headerManifestType); + size_t schemaHeaderTypeIndex = manifestInfo.DocumentSchemaHeader.SchemaHeader.find(headerManifestType) + 1; if (actualManifestType != expectedManifestType) { - errors.emplace_back(ValidationError::MessageContextValueLineLevelWithFile(ManifestError::SchemaHeaderManifestTypeMismatch, "", schemaHeader.ManifestType, schemaHeader.Line, actualManifestTypeIndex, errorLevel, schemaHeader.FileName)); + errors.emplace_back(ValidationError::MessageContextValueLineLevelWithFile(ManifestError::SchemaHeaderManifestTypeMismatch, "", headerManifestType, manifestInfo.DocumentSchemaHeader.Mark.line, schemaHeaderTypeIndex, errorLevel, manifestInfo.FileName)); } + return errors; + } + + std::vector ValidateSchemaHeaderVersion(const std::string& headerManifestVersion, const ManifestVer& expectedManifestVersion, const YamlManifestInfo& manifestInfo, ValidationError::Level errorLevel) + { + std::vector errors; + ManifestVer actualHeaderVersion(headerManifestVersion); + size_t schemaHeaderVersionIndex = manifestInfo.DocumentSchemaHeader.SchemaHeader.find(headerManifestVersion) + 1; + if (actualHeaderVersion != expectedManifestVersion) { - errors.emplace_back(ValidationError::MessageContextValueLineLevelWithFile(ManifestError::SchemaHeaderManifestVersionMismatch, "", schemaHeader.ManifestVersion, schemaHeader.Line, actualHeaderVersionIndex, errorLevel, schemaHeader.FileName)); + errors.emplace_back(ValidationError::MessageContextValueLineLevelWithFile(ManifestError::SchemaHeaderManifestVersionMismatch, "", headerManifestVersion, manifestInfo.DocumentSchemaHeader.Mark.line, schemaHeaderVersionIndex, errorLevel, manifestInfo.FileName)); } return errors; @@ -200,8 +193,7 @@ namespace AppInstaller::Manifest::YamlParser // Prefix schema ID with "schema=" to match the schema header URL pattern and compare it with the schema header URL schemaId = "$schema=" + schemaId; - if (std::equal(schemaId.begin(), schemaId.end(), schemaHeaderUrlString.begin(), schemaHeaderUrlString.end(), - [](char a, char b) { return tolower(a) == tolower(b); })) + if (Utility::CaseInsensitiveEquals(schemaId, schemaHeaderUrlString)) { return true; } @@ -210,20 +202,20 @@ namespace AppInstaller::Manifest::YamlParser return false; } - ValidationError GetSchemaHeaderUrlPatternMismatchError(const std::string& schemaHeaderUrlString,const ManifestSchemaHeader& manifestSchemaHeader, const ValidationError::Level& errorLevel) + ValidationError GetSchemaHeaderUrlPatternMismatchError(const std::string& schemaHeaderUrlString, const YamlManifestInfo& manifestInfo, const ValidationError::Level& errorLevel) { - size_t schemaHeaderUrlIndex = manifestSchemaHeader.SchemaHeaderString.find(schemaHeaderUrlString) + 1; + size_t schemaHeaderUrlIndex = manifestInfo.DocumentSchemaHeader.SchemaHeader.find(schemaHeaderUrlString) + 1; - return ValidationError::MessageContextValueLineLevelWithFile(ManifestError::SchemaHeaderUrlPatternMismatch, "", manifestSchemaHeader.SchemaHeaderString, manifestSchemaHeader.Line, schemaHeaderUrlIndex, errorLevel, manifestSchemaHeader.FileName); + return ValidationError::MessageContextValueLineLevelWithFile(ManifestError::SchemaHeaderUrlPatternMismatch, "", manifestInfo.DocumentSchemaHeader.SchemaHeader, manifestInfo.DocumentSchemaHeader.Mark.line, schemaHeaderUrlIndex, errorLevel, manifestInfo.FileName); } - std::vector ValidateSchemaHeaderUrl(const YamlManifestInfo& manifestInfo, const ManifestVer& manifestVersion, ManifestSchemaHeader& manifestSchemaHeader, const ValidationError::Level& errorLevel) + std::vector ValidateSchemaHeaderUrl(const YamlManifestInfo& manifestInfo, const ManifestVer& manifestVersion, const ValidationError::Level& errorLevel) { std::vector errors; std::string schemaHeaderUrlString; // Parse the schema header string to get the schema header URL - auto parserErrors = ParseSchemaHeaderString(manifestSchemaHeader, errorLevel, schemaHeaderUrlString); + auto parserErrors = ParseSchemaHeaderString(manifestInfo, errorLevel, schemaHeaderUrlString); std::move(parserErrors.begin(), parserErrors.end(), std::inserter(errors, errors.end())); if (!errors.empty()) @@ -237,21 +229,21 @@ namespace AppInstaller::Manifest::YamlParser // Parse the schema header URL to get the manifest type and version if (ParseSchemaHeaderUrl(schemaHeaderUrlString, manifestTypeString, manifestVersionString)) { - manifestSchemaHeader.ManifestType = std::move(manifestTypeString); - manifestSchemaHeader.ManifestVersion = std::move(manifestVersionString); + auto headerManifestTypeErrors = ValidateSchemaHeaderType(manifestTypeString, manifestInfo.ManifestType, manifestInfo, errorLevel); + std::move(headerManifestTypeErrors.begin(), headerManifestTypeErrors.end(), std::inserter(errors, errors.end())); - auto compareErrors = ValidateManifestSchemaHeaderDetails(manifestSchemaHeader, manifestInfo.ManifestType, manifestVersion, errorLevel); - std::move(compareErrors.begin(), compareErrors.end(), std::inserter(errors, errors.end())); + auto headerManifestVersionErrors = ValidateSchemaHeaderVersion(manifestVersionString, manifestVersion, manifestInfo, errorLevel); + std::move(headerManifestVersionErrors.begin(), headerManifestVersionErrors.end(), std::inserter(errors, errors.end())); // Finally, match the entire schema header URL with the schema ID in the schema file to ensure the URL domain matches the schema definition file. if (!IsValidSchemaHeaderUrl(schemaHeaderUrlString, manifestInfo, manifestVersion)) { - errors.emplace_back(GetSchemaHeaderUrlPatternMismatchError(schemaHeaderUrlString, manifestSchemaHeader, errorLevel)); + errors.emplace_back(GetSchemaHeaderUrlPatternMismatchError(schemaHeaderUrlString, manifestInfo, errorLevel)); } } else { - errors.emplace_back(GetSchemaHeaderUrlPatternMismatchError(schemaHeaderUrlString, manifestSchemaHeader, errorLevel)); + errors.emplace_back(GetSchemaHeaderUrlPatternMismatchError(schemaHeaderUrlString, manifestInfo, errorLevel)); } return errors; @@ -262,19 +254,13 @@ namespace AppInstaller::Manifest::YamlParser std::vector errors; std::string schemaHeaderString; - ManifestSchemaHeader schemaHeader{}; - schemaHeader.FileName = manifestInfo.FileName; - schemaHeader.SchemaHeaderString = manifestInfo.SchemaHeader.SchemaHeaderString; - schemaHeader.Line = manifestInfo.SchemaHeader.Mark.line; - schemaHeader.column = manifestInfo.SchemaHeader.Mark.column; - - if (schemaHeader.SchemaHeaderString.empty()) + if (manifestInfo.DocumentSchemaHeader.SchemaHeader.empty()) { errors.emplace_back(ValidationError::MessageLevelWithFile(ManifestError::SchemaHeaderNotFound, errorLevel, manifestInfo.FileName)); return errors; } - auto parserErrors = ValidateSchemaHeaderUrl(manifestInfo, manifestVersion, schemaHeader, errorLevel); + auto parserErrors = ValidateSchemaHeaderUrl(manifestInfo, manifestVersion, errorLevel); std::move(parserErrors.begin(), parserErrors.end(), std::inserter(errors, errors.end())); return errors; diff --git a/src/AppInstallerCommonCore/Manifest/YamlParser.cpp b/src/AppInstallerCommonCore/Manifest/YamlParser.cpp index 328d5bc031..a9bfc0b4f3 100644 --- a/src/AppInstallerCommonCore/Manifest/YamlParser.cpp +++ b/src/AppInstallerCommonCore/Manifest/YamlParser.cpp @@ -527,9 +527,9 @@ namespace AppInstaller::Manifest::YamlParser THROW_HR_IF_MSG(HRESULT_FROM_WIN32(ERROR_DIRECTORY_NOT_SUPPORTED), std::filesystem::is_directory(file.path()), "Subdirectory not supported in manifest path"); YamlManifestInfo manifestInfo; - YAML::DocumentRootWithSchema doc = YAML::LoadDocument(file.path()); + YAML::Document doc = YAML::LoadDocument(file.path()); manifestInfo.Root = doc.GetRoot(); - manifestInfo.SchemaHeader = doc.GetSchemaHeader(); + manifestInfo.DocumentSchemaHeader = doc.GetSchemaHeader(); manifestInfo.FileName = file.path().filename().u8string(); docList.emplace_back(std::move(manifestInfo)); } @@ -537,9 +537,9 @@ namespace AppInstaller::Manifest::YamlParser else { YamlManifestInfo manifestInfo; - YAML::DocumentRootWithSchema doc = YAML::LoadDocument(inputPath, manifestInfo.StreamSha256); + YAML::Document doc = YAML::LoadDocument(inputPath, manifestInfo.StreamSha256); manifestInfo.Root = doc.GetRoot(); - manifestInfo.SchemaHeader = doc.GetSchemaHeader(); + manifestInfo.DocumentSchemaHeader = doc.GetSchemaHeader(); manifestInfo.FileName = inputPath.filename().u8string(); docList.emplace_back(std::move(manifestInfo)); } @@ -562,9 +562,9 @@ namespace AppInstaller::Manifest::YamlParser try { YamlManifestInfo manifestInfo; - YAML::DocumentRootWithSchema doc = YAML::LoadDocument(input); + YAML::Document doc = YAML::LoadDocument(input); manifestInfo.Root = doc.GetRoot(); - manifestInfo.SchemaHeader = doc.GetSchemaHeader(); + manifestInfo.DocumentSchemaHeader = doc.GetSchemaHeader(); docList.emplace_back(std::move(manifestInfo)); } catch (const std::exception& e) diff --git a/src/AppInstallerCommonCore/Public/winget/ManifestYamlParser.h b/src/AppInstallerCommonCore/Public/winget/ManifestYamlParser.h index 7062054ab9..509b3bf86b 100644 --- a/src/AppInstallerCommonCore/Public/winget/ManifestYamlParser.h +++ b/src/AppInstallerCommonCore/Public/winget/ManifestYamlParser.h @@ -18,7 +18,7 @@ namespace AppInstaller::Manifest::YamlParser std::string FileName; // Schema header string found in the manifest file - YAML::DocumentSchemaHeader SchemaHeader; + YAML::DocumentSchemaHeader DocumentSchemaHeader; // The SHA256 hash of the stream Utility::SHA256::HashBuffer StreamSha256; diff --git a/src/AppInstallerSharedLib/Public/winget/Yaml.h b/src/AppInstallerSharedLib/Public/winget/Yaml.h index 17556efe57..585cda459e 100644 --- a/src/AppInstallerSharedLib/Public/winget/Yaml.h +++ b/src/AppInstallerSharedLib/Public/winget/Yaml.h @@ -241,19 +241,21 @@ namespace AppInstaller::YAML struct DocumentSchemaHeader { DocumentSchemaHeader() = default; - DocumentSchemaHeader(std::string schemaHeaderString, const Mark& mark) : SchemaHeaderString(std::move(schemaHeaderString)), Mark(mark) {} + DocumentSchemaHeader(std::string schemaHeaderString, const Mark& mark) : SchemaHeader(std::move(schemaHeaderString)), Mark(mark) {} - std::string SchemaHeaderString; + std::string SchemaHeader; Mark Mark; + static constexpr std::string_view YamlLanguageServerKey = "yaml-language-server"; }; - struct DocumentRootWithSchema + struct Document { - DocumentRootWithSchema() = default; - DocumentRootWithSchema(Node root, DocumentSchemaHeader schemaHeader) : m_root(std::move(root)), m_schemaHeader(std::move(schemaHeader)) {} + Document() = default; + Document(Node root, DocumentSchemaHeader schemaHeader) : m_root(std::move(root)), m_schemaHeader(std::move(schemaHeader)) {} + // Return r-values for move semantics Node GetRoot() const { return m_root; } - const DocumentSchemaHeader& GetSchemaHeader() const { return m_schemaHeader; } + DocumentSchemaHeader GetSchemaHeader() const { return m_schemaHeader; } private: Node m_root; @@ -267,10 +269,10 @@ namespace AppInstaller::YAML } // Loads from the input; returns the root node of the first document. - DocumentRootWithSchema LoadDocument(std::string_view input); - DocumentRootWithSchema LoadDocument(const std::string& input); - DocumentRootWithSchema LoadDocument(const std::filesystem::path& input); - DocumentRootWithSchema LoadDocument(const std::filesystem::path& input, Utility::SHA256::HashBuffer& hashOut); + Document LoadDocument(std::string_view input); + Document LoadDocument(const std::string& input); + Document LoadDocument(const std::filesystem::path& input); + Document LoadDocument(const std::filesystem::path& input, Utility::SHA256::HashBuffer& hashOut); // A YAML emitter. struct Emitter diff --git a/src/AppInstallerSharedLib/Yaml.cpp b/src/AppInstallerSharedLib/Yaml.cpp index e5c883bd6b..1326256ac9 100644 --- a/src/AppInstallerSharedLib/Yaml.cpp +++ b/src/AppInstallerSharedLib/Yaml.cpp @@ -99,6 +99,35 @@ namespace AppInstaller::YAML return Node::TagType::Unknown; } + + DocumentSchemaHeader ExtractSchemaHeaderFromYaml( const std::string& yamlDocument, size_t rootNodeLine) + { + std::istringstream input(yamlDocument); + std::string line; + size_t currentLine = 1; + + // Search for the schema header string in the comments before the root node. + while (currentLine < rootNodeLine && std::getline(input, line)) + { + std::string comment = Utility::Trim(line); + + // Check if the line is a comment + if (!comment.empty() && comment[0] == '#') + { + size_t pos = line.find(DocumentSchemaHeader::YamlLanguageServerKey); + + // Check if the comment contains the schema header string + if (pos != std::string::npos) + { + return DocumentSchemaHeader(std::move(comment), YAML::Mark{ currentLine, pos}); + } + } + + currentLine++; + } + + return {}; + } } Exception::Exception(Type type) : @@ -601,14 +630,17 @@ namespace AppInstaller::YAML return Load(input, &hashOut); } - DocumentRootWithSchema LoadDocument(std::string_view input) + Document LoadDocument(std::string_view input) { Wrapper::Parser parser(input); Wrapper::Document document = parser.Load(); if (document.HasRoot()) { - return { document.GetRoot(), document.GetSchemaHeader() }; + const Node root = document.GetRoot(); + const DocumentSchemaHeader schemaHeader = ExtractSchemaHeaderFromYaml(parser.GetEncodedInput(), root.Mark().line); + + return { root, schemaHeader }; } else { @@ -617,19 +649,22 @@ namespace AppInstaller::YAML } } - DocumentRootWithSchema LoadDocument(const std::string& input) + Document LoadDocument(const std::string& input) { return LoadDocument(static_cast(input)); } - DocumentRootWithSchema LoadDocument(std::istream& input, Utility::SHA256::HashBuffer* hashOut) + Document LoadDocument(std::istream& input, Utility::SHA256::HashBuffer* hashOut) { Wrapper::Parser parser(input, hashOut); Wrapper::Document document = parser.Load(); if (document.HasRoot()) { - return { document.GetRoot(), document.GetSchemaHeader() }; + const Node root = document.GetRoot(); + const DocumentSchemaHeader schemaHeader = ExtractSchemaHeaderFromYaml(parser.GetEncodedInput(), root.Mark().line); + + return { root, schemaHeader }; } else { @@ -638,19 +673,19 @@ namespace AppInstaller::YAML } } - DocumentRootWithSchema LoadDocument(const std::filesystem::path& input, Utility::SHA256::HashBuffer* hashOut) + Document LoadDocument(const std::filesystem::path& input, Utility::SHA256::HashBuffer* hashOut) { std::ifstream stream(input, std::ios_base::in | std::ios_base::binary); THROW_LAST_ERROR_IF(stream.fail()); return LoadDocument(stream, hashOut); } - DocumentRootWithSchema LoadDocument(const std::filesystem::path& input) + Document LoadDocument(const std::filesystem::path& input) { return LoadDocument(input, nullptr); } - DocumentRootWithSchema LoadDocument(const std::filesystem::path& input, Utility::SHA256::HashBuffer& hashOut) + Document LoadDocument(const std::filesystem::path& input, Utility::SHA256::HashBuffer& hashOut) { return LoadDocument(input, &hashOut); } diff --git a/src/AppInstallerSharedLib/YamlWrapper.cpp b/src/AppInstallerSharedLib/YamlWrapper.cpp index b6d1f854b7..a9664b0e0b 100644 --- a/src/AppInstallerSharedLib/YamlWrapper.cpp +++ b/src/AppInstallerSharedLib/YamlWrapper.cpp @@ -258,21 +258,6 @@ namespace AppInstaller::YAML::Wrapper } } - void Document::AddSchemaHeader(const DocumentSchemaHeader& schemaHeader) - { - m_schemaHeader = schemaHeader; - } - - bool Document::HasSchemaHeader() - { - return !m_schemaHeader.SchemaHeaderString.empty(); - } - - DocumentSchemaHeader Document::GetSchemaHeader() - { - return m_schemaHeader; - } - yaml_node_t* Document::GetNode(yaml_node_item_t index) { yaml_node_t* result = yaml_document_get_node(&m_document, index); @@ -334,12 +319,6 @@ namespace AppInstaller::YAML::Wrapper } } - DocumentSchemaHeader schemaHeader; - if (result.HasRoot() && FindManifestSchemaHeaderString(result.GetRoot().Mark().line, schemaHeader)) - { - result.AddSchemaHeader(schemaHeader); - } - return result; } @@ -408,37 +387,6 @@ namespace AppInstaller::YAML::Wrapper yaml_parser_set_encoding(&m_parser, YAML_UTF8_ENCODING); } - bool Parser::FindManifestSchemaHeaderString(size_t rootNodeLine, DocumentSchemaHeader& schemaHeader) - { - std::istringstream input(m_input); - std::string line; - size_t currentLine = 1; - - // Search for the schema header string in the comments before the root node. - while (currentLine < rootNodeLine && std::getline(input, line)) - { - std::string comment = Utility::Trim(line); - size_t pos = line.find(YamlLanguageServerKey); - - // Check if the line is a comment - if (!comment.empty() && comment[0] == '#') - { - // Check if the comment contains the schema header string - if (pos != std::string::npos) - { - schemaHeader.SchemaHeaderString = std::move(comment); - schemaHeader.Mark = { currentLine, pos }; - - return true; - } - } - - currentLine++; - } - - return false; - } - Event::~Event() { if (m_token) diff --git a/src/AppInstallerSharedLib/YamlWrapper.h b/src/AppInstallerSharedLib/YamlWrapper.h index 48fd903085..6fdc5f9f7d 100644 --- a/src/AppInstallerSharedLib/YamlWrapper.h +++ b/src/AppInstallerSharedLib/YamlWrapper.h @@ -55,22 +55,12 @@ namespace AppInstaller::YAML::Wrapper // Adds a pair to the mapping. void AppendMappingPair(int mapping, int key, int value); - // Adds a schema header to the document. - void AddSchemaHeader(const DocumentSchemaHeader& schemaHeader); - - // Determines whether the document has a schema header. - bool HasSchemaHeader(); - - // Gets the schema header string. - DocumentSchemaHeader GetSchemaHeader(); - private: // Gets the node referenced by the index. yaml_node_t* GetNode(yaml_node_item_t index); DestructionToken m_token; yaml_document_t m_document; - DocumentSchemaHeader m_schemaHeader = {}; }; // A libyaml yaml_parser_t. @@ -93,15 +83,16 @@ namespace AppInstaller::YAML::Wrapper // Loads the next document from the input, if one exists. Document Load(); + // Retrieves the input that was used to create the parser with the correct encoding scheme. + const std::string& GetEncodedInput() const { return m_input; } + private: // Determines the type of encoding in use, transforming the input as necessary. void PrepareInput(); - bool FindManifestSchemaHeaderString(size_t rootNodeLine, DocumentSchemaHeader& schemaHeader); DestructionToken m_token; yaml_parser_t m_parser; std::string m_input; - static constexpr std::string_view YamlLanguageServerKey = "yaml-language-server"; }; // A libyaml yaml_event_t. From db264b6352aa47bf335b42fb1e8af4866b71285e Mon Sep 17 00:00:00 2001 From: Madhusudhan Gumbalapura Sudarshan Date: Fri, 17 Jan 2025 11:59:48 -0800 Subject: [PATCH 18/18] Optimize YamlParser with move semantics - Improved the efficiency of the `YamlParser` by utilizing move semantics for the `Document` class in `Yaml.h`. - Updated `YamlParser.cpp` to call `GetRoot` with `std::move(doc).GetRoot()` to avoid unnecessary copying. - Modified `GetRoot` in `Yaml.h` to return an r-value reference (`Node&&`). - Also, updated `GetSchemaHeader` to return a constant reference (`const DocumentSchemaHeader&`) to reduce copying. --- src/AppInstallerCommonCore/Manifest/YamlParser.cpp | 6 +++--- src/AppInstallerSharedLib/Public/winget/Yaml.h | 5 +++-- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/src/AppInstallerCommonCore/Manifest/YamlParser.cpp b/src/AppInstallerCommonCore/Manifest/YamlParser.cpp index a9bfc0b4f3..86bbd23d84 100644 --- a/src/AppInstallerCommonCore/Manifest/YamlParser.cpp +++ b/src/AppInstallerCommonCore/Manifest/YamlParser.cpp @@ -528,7 +528,7 @@ namespace AppInstaller::Manifest::YamlParser YamlManifestInfo manifestInfo; YAML::Document doc = YAML::LoadDocument(file.path()); - manifestInfo.Root = doc.GetRoot(); + manifestInfo.Root = std::move(doc).GetRoot(); manifestInfo.DocumentSchemaHeader = doc.GetSchemaHeader(); manifestInfo.FileName = file.path().filename().u8string(); docList.emplace_back(std::move(manifestInfo)); @@ -538,7 +538,7 @@ namespace AppInstaller::Manifest::YamlParser { YamlManifestInfo manifestInfo; YAML::Document doc = YAML::LoadDocument(inputPath, manifestInfo.StreamSha256); - manifestInfo.Root = doc.GetRoot(); + manifestInfo.Root = std::move(doc).GetRoot(); manifestInfo.DocumentSchemaHeader = doc.GetSchemaHeader(); manifestInfo.FileName = inputPath.filename().u8string(); docList.emplace_back(std::move(manifestInfo)); @@ -563,7 +563,7 @@ namespace AppInstaller::Manifest::YamlParser { YamlManifestInfo manifestInfo; YAML::Document doc = YAML::LoadDocument(input); - manifestInfo.Root = doc.GetRoot(); + manifestInfo.Root = std::move(doc).GetRoot(); manifestInfo.DocumentSchemaHeader = doc.GetSchemaHeader(); docList.emplace_back(std::move(manifestInfo)); } diff --git a/src/AppInstallerSharedLib/Public/winget/Yaml.h b/src/AppInstallerSharedLib/Public/winget/Yaml.h index 585cda459e..2e4521430b 100644 --- a/src/AppInstallerSharedLib/Public/winget/Yaml.h +++ b/src/AppInstallerSharedLib/Public/winget/Yaml.h @@ -253,9 +253,10 @@ namespace AppInstaller::YAML Document() = default; Document(Node root, DocumentSchemaHeader schemaHeader) : m_root(std::move(root)), m_schemaHeader(std::move(schemaHeader)) {} + const DocumentSchemaHeader& GetSchemaHeader() const { return m_schemaHeader; } + // Return r-values for move semantics - Node GetRoot() const { return m_root; } - DocumentSchemaHeader GetSchemaHeader() const { return m_schemaHeader; } + Node&& GetRoot() && { return std::move(m_root); } private: Node m_root;