Skip to content
This repository was archived by the owner on Jan 21, 2022. It is now read-only.

NPE check for Password and Account Name field #384

Open
wants to merge 11 commits into
base: MAS-2.1.00
Choose a base branch
from
Open

NPE check for Password and Account Name field #384

Changes from 3 commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
3539bf8
NPE check for Password and Account Name field
Jun 17, 2021
5e4d89e
Throwing NPE if Account Name and Password is null
Jun 17, 2021
cc6e418
Throw IllegalArgument Exception while Account is null
Jun 17, 2021
43d9ef5
Added Logs to capture the exception inside the catch block
Jun 27, 2021
fae84fe
Printing the account type
Jun 28, 2021
ff87134
Fixing the null check
graju256 Jun 28, 2021
e1dac15
Printing the account name if it is not matching inside the loop
Jun 28, 2021
1f70116
printing the identifier in the logs
Jun 29, 2021
37ea576
Added Synchronized block
Jul 13, 2021
13dd503
Made the changes regarding synchronized block
Jul 13, 2021
b006774
Updated the synchronized block to include the key changes
Jul 14, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 5 additions & 3 deletions mas-foundation/src/main/java/com/ca/mas/core/storage/sharedstorage/AccountManagerUtil.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,16 +52,18 @@ public AccountManagerUtil(Context context, String accountName, boolean sharedSto
//Attempt to retrieve the account
Account[] accounts = mAccountManager.getAccountsByType(accountType);
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Apply synchronization to the block that consists of the below logic:
synchronized {

  • get-accounts
  • iterate
  • add-account-explicitly
    }

for (Account account : accounts) {
if (accountName.equals(account.name)) {
if (accountName != null &&accountName.equals(account.name)) {
String password = mAccountManager.getPassword(account);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you want to put some logging in this block also.
As later if password does not match, we make mAccount to null, and which eventually will go to the if(mAccount==null) block.

String savedPassword = identifier.toString();
if (password.equals(savedPassword)) {
if (password != null && password.equals(savedPassword)) {
mAccount = account;
}else {
// - case migration from old AccountManagerStoreDataSource
mAccount = null;
identifier = new SharedStorageIdentifier();
}
} else {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We might be throwing the exception for name mismatch. It's a mistake.

We need to rewrite the logic like below:

accountName is invariant to the loop. Hence, a null check for the accountName should be done outside the for-loop.

if (acoountName == null) {
throw new IllegalArgumentException("Invalid parameters, Account name cannot be null");
}

for (Account account : accounts) {
     if (accountName.equals(account.name)) {
     }
}

throw new IllegalArgumentException("Invalid parameters, Account name cannot be null");
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[optional], "Account name cannot be null" is sufficient here.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@graju256 @zakirjt Added some logs to capture some details inside the catch block. Can you please review that.

}
}

Expand Down Expand Up @@ -223,4 +225,4 @@ private String proxyKey(String key){

return retVal;
}
}
}