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

Null check sessionCredentials before saving #189

Closed
wants to merge 1 commit into from
Closed

Null check sessionCredentials before saving #189

wants to merge 1 commit into from

Conversation

ghost
Copy link

@ghost ghost commented Aug 17, 2016

When saving sessionCredentials a NullPointerException
will happen if sessionCredentials is null.
Null check sessionCredentials before saving.

When saving sessionCredentials a NullPointerException
will happen if sessionCredentials is null.
Null check sessionCredentials before saving.
@karthiksaligrama
Copy link
Contributor

@JimmyDahlqvist can you tell me how to reproduce the scenario where the session credentials are null? Ideally if you are using cognito they should never be null

@ghost
Copy link
Author

ghost commented Dec 21, 2016

@karthiksaligrama The problem happens if you migrate from unauthenticated to authenticated user, hence user log in with Google Account (federated identity). We in this case calls setLogins with the map with user authentication. This seems to clear the session credentials.

Also when looking at the code the function saveCredentials in https://github.com/aws/aws-sdk-android/blob/23c58741cfbcf8b2f06858b458765fa47abe967e/aws-android-sdk-core/src/main/java/com/amazonaws/auth/CognitoCachingCredentialsProvider.java
already do a null-check. However the null-pointer actually happens due to getSessionCredentitalsExpiration() returning null.
By null-checking before saveCredentials are called this is prevented.

@rubinbasha
Copy link

Greetings @JimmyDahlqvist .

I just wanted to ask you something about your pull request. We have the same issue as you with the NullPointerException and would be interesting to know how you did solve this and if there was/is/will be a merge happening between your pull and the master branch.
Also it would be interesting to know what are the consequences of having that null check, does the sdk have issues somewhere else than or does it just not get Push notifications anymore...

Best regards,
Rubin Basha

@dqvist
Copy link

dqvist commented Feb 2, 2017

Hi @rubinbasha

We added the null check and we are building the SDK from source in our project.
We have not seen any side effects with it.

Regards
Jimmy

@rubinbasha
Copy link

Hi @fernwilter thank you for the information, we also did the same thing as you did and are now building the SDK ourselves also. We too are not having any further issue so for the time being we will also keep it with a null check until the SDK handles the issue. I think they are already tackling the issue since they replied to me on this thread android/issues/247

@azsolinsky
Copy link

The underlying synchronization issue has been corrected in the AWS SDK for Android 2.3.9 Release. See the release notes for more details: https://aws.amazon.com/releasenotes/1894188094590101

@azsolinsky azsolinsky closed this Feb 3, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants