commit 457f84bf5ac7947a12c8b65879001cce1cdbfc43 parent 2b185be5192229b17e4fa8fd32c39d3c606a8d7d Author: JohnMcPMS <johnmcp@microsoft.com> Date: Wed, 1 May 2024 14:47:40 -0700 Block control codes and truncate longer configuration text blocks (#4436) ## Change All control codes in the range [0x0, 0x20) and the DELETE control code 0x7F (with the exceptions of the tab, line feed, and carriage return characters) will result in an error from the YAML parser. An alternative solution is to convert them to their control pictures, but it was decided that it was best to fail at this time. The configuration output for each unit during the "show" portion (used by almost all of the commands to display details about the file) will limit the amount of output allowed for any field that comes from an external source. Data from the file will present a warning that it was truncated just below its output. If anything is truncated, an overall "error" will be output as well. Diffstat:
20 files changed, 815 insertions(+), 410 deletions(-)
diff --git a/.github/actions/spelling/allow.txt b/.github/actions/spelling/allow.txt @@ -349,6 +349,7 @@ und UNICODESTRING uninstalling Unmarshal +unskipped untimes updatefile updatemanifest diff --git a/.github/actions/spelling/expect.txt b/.github/actions/spelling/expect.txt @@ -116,6 +116,7 @@ ECustom EFGH EFile efileresource +EMalicious endregion ENDSESSION EPester diff --git a/src/AppInstallerCLICore/Resources.h b/src/AppInstallerCLICore/Resources.h @@ -128,6 +128,8 @@ namespace AppInstaller::CLI::Resource WINGET_DEFINE_RESOURCE_STRINGID(ConfigurationWarning); WINGET_DEFINE_RESOURCE_STRINGID(ConfigurationWarningPromptApply); WINGET_DEFINE_RESOURCE_STRINGID(ConfigurationWarningPromptTest); + WINGET_DEFINE_RESOURCE_STRINGID(ConfigurationWarningSetViewTruncated); + WINGET_DEFINE_RESOURCE_STRINGID(ConfigurationWarningValueTruncated); WINGET_DEFINE_RESOURCE_STRINGID(ConfigureCommandLongDescription); WINGET_DEFINE_RESOURCE_STRINGID(ConfigureCommandShortDescription); WINGET_DEFINE_RESOURCE_STRINGID(ConfigureShowCommandLongDescription); diff --git a/src/AppInstallerCLICore/Workflows/ConfigurationFlow.cpp b/src/AppInstallerCLICore/Workflows/ConfigurationFlow.cpp @@ -106,7 +106,7 @@ namespace AppInstaller::CLI::Workflow return factory; } - std::optional<Utility::LocIndString> GetValueSetString(const ValueSet& valueSet, std::wstring_view value) + winrt::hstring GetValueSetString(const ValueSet& valueSet, std::wstring_view value) { if (valueSet.HasKey(value)) { @@ -114,7 +114,7 @@ namespace AppInstaller::CLI::Workflow IPropertyValue property = object.try_as<IPropertyValue>(); if (property && property.Type() == PropertyType::String) { - return Utility::LocIndString{ Utility::ConvertToUTF8(property.GetString()) }; + return property.GetString(); } } @@ -136,259 +136,344 @@ namespace AppInstaller::CLI::Workflow return {}; } - void OutputPropertyValue(OutputStream& out, const IPropertyValue property) + // Contains the output functions and tracks whether any fields needed to be truncated. + struct OutputHelper { - switch (property.Type()) + OutputHelper(Execution::Context& context) : m_context(context) {} + + size_t ValuesTruncated = 0; + + // Converts a string from the configuration API surface for output. + // All strings coming from the API are external data and not localizable by us. + Utility::LocIndString ConvertForOutput(const std::string& input, size_t maxLines) { - case PropertyType::String: - out << ' ' << Utility::ConvertToUTF8(property.GetString()) << '\n'; - break; - case PropertyType::Boolean: - out << ' ' << (property.GetBoolean() ? Utility::LocIndView("true") : Utility::LocIndView("false")) << '\n'; - break; - case PropertyType::Int64: - out << ' ' << property.GetInt64() << '\n'; - break; - default: - out << " [Debug:PropertyType="_liv << property.Type() << "]\n"_liv; - break; + bool truncated = false; + auto lines = Utility::SplitIntoLines(input); + + if (maxLines == 1 && lines.size() > 1) + { + // If the limit was one line, don't allow line breaks but do allow a second line of overflow + lines.resize(1); + maxLines = 2; + truncated = true; + } + + if (Utility::LimitOutputLines(lines, GetConsoleWidth(), maxLines)) + { + truncated = true; + } + + if (truncated) + { + ++ValuesTruncated; + } + + return Utility::LocIndString{ Utility::Join("\n", lines) }; } - } - void OutputValueSet(OutputStream& out, const ValueSet& valueSet, size_t indent); + Utility::LocIndString ConvertForOutput(const winrt::hstring& input, size_t maxLines) + { + return ConvertForOutput(Utility::ConvertToUTF8(input), maxLines); + } - void OutputValueSetAsArray(OutputStream& out, const ValueSet& valueSetArray, size_t indent) - { - Utility::LocIndString indentString{ std::string(indent, ' ') }; + Utility::LocIndString ConvertIdentifier(const winrt::hstring& input) + { + return ConvertForOutput(input, 1); + } + + Utility::LocIndString ConvertURI(const winrt::hstring& input) + { + return ConvertForOutput(input, 1); + } - std::vector<std::pair<int, winrt::Windows::Foundation::IInspectable>> arrayValues; - for (const auto& arrayValue : valueSetArray) + Utility::LocIndString ConvertValue(const winrt::hstring& input) { - if (arrayValue.Key() != L"treatAsArray") + return ConvertForOutput(input, 5); + } + + Utility::LocIndString ConvertDetailsIdentifier(const winrt::hstring& input) + { + return ConvertForOutput(Utility::ConvertControlCodesToPictures(Utility::ConvertToUTF8(input)), 1); + } + + Utility::LocIndString ConvertDetailsURI(const winrt::hstring& input) + { + return ConvertForOutput(Utility::ConvertControlCodesToPictures(Utility::ConvertToUTF8(input)), 1); + } + + Utility::LocIndString ConvertDetailsValue(const winrt::hstring& input) + { + return ConvertForOutput(Utility::ConvertControlCodesToPictures(Utility::ConvertToUTF8(input)), 5); + } + + void OutputValueWithTruncationWarningIfNeeded(const winrt::hstring& input) + { + size_t truncatedBefore = ValuesTruncated; + m_context.Reporter.Info() << ConvertValue(input) << '\n'; + + if (ValuesTruncated > truncatedBefore) { - arrayValues.emplace_back(std::make_pair(std::stoi(arrayValue.Key().c_str()), arrayValue.Value())); + m_context.Reporter.Warn() << Resource::String::ConfigurationWarningValueTruncated << std::endl; } } - std::sort( - arrayValues.begin(), - arrayValues.end(), - [](const std::pair<int, winrt::Windows::Foundation::IInspectable>& a, const std::pair<int, winrt::Windows::Foundation::IInspectable>& b) + void OutputPropertyValue(const IPropertyValue property) + { + switch (property.Type()) { - return a.first < b.first; - }); + case PropertyType::String: + m_context.Reporter.Info() << ' '; + OutputValueWithTruncationWarningIfNeeded(property.GetString()); + break; + case PropertyType::Boolean: + m_context.Reporter.Info() << ' ' << (property.GetBoolean() ? Utility::LocIndView("true") : Utility::LocIndView("false")) << '\n'; + break; + case PropertyType::Int64: + m_context.Reporter.Info() << ' ' << property.GetInt64() << '\n'; + break; + default: + m_context.Reporter.Info() << " [Debug:PropertyType="_liv << property.Type() << "]\n"_liv; + break; + } + } - for (const auto& arrayValue : arrayValues) + void OutputValueSetAsArray(const ValueSet& valueSetArray, size_t indent) { - auto arrayObject = arrayValue.second; - IPropertyValue arrayProperty = arrayObject.try_as<IPropertyValue>(); + Utility::LocIndString indentString{ std::string(indent, ' ') }; - out << indentString << "-"; - if (arrayProperty) + std::vector<std::pair<int, winrt::Windows::Foundation::IInspectable>> arrayValues; + for (const auto& arrayValue : valueSetArray) { - OutputPropertyValue(out, arrayProperty); + if (arrayValue.Key() != L"treatAsArray") + { + arrayValues.emplace_back(std::make_pair(std::stoi(arrayValue.Key().c_str()), arrayValue.Value())); + } } - else - { - ValueSet arraySubset = arrayObject.as<ValueSet>(); - auto size = arraySubset.Size(); - if (size > 0) + + std::sort( + arrayValues.begin(), + arrayValues.end(), + [](const std::pair<int, winrt::Windows::Foundation::IInspectable>& a, const std::pair<int, winrt::Windows::Foundation::IInspectable>& b) { - // First one is special. - auto first = arraySubset.First().Current(); - out << ' ' << Utility::ConvertToUTF8(first.Key()) << ':'; - - auto object = first.Value(); - IPropertyValue property = object.try_as<IPropertyValue>(); - if (property) - { - OutputPropertyValue(out, property); - } - else - { - // If not an IPropertyValue, it must be a ValueSet - ValueSet subset = object.as<ValueSet>(); - out << '\n'; - OutputValueSet(out, subset, indent + 4); - } + return a.first < b.first; + }); + + for (const auto& arrayValue : arrayValues) + { + auto arrayObject = arrayValue.second; + IPropertyValue arrayProperty = arrayObject.try_as<IPropertyValue>(); - if (size > 1) + m_context.Reporter.Info() << indentString << "-"; + if (arrayProperty) + { + OutputPropertyValue(arrayProperty); + } + else + { + ValueSet arraySubset = arrayObject.as<ValueSet>(); + auto size = arraySubset.Size(); + if (size > 0) { - arraySubset.Remove(first.Key()); - OutputValueSet(out, arraySubset, indent + 2); - arraySubset.Insert(first.Key(), first.Value()); + // First one is special. + auto first = arraySubset.First().Current(); + m_context.Reporter.Info() << ' ' << ConvertIdentifier(first.Key()) << ':'; + + auto object = first.Value(); + IPropertyValue property = object.try_as<IPropertyValue>(); + if (property) + { + OutputPropertyValue(property); + } + else + { + // If not an IPropertyValue, it must be a ValueSet + ValueSet subset = object.as<ValueSet>(); + m_context.Reporter.Info() << '\n'; + OutputValueSet(subset, indent + 4); + } + + if (size > 1) + { + arraySubset.Remove(first.Key()); + OutputValueSet(arraySubset, indent + 2); + arraySubset.Insert(first.Key(), first.Value()); + } } } } } - } - - void OutputValueSet(OutputStream& out, const ValueSet& valueSet, size_t indent) - { - Utility::LocIndString indentString{ std::string(indent, ' ') }; - for (const auto& value : valueSet) + void OutputValueSet(const ValueSet& valueSet, size_t indent) { - out << indentString << Utility::ConvertToUTF8(value.Key()) << ':'; + Utility::LocIndString indentString{ std::string(indent, ' ') }; - auto object = value.Value(); - - IPropertyValue property = object.try_as<IPropertyValue>(); - if (property) + for (const auto& value : valueSet) { - OutputPropertyValue(out, property); - } - else - { - // If not an IPropertyValue, it must be a ValueSet - ValueSet subset = object.as<ValueSet>(); - out << '\n'; - if (subset.HasKey(L"treatAsArray")) + m_context.Reporter.Info() << indentString << ConvertIdentifier(value.Key()) << ':'; + + auto object = value.Value(); + + IPropertyValue property = object.try_as<IPropertyValue>(); + if (property) { - OutputValueSetAsArray(out, subset, indent + 2); + OutputPropertyValue(property); } else { - OutputValueSet(out, subset, indent + 2); + // If not an IPropertyValue, it must be a ValueSet + ValueSet subset = object.as<ValueSet>(); + m_context.Reporter.Info() << '\n'; + if (subset.HasKey(L"treatAsArray")) + { + OutputValueSetAsArray(subset, indent + 2); + } + else + { + OutputValueSet(subset, indent + 2); + } } } } - } - - // Converts a string from the configuration API surface for output. - // All strings coming from the API are external data and not localizable by us. - Utility::LocIndString ConvertForOutput(const winrt::hstring& input) - { - return Utility::LocIndString{ Utility::ConvertToUTF8(input) }; - } - void OutputConfigurationUnitHeader(OutputStream& out, const ConfigurationUnit& unit, const winrt::hstring& name) - { - out << ConfigurationIntentEmphasis << ToResource(unit.Intent()) << " :: "_liv << ConfigurationUnitEmphasis << ConvertForOutput(name); - - winrt::hstring identifier = unit.Identifier(); - if (!identifier.empty()) + void OutputConfigurationUnitHeader(const ConfigurationUnit& unit, const winrt::hstring& name) { - out << " ["_liv << ConvertForOutput(identifier) << ']'; - } + m_context.Reporter.Info() << ConfigurationIntentEmphasis << ToResource(unit.Intent()) << " :: "_liv << ConfigurationUnitEmphasis << ConvertIdentifier(name); - out << '\n'; - } + winrt::hstring identifier = unit.Identifier(); + if (!identifier.empty()) + { + m_context.Reporter.Info() << " ["_liv << ConvertIdentifier(identifier) << ']'; + } - void OutputConfigurationUnitInformation(OutputStream& out, const ConfigurationUnit& unit) - { - IConfigurationUnitProcessorDetails details = unit.Details(); - ValueSet metadata = unit.Metadata(); + m_context.Reporter.Info() << '\n'; + } - if (details) + void OutputConfigurationUnitInformation(const ConfigurationUnit& unit) { - // -- Sample output when IConfigurationUnitProcessorDetails present -- - // Intent :: UnitType <from details> [Identifier] - // UnitDocumentationUri <if present> - // Description <from details first, directives second> - // "Module": ModuleName "by" Author / Publisher (IsLocal / ModuleSource) - // "Signed by": SigningCertificateChain (leaf subject CN) - // PublishedModuleUri / ModuleDocumentationUri <if present> - // ModuleDescription - OutputConfigurationUnitHeader(out, unit, details.UnitType()); - - auto unitDocumentationUri = details.UnitDocumentationUri(); - if (unitDocumentationUri) - { - out << " "_liv << ConvertForOutput(unitDocumentationUri.DisplayUri()) << '\n'; - } + IConfigurationUnitProcessorDetails details = unit.Details(); + ValueSet metadata = unit.Metadata(); - winrt::hstring unitDescriptionFromDetails = details.UnitDescription(); - if (!unitDescriptionFromDetails.empty()) + if (details) { - out << " "_liv << ConvertForOutput(unitDescriptionFromDetails) << '\n'; - } - else - { - auto unitDescriptionFromDirectives = GetValueSetString(metadata, s_Directive_Description); - if (unitDescriptionFromDirectives && !unitDescriptionFromDirectives.value().empty()) + // -- Sample output when IConfigurationUnitProcessorDetails present -- + // Intent :: UnitType <from details> [Identifier] + // UnitDocumentationUri <if present> + // Description <from details first, directives second> + // "Module": ModuleName "by" Author / Publisher (IsLocal / ModuleSource) + // "Signed by": SigningCertificateChain (leaf subject CN) + // PublishedModuleUri / ModuleDocumentationUri <if present> + // ModuleDescription + OutputConfigurationUnitHeader(unit, details.UnitType()); + + auto unitDocumentationUri = details.UnitDocumentationUri(); + if (unitDocumentationUri) { - out << " "_liv << unitDescriptionFromDirectives.value() << '\n'; + m_context.Reporter.Info() << " "_liv << ConvertDetailsURI(unitDocumentationUri.DisplayUri()) << '\n'; } - } - auto author = ConvertForOutput(details.Author()); - if (author.empty()) - { - author = ConvertForOutput(details.Publisher()); - } - if (details.IsLocal()) - { - out << " "_liv << Resource::String::ConfigurationModuleWithDetails(ConvertForOutput(details.ModuleName()), author, Resource::String::ConfigurationLocal) << '\n'; - } - else - { - out << " "_liv << Resource::String::ConfigurationModuleWithDetails(ConvertForOutput(details.ModuleName()), author, ConvertForOutput(details.ModuleSource())) << '\n'; - } + winrt::hstring unitDescriptionFromDetails = details.UnitDescription(); + if (!unitDescriptionFromDetails.empty()) + { + m_context.Reporter.Info() << " "_liv << ConvertDetailsValue(unitDescriptionFromDetails) << '\n'; + } + else + { + auto unitDescriptionFromDirectives = GetValueSetString(metadata, s_Directive_Description); + if (!unitDescriptionFromDirectives.empty()) + { + m_context.Reporter.Info() << " "_liv; + OutputValueWithTruncationWarningIfNeeded(unitDescriptionFromDirectives); + } + } + + auto author = ConvertDetailsIdentifier(details.Author()); + if (author.empty()) + { + author = ConvertDetailsIdentifier(details.Publisher()); + } + if (details.IsLocal()) + { + m_context.Reporter.Info() << " "_liv << Resource::String::ConfigurationModuleWithDetails(ConvertDetailsIdentifier(details.ModuleName()), author, Resource::String::ConfigurationLocal) << '\n'; + } + else + { + m_context.Reporter.Info() << " "_liv << Resource::String::ConfigurationModuleWithDetails(ConvertDetailsIdentifier(details.ModuleName()), author, ConvertDetailsIdentifier(details.ModuleSource())) << '\n'; + } - // TODO: Currently the signature information is only for the top files. Maybe each item should be tagged? - // TODO: Output signing information with additional details (like whether the certificate is trusted). Doing this with the validate command - // seems like a good time, as that will also need to do the check in order to inform the user on the validation. - // Just saying "Signed By: Foo" is going to lead to a false sense of trust if the signature is valid but not actually trusted. + // TODO: Currently the signature information is only for the top files. Maybe each item should be tagged? + // TODO: Output signing information with additional details (like whether the certificate is trusted). Doing this with the validate command + // seems like a good time, as that will also need to do the check in order to inform the user on the validation. + // Just saying "Signed By: Foo" is going to lead to a false sense of trust if the signature is valid but not actually trusted. - auto moduleUri = details.PublishedModuleUri(); - if (!moduleUri) - { - moduleUri = details.ModuleDocumentationUri(); - } - if (moduleUri) - { - out << " "_liv << ConvertForOutput(moduleUri.DisplayUri()) << '\n'; - } + auto moduleUri = details.PublishedModuleUri(); + if (!moduleUri) + { + moduleUri = details.ModuleDocumentationUri(); + } + if (moduleUri) + { + m_context.Reporter.Info() << " "_liv << ConvertDetailsURI(moduleUri.DisplayUri()) << '\n'; + } - winrt::hstring moduleDescription = details.ModuleDescription(); - if (!moduleDescription.empty()) - { - out << " "_liv << ConvertForOutput(moduleDescription) << '\n'; + winrt::hstring moduleDescription = details.ModuleDescription(); + if (!moduleDescription.empty()) + { + m_context.Reporter.Info() << " "_liv << ConvertDetailsValue(moduleDescription) << '\n'; + } } - } - else - { - // -- Sample output when no IConfigurationUnitProcessorDetails present -- - // Intent :: Type <from unit> [identifier] - // Description (from directives) - // "Module": module <directive> - OutputConfigurationUnitHeader(out, unit, unit.Type()); - - auto description = GetValueSetString(metadata, s_Directive_Description); - if (description && !description.value().empty()) + else { - out << " "_liv << description.value() << '\n'; + // -- Sample output when no IConfigurationUnitProcessorDetails present -- + // Intent :: Type <from unit> [identifier] + // Description (from directives) + // "Module": module <directive> + OutputConfigurationUnitHeader(unit, unit.Type()); + + auto description = GetValueSetString(metadata, s_Directive_Description); + if (!description.empty()) + { + m_context.Reporter.Info() << " "_liv; + OutputValueWithTruncationWarningIfNeeded(description); + } + + auto module = GetValueSetString(metadata, s_Directive_Module); + if (!module.empty()) + { + m_context.Reporter.Info() << " "_liv << Resource::String::ConfigurationModuleNameOnly(ConvertIdentifier(module)) << '\n'; + } } - auto module = GetValueSetString(metadata, s_Directive_Module); - if (module && !module.value().empty()) + // -- Sample output footer -- + // Dependencies: dep1, dep2, ... + // Settings: + // <... settings splat> + auto dependencies = unit.Dependencies(); + if (dependencies.Size() > 0) { - out << " "_liv << Resource::String::ConfigurationModuleNameOnly(module.value()) << '\n'; + std::ostringstream allDependencies; + for (const winrt::hstring& dependency : dependencies) + { + allDependencies << ' ' << ConvertIdentifier(dependency); + } + m_context.Reporter.Info() << " "_liv << Resource::String::ConfigurationDependencies(Utility::LocIndString{ std::move(allDependencies).str() }) << '\n'; } - } - // -- Sample output footer -- - // Dependencies: dep1, dep2, ... - // Settings: - // <... settings splat> - auto dependencies = unit.Dependencies(); - if (dependencies.Size() > 0) - { - std::ostringstream allDependencies; - for (const winrt::hstring& dependency : dependencies) + ValueSet settings = unit.Settings(); + if (settings.Size() > 0) { - allDependencies << ' ' << Utility::ConvertToUTF8(dependency); + m_context.Reporter.Info() << " "_liv << Resource::String::ConfigurationSettings << '\n'; + OutputValueSet(settings, 4); } - out << " "_liv << Resource::String::ConfigurationDependencies(Utility::LocIndString{ std::move(allDependencies).str() }) << '\n'; } - ValueSet settings = unit.Settings(); - if (settings.Size() > 0) - { - out << " "_liv << Resource::String::ConfigurationSettings << '\n'; - OutputValueSet(out, settings, 4); - } + private: + Execution::Context& m_context; + }; + + void OutputConfigurationUnitHeader(Execution::Context& context, const ConfigurationUnit& unit, const winrt::hstring& name) + { + OutputHelper helper{ context }; + helper.OutputConfigurationUnitHeader(unit, name); } void LogFailedGetConfigurationUnitDetails(const ConfigurationUnit& unit, const IConfigurationUnitResultInformation& resultInformation) @@ -701,8 +786,7 @@ namespace AppInstaller::CLI::Workflow { m_unitsSeen.insert(unitInstance); - OutputStream out = m_context.Reporter.Info(); - OutputConfigurationUnitHeader(out, unit, unit.Details() ? unit.Details().UnitType() : unit.Type()); + OutputConfigurationUnitHeader(m_context, unit, unit.Details() ? unit.Details().UnitType() : unit.Type()); } } @@ -758,10 +842,7 @@ namespace AppInstaller::CLI::Workflow EndProgress(); - { - OutputStream info = m_context.Reporter.Info(); - OutputConfigurationUnitHeader(info, unit, unit.Details() ? unit.Details().UnitType() : unit.Type()); - } + OutputConfigurationUnitHeader(m_context, unit, unit.Details() ? unit.Details().UnitType() : unit.Type()); switch (testResult) { @@ -988,7 +1069,7 @@ namespace AppInstaller::CLI::Workflow auto getDetailsOperation = configContext.Processor().GetSetDetailsAsync(configContext.Set(), ConfigurationUnitDetailFlags::ReadOnly); auto unification = CreateProgressCancellationUnification(std::move(progressScope), getDetailsOperation); - OutputStream out = context.Reporter.Info(); + OutputHelper outputHelper{ context }; uint32_t unitsShown = 0; getDetailsOperation.Progress([&](const IAsyncOperationWithProgress<GetConfigurationSetDetailsResult, GetConfigurationUnitDetailsResult>& operation, const GetConfigurationUnitDetailsResult&) @@ -1002,7 +1083,7 @@ namespace AppInstaller::CLI::Workflow { GetConfigurationUnitDetailsResult unitResult = unitResults.GetAt(unitsShown); LogFailedGetConfigurationUnitDetails(unitResult.Unit(), unitResult.ResultInformation()); - OutputConfigurationUnitInformation(out, unitResult.Unit()); + outputHelper.OutputConfigurationUnitInformation(unitResult.Unit()); } progressScope = context.Reporter.BeginAsyncProgress(true); @@ -1047,7 +1128,7 @@ namespace AppInstaller::CLI::Workflow { GetConfigurationUnitDetailsResult unitResult = unitResults.GetAt(unitsShown); LogFailedGetConfigurationUnitDetails(unitResult.Unit(), unitResult.ResultInformation()); - OutputConfigurationUnitInformation(out, unitResult.Unit()); + outputHelper.OutputConfigurationUnitInformation(unitResult.Unit()); } } } @@ -1057,7 +1138,13 @@ namespace AppInstaller::CLI::Workflow for (unitsShown; unitsShown < allUnits.Size(); ++unitsShown) { ConfigurationUnit unit = allUnits.GetAt(unitsShown); - OutputConfigurationUnitInformation(out, unit); + outputHelper.OutputConfigurationUnitInformation(unit); + } + + if (outputHelper.ValuesTruncated) + { + // Using error to make this stand out from other warnings + context.Reporter.Error() << Resource::String::ConfigurationWarningSetViewTruncated << std::endl; } } @@ -1183,8 +1270,7 @@ namespace AppInstaller::CLI::Workflow { ConfigurationUnit unit = unitResult.Unit(); - auto out = context.Reporter.Info(); - OutputConfigurationUnitHeader(out, unit, unit.Type()); + OutputConfigurationUnitHeader(context, unit, unit.Type()); switch (resultCode) { @@ -1314,14 +1400,13 @@ namespace AppInstaller::CLI::Workflow { if (needsHeader) { - auto out = context.Reporter.Info(); - OutputConfigurationUnitHeader(out, unit, unit.Type()); + OutputConfigurationUnitHeader(context, unit, unit.Type()); needsHeader = false; foundIssue = true; } }; - if (GetValueSetString(unit.Metadata(), s_Directive_Module).value_or(Utility::LocIndString{}).empty()) + if (GetValueSetString(unit.Metadata(), s_Directive_Module).empty()) { outputHeaderIfNeeded(); context.Reporter.Warn() << " "_liv << Resource::String::ConfigurationUnitModuleNotProvidedWarning << std::endl; diff --git a/src/AppInstallerCLIE2ETests/ConfigureShowCommand.cs b/src/AppInstallerCLIE2ETests/ConfigureShowCommand.cs @@ -120,5 +120,18 @@ namespace AppInstallerCLIE2ETests Assert.AreEqual(0, result.ExitCode); Assert.True(result.StdOut.Contains(Constants.TestRepoName)); } + + /// <summary> + /// This test ensures that there is not significant overflow from large strings in the configuration file. + /// </summary> + [Test] + public void ShowTruncatedDetailsAndFileContent() + { + var result = TestCommon.RunAICLICommand("configure show", $"{TestCommon.GetTestDataFile("Configuration\\LargeContentStrings.yml")} --verbose"); + Assert.AreEqual(0, result.ExitCode); + Assert.True(result.StdOut.Contains("<this value has been truncated; inspect the file contents for the complete text>")); + Assert.True(result.StdOut.Contains("Some of the data present in the configuration file was truncated for this output; inspect the file contents for the complete content.")); + Assert.False(result.StdOut.Contains("Line5")); + } } } diff --git a/src/AppInstallerCLIE2ETests/TestData/Configuration/LargeContentStrings.yml b/src/AppInstallerCLIE2ETests/TestData/Configuration/LargeContentStrings.yml @@ -0,0 +1,22 @@ +properties: + configurationVersion: 0.2 + resources: + - resource: xE2EMalicious/E2EMalicious + id: firstfirstfirstfirstfirstfirstfirstfirstfirstfirstfirstfirstfirstfirstfirstfirstfirstfirstfirstfirstfirstfirstfirstfirstfirstfirstfirstfirstfirstfirstfirstfirstfirstfirstfirstfirstfirstfirstfirstfirstfirstfirstfirstfirstfirstfirstfirstfirstfirstfirstfirstfirstfirstfirstfirstfirstfirstfirstfirstfirstfirstfirstfirstfirstfirstfirstfirstfirstfirstfirstfirstfirstfirstfirstfirstfirstfirstfirstfirstfirst + directives: + repository: AppInstallerCLIE2ETestsRepo + description: "Line1\nLine2\nLine3Line3Line3Line3Line3Line3Line3Line3Line3Line3Line3Line3Line3Line3Line3Line3Line3Line3Line3Line3Line3Line3Line3Line3Line3Line3Line3Line3Line3Line3Line3Line3Line3Line3Line3Line3Line3Line3Line3Line3Line3Line3\nLine4\nLine5\nLine6" + settings: + key: Foo + description: "Line1\nLine2\nLine3Line3Line3Line3Line3Line3Line3Line3Line3Line3Line3Line3Line3Line3Line3Line3Line3Line3Line3Line3Line3Line3Line3Line3Line3Line3Line3Line3Line3Line3Line3Line3Line3Line3Line3Line3Line3Line3Line3Line3Line3Line3\nLine4\nLine5\nLine6" + - resource: Unknown + dependsOn: + - firstfirstfirstfirstfirstfirstfirstfirstfirstfirstfirstfirstfirstfirstfirstfirstfirstfirstfirstfirstfirstfirstfirstfirstfirstfirstfirstfirstfirstfirstfirstfirstfirstfirstfirstfirstfirstfirstfirstfirstfirstfirstfirstfirstfirstfirstfirstfirstfirstfirstfirstfirstfirstfirstfirstfirstfirstfirstfirstfirstfirstfirstfirstfirstfirstfirstfirstfirstfirstfirstfirstfirstfirstfirstfirstfirstfirstfirstfirstfirst + directives: + module: UnknownUnknownUnknownUnknownUnknownUnknownUnknownUnknownUnknownUnknownUnknownUnknownUnknownUnknownUnknownUnknownUnknownUnknownUnknownUnknownUnknownUnknownUnknownUnknownUnknownUnknownUnknownUnknownUnknownUnknownUnknownUnknownUnknownUnknownUnknownUnknown + repository: AppInstallerCLIE2ETestsRepo + description: "Line1\nLine2\nLine3Line3Line3Line3Line3Line3Line3Line3Line3Line3Line3Line3Line3Line3Line3Line3Line3Line3Line3Line3Line3Line3Line3Line3Line3Line3Line3Line3Line3Line3Line3Line3Line3Line3Line3Line3Line3Line3Line3Line3Line3Line3\nLine4\nLine5\nLine6" + settings: + Path: ${WinGetConfigRoot}\ConfigServerUnexpectedExit.txt + Content: Contents! + description: "Line1\nLine2\nLine3Line3Line3Line3Line3Line3Line3Line3Line3Line3Line3Line3Line3Line3Line3Line3Line3Line3Line3Line3Line3Line3Line3Line3Line3Line3Line3Line3Line3Line3Line3Line3Line3Line3Line3Line3Line3Line3Line3Line3Line3Line3\nLine4\nLine5\nLine6" diff --git a/src/AppInstallerCLIE2ETests/TestData/Configuration/Modules/xE2EMalicious/xE2EMalicious.psd1 b/src/AppInstallerCLIE2ETests/TestData/Configuration/Modules/xE2EMalicious/xE2EMalicious.psd1 @@ -0,0 +1,33 @@ +# +# Module manifest for module 'xE2ETestResource' +# + +@{ + +RootModule = 'xE2EMalicious.psm1' +ModuleVersion = '0.0.0.1' +GUID = 'a0be43e8-ac22-4244-8efc-7263dfa50b92' +CompatiblePSEditions = 'Core' +Author = "WinGet Dev Team" +CompanyName = 'Microsoft Corporation' +Copyright = '(c) Microsoft Corporation. All rights reserved.' +Description = "PowerShell module with DSC resources for unit tests | PowerShell module with DSC resources for unit tests | PowerShell module with DSC resources for unit tests | PowerShell module with DSC resources for unit tests | PowerShell module with DSC resources for unit tests | PowerShell module with DSC resources for unit tests | PowerShell module with DSC resources for unit tests | PowerShell module with DSC resources for unit tests | PowerShell module with DSC resources for unit tests | PowerShell module with DSC resources for unit tests | PowerShell module with DSC resources for unit tests | PowerShell module with DSC resources for unit tests | PowerShell module with DSC resources for unit tests | PowerShell module with DSC resources for unit tests | PowerShell module with DSC resources for unit tests | PowerShell module with DSC resources for unit tests | PowerShell module with DSC resources for unit tests | PowerShell module with DSC resources for unit tests | PowerShell module with DSC resources for unit tests | PowerShell module with DSC resources for unit tests | PowerShell module with DSC resources for unit tests | PowerShell module with DSC resources for unit tests | PowerShell module with DSC resources for unit tests | PowerShell module with DSC resources for unit tests | PowerShell module with DSC resources for unit tests | PowerShell module with DSC resources for unit tests | PowerShell module with DSC resources for unit tests | PowerShell module with DSC resources for unit tests | PowerShell module with DSC resources for unit tests | PowerShell module with DSC resources for unit tests | PowerShell module with DSC resources for unit tests | PowerShell module with DSC resources for unit tests | PowerShell module with DSC resources for unit tests | PowerShell module with DSC resources for unit tests | PowerShell module with DSC resources for unit tests | PowerShell module with DSC resources for unit tests | PowerShell module with DSC resources for unit tests | PowerShell module with DSC resources for unit tests | PowerShell module with DSC resources for unit tests | PowerShell module with DSC resources for unit tests | PowerShell module with DSC resources for unit tests | PowerShell module with DSC resources for unit tests | PowerShell module with DSC resources for unit tests | PowerShell module with DSC resources for unit tests | PowerShell module with DSC resources for unit tests | PowerShell module with DSC resources for unit tests | PowerShell module with DSC resources for unit tests | PowerShell module with DSC resources for unit tests | PowerShell module with DSC resources for unit tests | PowerShell module with DSC resources for unit tests | PowerShell module with DSC resources for unit tests | PowerShell module with DSC resources for unit tests | PowerShell module with DSC resources for unit tests | PowerShell module with DSC resources for unit tests | PowerShell module with DSC resources for unit tests | PowerShell module with DSC resources for unit tests | PowerShell module with DSC resources for unit tests | PowerShell module with DSC resources for unit tests | PowerShell module with DSC resources for unit tests | PowerShell module with DSC resources for unit tests | PowerShell module with DSC resources for unit tests | PowerShell module with DSC resources for unit tests | PowerShell module with DSC resources for unit tests | PowerShell module with DSC resources for unit tests | PowerShell module with DSC resources for unit tests | PowerShell module with DSC resources for unit tests | PowerShell module with DSC resources for unit tests | PowerShell module with DSC resources for unit tests | PowerShell module with DSC resources for unit tests | PowerShell module with DSC resources for unit tests | PowerShell module with DSC resources for unit tests | PowerShell module with DSC resources for unit tests | PowerShell module with DSC resources for unit tests | PowerShell module with DSC resources for unit tests | PowerShell module with DSC resources for unit tests | PowerShell module with DSC resources for unit tests | PowerShell module with DSC resources for unit tests | PowerShell module with DSC resources for unit tests" +PowerShellVersion = '7.2' +FunctionsToExport = @() +CmdletsToExport = @() +DscResourcesToExport = @( + 'E2EMalicious' +) +HelpInfoURI = 'https://www.contoso.com/help' + +# Private data to pass to the module specified in RootModule/ModuleToProcess. This may also contain a PSData hashtable with additional module metadata used by PowerShell. +PrivateData = @{ + + PSData = @{ + ProjectUri = 'https://github.com/microsoft/winget-cli' + IconUri = 'https://www.contoso.com/icons/icon.png' + } + +} + +} diff --git a/src/AppInstallerCLIE2ETests/TestData/Configuration/Modules/xE2EMalicious/xE2EMalicious.psm1 b/src/AppInstallerCLIE2ETests/TestData/Configuration/Modules/xE2EMalicious/xE2EMalicious.psm1 @@ -0,0 +1,86 @@ +# E2E module with resources. + +enum Ensure +{ + Absent + Present +} + +# This resource just checks if a file is there or not with and if its with the specified content. +[DscResource()] +class E2EMalicious +{ + [DscProperty(Key)] + [string] $Path + + [DscProperty()] + [Ensure] $Ensure = [Ensure]::Present + + [DscProperty()] + [string] $Content = $null + + [E2EFileResource] Get() + { + if ([string]::IsNullOrEmpty($this.Path)) + { + throw + } + + $fileContent = $null + if (Test-Path -Path $this.Path -PathType Leaf) + { + $fileContent = Get-Content $this.Path -Raw + } + + $result = @{ + Path = $this.Path + Content = $fileContent + } + + return $result + } + + [bool] Test() + { + $get = $this.Get() + + if (Test-Path -Path $this.Path -PathType Leaf) + { + if ($this.Ensure -eq [Ensure]::Present) + { + return $this.Content -eq $get.Content + } + } + elseif ($this.Ensure -eq [Ensure]::Absent) + { + return $true + } + + return $false + } + + [void] Set() + { + if (-not $this.Test()) + { + if (Test-Path -Path $this.Path -PathType Leaf) + { + if ($this.Ensure -eq [Ensure]::Present) + { + Set-Content $this.Path $this.Content -NoNewline + } + else + { + Remove-Item $this.Path + } + } + else + { + if ($this.Ensure -eq [Ensure]::Present) + { + Set-Content $this.Path $this.Content -NoNewline + } + } + } + } +} diff --git a/src/AppInstallerCLIPackage/Shared/Strings/en-us/winget.resw b/src/AppInstallerCLIPackage/Shared/Strings/en-us/winget.resw @@ -2889,4 +2889,11 @@ Please specify one of them using the --source option to proceed.</value> <data name="MSStoreDownloadPackageNotFound" xml:space="preserve"> <value>The MSStore package could not be found.</value> </data> + <data name="ConfigurationWarningSetViewTruncated" xml:space="preserve"> + <value>Some of the data present in the configuration file was truncated for this output; inspect the file contents for the complete content.</value> + </data> + <data name="ConfigurationWarningValueTruncated" xml:space="preserve"> + <value><this value has been truncated; inspect the file contents for the complete text></value> + <comment>Keep some form of separator like the "<>" around the text so that it stands out from the preceding text.</comment> + </data> </root> \ No newline at end of file diff --git a/src/AppInstallerCLITests/AppInstallerCLITests.vcxproj b/src/AppInstallerCLITests/AppInstallerCLITests.vcxproj @@ -325,6 +325,7 @@ <ClCompile Include="TestCommon.cpp" /> <ClCompile Include="WorkflowCommon.cpp" /> <ClCompile Include="WorkflowGroupPolicy.cpp" /> + <ClCompile Include="Yaml.cpp" /> <ClCompile Include="YamlManifest.cpp" /> </ItemGroup> <ItemGroup> @@ -929,40 +930,34 @@ <DeploymentContent>true</DeploymentContent> </CopyFileToFolders> <CopyFileToFolders Include="TestData\Shadow\V1_5\ManifestV1_5-Shadow-DefaultLocale.yaml"> - <DeploymentContent Condition="'$(Configuration)|$(Platform)'=='Debug|x64'">true</DeploymentContent> - <FileType>Document</FileType> + <DeploymentContent>true</DeploymentContent> </CopyFileToFolders> <CopyFileToFolders Include="TestData\Shadow\V1_5\ManifestV1_5-Shadow-Locale.yaml"> - <DeploymentContent Condition="'$(Configuration)|$(Platform)'=='Debug|x64'">true</DeploymentContent> - <FileType>Document</FileType> + <DeploymentContent>true</DeploymentContent> </CopyFileToFolders> <CopyFileToFolders Include="TestData\Shadow\V1_5\ManifestV1_5-Shadow-Locale2.yaml"> - <DeploymentContent Condition="'$(Configuration)|$(Platform)'=='Debug|x64'">true</DeploymentContent> - <FileType>Document</FileType> + <DeploymentContent>true</DeploymentContent> </CopyFileToFolders> <CopyFileToFolders Include="TestData\Shadow\V1_5\ManifestV1_5-Shadow-Shadow.yaml"> - <DeploymentContent Condition="'$(Configuration)|$(Platform)'=='Debug|x64'">true</DeploymentContent> - <FileType>Document</FileType> + <DeploymentContent>true</DeploymentContent> </CopyFileToFolders> <CopyFileToFolders Include="TestData\Shadow\V1_5\ManifestV1_5-Shadow-Shadow2.yaml"> - <DeploymentContent Condition="'$(Configuration)|$(Platform)'=='Debug|x64'">true</DeploymentContent> - <FileType>Document</FileType> + <DeploymentContent>true</DeploymentContent> </CopyFileToFolders> <CopyFileToFolders Include="TestData\Shadow\V1_5\ManifestV1_5-Shadow-Installer.yaml"> - <DeploymentContent Condition="'$(Configuration)|$(Platform)'=='Debug|x64'">true</DeploymentContent> - <FileType>Document</FileType> + <DeploymentContent>true</DeploymentContent> </CopyFileToFolders> <CopyFileToFolders Include="TestData\Node-Merge.yaml"> - <DeploymentContent Condition="'$(Configuration)|$(Platform)'=='Debug|x64'">true</DeploymentContent> - <FileType>Document</FileType> + <DeploymentContent>true</DeploymentContent> </CopyFileToFolders> <CopyFileToFolders Include="TestData\Node-Merge2.yaml"> - <DeploymentContent Condition="'$(Configuration)|$(Platform)'=='Debug|x64'">true</DeploymentContent> - <FileType>Document</FileType> + <DeploymentContent>true</DeploymentContent> </CopyFileToFolders> <CopyFileToFolders Include="TestData\Node-Mapping.yaml"> - <DeploymentContent Condition="'$(Configuration)|$(Platform)'=='Debug|x64'">true</DeploymentContent> - <FileType>Document</FileType> + <DeploymentContent>true</DeploymentContent> + </CopyFileToFolders> + <CopyFileToFolders Include="TestData\ContainsEscapeControlCode.yaml"> + <DeploymentContent>true</DeploymentContent> </CopyFileToFolders> </ItemGroup> <ItemGroup> diff --git a/src/AppInstallerCLITests/AppInstallerCLITests.vcxproj.filters b/src/AppInstallerCLITests/AppInstallerCLITests.vcxproj.filters @@ -347,6 +347,9 @@ <ClCompile Include="PackageVersionDataManifest.cpp"> <Filter>Source Files\Common</Filter> </ClCompile> + <ClCompile Include="Yaml.cpp"> + <Filter>Source Files\Common</Filter> + </ClCompile> </ItemGroup> <ItemGroup> <None Include="PropertySheet.props" /> @@ -978,5 +981,8 @@ <CopyFileToFolders Include="TestData\Node-Mapping.yaml"> <Filter>TestData</Filter> </CopyFileToFolders> + <CopyFileToFolders Include="TestData\ContainsEscapeControlCode.yaml"> + <Filter>TestData</Filter> + </CopyFileToFolders> </ItemGroup> </Project> \ No newline at end of file diff --git a/src/AppInstallerCLITests/Strings.cpp b/src/AppInstallerCLITests/Strings.cpp @@ -249,7 +249,7 @@ TEST_CASE("Format", "[strings]") REQUIRE("First {1}" == Format("{0} {1}", "First")); } -TEST_CASE("SplitIntoLines", "[string]") +TEST_CASE("SplitIntoLines", "[strings]") { REQUIRE(SplitIntoLines("Boring test") == std::vector<std::string>{ "Boring test" }); REQUIRE(SplitIntoLines( @@ -261,7 +261,7 @@ TEST_CASE("SplitIntoLines", "[string]") == std::vector<std::string>{ "You want my treasure?", "You can have it!", "I left everything I gathered in one place!", "You just have to find it!" }); } -TEST_CASE("SplitWithSeparator", "[string]") +TEST_CASE("SplitWithSeparator", "[strings]") { std::vector<std::string> test1 = Split("first;second;third", ';'); REQUIRE(test1.size() == 3); @@ -285,10 +285,40 @@ TEST_CASE("SplitWithSeparator", "[string]") REQUIRE(test4[1] == "spaces"); } -TEST_CASE("ConvertGuid", "[string]") +TEST_CASE("ConvertGuid", "[strings]") { std::string validGuidString = "{4d1e55b2-f16f-11cf-88cb-001111000030}"; GUID guid = { 0x4d1e55b2, 0xf16f, 0x11cf, 0x88, 0xcb, 0x00, 0x11, 0x11, 0x00, 0x00, 0x30 }; REQUIRE(CaseInsensitiveEquals(ConvertGuidToString(guid), validGuidString)); } + +TEST_CASE("FindControlCodeToConvert", "[strings]") +{ + REQUIRE(FindControlCodeToConvert("No codes") == std::string::npos); + REQUIRE(FindControlCodeToConvert("Allowed codes: \t\r\n") == std::string::npos); + REQUIRE(FindControlCodeToConvert("\x1bSkipped code", 1) == std::string::npos); + + REQUIRE(FindControlCodeToConvert("\x1bUnskipped code") == 0); + REQUIRE(FindControlCodeToConvert("Escape code: \x1b") == 13); + + std::string_view allCodes{ "\x0\x1\x2\x3\x4\x5\x6\x7\x8\xb\xc\xe\xf\x10\x11\x12\x13\x14\x15\x16\x17\x18\x19\x1a\x1b\x1c\x1d\x1e\x1f\x7f"sv }; + for (size_t i = 0; i < allCodes.length(); ++i) + { + REQUIRE(FindControlCodeToConvert(allCodes, i) == i); + } +} + +TEST_CASE("ConvertControlCodesToPictures", "[strings]") +{ + REQUIRE(ConvertControlCodesToPictures("No codes") == "No codes"); + REQUIRE(ConvertControlCodesToPictures("Allowed codes: \t\r\n") == "Allowed codes: \t\r\n"); + + REQUIRE(ConvertControlCodesToPictures("\x1b Code First") == ConvertToUTF8(L"\x241b Code First")); + REQUIRE(ConvertControlCodesToPictures("Escape code: \x1b") == ConvertToUTF8(L"Escape code: \x241b")); + + std::string_view allCodes{ "\x0\x1\x2\x3\x4\x5\x6\x7\x8\xb\xc\xe\xf\x10\x11\x12\x13\x14\x15\x16\x17\x18\x19\x1a\x1b\x1c\x1d\x1e\x1f\x7f"sv }; + std::wstring_view allPictures{ L"\x2400\x2401\x2402\x2403\x2404\x2405\x2406\x2407\x2408\x240b\x240c\x240e\x240f\x2410\x2411\x2412\x2413\x2414\x2415\x2416\x2417\x2418\x2419\x241a\x241b\x241c\x241d\x241e\x241f\x2421"sv }; + + REQUIRE(ConvertControlCodesToPictures(allCodes) == ConvertToUTF8(allPictures)); +} diff --git a/src/AppInstallerCLITests/TestData/ContainsEscapeControlCode.yaml b/src/AppInstallerCLITests/TestData/ContainsEscapeControlCode.yaml @@ -0,0 +1 @@ +key: "This is the ESCAPE control code: \x1b" diff --git a/src/AppInstallerCLITests/Yaml.cpp b/src/AppInstallerCLITests/Yaml.cpp @@ -0,0 +1,175 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. +#include "pch.h" +#include "TestCommon.h" +#include <AppInstallerErrors.h> +#include <winget/Yaml.h> + +using namespace TestCommon; +using namespace AppInstaller::Utility; +using namespace AppInstaller::YAML; + + +TEST_CASE("YamlParserTypes", "[YAML]") +{ + auto document = AppInstaller::YAML::Load(TestDataFile("Node-Types.yaml")); + + auto intUnquoted = document["IntegerUnquoted"]; + CHECK(intUnquoted.GetTagType() == Node::TagType::Int); + + auto intSingleQuoted = document["IntegerSingleQuoted"]; + CHECK(intSingleQuoted.GetTagType() == Node::TagType::Str); + + auto intDoubleQuoted = document["IntegerDoubleQuoted"]; + CHECK(intDoubleQuoted.GetTagType() == Node::TagType::Str); + + auto boolTrue = document["BooleanTrue"]; + CHECK(boolTrue.GetTagType() == Node::TagType::Bool); + + auto strTrue = document["StringTrue"]; + CHECK(strTrue.GetTagType() == Node::TagType::Str); + + auto boolFalse = document["BooleanFalse"]; + CHECK(boolFalse.GetTagType() == Node::TagType::Bool); + + auto strFalse = document["StringFalse"]; + CHECK(strFalse.GetTagType() == Node::TagType::Str); + + auto localTag = document["LocalTag"]; + CHECK(localTag.GetTagType() == Node::TagType::Unknown); +} + +TEST_CASE("YamlMergeMappingNode", "[YAML]") +{ + auto document = Load(TestDataFile("Node-Mapping.yaml")); + + auto mergeNode = document["MergeNode"]; + auto mergeNode2 = document["MergeNode2"]; + + REQUIRE(3 == mergeNode.size()); + REQUIRE(2 == mergeNode2.size()); + + mergeNode.MergeMappingNode(mergeNode2); + + REQUIRE(5 == mergeNode.size()); +} + +TEST_CASE("YamlMergeMappingNode_CaseInsensitive", "[YAML]") +{ + auto document = Load(TestDataFile("Node-Mapping.yaml")); + + auto mergeNode = document["MergeNode"]; + auto mergeNode2 = document["MergeNode2"]; + + REQUIRE(3 == mergeNode.size()); + REQUIRE(2 == mergeNode2.size()); + + mergeNode.MergeMappingNode(mergeNode2, true); + + REQUIRE(4 == mergeNode.size()); +} + +TEST_CASE("YamlMergeSequenceNode", "[YAML]") +{ + auto document = Load(TestDataFile("Node-Merge.yaml")); + auto document2 = Load(TestDataFile("Node-Merge2.yaml")); + + REQUIRE(3 == document["StrawHats"].size()); + REQUIRE(2 == document2["StrawHats"].size()); + + // Internally will call MergeMappingNode. + document["StrawHats"].MergeSequenceNode(document2["StrawHats"], "Bounty"); + REQUIRE(5 == document["StrawHats"].size()); +} + +TEST_CASE("YamlMergeSequenceNode_CaseInsensitive", "[YAML]") +{ + auto document = Load(TestDataFile("Node-Merge.yaml")); + auto document2 = Load(TestDataFile("Node-Merge2.yaml")); + + REQUIRE(3 == document["StrawHats"].size()); + REQUIRE(2 == document2["StrawHats"].size()); + + // Internally will call MergeMappingNode. + document["StrawHats"].MergeSequenceNode(document2["StrawHats"], "Name", true); + REQUIRE(4 == document["StrawHats"].size()); + + auto luffy = std::find_if( + document["StrawHats"].Sequence().begin(), + document["StrawHats"].Sequence().end(), + [](auto const& n) { return n["Name"].as<std::string>() == "Monkey D Luffy"; }); + REQUIRE(luffy != document["StrawHats"].Sequence().end()); + + // From original node + REQUIRE((*luffy)["Bounty"].as<std::string>() == "3,000,000,000"); + + // From merged node + REQUIRE((*luffy)["Fruit"].as<std::string>() == "Gomu Gomu no Mi"); +} + +TEST_CASE("YamlMergeNode_MergeSequenceNoKey", "[YAML]") +{ + auto document = Load(TestDataFile("Node-Merge.yaml")); + auto document2 = Load(TestDataFile("Node-Merge2.yaml")); + + REQUIRE_THROWS_HR(document["StrawHats"].MergeSequenceNode(document2["StrawHats"], "Power"), APPINSTALLER_CLI_ERROR_YAML_INVALID_DATA); +} + +TEST_CASE("YamlMappingNode", "[YAML]") +{ + auto document = Load(TestDataFile("Node-Mapping.yaml")); + + auto node = document["key"]; + REQUIRE(node.as<std::string>() == "value"); + + auto node2 = document.GetChildNode("KEY"); + REQUIRE(node2.as<std::string>() == "value"); + + auto node3 = document.GetChildNode("key"); + REQUIRE(node3.as<std::string>() == "value"); + + auto node4 = document.GetChildNode("kEy"); + REQUIRE(node4.as<std::string>() == "value"); + + auto node5 = document.GetChildNode("fake"); + REQUIRE(node5.IsNull()); + + auto node6 = document["repeatedkey"]; + REQUIRE(node6.as<std::string>() == "repeated value"); + REQUIRE_THROWS_HR(document.GetChildNode("repeatedkey"), APPINSTALLER_CLI_ERROR_YAML_DUPLICATE_MAPPING_KEY); + + REQUIRE_THROWS_HR(document.GetChildNode("RepeatedKey"), APPINSTALLER_CLI_ERROR_YAML_DUPLICATE_MAPPING_KEY); + REQUIRE_THROWS_HR(document["RepeatedKey"], APPINSTALLER_CLI_ERROR_YAML_DUPLICATE_MAPPING_KEY); +} + +TEST_CASE("YamlMappingNode_const", "[YAML]") +{ + const auto document = Load(TestDataFile("Node-Mapping.yaml")); + + auto node = document["key"]; + REQUIRE(node.as<std::string>() == "value"); + + auto node2 = document.GetChildNode("KEY"); + REQUIRE(node2.as<std::string>() == "value"); + + auto node3 = document.GetChildNode("key"); + REQUIRE(node3.as<std::string>() == "value"); + + auto node4 = document.GetChildNode("kEy"); + REQUIRE(node4.as<std::string>() == "value"); + + auto node5 = document.GetChildNode("fake"); + REQUIRE(node5.IsNull()); + + auto node6 = document["repeatedkey"]; + REQUIRE(node6.as<std::string>() == "repeated value"); + REQUIRE_THROWS_HR(document.GetChildNode("repeatedkey"), APPINSTALLER_CLI_ERROR_YAML_DUPLICATE_MAPPING_KEY); + + REQUIRE_THROWS_HR(document.GetChildNode("RepeatedKey"), APPINSTALLER_CLI_ERROR_YAML_DUPLICATE_MAPPING_KEY); + REQUIRE_THROWS_HR(document["RepeatedKey"], APPINSTALLER_CLI_ERROR_YAML_DUPLICATE_MAPPING_KEY); +} + +TEST_CASE("YamlContainsEscapeControlCode", "[YAML]") +{ + REQUIRE_THROWS_HR(Load(TestDataFile("ContainsEscapeControlCode.yaml")), APPINSTALLER_CLI_ERROR_LIBYAML_ERROR); +} diff --git a/src/AppInstallerCLITests/YamlManifest.cpp b/src/AppInstallerCLITests/YamlManifest.cpp @@ -1789,162 +1789,3 @@ TEST_CASE("ShadowManifest_NotVerifiedPublisher", "[ShadowManifest]") TempFile mergedManifestFile{ "merged.yaml" }; REQUIRE_THROWS_MATCHES(YamlParser::CreateFromPath(multiFileDirectory, validateOption, mergedManifestFile), ManifestException, ManifestExceptionMatcher("Field usage requires verified publishers. [Icons]")); } - -TEST_CASE("YamlParserTypes", "[YAML]") -{ - auto document = AppInstaller::YAML::Load(TestDataFile("Node-Types.yaml")); - - auto intUnquoted = document["IntegerUnquoted"]; - CHECK(intUnquoted.GetTagType() == Node::TagType::Int); - - auto intSingleQuoted = document["IntegerSingleQuoted"]; - CHECK(intSingleQuoted.GetTagType() == Node::TagType::Str); - - auto intDoubleQuoted = document["IntegerDoubleQuoted"]; - CHECK(intDoubleQuoted.GetTagType() == Node::TagType::Str); - - auto boolTrue = document["BooleanTrue"]; - CHECK(boolTrue.GetTagType() == Node::TagType::Bool); - - auto strTrue = document["StringTrue"]; - CHECK(strTrue.GetTagType() == Node::TagType::Str); - - auto boolFalse = document["BooleanFalse"]; - CHECK(boolFalse.GetTagType() == Node::TagType::Bool); - - auto strFalse = document["StringFalse"]; - CHECK(strFalse.GetTagType() == Node::TagType::Str); - - auto localTag = document["LocalTag"]; - CHECK(localTag.GetTagType() == Node::TagType::Unknown); -} - -TEST_CASE("YamlMergeMappingNode", "[YAML]") -{ - auto document = Load(TestDataFile("Node-Mapping.yaml")); - - auto mergeNode = document["MergeNode"]; - auto mergeNode2 = document["MergeNode2"]; - - REQUIRE(3 == mergeNode.size()); - REQUIRE(2 == mergeNode2.size()); - - mergeNode.MergeMappingNode(mergeNode2); - - REQUIRE(5 == mergeNode.size()); -} - -TEST_CASE("YamlMergeMappingNode_CaseInsensitive", "[YAML]") -{ - auto document = Load(TestDataFile("Node-Mapping.yaml")); - - auto mergeNode = document["MergeNode"]; - auto mergeNode2 = document["MergeNode2"]; - - REQUIRE(3 == mergeNode.size()); - REQUIRE(2 == mergeNode2.size()); - - mergeNode.MergeMappingNode(mergeNode2, true); - - REQUIRE(4 == mergeNode.size()); -} - -TEST_CASE("YamlMergeSequenceNode", "[YAML]") -{ - auto document = Load(TestDataFile("Node-Merge.yaml")); - auto document2 = Load(TestDataFile("Node-Merge2.yaml")); - - REQUIRE(3 == document["StrawHats"].size()); - REQUIRE(2 == document2["StrawHats"].size()); - - // Internally will call MergeMappingNode. - document["StrawHats"].MergeSequenceNode(document2["StrawHats"], "Bounty"); - REQUIRE(5 == document["StrawHats"].size()); -} - -TEST_CASE("YamlMergeSequenceNode_CaseInsensitive", "[YAML]") -{ - auto document = Load(TestDataFile("Node-Merge.yaml")); - auto document2 = Load(TestDataFile("Node-Merge2.yaml")); - - REQUIRE(3 == document["StrawHats"].size()); - REQUIRE(2 == document2["StrawHats"].size()); - - // Internally will call MergeMappingNode. - document["StrawHats"].MergeSequenceNode(document2["StrawHats"], "Name", true); - REQUIRE(4 == document["StrawHats"].size()); - - auto luffy = std::find_if( - document["StrawHats"].Sequence().begin(), - document["StrawHats"].Sequence().end(), - [](auto const& n) { return n["Name"].as<std::string>() == "Monkey D Luffy"; }); - REQUIRE(luffy != document["StrawHats"].Sequence().end()); - - // From original node - REQUIRE((*luffy)["Bounty"].as<std::string>() == "3,000,000,000"); - - // From merged node - REQUIRE((*luffy)["Fruit"].as<std::string>() == "Gomu Gomu no Mi"); -} - -TEST_CASE("YamlMergeNode_MergeSequenceNoKey", "[YAML]") -{ - auto document = Load(TestDataFile("Node-Merge.yaml")); - auto document2 = Load(TestDataFile("Node-Merge2.yaml")); - - REQUIRE_THROWS_HR(document["StrawHats"].MergeSequenceNode(document2["StrawHats"], "Power"), APPINSTALLER_CLI_ERROR_YAML_INVALID_DATA); -} - -TEST_CASE("YamlMappingNode", "[YAML]") -{ - auto document = Load(TestDataFile("Node-Mapping.yaml")); - - auto node = document["key"]; - REQUIRE(node.as<std::string>() == "value"); - - auto node2 = document.GetChildNode("KEY"); - REQUIRE(node2.as<std::string>() == "value"); - - auto node3 = document.GetChildNode("key"); - REQUIRE(node3.as<std::string>() == "value"); - - auto node4 = document.GetChildNode("kEy"); - REQUIRE(node4.as<std::string>() == "value"); - - auto node5 = document.GetChildNode("fake"); - REQUIRE(node5.IsNull()); - - auto node6 = document["repeatedkey"]; - REQUIRE(node6.as<std::string>() == "repeated value"); - REQUIRE_THROWS_HR(document.GetChildNode("repeatedkey"), APPINSTALLER_CLI_ERROR_YAML_DUPLICATE_MAPPING_KEY); - - REQUIRE_THROWS_HR(document.GetChildNode("RepeatedKey"), APPINSTALLER_CLI_ERROR_YAML_DUPLICATE_MAPPING_KEY); - REQUIRE_THROWS_HR(document["RepeatedKey"], APPINSTALLER_CLI_ERROR_YAML_DUPLICATE_MAPPING_KEY); -} - -TEST_CASE("YamlMappingNode_const", "[YAML]") -{ - const auto document = Load(TestDataFile("Node-Mapping.yaml")); - - auto node = document["key"]; - REQUIRE(node.as<std::string>() == "value"); - - auto node2 = document.GetChildNode("KEY"); - REQUIRE(node2.as<std::string>() == "value"); - - auto node3 = document.GetChildNode("key"); - REQUIRE(node3.as<std::string>() == "value"); - - auto node4 = document.GetChildNode("kEy"); - REQUIRE(node4.as<std::string>() == "value"); - - auto node5 = document.GetChildNode("fake"); - REQUIRE(node5.IsNull()); - - auto node6 = document["repeatedkey"]; - REQUIRE(node6.as<std::string>() == "repeated value"); - REQUIRE_THROWS_HR(document.GetChildNode("repeatedkey"), APPINSTALLER_CLI_ERROR_YAML_DUPLICATE_MAPPING_KEY); - - REQUIRE_THROWS_HR(document.GetChildNode("RepeatedKey"), APPINSTALLER_CLI_ERROR_YAML_DUPLICATE_MAPPING_KEY); - REQUIRE_THROWS_HR(document["RepeatedKey"], APPINSTALLER_CLI_ERROR_YAML_DUPLICATE_MAPPING_KEY); -} diff --git a/src/AppInstallerSharedLib/AppInstallerStrings.cpp b/src/AppInstallerSharedLib/AppInstallerStrings.cpp @@ -762,9 +762,11 @@ namespace AppInstaller::Utility // If not, round up to the next line count (by rounding down through integer division after subtracting 1 + 1). size_t currentLineActualLineCount = (currentLineWidth ? (currentLineWidth - 1) / lineWidth : 0) + 1; - // The current line may be too big to be the last line. + // The current line may be too big to be the last line, or it may be just the right size but we will end up trimming + // additional lines. In either case, append an ellipsis to indicate that we trimmed the value. size_t availableLines = maximum - totalLines; - if (currentLineActualLineCount > availableLines) + if (currentLineActualLineCount > availableLines || + (currentLineActualLineCount == availableLines && currentLine != lines.size() - 1)) { size_t actualWidth = 0; std::string trimmedLine = UTF8TrimRightToColumnWidth(lines[currentLine], (availableLines * lineWidth) - 1, actualWidth); @@ -827,7 +829,8 @@ namespace AppInstaller::Utility return result; } - LocIndString Join(LocIndView separator, const std::vector<LocIndString>& vector) + template <typename StringLike> + static std::string JoinInternal(std::string_view separator, const std::vector<StringLike>& vector) { auto vectorSize = vector.size(); if (vectorSize == 0) @@ -841,7 +844,17 @@ namespace AppInstaller::Utility { ssJoin << separator << vector[i]; } - return LocIndString{ ssJoin.str() }; + return ssJoin.str(); + } + + LocIndString Join(LocIndView separator, const std::vector<LocIndString>& vector) + { + return LocIndString{ JoinInternal(separator, vector) }; + } + + std::string Join(std::string_view separator, const std::vector<std::string>& vector) + { + return JoinInternal(separator, vector); } std::vector<std::string> Split(const std::string& input, char separator, bool trim) @@ -909,4 +922,74 @@ namespace AppInstaller::Utility return false; } } + + size_t FindControlCodeToConvert(std::string_view input, size_t offset) + { + size_t nextControl = offset; + while (nextControl < input.size()) + { + char currentChar = input[nextControl]; + + // Convert all low controls except tab, line feed and carriage return + if (currentChar >= 0 && currentChar < 0x20 && + currentChar != '\t' && + currentChar != '\n' && + currentChar != '\r') + { + break; + } + + // Convert the Delete control + if (currentChar == 0x7F) + { + break; + } + + ++nextControl; + } + + return nextControl < input.size() ? nextControl : std::string::npos; + } + + std::string ConvertControlCodesToPictures(std::string_view input) + { + std::string result; + size_t pos = 0; + + while (pos < input.size()) + { + size_t nextControl = FindControlCodeToConvert(input, pos); + + if (nextControl == std::string::npos) + { + // No more control codes found + result += input.substr(pos); + break; + } + else + { + result += input.substr(pos, nextControl - pos); + + char currentChar = input[nextControl]; + + if (currentChar >= 0 && currentChar < 0x20) + { + // ASCII 0x00 - 0x1F => UTF-8 0x2400 - 0x241F + // Then manually converted to UTF-8 since only the last character is affected + result += '\xE2'; + result += '\x90'; + result += ('\x80' + currentChar); + } + else if (currentChar == 0x7F) + { + // UTF-8 for control picture of DELETE + result += "\xE2\x90\xA1"; + } + + pos = nextControl + 1; + } + } + + return result; + } } diff --git a/src/AppInstallerSharedLib/Public/AppInstallerStrings.h b/src/AppInstallerSharedLib/Public/AppInstallerStrings.h @@ -254,6 +254,9 @@ namespace AppInstaller::Utility // Join a string vector using the provided separator. LocIndString Join(LocIndView separator, const std::vector<LocIndString>& vector); + // Join a string vector using the provided separator. + std::string Join(std::string_view separator, const std::vector<std::string>& vector); + // Splits the string using the provided separator. Entries can also be trimmed. std::vector<std::string> Split(const std::string& input, char separator, bool trim = false); @@ -278,4 +281,12 @@ namespace AppInstaller::Utility // Converts the input string to a DWORD value using std::stoul and returns a boolean value based on the resulting DWORD value. bool IsDwordFlagSet(const std::string& value); + + // Finds the next control code index that would be replaced. + // Returns std::string::npos if not found. + size_t FindControlCodeToConvert(std::string_view input, size_t offset = 0); + + // Converts most control codes in the input to their corresponding control picture in the output. + // Exempts tab, line feed, and carriage return from being replaced. + std::string ConvertControlCodesToPictures(std::string_view input); } diff --git a/src/AppInstallerSharedLib/Public/winget/Yaml.h b/src/AppInstallerSharedLib/Public/winget/Yaml.h @@ -38,7 +38,8 @@ namespace AppInstaller::YAML Parser, Composer, Writer, - Emitter + Emitter, + Policy, }; // Should only be used for Memory. diff --git a/src/AppInstallerSharedLib/Yaml.cpp b/src/AppInstallerSharedLib/Yaml.cpp @@ -45,6 +45,8 @@ namespace AppInstaller::YAML return "Writer"sv; case Exception::Type::Emitter: return "Emitter"sv; + case Exception::Type::Policy: + return "Policy"sv; } return "Unknown"sv; diff --git a/src/AppInstallerSharedLib/YamlWrapper.cpp b/src/AppInstallerSharedLib/YamlWrapper.cpp @@ -53,26 +53,36 @@ namespace AppInstaller::YAML::Wrapper THROW_HR(E_UNEXPECTED); } - std::string ConvertYamlString(yaml_char_t* string, size_t length = std::string::npos) + Mark ConvertMark(const yaml_mark_t& mark) { + return { mark.line + 1, mark.column + 1 }; + } + + std::string ConvertYamlString(yaml_char_t* string, const yaml_mark_t& mark, size_t length = std::string::npos) + { + std::string_view resultView; + if (length == std::string::npos) { - return { reinterpret_cast<char*>(string) }; + resultView = { reinterpret_cast<char*>(string) }; } else { - return { reinterpret_cast<char*>(string), length }; + resultView = { reinterpret_cast<char*>(string), length }; } - } - std::string ConvertScalarToString(yaml_node_t* node) - { - return ConvertYamlString(node->data.scalar.value, node->data.scalar.length); + size_t invalidCharacter = Utility::FindControlCodeToConvert(resultView); + if (invalidCharacter != std::string::npos) + { + THROW_EXCEPTION(Exception(Exception::Type::Policy, "unsupported control character", ConvertMark(mark))); + } + + return std::string{ resultView }; } - Mark ConvertMark(const yaml_mark_t& mark) + std::string ConvertScalarToString(yaml_node_t* node, const yaml_mark_t& mark) { - return { mark.line + 1, mark.column + 1 }; + return ConvertYamlString(node->data.scalar.value, mark, node->data.scalar.length); } } @@ -115,7 +125,7 @@ namespace AppInstaller::YAML::Wrapper return {}; } - Node result(ConvertNodeType(root->type), ConvertYamlString(root->tag), ConvertMark(root->start_mark)); + Node result(ConvertNodeType(root->type), ConvertYamlString(root->tag, root->start_mark), ConvertMark(root->start_mark)); struct StackItem { @@ -142,7 +152,7 @@ namespace AppInstaller::YAML::Wrapper break; case YAML_SCALAR_NODE: stackItem.node->SetScalar( - ConvertScalarToString(stackItem.yamlNode), + ConvertScalarToString(stackItem.yamlNode, stackItem.yamlNode->start_mark), stackItem.yamlNode->data.scalar.style == YAML_SINGLE_QUOTED_SCALAR_STYLE || stackItem.yamlNode->data.scalar.style == YAML_DOUBLE_QUOTED_SCALAR_STYLE); pop = true; @@ -153,7 +163,7 @@ namespace AppInstaller::YAML::Wrapper if (child < stackItem.yamlNode->data.sequence.items.top) { yaml_node_t* childYamlNode = GetNode(*child); - Node& childNode = stackItem.node->AddSequenceNode(ConvertNodeType(childYamlNode->type), ConvertYamlString(childYamlNode->tag), ConvertMark(childYamlNode->start_mark)); + Node& childNode = stackItem.node->AddSequenceNode(ConvertNodeType(childYamlNode->type), ConvertYamlString(childYamlNode->tag, childYamlNode->start_mark), ConvertMark(childYamlNode->start_mark)); resultStack.emplace(childYamlNode, &childNode); } else @@ -171,12 +181,12 @@ namespace AppInstaller::YAML::Wrapper yaml_node_t* keyYamlNode = GetNode(child->key); THROW_HR_IF(APPINSTALLER_CLI_ERROR_YAML_INVALID_MAPPING_KEY, keyYamlNode->type != YAML_SCALAR_NODE); - Node keyNode(ConvertNodeType(keyYamlNode->type), ConvertYamlString(keyYamlNode->tag), ConvertMark(keyYamlNode->start_mark)); - keyNode.SetScalar(ConvertScalarToString(keyYamlNode)); + Node keyNode(ConvertNodeType(keyYamlNode->type), ConvertYamlString(keyYamlNode->tag, keyYamlNode->start_mark), ConvertMark(keyYamlNode->start_mark)); + keyNode.SetScalar(ConvertScalarToString(keyYamlNode, keyYamlNode->start_mark)); yaml_node_t* valueYamlNode = GetNode(child->value); - Node& childNode = stackItem.node->AddMappingNode(std::move(keyNode), ConvertNodeType(valueYamlNode->type), ConvertYamlString(valueYamlNode->tag), ConvertMark(valueYamlNode->start_mark)); + Node& childNode = stackItem.node->AddMappingNode(std::move(keyNode), ConvertNodeType(valueYamlNode->type), ConvertYamlString(valueYamlNode->tag, valueYamlNode->start_mark), ConvertMark(valueYamlNode->start_mark)); resultStack.emplace(valueYamlNode, &childNode); } else