AAPT2: treat manifest validation errors as warnings when asked
Bug: 65670329
Test: updated
Change-Id: Ic554cc20134fce66aa9ddf8d16ddffe0131c50e9
diff --git a/tools/aapt2/cmd/Link.cpp b/tools/aapt2/cmd/Link.cpp
index becbfc2..72e07dc 100644
--- a/tools/aapt2/cmd/Link.cpp
+++ b/tools/aapt2/cmd/Link.cpp
@@ -2121,6 +2121,9 @@
&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:<config>[,<config>[...]].\n"
diff --git a/tools/aapt2/link/ManifestFixer.cpp b/tools/aapt2/link/ManifestFixer.cpp
index a68df1d..da05dc3 100644
--- a/tools/aapt2/link/ManifestFixer.cpp
+++ b/tools/aapt2/link/ManifestFixer.cpp
@@ -430,7 +430,10 @@
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 f5715f6..0caa52e 100644
--- a/tools/aapt2/link/ManifestFixer.h
+++ b/tools/aapt2/link/ManifestFixer.h
@@ -57,6 +57,11 @@
// The version codename of the framework being compiled against to set for
// 'android:compileSdkVersionCodename' in the <manifest> tag.
Maybe<std::string> 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 1320dcd..c6f895b 100644
--- a/tools/aapt2/link/ManifestFixer_test.cpp
+++ b/tools/aapt2/link/ManifestFixer_test.cpp
@@ -112,7 +112,9 @@
}
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<xml::XmlResource> doc = VerifyWithOptions(R"EOF(
<manifest xmlns:android="http://schemas.android.com/apk/res/android"
@@ -193,7 +195,9 @@
}
TEST_F(ManifestFixerTest, UsesSdkMustComeBeforeApplication) {
- 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<xml::XmlResource> doc = VerifyWithOptions(R"EOF(
<manifest xmlns:android="http://schemas.android.com/apk/res/android"
package="android">
@@ -467,4 +471,27 @@
EXPECT_THAT(attr->value, StrEq("P"));
}
+TEST_F(ManifestFixerTest, UnexpectedElementsInManifest) {
+ std::string input = R"(
+ <manifest xmlns:android="http://schemas.android.com/apk/res/android"
+ package="android">
+ <beep/>
+ </manifest>)";
+ ManifestFixerOptions options;
+ options.warn_validation = true;
+
+ // Unexpected element should result in a warning if the flag is set to 'true'.
+ std::unique_ptr<xml::XmlResource> 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 602a902..cb844f0 100644
--- a/tools/aapt2/xml/XmlActionExecutor.cpp
+++ b/tools/aapt2/xml/XmlActionExecutor.cpp
@@ -66,7 +66,7 @@
continue;
}
- if (policy == XmlActionExecutorPolicy::kWhitelist) {
+ if (policy != XmlActionExecutorPolicy::kNone) {
DiagMessage error_msg(child_el->line_number);
error_msg << "unexpected element ";
PrintElementToDiagMessage(child_el, &error_msg);
@@ -74,8 +74,14 @@
for (const StringPiece& element : *bread_crumb) {
error_msg << "<" << element << ">";
}
- 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 df70100..f689b2a 100644
--- a/tools/aapt2/xml/XmlActionExecutor.h
+++ b/tools/aapt2/xml/XmlActionExecutor.h
@@ -34,10 +34,15 @@
// 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