kotlin-inject icon indicating copy to clipboard operation
kotlin-inject copied to clipboard

Critical obfuscation issue.

Open Monabr opened this issue 1 year ago • 9 comments

Hello. You have a great library, but there is a problem that prevents me from using it.

The generated code uses the full class name including the package as a key. This should not happen after obfuscation! Please change this behavior.

As a solution, I am ready to see a functionality where I would manually assign a key to each class. This could be made by an optional parameter in the annotation or by some additional annotation.

I believe this is a critical issue and should be fixed as soon as possible!

Monabr avatar Jul 25 '24 22:07 Monabr

They really just need to be unique per type access, so some type of hashing my work here. Alternatively it looks like hilt solved a similar issue https://github.com/google/dagger/issues/3197 though will have to dig to figure out what code changes they actually did.

evant avatar Jul 26 '24 02:07 evant

@evant Could you tell me how fixable this problem is, should I wait for the nearest time? I am pressed for time and at the moment I am choosing between your library and service locators, which I would really like to avoid using.

Monabr avatar Jul 26 '24 09:07 Monabr

@evant Here is change commit they made https://github.com/google/dagger/compare/1ece31e5ce8a3de25930cc10a8465af183ff276c..2638852a4b499eb7858fd8227595d6e306c0934b

Monabr avatar Jul 26 '24 09:07 Monabr

For Proguard/R8, there's the -adaptclassstrings option. The strings containing class names are modified to their obfuscated version. (source)

For example, in a generated mapping file, I have:

[...]
me.tatarka.inject.internal.LazyMap -> dR0:
# {"id":"sourceFile","fileName":"LazyMap.kt"}
[...]

And in an obfuscated component class, I see:

public final class bt0 implements c22, ut1 {
   // --> LazyMap
   public final dR0 b;

   public final lAi C() {
      // --> LazyMap usage
      return (lAi)this.b.a("lAi", bt0::a);
   }
[...]

It doesn't work if the key has backticks or if it points to a generic type, though.

asapha avatar Jul 26 '24 16:07 asapha

It doesn't work if the key has backticks or if it points to a generic type, though.

Hm, I wonder if doing something like

"com.example.MyClass" + "<" + "com.example.MyArg" + ">"

would help it pick up generic types properly

evant avatar Jul 26 '24 16:07 evant

@Monabr

Could you tell me how fixable this problem is, should I wait for the nearest time?

I can make no promises on timelines, but this does seem fixable, pr's welcome.

evant avatar Jul 26 '24 16:07 evant

For Proguard/R8, there's the -adaptclassstrings option.

Are you talking about this as a temporary solution until the library is changed or do you mean that the dagger library team used this? If you mean the second option - it seems like they used something else.

Monabr avatar Jul 27 '24 01:07 Monabr

@evant Want to highlight that Dagger uses Map<Class<?>, Boolean> instead of strings keys. So I believe this could be implemented in this library.

Monabr avatar Sep 06 '24 21:09 Monabr

You'd need to make sure generics are handled correctly, but yes that's a possible solution

evant avatar Sep 07 '24 01:09 evant