[geode] branch support/1.12 updated: GEODE-8261: Added a null check for the proxyID. (#5251)

[es...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

eshu11 pushed a commit to branch support/1.12
in repository https://gitbox.apache.org/repos/asf/geode.git


The following commit(s) were added to refs/heads/support/1.12 by this push:
     new d682b66  GEODE-8261: Added a null check for the proxyID. (#5251)
d682b66 is described below

commit d682b662ee25e4894deafd0ac30a9cb63961fe8c
Author: Nabarun Nag <na...@users.noreply.github.com>
AuthorDate: Tue Jun 16 14:48:25 2020 -0700

    GEODE-8261: Added a null check for the proxyID. (#5251)
    
    * During shutdown operations proxyID can be null.
    	* Executing operations using this null value can cause NPEs
    	* The solution is to skip all operations if the proxyID value is null.
    
    (cherry picked from commit 7609cfaefcaed12ce4bdc9450859e690e1c79815)
---
 .../geode/internal/cache/ha/HARegionQueue.java     | 36 ++++++++++++----------
 .../geode/internal/cache/ha/HARegionQueueTest.java | 18 +++++++++++
 2 files changed, 37 insertions(+), 17 deletions(-)

diff --git a/geode-core/src/main/java/org/apache/geode/internal/cache/ha/HARegionQueue.java b/geode-core/src/main/java/org/apache/geode/internal/cache/ha/HARegionQueue.java
index d538c48..350bc5b 100755
--- a/geode-core/src/main/java/org/apache/geode/internal/cache/ha/HARegionQueue.java
+++ b/geode-core/src/main/java/org/apache/geode/internal/cache/ha/HARegionQueue.java
@@ -3432,26 +3432,28 @@ public class HARegionQueue implements RegionQueue {
     }
   }
 
-  private void addClientCQsAndInterestList(ClientUpdateMessageImpl msg,
+  protected void addClientCQsAndInterestList(ClientUpdateMessageImpl msg,
       HAEventWrapper haEventWrapper, Map haContainer, String regionName) {
 
     ClientProxyMembershipID proxyID = ((HAContainerWrapper) haContainer).getProxyID(regionName);
-    if (haEventWrapper.getClientCqs() != null) {
-      CqNameToOp clientCQ = haEventWrapper.getClientCqs().get(proxyID);
-      if (clientCQ != null) {
-        msg.addClientCqs(proxyID, clientCQ);
-      }
-    }
-
-    // This is a remote HAEventWrapper.
-    // Add new Interested client lists.
-    ClientUpdateMessageImpl clientMsg =
-        (ClientUpdateMessageImpl) haEventWrapper.getClientUpdateMessage();
-    if (clientMsg != null) {
-      if (clientMsg.isClientInterestedInUpdates(proxyID)) {
-        msg.addClientInterestList(proxyID, true);
-      } else if (clientMsg.isClientInterestedInInvalidates(proxyID)) {
-        msg.addClientInterestList(proxyID, false);
+    if (proxyID != null) {
+      if (haEventWrapper.getClientCqs() != null) {
+        CqNameToOp clientCQ = haEventWrapper.getClientCqs().get(proxyID);
+        if (clientCQ != null) {
+          msg.addClientCqs(proxyID, clientCQ);
+        }
+      }
+
+      // This is a remote HAEventWrapper.
+      // Add new Interested client lists.
+      ClientUpdateMessageImpl clientMsg =
+          (ClientUpdateMessageImpl) haEventWrapper.getClientUpdateMessage();
+      if (clientMsg != null) {
+        if (clientMsg.isClientInterestedInUpdates(proxyID)) {
+          msg.addClientInterestList(proxyID, true);
+        } else if (clientMsg.isClientInterestedInInvalidates(proxyID)) {
+          msg.addClientInterestList(proxyID, false);
+        }
       }
     }
   }
diff --git a/geode-core/src/test/java/org/apache/geode/internal/cache/ha/HARegionQueueTest.java b/geode-core/src/test/java/org/apache/geode/internal/cache/ha/HARegionQueueTest.java
index 2d469cb..67a0a1a 100644
--- a/geode-core/src/test/java/org/apache/geode/internal/cache/ha/HARegionQueueTest.java
+++ b/geode-core/src/test/java/org/apache/geode/internal/cache/ha/HARegionQueueTest.java
@@ -17,6 +17,8 @@ package org.apache.geode.internal.cache.ha;
 import static junit.framework.TestCase.assertEquals;
 import static org.mockito.ArgumentMatchers.any;
 import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.times;
+import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.when;
 
 import java.io.IOException;
@@ -32,6 +34,8 @@ import org.apache.geode.internal.cache.EventID;
 import org.apache.geode.internal.cache.HARegion;
 import org.apache.geode.internal.cache.InternalCache;
 import org.apache.geode.internal.cache.tier.sockets.CacheClientNotifier;
+import org.apache.geode.internal.cache.tier.sockets.ClientUpdateMessageImpl;
+import org.apache.geode.internal.cache.tier.sockets.HAEventWrapper;
 import org.apache.geode.internal.statistics.StatisticsClock;
 import org.apache.geode.internal.util.concurrent.StoppableReentrantReadWriteLock;
 import org.apache.geode.test.junit.categories.ClientSubscriptionTest;
@@ -73,6 +77,20 @@ public class HARegionQueueTest {
   }
 
   @Test
+  public void whenProxyIDisNullThenItIsNotAddedToClientInterestList() {
+    ClientUpdateMessageImpl clientUpdateMessage = mock(ClientUpdateMessageImpl.class);
+    HAEventWrapper haEventWrapper = mock(HAEventWrapper.class);
+    HAContainerWrapper haContainerWrapper = mock(HAContainerWrapper.class);
+    String regionName = "mockRegion";
+    when(haContainerWrapper.getProxyID(any())).thenReturn(null);
+    haRegionQueue.addClientCQsAndInterestList(clientUpdateMessage, haEventWrapper,
+        haContainerWrapper, regionName);
+    verify(haEventWrapper, times(0)).getClientCqs();
+    verify(haEventWrapper, times(0)).getClientUpdateMessage();
+
+  }
+
+  @Test
   public void conflateConflatableEntriesAndDoNotConflateNonConflatableEntries() throws Exception {
     EventID eventId1 = new EventID(new byte[] {1}, 1, 1);
     EventID eventId2 = new EventID(new byte[] {1}, 1, 2);