From ad9e1324ff2c459d0ee6ee571d4a3e458c02cc81 Mon Sep 17 00:00:00 2001 From: Izabela Orlowska Date: Tue, 19 Dec 2017 16:22:42 +0000 Subject: [PATCH] AAPT2: treat manifest validation errors as warnings when asked Bug: 65670329 Test: updated Change-Id: Ic554cc20134fce66aa9ddf8d16ddffe0131c50e9 --- tools/aapt2/cmd/Link.cpp | 3 +++ tools/aapt2/link/ManifestFixer.cpp | 5 +++- tools/aapt2/link/ManifestFixer.h | 5 ++++ tools/aapt2/link/ManifestFixer_test.cpp | 31 +++++++++++++++++++++++-- tools/aapt2/xml/XmlActionExecutor.cpp | 12 +++++++--- tools/aapt2/xml/XmlActionExecutor.h | 9 +++++-- 6 files changed, 57 insertions(+), 8 deletions(-) diff --git a/tools/aapt2/cmd/Link.cpp b/tools/aapt2/cmd/Link.cpp index becbfc255ed14..72e07dc237252 100644 --- a/tools/aapt2/cmd/Link.cpp +++ b/tools/aapt2/cmd/Link.cpp @@ -2121,6 +2121,9 @@ int Link(const std::vector& args, IDiagnostics* diagnostics) { &options.manifest_fixer_options.rename_instrumentation_target_package) .OptionalFlagList("-0", "File extensions not to compress.", &options.extensions_to_not_compress) + .OptionalSwitch("--warn-manifest-validation", + "Treat manifest validation errors as warnings.", + &options.manifest_fixer_options.warn_validation) .OptionalFlagList("--split", "Split resources matching a set of configs out to a Split APK.\n" "Syntax: path/to/output.apk:[,[...]].\n" diff --git a/tools/aapt2/link/ManifestFixer.cpp b/tools/aapt2/link/ManifestFixer.cpp index a68df1dbc998d..da05dc328f7f6 100644 --- a/tools/aapt2/link/ManifestFixer.cpp +++ b/tools/aapt2/link/ManifestFixer.cpp @@ -430,7 +430,10 @@ bool ManifestFixer::Consume(IAaptContext* context, xml::XmlResource* doc) { return false; } - if (!executor.Execute(xml::XmlActionExecutorPolicy::kWhitelist, context->GetDiagnostics(), doc)) { + xml::XmlActionExecutorPolicy policy = options_.warn_validation + ? xml::XmlActionExecutorPolicy::kWhitelistWarning + : xml::XmlActionExecutorPolicy::kWhitelist; + if (!executor.Execute(policy, context->GetDiagnostics(), doc)) { return false; } diff --git a/tools/aapt2/link/ManifestFixer.h b/tools/aapt2/link/ManifestFixer.h index f5715f605b041..0caa52eaf6509 100644 --- a/tools/aapt2/link/ManifestFixer.h +++ b/tools/aapt2/link/ManifestFixer.h @@ -57,6 +57,11 @@ struct ManifestFixerOptions { // The version codename of the framework being compiled against to set for // 'android:compileSdkVersionCodename' in the tag. Maybe compile_sdk_version_codename; + + // Wether validation errors should be treated only as warnings. If this is 'true', then an + // incorrect node will not result in an error, but only as a warning, and the parsing will + // continue. + bool warn_validation = false; }; // Verifies that the manifest is correctly formed and inserts defaults where specified with diff --git a/tools/aapt2/link/ManifestFixer_test.cpp b/tools/aapt2/link/ManifestFixer_test.cpp index 1320dcd2a170e..c6f895bb52ddf 100644 --- a/tools/aapt2/link/ManifestFixer_test.cpp +++ b/tools/aapt2/link/ManifestFixer_test.cpp @@ -112,7 +112,9 @@ TEST_F(ManifestFixerTest, AllowMetaData) { } TEST_F(ManifestFixerTest, UseDefaultSdkVersionsIfNonePresent) { - ManifestFixerOptions options = {std::string("8"), std::string("22")}; + ManifestFixerOptions options; + options.min_sdk_version_default = std::string("8"); + options.target_sdk_version_default = std::string("22"); std::unique_ptr doc = VerifyWithOptions(R"EOF( doc = VerifyWithOptions(R"EOF( @@ -467,4 +471,27 @@ TEST_F(ManifestFixerTest, InsertCompileSdkVersions) { EXPECT_THAT(attr->value, StrEq("P")); } +TEST_F(ManifestFixerTest, UnexpectedElementsInManifest) { + std::string input = R"( + + + )"; + ManifestFixerOptions options; + options.warn_validation = true; + + // Unexpected element should result in a warning if the flag is set to 'true'. + std::unique_ptr manifest = VerifyWithOptions(input, options); + ASSERT_THAT(manifest, NotNull()); + + // Unexpected element should result in an error if the flag is set to 'false'. + options.warn_validation = false; + manifest = VerifyWithOptions(input, options); + ASSERT_THAT(manifest, IsNull()); + + // By default the flag should be set to 'false'. + manifest = Verify(input); + ASSERT_THAT(manifest, IsNull()); +} + } // namespace aapt diff --git a/tools/aapt2/xml/XmlActionExecutor.cpp b/tools/aapt2/xml/XmlActionExecutor.cpp index 602a902bfc3e8..cb844f085ecc2 100644 --- a/tools/aapt2/xml/XmlActionExecutor.cpp +++ b/tools/aapt2/xml/XmlActionExecutor.cpp @@ -66,7 +66,7 @@ bool XmlNodeAction::Execute(XmlActionExecutorPolicy policy, std::vectorline_number); error_msg << "unexpected element "; PrintElementToDiagMessage(child_el, &error_msg); @@ -74,8 +74,14 @@ bool XmlNodeAction::Execute(XmlActionExecutorPolicy policy, std::vector"; } - diag->Error(error_msg); - error = true; + if (policy == XmlActionExecutorPolicy::kWhitelistWarning) { + // Treat the error only as a warning. + diag->Warn(error_msg); + } else { + // Policy is XmlActionExecutorPolicy::kWhitelist, we should fail. + diag->Error(error_msg); + error = true; + } } } } diff --git a/tools/aapt2/xml/XmlActionExecutor.h b/tools/aapt2/xml/XmlActionExecutor.h index df70100e882a5..f689b2a3eaa85 100644 --- a/tools/aapt2/xml/XmlActionExecutor.h +++ b/tools/aapt2/xml/XmlActionExecutor.h @@ -34,10 +34,15 @@ enum class XmlActionExecutorPolicy { // Actions are run if elements are matched, errors occur only when actions return false. kNone, - // The actions defined must match and run. If an element is found that does - // not match an action, an error occurs. + // The actions defined must match and run. If an element is found that does not match an action, + // an error occurs. // Note: namespaced elements are always ignored. kWhitelist, + + // The actions defined should match and run. if an element is found that does not match an + // action, a warning is printed. + // Note: namespaced elements are always ignored. + kWhitelistWarning, }; // Contains the actions to perform at this XML node. This is a recursive data structure that