[aapt2] Ensure that attributes have declared namespaces when fixing manifest
aapt::ManifestFixer might add attributes from
http://schemas.android.com/apk/res/android when none existed previously.
When that happens, something needs to ensure that the schema has a
declared xmlns prefix, otherwise this:
https://github.com/bazelbuild/bazel/blob/bdb3e97c47188ed4e5b472a3b7393e9588c93011/src/tools/android/java/com/google/devtools/build/android/aapt2/ResourceLinker.java#L560
will crash. It's a bit odd that Bazel converts back and forth between
protobuf and XML, but as long as the fields exist in Resources.proto,
they should be set.
Bug: 133004752
Change-Id: I73fb3f713be34c9f81753fde2fd24c4524656b7d
Tested: aapt2_tests
diff --git a/tools/aapt2/link/ManifestFixer.cpp b/tools/aapt2/link/ManifestFixer.cpp
index 49909f6..11150dd 100644
--- a/tools/aapt2/link/ManifestFixer.cpp
+++ b/tools/aapt2/link/ManifestFixer.cpp
@@ -214,6 +214,33 @@
return true;
}
+// Ensure that 'ns_decls' contains a declaration for 'uri', using 'prefix' as
+// the xmlns prefix if possible.
+static void EnsureNamespaceIsDeclared(const std::string& prefix, const std::string& uri,
+ std::vector<xml::NamespaceDecl>* ns_decls) {
+ if (std::find_if(ns_decls->begin(), ns_decls->end(), [&](const xml::NamespaceDecl& ns_decl) {
+ return ns_decl.uri == uri;
+ }) != ns_decls->end()) {
+ return;
+ }
+
+ std::set<std::string> used_prefixes;
+ for (const auto& ns_decl : *ns_decls) {
+ used_prefixes.insert(ns_decl.prefix);
+ }
+
+ // Make multiple attempts in the unlikely event that 'prefix' is already taken.
+ std::string disambiguator;
+ for (int i = 0; i < used_prefixes.size() + 1; i++) {
+ std::string attempted_prefix = prefix + disambiguator;
+ if (used_prefixes.find(attempted_prefix) == used_prefixes.end()) {
+ ns_decls->push_back(xml::NamespaceDecl{attempted_prefix, uri});
+ return;
+ }
+ disambiguator = std::to_string(i);
+ }
+}
+
bool ManifestFixer::BuildRules(xml::XmlActionExecutor* executor,
IDiagnostics* diag) {
// First verify some options.
@@ -262,6 +289,8 @@
manifest_action.Action(VerifyManifest);
manifest_action.Action(FixCoreAppAttribute);
manifest_action.Action([&](xml::Element* el) -> bool {
+ EnsureNamespaceIsDeclared("android", xml::kSchemaAndroid, &el->namespace_decls);
+
if (options_.version_name_default) {
if (options_.replace_version) {
el->RemoveAttribute(xml::kSchemaAndroid, "versionName");
diff --git a/tools/aapt2/link/ManifestFixer_test.cpp b/tools/aapt2/link/ManifestFixer_test.cpp
index 3f1ee36..3af06f5 100644
--- a/tools/aapt2/link/ManifestFixer_test.cpp
+++ b/tools/aapt2/link/ManifestFixer_test.cpp
@@ -727,8 +727,7 @@
}
TEST_F(ManifestFixerTest, InsertCompileSdkVersions) {
- std::string input = R"(
- <manifest xmlns:android="http://schemas.android.com/apk/res/android" package="android" />)";
+ std::string input = R"(<manifest package="com.pkg" />)";
ManifestFixerOptions options;
options.compile_sdk_version = {"28"};
options.compile_sdk_version_codename = {"P"};
@@ -736,6 +735,12 @@
std::unique_ptr<xml::XmlResource> manifest = VerifyWithOptions(input, options);
ASSERT_THAT(manifest, NotNull());
+ // There should be a declaration of kSchemaAndroid, even when the input
+ // didn't have one.
+ EXPECT_EQ(manifest->root->namespace_decls.size(), 1);
+ EXPECT_EQ(manifest->root->namespace_decls[0].prefix, "android");
+ EXPECT_EQ(manifest->root->namespace_decls[0].uri, xml::kSchemaAndroid);
+
xml::Attribute* attr = manifest->root->FindAttribute(xml::kSchemaAndroid, "compileSdkVersion");
ASSERT_THAT(attr, NotNull());
EXPECT_THAT(attr->value, StrEq("28"));
@@ -782,6 +787,27 @@
EXPECT_THAT(attr->value, StrEq("P"));
}
+TEST_F(ManifestFixerTest, AndroidPrefixAlreadyUsed) {
+ std::string input =
+ R"(<manifest package="com.pkg"
+ xmlns:android="http://schemas.android.com/apk/prv/res/android"
+ android:private_attr="foo" />)";
+ ManifestFixerOptions options;
+ options.compile_sdk_version = {"28"};
+ options.compile_sdk_version_codename = {"P"};
+
+ std::unique_ptr<xml::XmlResource> manifest = VerifyWithOptions(input, options);
+ ASSERT_THAT(manifest, NotNull());
+
+ // Make sure that we don't redefine "android".
+ EXPECT_EQ(manifest->root->namespace_decls.size(), 2);
+ EXPECT_EQ(manifest->root->namespace_decls[0].prefix, "android");
+ EXPECT_EQ(manifest->root->namespace_decls[0].uri,
+ "http://schemas.android.com/apk/prv/res/android");
+ EXPECT_EQ(manifest->root->namespace_decls[1].prefix, "android0");
+ EXPECT_EQ(manifest->root->namespace_decls[1].uri, xml::kSchemaAndroid);
+}
+
TEST_F(ManifestFixerTest, UnexpectedElementsInManifest) {
std::string input = R"(
<manifest xmlns:android="http://schemas.android.com/apk/res/android"