Alternative annotation for custom names
wbonnefond opened this issue · 9 comments
I'm in the process of evaluating Moshi + Kotshi for a large Android project that currently uses Jackson. As part of this I'd like to add obfuscation to both our Kotlin DTO classes and their generated Kotshi*JsonAdapter's using Dexguard/Proguard. As it currently exists, Kotshi allows for full obfuscation of this code with one exception. The Moshi @Json annotation that Kotshi uses for defining custom names is a runtime annotation meant for reflection which means that it will show up in the compiled application code along with the value supplied for the JSON name.
Would you accept a change to either add an alternative source annotation for this purpose or replace the usage of @Json completely? Obviously replacing the annotation would be a breaking change, but it would also remove unused code from the compiled application.
I'm envisioning an annotation like this:
@Target(AnnotationTarget.FIELD)
@MustBeDocumented
@Retention(AnnotationRetention.SOURCE)
annotation class JsonSerializedName(
val name: String
)
Kotshi only uses that annotation during compile time and you can safely instruct your shrinker to remove them on all @JsonSerializable classes.
Some other shrinkers might have this ability, but I don't believe such a targeted removal is possible using Dexguard. ClassJsonAdapter, one of Moshi's built-in adapters, references the @Json annotation so it's not able to be stripped.
I've tried dozens of dexguard configurations to try to remove the runtime annotation, but switching to a new source annotation was the only way I was able to successfully keep it out of the compiled code.
If you'd like to see what it looks like I can open a PR that adds this in a backwards-compatible way.
I'm guessing you have the -keepattributes *Annotation* option enabled (or a similar one). Instead of this you could keep annotations on a case by case basis as dexguard/proguard removes annotations by default.
That's correct, we are using -keepattributes RuntimeVisibleAnnotations since there are more than a few runtime annotations that we do rely on. A case by case approach seems like a possible workaround, but it's tedious and difficult to maintain at scale. Included libraries can also define their own proguard rules which get included in the final app. Both material components and retrofit define -keepattributes RuntimeVisibleAnnotations as well.
I'd much prefer to update Kotshi to not use a runtime annotation when it only needs that information at compile time.
Yeah, that makes sense of course. I'm not super keep on adding another annotation when one already exists. To me the value of this seems extremely small as the generated adapter will already have the JSON names in plain text even if you used a compile time only annotation.
For unobfuscated code, I agree there is no benefit. But apps that use Dexguard are able to obfuscate the generated adapters completely (including the JSON names).
The code is obfuscated yes but the strings used to parse the JSON still remain so you'll still be able to infer the JSON names even if the annotation was removed.
There's an -encryptstrings rule that can encrypt/obfuscate strings in specific classes. I can share an example of the resulting obfuscated code.
Right, fair enough. But it's trivial to reverse this by just loading the class and checking the field values. My point is that introducing a new, duplicate, feature just for this purpose isn't something I'd want to do since the value, in my opinion, is minuscule. Another option you could perhaps try is to shade Moshi and use a version where you change the retention of the Json annotation yourself. Alternatively you can fork Kotshi and make the required changes and use that build.