-
Notifications
You must be signed in to change notification settings - Fork 4.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Adjust ProGuard default rules and shrinking tests #2420
Changes from 2 commits
d8d2471
b932a65
a206d90
91772ee
6feba4c
f5957a2
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -313,6 +313,8 @@ Note: For newer Gson versions these rules might be applied automatically; make s | |
|
||
**Symptom:** A `JsonIOException` with the message 'Abstract classes can't be instantiated!' is thrown; the class mentioned in the exception message is not actually `abstract` in your source code, and you are using the code shrinking tool R8 (Android app builds normally have this configured by default). | ||
|
||
Note: If the class which you are trying to deserialize is actually abstract, then this exception is probably unrelated to R8 and you will have to implement a custom [`InstanceCreator`](https://javadoc.io/doc/com.google.code.gson/gson/latest/com.google.gson/com/google/gson/InstanceCreator.html) or [`TypeAdapter`](https://javadoc.io/doc/com.google.code.gson/gson/latest/com.google.gson/com/google/gson/TypeAdapter.html) which creates an instance of a non-abstract subclass of the class. | ||
|
||
**Reason:** The code shrinking tool R8 performs optimizations where it removes the no-args constructor from a class and makes the class `abstract`. Due to this Gson cannot create an instance of the class. | ||
|
||
**Solution:** Make sure the class has a no-args constructor, then adjust your R8 configuration file to keep the constructor of the class. For example: | ||
|
@@ -324,6 +326,8 @@ Note: For newer Gson versions these rules might be applied automatically; make s | |
} | ||
``` | ||
|
||
You can also use `<init>(...);` to keep all constructors of that class, but then you might actually rely on JDK Unsafe to create classes without no-args constructor, see [`GsonBuilder.disableJdkUnsafe()`](https://javadoc.io/doc/com.google.code.gson/gson/latest/com.google.gson/com/google/gson/GsonBuilder.html#disableJdkUnsafe()) for more information. | ||
|
||
For Android you can add this rule to the `proguard-rules.pro` file, see also the [Android documentation](https://developer.android.com/build/shrink-code#keep-code). In case the class name in the exception message is obfuscated, see the Android documentation about [retracing](https://developer.android.com/build/shrink-code#retracing). | ||
|
||
Note: If the class which you are trying to deserialize is actually abstract, then this exception is probably unrelated to R8 and you will have to implement a custom [`InstanceCreator`](https://javadoc.io/doc/com.google.code.gson/gson/latest/com.google.gson/com/google/gson/InstanceCreator.html) or [`TypeAdapter`](https://javadoc.io/doc/com.google.code.gson/gson/latest/com.google.gson/com/google/gson/TypeAdapter.html) which creates an instance of a non-abstract subclass of the class. | ||
For Android you can alternatively use the [`@Keep` annotation](https://developer.android.com/studio/write/annotations#keep) on the class or constructor you want to keep. That might be easier than having to maintain a custom R8 configuration. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is not only for Android. You can also use R8 (or ProGuard) to shrink class files. Also please change There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Maybe this paragraph is worded a bit misleading then. What I meant was that Android developers can use Android's Because this is also written in the context of Android, I am not sure if it is necessary to specify the fully qualified package name. I also preferred linking to the Android Developers page instead of the Javadoc for that There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks, linking to DAC is a good idea, and also keeping Android is actually fine, as the annotation is part of an Android library. |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -15,3 +15,6 @@ tools. Have a look at R8's Compatibility FAQ, and especially at the [Gson sectio | |
Note that newer Gson versions apply some of the rules shown in `proguard.cfg` automatically by default, | ||
see the file [`gson/META-INF/proguard/gson.pro`](/gson/src/main/resources/META-INF/proguard/gson.pro) for | ||
the Gson version you are using. | ||
|
||
An alternative to writing custom keep rules for your classes in the ProGuard configuration can be to use | ||
Android's [`@Keep` annotation](https://developer.android.com/studio/write/annotations#keep). | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Again, please change change |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -13,7 +13,7 @@ | |
|
||
# Keep Gson annotations | ||
# Note: Cannot perform finer selection here to only cover Gson annotations, see also https://stackoverflow.com/q/47515093 | ||
-keepattributes *Annotation* | ||
-keepattributes RuntimeVisibleAnnotations,AnnotationDefault | ||
|
||
|
||
### The following rules are needed for R8 in "full mode" which only adheres to `-keepattribtues` if | ||
|
@@ -24,18 +24,20 @@ | |
-keep class com.google.gson.reflect.TypeToken { *; } | ||
|
||
# Keep any (anonymous) classes extending TypeToken | ||
-keep class * extends com.google.gson.reflect.TypeToken | ||
-keep,allowobfuscation class * extends com.google.gson.reflect.TypeToken | ||
|
||
# Keep classes with @JsonAdapter annotation | ||
-keep @com.google.gson.annotations.JsonAdapter class * | ||
-keep,allowobfuscation,allowoptimization @com.google.gson.annotations.JsonAdapter class * | ||
|
||
# Keep fields with @SerializedName annotation, but allow obfuscation of their names | ||
-keepclassmembers,allowobfuscation class * { | ||
@com.google.gson.annotations.SerializedName <fields>; | ||
} | ||
|
||
# Keep fields with any other Gson annotation | ||
-keepclassmembers class * { | ||
# Also allow obfuscation, assuming that users will additionally use @SerializedName or | ||
# other means to preserve the field names | ||
-keepclassmembers,allowobfuscation class * { | ||
@com.google.gson.annotations.Expose <fields>; | ||
@com.google.gson.annotations.JsonAdapter <fields>; | ||
@com.google.gson.annotations.Since <fields>; | ||
|
@@ -44,15 +46,25 @@ | |
|
||
# Keep no-args constructor of classes which can be used with @JsonAdapter | ||
# By default their no-args constructor is invoked to create an adapter instance | ||
-keep class * extends com.google.gson.TypeAdapter { | ||
-keepclassmembers class * extends com.google.gson.TypeAdapter { | ||
<init>(); | ||
} | ||
-keep class * implements com.google.gson.TypeAdapterFactory { | ||
-keepclassmembers class * implements com.google.gson.TypeAdapterFactory { | ||
<init>(); | ||
} | ||
-keep class * implements com.google.gson.JsonSerializer { | ||
-keepclassmembers class * implements com.google.gson.JsonSerializer { | ||
<init>(); | ||
} | ||
-keep class * implements com.google.gson.JsonDeserializer { | ||
-keepclassmembers class * implements com.google.gson.JsonDeserializer { | ||
<init>(); | ||
} | ||
|
||
# If a class still exists after shrinking, and has fields annotated with @SerializedName, | ||
# keep the no-args constructor | ||
# Note: This behavior is not officially documented somewhere; based on https://issuetracker.google.com/issues/150189783#comment11 | ||
# and discussion in https://github.com/google/gson/pull/2397 | ||
-if class * | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This seems to work as expected for R8, but it is still to me a bit unclear how There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
What I tried to point out here is that reading
Right, a class without no-args constructor will with the current Gson ProGuard configuration cause R8 to still make the class abstract, so Unsafe cannot be used. I personally think that this is actually fine to highlight that users should add a no-args constructor (otherwise they would implicitly rely on Unsafe). I have added a test for this now and adjusted the guides slightly. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The Suggesting to always have a default constructor makes sense (I saw the change with the tests and improved error message). The fallback to using Unsafe will then only be needed if using GSON with libraries the developer does not have control over. That could be solved by byte code rewriting injecting the default constructor, but I guess that is out of scope of GSON. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks for the clarification! I have adjusted the comment in |
||
-keepclasseswithmembers,allowobfuscation,allowoptimization class <1> { | ||
<init>(); | ||
@com.google.gson.annotations.SerializedName <fields>; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe change "rely on JDK Unsafe" to "rely on
sun.misc.Unsafe
on both JDK and Android".UnsafeAllocator.create()
also try some Android specific magic on older Dalvik VMs, but they are becoming quite rare, so I don't think it is needed to mention that.