Skip to content
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

Crashlytics NDK Proguard config is incorrect #6593

Open
nathan3d opened this issue Dec 11, 2024 · 6 comments
Open

Crashlytics NDK Proguard config is incorrect #6593

nathan3d opened this issue Dec 11, 2024 · 6 comments

Comments

@nathan3d
Copy link

[READ] Step 1: Are you in the right place?

Issues filed here should be about bugs in the code in this repository.
If you have a general question, need help debugging, or fall into some
other category use one of these other channels:

  • For general technical questions, post a question on StackOverflow
    with the firebase tag.
  • For general Firebase discussion, use the firebase-talk
    google group.
  • For help troubleshooting your application that does not fall under one
    of the above categories, reach out to the personalized
    Firebase support channel.

[REQUIRED] Step 2: Describe your environment

  • Android Studio version: Jellyfish
  • Firebase Component: Crashlytics NDK
  • Component version: 19.2.1

[REQUIRED] Step 3: Describe the problem

The keep rules for the Crashlytics NDK are limited to public classes, but at the minimum JniNativeApi is not a public class, so the included rules don't actually apply.

The Android Gradle Plugin includes this rule that mitigates the issue when used, but for builds using other build systems, this may not be present.

-keepclasseswithmembernames,includedescriptorclasses class * {
    native <methods>;
}
@lehcar09
Copy link
Contributor

Hi @nathan3d, thank you for reaching out. According to Firebase SDK docs,

Firebase SDKs do not proguard themselves, but support proguarding. Firebase SDKs themselves are proguard friendly, but the dependencies of Firebase SDKs may not be.

From what I gather, Android Gradle Plugin (AGP) seamlessly integrates ProGuard into the build process; other build systems require manual configuration. For more information, you can check this for reference.

However, based on proguard documentation for processing native methods,

ProGuard doesn't look at your native code, so it won't automatically preserve the classes or class members that are invoked by the native code.

I suggest checking DexGuard, this is a commercial extension of ProGuard. DexGuard is specialized for Android applications and libraries: it optimizes and obfuscates not just the bytecode, but also the manifest file, resources, resource files, asset files, and native libraries.

@nathan3d
Copy link
Author

Hi @lehcar09,

There are Proguard/R8 rules for the NDK published as part of the project here: https://github.com/firebase/firebase-android-sdk/blob/main/firebase-crashlytics-ndk/firebase-crashlytics-ndk-proguard.txt.

They're just not correct.

@lehcar09
Copy link
Contributor

Thank you for your clarification. Based on my understanding with proguard, the rule;

-keepclasseswithmembernames,includedescriptorclasses class * { native <methods>; } 

aims to keep all classes with native methods across your entire project. While the rule,

-keep public class com.google.firebase.crashlytics.ndk.JniNativeApi { native <methods>; }

specific rules for Firebase Crashlytics that are designed to preserve particular classes and their native methods. However, adding the broad rule might have side effects on optimizations. I’ll raise this to our engineers and see what we can do here. Thanks!

@nathan3d
Copy link
Author

Right. The problem is JNINativeApi isn't public, so this rule has no effect.
Replacing:
-keep public class com.google.firebase.crashlytics.ndk.JniNativeApi { native <methods>; }
with:
-keep class com.google.firebase.crashlytics.ndk.JniNativeApi { native <methods>; }
would fix the issue, at least for JniNativeApi. I think some of the others may need updating as well. I haven't had a chance to verify all of the rules.

@lehcar09
Copy link
Contributor

Hey @nathan3d, were currently working on this. Unfortunately, we are having trouble in reproducing the issue. By any chance, can you share an MCVE to help us investigate the issue? Thanks!

@google-oss-bot
Copy link
Contributor

Hey @nathan3d. We need more information to resolve this issue but there hasn't been an update in 5 weekdays. I'm marking the issue as stale and if there are no new updates in the next 5 days I will close it automatically.

If you have more information that will help us get to the bottom of this, just add a comment!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants