From 7e5236dc563595f1dca7ed3e6cb87b6ce995c402 Mon Sep 17 00:00:00 2001 From: Ryan Mitchell Date: Tue, 25 Sep 2018 15:20:59 -0700 Subject: Restore Proguard behavior and add minimal flag We previously changed AAPT2 to correctly only generate keep rules for the constructors required to inflate the different views. This cause projects that did not have keep rules for the other constructors that were accessed through reflection to have runtime crashes. This change adds a flag to the link stage (--proguard-minimal-keep-rules) that allows AAPT2 to only keep the constructors required for layout inflation. If the flag is not present, then AAPT2 will generate less specific keep rules than keep all constructors. Bug: 116201243 Test: aapt2_tests Change-Id: I8bb5cdf8446518ab153ea988e1243ca9494258c7 --- tools/aapt2/java/ProguardRules.cpp | 14 +++-- tools/aapt2/java/ProguardRules.h | 4 +- tools/aapt2/java/ProguardRules_test.cpp | 94 +++++++++++++++++++++++++-------- 3 files changed, 83 insertions(+), 29 deletions(-) (limited to 'tools/aapt2/java') diff --git a/tools/aapt2/java/ProguardRules.cpp b/tools/aapt2/java/ProguardRules.cpp index d40795accf79..52e168ed47aa 100644 --- a/tools/aapt2/java/ProguardRules.cpp +++ b/tools/aapt2/java/ProguardRules.cpp @@ -384,7 +384,7 @@ bool CollectProguardRules(IAaptContext* context_, xml::XmlResource* res, KeepSet return true; } -void WriteKeepSet(const KeepSet& keep_set, OutputStream* out) { +void WriteKeepSet(const KeepSet& keep_set, OutputStream* out, bool minimal_keep) { Printer printer(out); for (const auto& entry : keep_set.manifest_class_set_) { for (const UsageLocation& location : entry.second) { @@ -406,15 +406,19 @@ 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.name).Print(" { (") - .Print(entry.first.signature).Println("); }"); + + printer.Print("-keep class ").Print(entry.first.name).Print(" { ("); + printer.Print((minimal_keep) ? entry.first.signature : "..."); + printer.Println("); }"); } } else { for (const UsageLocation& location : entry.second) { printer.Print("# Referenced at ").Println(location.source.to_string()); } - printer.Print("-keep class ").Print(entry.first.name).Print(" { (") - .Print(entry.first.signature).Println("); }"); + + printer.Print("-keep class ").Print(entry.first.name).Print(" { ("); + printer.Print((minimal_keep) ? entry.first.signature : "..."); + printer.Println("); }"); } printer.Println(); } diff --git a/tools/aapt2/java/ProguardRules.h b/tools/aapt2/java/ProguardRules.h index 01dad0b08aea..38b4860d1d61 100644 --- a/tools/aapt2/java/ProguardRules.h +++ b/tools/aapt2/java/ProguardRules.h @@ -70,7 +70,7 @@ class KeepSet { } private: - friend void WriteKeepSet(const KeepSet& keep_set, io::OutputStream* out); + friend void WriteKeepSet(const KeepSet& keep_set, io::OutputStream* out, bool minimal_keep); friend bool CollectLocations(const UsageLocation& location, const KeepSet& keep_set, std::set* locations); @@ -89,7 +89,7 @@ bool CollectProguardRules(IAaptContext* context, xml::XmlResource* res, KeepSet* bool CollectResourceReferences(IAaptContext* context, ResourceTable* table, KeepSet* keep_set); -void WriteKeepSet(const KeepSet& keep_set, io::OutputStream* out); +void WriteKeepSet(const KeepSet& keep_set, io::OutputStream* out, bool minimal_keep); bool CollectLocations(const UsageLocation& location, const KeepSet& keep_set, std::set* locations); diff --git a/tools/aapt2/java/ProguardRules_test.cpp b/tools/aapt2/java/ProguardRules_test.cpp index 83c72d89bb62..3d93cb1dd43b 100644 --- a/tools/aapt2/java/ProguardRules_test.cpp +++ b/tools/aapt2/java/ProguardRules_test.cpp @@ -26,10 +26,10 @@ using ::testing::Not; namespace aapt { -std::string GetKeepSetString(const proguard::KeepSet& set) { +std::string GetKeepSetString(const proguard::KeepSet& set, bool minimal_rules) { std::string out; StringOutputStream sout(&out); - proguard::WriteKeepSet(set, &sout); + proguard::WriteKeepSet(set, &sout, minimal_rules); sout.Flush(); return out; } @@ -53,8 +53,17 @@ TEST(ProguardRulesTest, ManifestRuleDefaultConstructorOnly) { proguard::KeepSet set; ASSERT_TRUE(proguard::CollectProguardRulesForManifest(manifest.get(), &set, false)); - std::string actual = GetKeepSetString(set); + std::string actual = GetKeepSetString(set, /** minimal_rules */ false); + EXPECT_THAT(actual, HasSubstr("-keep class com.foo.BarAppComponentFactory { (); }")); + EXPECT_THAT(actual, HasSubstr("-keep class com.foo.BarBackupAgent { (); }")); + EXPECT_THAT(actual, HasSubstr("-keep class com.foo.BarApplication { (); }")); + EXPECT_THAT(actual, HasSubstr("-keep class com.foo.BarActivity { (); }")); + EXPECT_THAT(actual, HasSubstr("-keep class com.foo.BarService { (); }")); + EXPECT_THAT(actual, HasSubstr("-keep class com.foo.BarReceiver { (); }")); + EXPECT_THAT(actual, HasSubstr("-keep class com.foo.BarProvider { (); }")); + EXPECT_THAT(actual, HasSubstr("-keep class com.foo.BarInstrumentation { (); }")); + actual = GetKeepSetString(set, /** minimal_rules */ true); EXPECT_THAT(actual, HasSubstr("-keep class com.foo.BarAppComponentFactory { (); }")); EXPECT_THAT(actual, HasSubstr("-keep class com.foo.BarBackupAgent { (); }")); EXPECT_THAT(actual, HasSubstr("-keep class com.foo.BarApplication { (); }")); @@ -75,8 +84,10 @@ TEST(ProguardRulesTest, FragmentNameRuleIsEmitted) { proguard::KeepSet set; ASSERT_TRUE(proguard::CollectProguardRules(context.get(), layout.get(), &set)); - std::string actual = GetKeepSetString(set); + std::string actual = GetKeepSetString(set, /** minimal_rules */ false); + EXPECT_THAT(actual, HasSubstr("-keep class com.foo.Bar { (...); }")); + actual = GetKeepSetString(set, /** minimal_rules */ true); EXPECT_THAT(actual, HasSubstr("-keep class com.foo.Bar { (); }")); } @@ -89,8 +100,10 @@ TEST(ProguardRulesTest, FragmentClassRuleIsEmitted) { proguard::KeepSet set; ASSERT_TRUE(proguard::CollectProguardRules(context.get(), layout.get(), &set)); - std::string actual = GetKeepSetString(set); + std::string actual = GetKeepSetString(set, /** minimal_rules */ false); + EXPECT_THAT(actual, HasSubstr("-keep class com.foo.Bar { (...); }")); + actual = GetKeepSetString(set, /** minimal_rules */ true); EXPECT_THAT(actual, HasSubstr("-keep class com.foo.Bar { (); }")); } @@ -105,8 +118,11 @@ TEST(ProguardRulesTest, FragmentNameAndClassRulesAreEmitted) { proguard::KeepSet set; ASSERT_TRUE(proguard::CollectProguardRules(context.get(), layout.get(), &set)); - std::string actual = GetKeepSetString(set); + std::string actual = GetKeepSetString(set, /** minimal_rules */ false); + EXPECT_THAT(actual, HasSubstr("-keep class com.foo.Bar { (...); }")); + EXPECT_THAT(actual, HasSubstr("-keep class com.foo.Baz { (...); }")); + actual = GetKeepSetString(set, /** minimal_rules */ true); EXPECT_THAT(actual, HasSubstr("-keep class com.foo.Bar { (); }")); EXPECT_THAT(actual, HasSubstr("-keep class com.foo.Baz { (); }")); } @@ -133,7 +149,12 @@ TEST(ProguardRulesTest, NavigationFragmentNameAndClassRulesAreEmitted) { proguard::KeepSet set; ASSERT_TRUE(proguard::CollectProguardRules(context.get(), navigation.get(), &set)); - std::string actual = GetKeepSetString(set); + std::string actual = GetKeepSetString(set, /** minimal_rules */ false); + EXPECT_THAT(actual, HasSubstr("-keep class com.package.Foo { (...); }")); + EXPECT_THAT(actual, HasSubstr("-keep class com.package.Bar { (...); }")); + EXPECT_THAT(actual, HasSubstr("-keep class com.base.Nested { (...); }")); + + actual = GetKeepSetString(set, /** minimal_rules */ true); EXPECT_THAT(actual, HasSubstr("-keep class com.package.Foo { (...); }")); EXPECT_THAT(actual, HasSubstr("-keep class com.package.Bar { (...); }")); EXPECT_THAT(actual, HasSubstr("-keep class com.base.Nested { (...); }")); @@ -150,8 +171,10 @@ TEST(ProguardRulesTest, CustomViewRulesAreEmitted) { proguard::KeepSet set; ASSERT_TRUE(proguard::CollectProguardRules(context.get(), layout.get(), &set)); - std::string actual = GetKeepSetString(set); + std::string actual = GetKeepSetString(set, /** minimal_rules */ false); + EXPECT_THAT(actual, HasSubstr("-keep class com.foo.Bar { (...); }")); + actual = GetKeepSetString(set, /** minimal_rules */ true); EXPECT_THAT(actual, HasSubstr( "-keep class com.foo.Bar { (android.content.Context, android.util.AttributeSet); }")); } @@ -188,11 +211,16 @@ TEST(ProguardRulesTest, IncludedLayoutRulesAreConditional) { ASSERT_TRUE(proguard::CollectProguardRules(context.get(), bar_layout.get(), &set)); ASSERT_TRUE(proguard::CollectProguardRules(context.get(), foo_layout.get(), &set)); - std::string actual = GetKeepSetString(set); + std::string actual = GetKeepSetString(set, /** minimal_rules */ false); + EXPECT_THAT(actual, HasSubstr("-if class **.R$layout")); + EXPECT_THAT(actual, HasSubstr("-keep class com.foo.Bar { (...); }")); + EXPECT_THAT(actual, HasSubstr("int foo")); + EXPECT_THAT(actual, HasSubstr("int bar")); + actual = GetKeepSetString(set, /** minimal_rules */ true); EXPECT_THAT(actual, HasSubstr("-if class **.R$layout")); EXPECT_THAT(actual, HasSubstr( - "-keep class com.foo.Bar { (android.content.Context, android.util.AttributeSet); }")); + "-keep class com.foo.Bar { (android.content.Context, android.util.AttributeSet); }")); EXPECT_THAT(actual, HasSubstr("int foo")); EXPECT_THAT(actual, HasSubstr("int bar")); } @@ -209,10 +237,16 @@ TEST(ProguardRulesTest, AliasedLayoutRulesAreConditional) { set.AddReference({test::ParseNameOrDie("layout/bar"), {}}, layout->file.name); ASSERT_TRUE(proguard::CollectProguardRules(context.get(), layout.get(), &set)); - std::string actual = GetKeepSetString(set); + std::string actual = GetKeepSetString(set, /** minimal_rules */ false); + EXPECT_THAT(actual, HasSubstr( + "-keep class com.foo.Bar { (...); }")); + EXPECT_THAT(actual, HasSubstr("-if class **.R$layout")); + EXPECT_THAT(actual, HasSubstr("int foo")); + EXPECT_THAT(actual, HasSubstr("int bar")); + actual = GetKeepSetString(set, /** minimal_rules */ true); EXPECT_THAT(actual, HasSubstr( - "-keep class com.foo.Bar { (android.content.Context, android.util.AttributeSet); }")); + "-keep class com.foo.Bar { (android.content.Context, android.util.AttributeSet); }")); EXPECT_THAT(actual, HasSubstr("-if class **.R$layout")); EXPECT_THAT(actual, HasSubstr("int foo")); EXPECT_THAT(actual, HasSubstr("int bar")); @@ -230,11 +264,14 @@ TEST(ProguardRulesTest, NonLayoutReferencesAreUnconditional) { set.AddReference({test::ParseNameOrDie("style/MyStyle"), {}}, layout->file.name); ASSERT_TRUE(proguard::CollectProguardRules(context.get(), layout.get(), &set)); - std::string actual = GetKeepSetString(set); + std::string actual = GetKeepSetString(set, /** minimal_rules */ false); + EXPECT_THAT(actual, Not(HasSubstr("-if"))); + EXPECT_THAT(actual, HasSubstr("-keep class com.foo.Bar { (...); }")); + actual = GetKeepSetString(set, /** minimal_rules */ true); EXPECT_THAT(actual, Not(HasSubstr("-if"))); EXPECT_THAT(actual, HasSubstr( - "-keep class com.foo.Bar { (android.content.Context, android.util.AttributeSet); }")); + "-keep class com.foo.Bar { (android.content.Context, android.util.AttributeSet); }")); } TEST(ProguardRulesTest, ViewOnClickRuleIsEmitted) { @@ -247,10 +284,13 @@ TEST(ProguardRulesTest, ViewOnClickRuleIsEmitted) { proguard::KeepSet set; ASSERT_TRUE(proguard::CollectProguardRules(context.get(), layout.get(), &set)); - std::string actual = GetKeepSetString(set); - + std::string actual = GetKeepSetString(set, /** minimal_rules */ false); EXPECT_THAT(actual, HasSubstr( "-keepclassmembers class * { *** bar_method(android.view.View); }")); + + actual = GetKeepSetString(set, /** minimal_rules */ true); + EXPECT_THAT(actual, HasSubstr( + "-keepclassmembers class * { *** bar_method(android.view.View); }")); } TEST(ProguardRulesTest, MenuRulesAreEmitted) { @@ -267,10 +307,16 @@ TEST(ProguardRulesTest, MenuRulesAreEmitted) { proguard::KeepSet set; ASSERT_TRUE(proguard::CollectProguardRules(context.get(), menu.get(), &set)); - std::string actual = GetKeepSetString(set); + std::string actual = GetKeepSetString(set, /** minimal_rules */ false); + EXPECT_THAT(actual, HasSubstr( + "-keepclassmembers class * { *** on_click(android.view.MenuItem); }")); + EXPECT_THAT(actual, HasSubstr("-keep class com.foo.Bar { (...); }")); + EXPECT_THAT(actual, HasSubstr("-keep class com.foo.Baz { (...); }")); + EXPECT_THAT(actual, Not(HasSubstr("com.foo.Bat"))); + actual = GetKeepSetString(set, /** minimal_rules */ true); EXPECT_THAT(actual, HasSubstr( - "-keepclassmembers class * { *** on_click(android.view.MenuItem); }")); + "-keepclassmembers class * { *** on_click(android.view.MenuItem); }")); EXPECT_THAT(actual, HasSubstr("-keep class com.foo.Bar { (android.content.Context); }")); EXPECT_THAT(actual, HasSubstr("-keep class com.foo.Baz { (android.content.Context); }")); EXPECT_THAT(actual, Not(HasSubstr("com.foo.Bat"))); @@ -287,10 +333,12 @@ TEST(ProguardRulesTest, TransitionPathMotionRulesAreEmitted) { proguard::KeepSet set; ASSERT_TRUE(proguard::CollectProguardRules(context.get(), transition.get(), &set)); - std::string actual = GetKeepSetString(set); + std::string actual = GetKeepSetString(set, /** minimal_rules */ false); + EXPECT_THAT(actual, HasSubstr("-keep class com.foo.Bar { (...); }")); + actual = GetKeepSetString(set, /** minimal_rules */ true); EXPECT_THAT(actual, HasSubstr( - "-keep class com.foo.Bar { (android.content.Context, android.util.AttributeSet); }")); + "-keep class com.foo.Bar { (android.content.Context, android.util.AttributeSet); }")); } TEST(ProguardRulesTest, TransitionRulesAreEmitted) { @@ -304,10 +352,12 @@ TEST(ProguardRulesTest, TransitionRulesAreEmitted) { proguard::KeepSet set; ASSERT_TRUE(proguard::CollectProguardRules(context.get(), transitionSet.get(), &set)); - std::string actual = GetKeepSetString(set); + std::string actual = GetKeepSetString(set, /** minimal_rules */ false); + EXPECT_THAT(actual, HasSubstr("-keep class com.foo.Bar { (...); }")); + actual = GetKeepSetString(set, /** minimal_rules */ true); EXPECT_THAT(actual, HasSubstr( - "-keep class com.foo.Bar { (android.content.Context, android.util.AttributeSet); }")); + "-keep class com.foo.Bar { (android.content.Context, android.util.AttributeSet); }")); } } // namespace aapt -- cgit v1.2.3-59-g8ed1b