kotlin-inject
kotlin-inject copied to clipboard
Critical obfuscation issue.
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!
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 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.
@evant Here is change commit they made https://github.com/google/dagger/compare/1ece31e5ce8a3de25930cc10a8465af183ff276c..2638852a4b499eb7858fd8227595d6e306c0934b
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.
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
@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.
For Proguard/R8, there's the
-adaptclassstringsoption.
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.
@evant Want to highlight that Dagger uses Map<Class<?>, Boolean> instead of strings keys. So I believe this could be implemented in this library.
You'd need to make sure generics are handled correctly, but yes that's a possible solution