Repository navigation
fix: accept YAML block-scalar trailing line breaks in schema patterns - #6596
lsmlhi_25 (FOWEPJF255) wants to merge 2 commits into
Conversation
|
Azure Pipelines: There may be pipelines that require an authorized user to comment /azp run to run. |
This comment has been minimized.
This comment has been minimized.
Kaleb Luedtke (Trenly)
left a comment
Kaleb Luedtke (Trenly)
left a comment
There was a problem hiding this comment.
The upstream PR defines an option to simply recert to multiline matching, why not use that? Or, where the regex check actually takes place, why not specify rhe multiline option?
It feels to me like mutating the strings for each individual occurence would not lead to consistent behaviors
| Installers: | ||
| - Architecture: x86 | ||
| InstallerUrl: > | ||
| https://rubengustorage.blob.core.windows.net/publiccontainer/msixsdk-x86.msi |
There was a problem hiding this comment.
I don’t think this should be using a hardcoded live URL - Don’t other tests use placeholders that get resolved at test runtime?
There was a problem hiding this comment.
Updated in ff8a491. This fixture only exercises manifest parsing and schema validation and never downloads the URL, so I replaced the live endpoint with the repository's established https://ThisIsNotUsed test sentinel.
| { | ||
| { "Arch", [](const YAML::Node& value, const VariantManifestPtr& v)->ValidationErrors { variant_ptr<ManifestInstaller>(v)->Arch = Utility::ConvertToArchitectureEnum(value.as<std::string>()); return {}; } }, | ||
| { "Url", [](const YAML::Node& value, const VariantManifestPtr& v)->ValidationErrors { variant_ptr<ManifestInstaller>(v)->Url = value.as<std::string>(); return {}; } }, | ||
| { "Url", [](const YAML::Node& value, const VariantManifestPtr& v)->ValidationErrors { variant_ptr<ManifestInstaller>(v)->Url = Utility::Trim(value.as<std::string>()); return {}; } }, |
There was a problem hiding this comment.
Applying the trim only to specific fields seems prone to failure. What happens when a new pattern field is added in the future ? The user would have to remember to add the trim?
There was a problem hiding this comment.
Updated in ff8a491. I removed the field-specific trimming and configured the shared Valijson ValidatorT with a multiline std::regex engine instead. This applies the anchor behavior centrally to every schema pattern, including future fields, while preserving the original manifest scalar value; the regression assertion now checks that the trailing newline remains.
📖 Description
YAML block scalars that use clip chomping (
>/|) keep one trailing line break. Schemapatterns such asInstallerUrl(^([Hh][Tt][Tt][Pp][Ss]?)://.+$) no longer match that break after the MSVCstd::regexECMAScript change, so previously valid manifests fail schema validation.This change configures the shared Valijson regex engine with the ECMAScript
multilineoption. Schema anchors therefore retain their earlier behavior for all current and future pattern fields, while manifest values remain unchanged. The regression test verifies both successful validation and preservation of the YAML block scalar's trailing newline.🔗 References
Resolves #6592
🔍 Validation
Manifest-Good-InstallerUrl-BlockScalar.yamlandReadGoodManifestInstallerUrlBlockScalar, and included the fixture inReadGoodManifests.https://ThisIsNotUsedsentinel.git diff --check origin/master...HEADpassed.AppInstallerCLITestsbuild was attempted with MSBuild, but this machine does not have the Visual C++ targets (Microsoft.Cpp.Default.props) installed, so compilation and the Catch2 test could not run locally. GitHub CI is the build/test authority for this update.✅ Checklist
🤖 AI Assistance
AI assistance helped investigate Valijson's custom regex engine, draft the centralized multiline-regex fix and regression test, and update this description. I reviewed the implementation against issue #6592, the repository's contribution guidance, and the Valijson v1.0.4 interface before submission.
📋 Issue Type
Microsoft Reviewers: Open in CodeFlow