Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 98100c38 authored by Jake Wharton's avatar Jake Wharton
Browse files

Only keep methods with correct signature for more types 

- For transition and pathMotion attributes the method must have Context and AttributeSet parameters.
- For actionViewClass and actionProviderClass attributes the constructor must have a single Context parameter.
- For Fragment's class or name attributes the constructor must have zero parameters.

Bug: 37123156
Test: make aapt2_tests
Change-Id: I34017abd182867ba95172835051d114cb2f3b3ac
parent 55f09811

tools/aapt2/java/ProguardRules.cpp

+39 −27
Original line number Diff line number Diff line
@@ -50,11 +50,11 @@ class BaseVisitor : public xml::Visitor { 
        // This is a custom view, let's figure out the class name from this.

        std::string package = maybe_package.value().package + "." + node->name;

        if (util::IsJavaClassName(package)) {

          AddClass(node->line_number, package);

          AddClass(node->line_number, package, "...");

        }

      }

    } else if (util::IsJavaClassName(node->name)) {

      AddClass(node->line_number, node->name);

      AddClass(node->line_number, node->name, "...");

    }



    for (const auto& child : node->children) {
@@ -75,8 +75,10 @@ class BaseVisitor : public xml::Visitor { 
  ResourceFile file_;

  KeepSet* keep_set_;



  virtual void AddClass(size_t line_number, const std::string& class_name) {

    keep_set_->AddConditionalClass({file_.name, file_.source.WithLine(line_number)}, class_name);

  virtual void AddClass(size_t line_number, const std::string& class_name,

                        const std::string& ctor_signature) {

    keep_set_->AddConditionalClass({file_.name, file_.source.WithLine(line_number)},

        {class_name, ctor_signature});

  }



  void AddMethod(size_t line_number, const std::string& method_name,
@@ -106,27 +108,33 @@ class LayoutVisitor : public BaseVisitor { 
  }



  void Visit(xml::Element* node) override {

    bool check_class = false;

    bool check_name = false;

    bool is_view = false;

    bool is_fragment = false;

    if (node->namespace_uri.empty()) {

      if (node->name == "view") {

        check_class = true;

        is_view = true;

      } else if (node->name == "fragment") {

        check_class = check_name = true;

        is_fragment = true;

      }

    } else if (node->namespace_uri == xml::kSchemaAndroid) {

      check_name = node->name == "fragment";

      is_fragment = node->name == "fragment";

    }



    for (const auto& attr : node->attributes) {

      if (check_class && attr.namespace_uri.empty() && attr.name == "class" &&

          util::IsJavaClassName(attr.value)) {

        AddClass(node->line_number, attr.value);

      } else if (check_name && attr.namespace_uri == xml::kSchemaAndroid &&

                 attr.name == "name" && util::IsJavaClassName(attr.value)) {

        AddClass(node->line_number, attr.value);

      } else if (attr.namespace_uri == xml::kSchemaAndroid &&

                 attr.name == "onClick") {

      if (attr.namespace_uri.empty() && attr.name == "class") {

        if (util::IsJavaClassName(attr.value)) {

          if (is_view) {

            AddClass(node->line_number, attr.value,

                "android.content.Context, android.util.AttributeSet");

          } else if (is_fragment) {

            AddClass(node->line_number, attr.value, "");

          }

        }

      } else if (attr.namespace_uri == xml::kSchemaAndroid && attr.name == "name") {

        if (is_fragment && util::IsJavaClassName(attr.value)) {

          AddClass(node->line_number, attr.value, "");

        }

      } else if (attr.namespace_uri == xml::kSchemaAndroid && attr.name == "onClick") {

        AddMethod(node->line_number, attr.value, "android.view.View");

      }

    }
@@ -149,7 +157,7 @@ class MenuVisitor : public BaseVisitor { 
        if (attr.namespace_uri == xml::kSchemaAndroid) {

          if ((attr.name == "actionViewClass" || attr.name == "actionProviderClass") &&

              util::IsJavaClassName(attr.value)) {

            AddClass(node->line_number, attr.value);

            AddClass(node->line_number, attr.value, "android.content.Context");

          } else if (attr.name == "onClick") {

            AddMethod(node->line_number, attr.value, "android.view.MenuItem");

          }
@@ -180,7 +188,7 @@ class XmlResourceVisitor : public BaseVisitor { 
      xml::Attribute* attr =

          node->FindAttribute(xml::kSchemaAndroid, "fragment");

      if (attr && util::IsJavaClassName(attr->value)) {

        AddClass(node->line_number, attr->value);

        AddClass(node->line_number, attr->value, "");

      }

    }
@@ -202,7 +210,7 @@ class NavigationVisitor : public BaseVisitor { 
    if (attr != nullptr && !attr->value.empty()) {

      std::string name = (attr->value[0] == '.') ? package_ + attr->value : attr->value;

      if (util::IsJavaClassName(name)) {

        AddClass(node->line_number, name);

        AddClass(node->line_number, name, "...");

      }

    }
@@ -225,7 +233,8 @@ class TransitionVisitor : public BaseVisitor { 
    if (check_class) {

      xml::Attribute* attr = node->FindAttribute({}, "class");

      if (attr && util::IsJavaClassName(attr->value)) {

        AddClass(node->line_number, attr->value);

        AddClass(node->line_number, attr->value,

            "android.content.Context, android.util.AttributeSet");

      }

    }
@@ -256,14 +265,14 @@ class ManifestVisitor : public BaseVisitor { 
        if (attr) {

          Maybe<std::string> result = util::GetFullyQualifiedClassName(package_, attr->value);

          if (result) {

            AddClass(node->line_number, result.value());

            AddClass(node->line_number, result.value(), "");

          }

        }

        attr = node->FindAttribute(xml::kSchemaAndroid, "appComponentFactory");

        if (attr) {

          Maybe<std::string> result = util::GetFullyQualifiedClassName(package_, attr->value);

          if (result) {

            AddClass(node->line_number, result.value());

            AddClass(node->line_number, result.value(), "");

          }

        }

        if (main_dex_only_) {
@@ -294,7 +303,7 @@ class ManifestVisitor : public BaseVisitor { 
        if (get_name) {

          Maybe<std::string> result = util::GetFullyQualifiedClassName(package_, attr->value);

          if (result) {

            AddClass(node->line_number, result.value());

            AddClass(node->line_number, result.value(), "");

          }

        }

      }
@@ -302,7 +311,8 @@ class ManifestVisitor : public BaseVisitor { 
    BaseVisitor::Visit(node);

  }



  virtual void AddClass(size_t line_number, const std::string& class_name) override {

  virtual void AddClass(size_t line_number, const std::string& class_name,

                        const std::string& ctor_signature) override {

    keep_set_->AddManifestClass({file_.name, file_.source.WithLine(line_number)}, class_name);

  }
@@ -390,13 +400,15 @@ void WriteKeepSet(const KeepSet& keep_set, OutputStream* out) { 
        printer.Print("-if class **.R$layout { int ")

            .Print(JavaClassGenerator::TransformToFieldName(location.name.entry))

            .Println("; }");

        printer.Print("-keep class ").Print(entry.first).Println(" { <init>(...); }");

        printer.Print("-keep class ").Print(entry.first.name).Print(" { <init>(")

            .Print(entry.first.signature).Println("); }");

      }

    } else {

      for (const UsageLocation& location : entry.second) {

        printer.Print("# Referenced at ").Println(location.source.to_string());

      }

      printer.Print("-keep class ").Print(entry.first).Println(" { <init>(...); }");

      printer.Print("-keep class ").Print(entry.first.name).Print(" { <init>(")

          .Print(entry.first.signature).Println("); }");

    }

    printer.Println();

  }

tools/aapt2/java/ProguardRules.h

+4 −3
Original line number Diff line number Diff line
@@ -56,8 +56,9 @@ class KeepSet { 
    manifest_class_set_[class_name].insert(file);

  }



  inline void AddConditionalClass(const UsageLocation& file, const std::string& class_name) {

    conditional_class_set_[class_name].insert(file);

  inline void AddConditionalClass(const UsageLocation& file,

                                  const NameAndSignature& class_and_signature) {

    conditional_class_set_[class_and_signature].insert(file);

  }



  inline void AddMethod(const UsageLocation& file, const NameAndSignature& name_and_signature) {
@@ -77,7 +78,7 @@ class KeepSet { 
  bool conditional_keep_rules_ = false;

  std::map<std::string, std::set<UsageLocation>> manifest_class_set_;

  std::map<NameAndSignature, std::set<UsageLocation>> method_set_;

  std::map<std::string, std::set<UsageLocation>> conditional_class_set_;

  std::map<NameAndSignature, std::set<UsageLocation>> conditional_class_set_;

  std::map<ResourceName, std::set<UsageLocation>> reference_set_;

};

tools/aapt2/java/ProguardRules_test.cpp

+10 −8
Original line number Diff line number Diff line
@@ -77,7 +77,7 @@ TEST(ProguardRulesTest, FragmentNameRuleIsEmitted) { 


  std::string actual = GetKeepSetString(set);



  EXPECT_THAT(actual, HasSubstr("-keep class com.foo.Bar { <init>(...); }"));

  EXPECT_THAT(actual, HasSubstr("-keep class com.foo.Bar { <init>(); }"));

}



TEST(ProguardRulesTest, FragmentClassRuleIsEmitted) {
@@ -91,7 +91,7 @@ TEST(ProguardRulesTest, FragmentClassRuleIsEmitted) { 


  std::string actual = GetKeepSetString(set);



  EXPECT_THAT(actual, HasSubstr("-keep class com.foo.Bar { <init>(...); }"));

  EXPECT_THAT(actual, HasSubstr("-keep class com.foo.Bar { <init>(); }"));

}



TEST(ProguardRulesTest, FragmentNameAndClassRulesAreEmitted) {
@@ -107,8 +107,8 @@ TEST(ProguardRulesTest, FragmentNameAndClassRulesAreEmitted) { 


  std::string actual = GetKeepSetString(set);



  EXPECT_THAT(actual, HasSubstr("-keep class com.foo.Bar { <init>(...); }"));

  EXPECT_THAT(actual, HasSubstr("-keep class com.foo.Baz { <init>(...); }"));

  EXPECT_THAT(actual, HasSubstr("-keep class com.foo.Bar { <init>(); }"));

  EXPECT_THAT(actual, HasSubstr("-keep class com.foo.Baz { <init>(); }"));

}



TEST(ProguardRulesTest, NavigationFragmentNameAndClassRulesAreEmitted) {
@@ -267,8 +267,8 @@ TEST(ProguardRulesTest, MenuRulesAreEmitted) { 


  EXPECT_THAT(actual, HasSubstr(

      "-keepclassmembers class * { *** on_click(android.view.MenuItem); }"));

  EXPECT_THAT(actual, HasSubstr("-keep class com.foo.Bar { <init>(...); }"));

  EXPECT_THAT(actual, HasSubstr("-keep class com.foo.Baz { <init>(...); }"));

  EXPECT_THAT(actual, HasSubstr("-keep class com.foo.Bar { <init>(android.content.Context); }"));

  EXPECT_THAT(actual, HasSubstr("-keep class com.foo.Baz { <init>(android.content.Context); }"));

  EXPECT_THAT(actual, Not(HasSubstr("com.foo.Bat")));

}
@@ -285,7 +285,8 @@ TEST(ProguardRulesTest, TransitionPathMotionRulesAreEmitted) { 


  std::string actual = GetKeepSetString(set);



  EXPECT_THAT(actual, HasSubstr("-keep class com.foo.Bar { <init>(...); }"));

  EXPECT_THAT(actual, HasSubstr(

      "-keep class com.foo.Bar { <init>(android.content.Context, android.util.AttributeSet); }"));

}



TEST(ProguardRulesTest, TransitionRulesAreEmitted) {
@@ -301,7 +302,8 @@ TEST(ProguardRulesTest, TransitionRulesAreEmitted) { 


  std::string actual = GetKeepSetString(set);



  EXPECT_THAT(actual, HasSubstr("-keep class com.foo.Bar { <init>(...); }"));

  EXPECT_THAT(actual, HasSubstr(

      "-keep class com.foo.Bar { <init>(android.content.Context, android.util.AttributeSet); }"));

}



}  // namespace aapt