Use newly created nullsage equalsIgnoreCase.

In reference to #1550 * The only way I can understand the tokenCacheItem to be null is if something mutates the map unsafely. * Create a nullsafe equalsIgnoreCase utility in common and use it here.
AzureAD · Aug 11, 2020 · 15a76c0 · 15a76c0
1 parent 841d0fc
commit 15a76c0
Show file tree

Hide file tree

Showing 3 changed files with 11 additions and 25 deletions.
diff --git a/adal/src/main/java/com/microsoft/aad/adal/AcquireTokenRequest.java b/adal/src/main/java/com/microsoft/aad/adal/AcquireTokenRequest.java
@@ -727,7 +727,7 @@ private void verifyBrokerRedirectUri(final AuthenticationRequest request) throws
             throw new UsageAuthenticationException(ADALError.DEVELOPER_REDIRECTURI_INVALID, "The redirectUri is null or blank.");
         }
 
-        if (inputUri.equalsIgnoreCase(AuthenticationConstants.Broker.BROKER_REDIRECT_URI)) {
+        if (AuthenticationConstants.Broker.BROKER_REDIRECT_URI.equalsIgnoreCase(inputUri)) {
             // TODO: Clean this up once we migrate all Logger functions to the common one.
             com.microsoft.identity.common.internal.logging.Logger.info(TAG + methodName, "This is a broker redirectUri. Bypass the check.");
             return;

diff --git a/adal/src/main/java/com/microsoft/aad/adal/MemoryTokenCacheStore.java b/adal/src/main/java/com/microsoft/aad/adal/MemoryTokenCacheStore.java
@@ -26,9 +26,9 @@
 import java.io.IOException;
 import java.io.ObjectInputStream;
 import java.io.ObjectOutputStream;
-import java.util.HashMap;
 import java.util.Iterator;
 import java.util.Map;
+import java.util.concurrent.ConcurrentHashMap;
 
 /**
  * tokenCacheItem is not persisted. Memory cache does not keep static items.
@@ -42,9 +42,7 @@ public class MemoryTokenCacheStore implements ITokenCacheStore {
 
     private static final String TAG = "MemoryTokenCacheStore";
 
-    private final Map<String, TokenCacheItem> mCache = new HashMap<>();
-
-    private transient Object mCacheLock = new Object();
+    private final Map<String, TokenCacheItem> mCache = new ConcurrentHashMap<>(16, 0.75f, 1);
 
     /**
      * Creates MemoryTokenCacheStore.
@@ -59,9 +57,7 @@ public TokenCacheItem getItem(String key) {
         }
 
         Logger.i(TAG, "Get Item from cache. ", "Key:" + key);
-        synchronized (mCacheLock) {
-            return mCache.get(key);
-        }
+        return mCache.get(key);
     }
 
     @Override
@@ -75,9 +71,7 @@ public void setItem(String key, TokenCacheItem item) {
         }
 
         Logger.i(TAG, "Set Item to cache. ", "Key: " + key);
-        synchronized (mCacheLock) {
-            mCache.put(key, item);
-        }
+        mCache.put(key, item);
     }
 
     @Override
@@ -87,17 +81,13 @@ public void removeItem(String key) {
         }
 
         Logger.i(TAG, "Remove Item from cache. ", "Key:" + key.hashCode());
-        synchronized (mCacheLock) {
-            mCache.remove(key);
-        }
+        mCache.remove(key);
     }
 
     @Override
     public void removeAll() {
         Logger.v(TAG, "Remove all items from cache.");
-        synchronized (mCacheLock) {
-            mCache.clear();
-        }
+        mCache.clear();
     }
 
     private synchronized void writeObject(ObjectOutputStream out) throws IOException {
@@ -108,7 +98,6 @@ private void readObject(ObjectInputStream inputStream) throws IOException,
             ClassNotFoundException {
         inputStream.defaultReadObject();
 
-        mCacheLock = new Object();
     }
 
     @Override
@@ -118,16 +107,12 @@ public boolean contains(String key) {
         }
 
         Logger.i(TAG, "contains Item from cache.", "Key: " + key);
-        synchronized (mCacheLock) {
-            return mCache.get(key) != null;
-        }
+        return mCache.get(key) != null;
     }
 
     @Override
     public Iterator<TokenCacheItem> getAll() {
         Logger.v(TAG, "Retrieving all items from cache. ");
-        synchronized (mCacheLock) {
-            return mCache.values().iterator();
-        }
+        return mCache.values().iterator();
     }
 }
diff --git a/adal/src/main/java/com/microsoft/aad/adal/TokenCacheAccessor.java b/adal/src/main/java/com/microsoft/aad/adal/TokenCacheAccessor.java
@@ -43,6 +43,7 @@
 import com.microsoft.identity.common.internal.providers.microsoft.azureactivedirectory.AzureActiveDirectoryOAuth2Configuration;
 import com.microsoft.identity.common.internal.providers.microsoft.azureactivedirectory.AzureActiveDirectoryOAuth2Strategy;
 import com.microsoft.identity.common.internal.providers.microsoft.azureactivedirectory.AzureActiveDirectoryTokenResponse;
+import com.microsoft.identity.common.internal.util.StringUtil;
 
 import java.net.MalformedURLException;
 import java.net.URL;
@@ -437,7 +438,7 @@ boolean isMultipleMRRTsMatchingGivenApp(final String clientId) {
         final List<TokenCacheItem> mrrtsMatchingRequest = new ArrayList<>();
         while (allItems.hasNext()) {
             final TokenCacheItem tokenCacheItem = allItems.next();
-            if (tokenCacheItem.getAuthority().equalsIgnoreCase(mAuthority) && tokenCacheItem.getClientId().equalsIgnoreCase(clientId)
+            if (StringUtil.equalsIgnoreCase(tokenCacheItem.getAuthority(), mAuthority) && StringUtil.equalsIgnoreCase(tokenCacheItem.getClientId(), clientId)
                     && (tokenCacheItem.getIsMultiResourceRefreshToken() || StringExtensions.isNullOrBlank(tokenCacheItem.getResource()))) {
                 mrrtsMatchingRequest.add(tokenCacheItem);
             }