apptentive / apptentive-android

Apptentive Android SDK
http://www.apptentive.com
BSD 3-Clause "New" or "Revised" License
65 stars 64 forks source link

Android 12 non-SDK restrictions reflection is unsupported #233

Open rid-x opened 2 years ago

rid-x commented 2 years ago

After analyzing my app with the veridex tool, I found that there are two use cases for the reflection API in the Apptentive Android SDK.

Reflection unsupported Landroid/graphics/Typeface;->sSystemFontMap use(s):
  Lcom/apptentive/android/sdk/util/Util;->replaceDefaultFont(Landroid/content/Context;Ljava/lang/String;)V
  Lcom/apptentive/android/sdk/util/Util;->replaceDefaultFont(Landroid/content/Context;Ljava/lang/String;)V

The reason for this log is reflection field API usage getDeclaredField and it could break the app on Android 12. Here you can see the code snippet from Apptentive Android SDK.

com.apptentive.android.sdk.util.Util#replaceDefaultFont

try {
    final Field staticField = Typeface.class.getDeclaredField("sSystemFontMap");
    staticField.setAccessible(true);
    staticField.set(null, newMap);
} catch (NoSuchFieldException e) {
    ApptentiveLog.e(e, "Exception replacing system font");
    logException(e);
} catch (IllegalAccessException e) {
    ApptentiveLog.e(e, "Exception replacing system font");
    logException(e);
}
.....
try {
    final Field staticField = Typeface.class.getDeclaredField(staticTypefaceFieldName);
    staticField.setAccessible(true);
    staticField.set(null, newTypeface);
} catch (NoSuchFieldException e) {
    ApptentiveLog.e(e, "Exception replacing system font");
    logException(e);
} catch (IllegalAccessException e) {
    ApptentiveLog.e(e, "Exception replacing system font");
    logException(e);
}

The functionality of the replaceDefaultFont method, which currently relies on reflection, needs to be replaced with some supported API.

CaseyApptentive commented 2 years ago

Hi @rid-hrant,

Thanks for reaching out about this. I'll take a closer look at this with my engineering team.

This is the first report we've heard with this issue, though many of our customers are using our latest Android SDK (5.7.1) and have customers on Android 12. Is this causing any issues for your app in production, development, or both? Any other details you could give would be helpful.

Thanks again!

rid-x commented 2 years ago

Hi @CaseyApptentive, I am currently working on researching Android 12 changes to target our application. I'm not using replaceDefaultFont(Context context, String fontFilePath) method in my application but for users who will use this method, I think there is possibility of an issue as there are new reflection restrictions starting from Android 12.

CaseyApptentive commented 2 years ago

Thanks for the follow up, @rid-hrant. Let me discuss this with my team and get back to you.

Stay tuned!

HarryAWoodworth commented 2 years ago

Hi @rid-hrant ! I have spoken with our engineering team and we have assessed the issue that you brought up. The replaceDefaultClient function uses the getDeclaredField method that utilizes reflection. The function was written to test the system default font overriding functionality and could be extended to SDK users so it was left public. It is safe to be removed in our next release.

Our engineer tested the function on Andoroid 12 to check if it would cause an issue, and found that it fails silently before hitting getDeclaredField, and therefore is non usable in the present and does not cause issues on Android 12.

Thank you so much for bringing this to our attention!

rid-x commented 2 years ago

Hey, @HarryAWoodworth thx for your response, and thx for testing it on android 12. I suppose in your message you mean replaceDefaultFont, not replaceDefaultClient, am I right?

HarryAWoodworth commented 2 years ago

@rid-hrant Yes, my apologies, I did mean replaceDefaultFont! Thank you for catching that