AAPT2: <uses-library /> and <library /> don't need to be Java packages

- Also fixes issue where a failure checking the Java package of an attribute
would yield no error message.

Bug: 70722199
Test: make aapt2_tests
Change-Id: I417d5ffe18c8d4ba2f1638f9fb9ba29278ec8e94

diff --git a/tools/aapt2/integration-tests/AutoVersionTest/AndroidManifest.xml b/tools/aapt2/integration-tests/AutoVersionTest/AndroidManifest.xml
index e66d709..dd45239 100644 (file)
--- a/tools/aapt2/integration-tests/AutoVersionTest/AndroidManifest.xml
+++ b/tools/aapt2/integration-tests/AutoVersionTest/AndroidManifest.xml
@@ -18,4 +18,7 @@
     package="com.android.aapt.autoversiontest">
 
     <uses-sdk android:minSdkVersion="7" />
+    <application>
+        <uses-library android:name="clockwork-system" />
+    </application>
 </manifest>

diff --git a/tools/aapt2/link/ManifestFixer.cpp b/tools/aapt2/link/ManifestFixer.cpp
index da05dc3..ccc3470 100644 (file)
--- a/tools/aapt2/link/ManifestFixer.cpp
+++ b/tools/aapt2/link/ManifestFixer.cpp
@@ -29,6 +29,22 @@ using android::StringPiece;
 
 namespace aapt {
 
+static bool RequiredNameIsNotEmpty(xml::Element* el, SourcePathDiagnostics* diag) {
+  xml::Attribute* attr = el->FindAttribute(xml::kSchemaAndroid, "name");
+  if (attr == nullptr) {
+    diag->Error(DiagMessage(el->line_number)
+                << "<" << el->name << "> is missing attribute 'android:name'");
+    return false;
+  }
+
+  if (attr->value.empty()) {
+    diag->Error(DiagMessage(el->line_number)
+                << "attribute 'android:name' in <" << el->name << "> tag must not be empty");
+    return false;
+  }
+  return true;
+}
+
 // This is how PackageManager builds class names from AndroidManifest.xml entries.
 static bool NameIsJavaClassName(xml::Element* el, xml::Attribute* attr,
                                 SourcePathDiagnostics* diag) {
@@ -59,21 +75,29 @@ static bool OptionalNameIsJavaClassName(xml::Element* el, SourcePathDiagnostics*
 }
 
 static bool RequiredNameIsJavaClassName(xml::Element* el, SourcePathDiagnostics* diag) {
-  if (xml::Attribute* attr = el->FindAttribute(xml::kSchemaAndroid, "name")) {
-    return NameIsJavaClassName(el, attr, diag);
+  xml::Attribute* attr = el->FindAttribute(xml::kSchemaAndroid, "name");
+  if (attr == nullptr) {
+    diag->Error(DiagMessage(el->line_number)
+                << "<" << el->name << "> is missing attribute 'android:name'");
+    return false;
   }
-  diag->Error(DiagMessage(el->line_number)
-              << "<" << el->name << "> is missing attribute 'android:name'");
-  return false;
+  return NameIsJavaClassName(el, attr, diag);
 }
 
 static bool RequiredNameIsJavaPackage(xml::Element* el, SourcePathDiagnostics* diag) {
-  if (xml::Attribute* attr = el->FindAttribute(xml::kSchemaAndroid, "name")) {
-    return util::IsJavaPackageName(attr->value);
+  xml::Attribute* attr = el->FindAttribute(xml::kSchemaAndroid, "name");
+  if (attr == nullptr) {
+    diag->Error(DiagMessage(el->line_number)
+                << "<" << el->name << "> is missing attribute 'android:name'");
+    return false;
   }
-  diag->Error(DiagMessage(el->line_number)
-              << "<" << el->name << "> is missing attribute 'android:name'");
-  return false;
+
+  if (!util::IsJavaPackageName(attr->value)) {
+    diag->Error(DiagMessage(el->line_number) << "attribute 'android:name' in <" << el->name
+                                             << "> tag must be a valid Java package name");
+    return false;
+  }
+  return true;
 }
 
 static xml::XmlNodeAction::ActionFuncWithDiag RequiredAndroidAttribute(const std::string& attr) {
@@ -213,8 +237,8 @@ bool ManifestFixer::BuildRules(xml::XmlActionExecutor* executor,
 
   // Common <intent-filter> actions.
   xml::XmlNodeAction intent_filter_action;
-  intent_filter_action["action"];
-  intent_filter_action["category"];
+  intent_filter_action["action"].Action(RequiredNameIsNotEmpty);
+  intent_filter_action["category"].Action(RequiredNameIsNotEmpty);
   intent_filter_action["data"];
 
   // Common <meta-data> actions.
@@ -317,8 +341,8 @@ bool ManifestFixer::BuildRules(xml::XmlActionExecutor* executor,
   xml::XmlNodeAction& application_action = manifest_action["application"];
   application_action.Action(OptionalNameIsJavaClassName);
 
-  application_action["uses-library"].Action(RequiredNameIsJavaPackage);
-  application_action["library"].Action(RequiredNameIsJavaPackage);
+  application_action["uses-library"].Action(RequiredNameIsNotEmpty);
+  application_action["library"].Action(RequiredNameIsNotEmpty);
 
   xml::XmlNodeAction& static_library_action = application_action["static-library"];
   static_library_action.Action(RequiredNameIsJavaPackage);

diff --git a/tools/aapt2/link/ManifestFixer_test.cpp b/tools/aapt2/link/ManifestFixer_test.cpp
index c6f895b..ed98d71 100644 (file)
--- a/tools/aapt2/link/ManifestFixer_test.cpp
+++ b/tools/aapt2/link/ManifestFixer_test.cpp
@@ -494,4 +494,34 @@ TEST_F(ManifestFixerTest, UnexpectedElementsInManifest) {
   ASSERT_THAT(manifest, IsNull());
 }
 
+
+TEST_F(ManifestFixerTest, UsesLibraryMustHaveNonEmptyName) {
+  std::string input = R"(
+      <manifest xmlns:android="http://schemas.android.com/apk/res/android"
+          package="android">
+        <application>
+          <uses-library android:name="" />
+        </application>
+      </manifest>)";
+  EXPECT_THAT(Verify(input), IsNull());
+
+  input = R"(
+      <manifest xmlns:android="http://schemas.android.com/apk/res/android"
+          package="android">
+        <application>
+          <uses-library />
+        </application>
+      </manifest>)";
+  EXPECT_THAT(Verify(input), IsNull());
+
+  input = R"(
+       <manifest xmlns:android="http://schemas.android.com/apk/res/android"
+           package="android">
+         <application>
+           <uses-library android:name="blahhh" />
+         </application>
+       </manifest>)";
+  EXPECT_THAT(Verify(input), NotNull());
+}
+
 }  // namespace aapt