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

Is there missing proguard rules ? #676

Closed
6 tasks done
bennycao opened this issue Jul 25, 2023 · 13 comments
Closed
6 tasks done

Is there missing proguard rules ? #676

bennycao opened this issue Jul 25, 2023 · 13 comments
Labels
bug This points to a verified bug in the code

Comments

@bennycao
Copy link
Contributor

Checklist

Description

Hi, we are seeing in our error logs exception being thrown

java.lang.NullPointerException: Attempt to invoke virtual method 'java.lang.annotation.Annotation java.lang.reflect.Field.getAnnotation(java.lang.Class)' on a null object reference at com.auth0.android.request.internal.JsonRequiredTypeAdapterFactory$1.read(JsonRequiredTypeAdapterFactory.java:35) at com.google.gson.TypeAdapter$1.read(TypeAdapter.java:199) at com.google.gson.internal.bind.ReflectiveTypeAdapterFactory$1.read(ReflectiveTypeAdapterFactory.java:131) at com.google.gson.internal.bind.ReflectiveTypeAdapterFactory$Adapter.read(ReflectiveTypeAdapterFactory.java:222) at com.auth0.android.request.internal.JsonRequiredTypeAdapterFactory$1.read(JsonRequiredTypeAdapterFactory.java:31) at com.google.gson.TypeAdapter$1.read(TypeAdapter.java:199) at com.google.gson.Gson.fromJson(Gson.java:963) at com.google.gson.Gson.fromJson(Gson.java:928) at com.google.gson.Gson.fromJson(Gson.java:877) at com.google.gson.Gson.fromJson(Gson.java:848) at com.auth0.android.authentication.storage.SecureCredentialsManager.continueGetCredentials$lambda-3(SecureCredentialsManager.kt:492) at com.auth0.android.authentication.storage.SecureCredentialsManager.$r8$lambda$6WE7o-y4gxPLV-YF4cOhR7EawOA(SecureCredentialsManager.kt:1) at com.auth0.android.authentication.storage.SecureCredentialsManager$$InternalSyntheticLambda$1$509647de71d71de33da01830616eb550f5518df260cce6beb4062f02e8cc048d$0.run(SecureCredentialsManager:13) at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1167) at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:641) at java.lang.Thread.run(Thread.java:923)

It seems to have increased from version 2.9.3 and for some reason only seeing issues for android 10,11 and 12.

Is there some proguard rules missing ?

Reproduction

cannot seem to reproduce locally

Additional context

No response

Auth0.Android version

2.9.3

Android version(s)

10,11,12

@bennycao bennycao added the bug This points to a verified bug in the code label Jul 25, 2023
@poovamraj
Copy link
Contributor

@bennycao The issue is not reproducing and this happens OptionalCredentials. This is different from how we deserialize Credentials.

Are you able to reproduce this consistently? Do you have 1 fat APK or do you have seperate ones based on architecture and is there any pattern on this.

Any details for us to reproduce this would be great.

@bennycao
Copy link
Contributor Author

@bennycao The issue is not reproducing and this happens OptionalCredentials. This is different from how we deserialize Credentials.

Are you able to reproduce this consistently? Do you have 1 fat APK or do you have seperate ones based on architecture and is there any pattern on this.

Any details for us to reproduce this would be great.

Hi @poovamraj , we haven't been able to locally reproduce the error. We have the 1 apk.
Could a first step be to guard against the NullPointerException here https://github.com/auth0/Auth0.Android/blob/main/auth0/src/main/java/com/auth0/android/request/internal/JsonRequiredTypeAdapterFactory.java#L35 ?

@poovamraj
Copy link
Contributor

Patching for an issue without understanding the underlying cause could backfire in the future. I am not sure how to proceed further since we already have proguard rules in place to avoid issues. Could this issue be on the application end?

@poovamraj
Copy link
Contributor

@bennycao anything on this?

@bennycao
Copy link
Contributor Author

bennycao commented Oct 9, 2023

Hi @poovamraj , we still are unable to reproduce this locally but it still does happen in production.
The concern here is that it throws a unhandled exception.

If it is on the application end, is there something that we need to add to proguard or the like ? I wouldn't of thought we needed to.

The interesting part is it only shows up on our production logs for OS versions 10, 11 and 12. Is there something there ?

@zsoltboldizsar
Copy link

Hi @bennycao. First of all thank you for opening this issue. I've recently updated to 2.10.2 and I'm wondering if your issue appears on all versions (2.9.3+) or 2.9.3 specifically? Thank you for your time.

@bennycao
Copy link
Contributor Author

Hi @bennycao. First of all thank you for opening this issue. I've recently updated to 2.10.2 and I'm wondering if your issue appears on all versions (2.9.3+) or 2.9.3 specifically? Thank you for your time.

Hi @zsoltboldizsar , we are already on 2.10.1 and it still happens on this version. Also its been happening before version 2.9.3, can't pinpoint a particular version as our logs only retain so much but definitely well before 2.9.3

@zsoltboldizsar
Copy link

zsoltboldizsar commented Oct 11, 2023

Hi @zsoltboldizsar , we are already on 2.10.1 and it still happens on this version. Also its been happening before version 2.9.3, can't pinpoint a particular version as our logs only retain so much but definitely well before 2.9.3

Thanks for your answer @bennycao, interesting situation for sure 🤔.

Worth noting, that in a 9 month time-frame we never experienced this issue using v2.9.0 in an app with a small user base of 500+ users. Now that I updated to 2.10.2 I was worried whether it would happen to us as well. Will keep a close eye on it and share if the issue shows up.

@poovamraj
Copy link
Contributor

Hi @bennycao considering this has been open for quite sometime and we are not able to close this issue and no one else has also reported about this. We will close this issue now. But feel free to comment here if you have more details and we can reopen this issue. Thanks for understanding.

@mattt-seek
Copy link

mattt-seek commented Dec 5, 2023

Based on the stacktrace there appears to be a call to the reflection API without a null check, here (but it's not obvious why we'd have a null there):

@bennycao
Copy link
Contributor Author

bennycao commented Dec 5, 2023

@poovamraj we still don't have a reproduceable scenario for this issue, but exception is increasing and is not good that it produces a unhandled exception.

As the exception is within a new Thread, we cannot catch the exception as a consumer.
Therefore, can you add some sort of default exception handling whether by e.g.

        Thread.setDefaultUncaughtExceptionHandler { _, ex ->
            callback.onFailure(
                CredentialsManagerException("Handled uncaught exception", ex)
            )
        }

before this line https://github.com/auth0/Auth0.Android/blob/main/auth0/src/main/java/com/auth0/android/authentication/storage/SecureCredentialsManager.kt#L515

or

wrapping the line of concern here https://github.com/auth0/Auth0.Android/blob/main/auth0/src/main/java/com/auth0/android/authentication/storage/SecureCredentialsManager.kt#L550
with

            val bridgeCredentials: OptionalCredentials

            try {
                bridgeCredentials = gson.fromJson(json, OptionalCredentials::class.java)
            } catch (ex: NullPointerException) {
                callback.onFailure(CredentialsManagerException("Invalid credentials from storage."))
                decryptCallback = null
                return@execute
            }

This will enable us to handle the exception gracefully

@poovamraj
Copy link
Contributor

@bennycao Would be great if you can send us a PR with the 2nd solution as we are bit tied now and this issue is absolutely not reproducing for anyone..

The error message has to be more fine tuned though but we can discuss it in the PR.

@bennycao
Copy link
Contributor Author

bennycao commented Dec 6, 2023

Thanks @poovamraj i've created a PR #701

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug This points to a verified bug in the code
Projects
None yet
Development

No branches or pull requests

4 participants