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

symat pushed a commit to branch branch-3.7
in repository https://gitbox.apache.org/repos/asf/zookeeper.git


The following commit(s) were added to refs/heads/branch-3.7 by this push:
     new 56f807f  ZOOKEEPER-3988: 
rg.apache.zookeeper.server.NettyServerCnxn.receiveMessage throws 
NullPointerException
56f807f is described below

commit 56f807f909f2b7799ced0fcdd4c1073c9a1acc0d
Author: Enrico Olivelli <[email protected]>
AuthorDate: Tue Jan 25 12:48:34 2022 +0000

    ZOOKEEPER-3988: rg.apache.zookeeper.server.NettyServerCnxn.receiveMessage 
throws NullPointerException
    
    Modifications:
    - prevent the NPE, the code that throws NPE is only to record some metrics 
for non TLS requests
    
    Related to:
    - apache/pulsar#11070
    - https://github.com/pravega/zookeeper-operator/issues/393
    
    Author: Enrico Olivelli <[email protected]>
    
    Reviewers: Nicolo² Boschi <[email protected]>, Andor Molnar 
<[email protected]>, Mate Szalay-Beko <[email protected]>
    
    Closes #1798 from eolivelli/fix/ZOOKEEPER-3988-npe
    
    (cherry picked from commit 957f8fc0afbeca638f13f6fb739e49a921da2b9d)
    Signed-off-by: Mate Szalay-Beko <[email protected]>
---
 .../zookeeper/server/NettyServerCnxnFactory.java   | 18 +++++++++-----
 .../zookeeper/server/NettyServerCnxnTest.java      | 28 ++++++++++++++--------
 2 files changed, 30 insertions(+), 16 deletions(-)

diff --git 
a/zookeeper-server/src/main/java/org/apache/zookeeper/server/NettyServerCnxnFactory.java
 
b/zookeeper-server/src/main/java/org/apache/zookeeper/server/NettyServerCnxnFactory.java
index 0891021..6b27132 100644
--- 
a/zookeeper-server/src/main/java/org/apache/zookeeper/server/NettyServerCnxnFactory.java
+++ 
b/zookeeper-server/src/main/java/org/apache/zookeeper/server/NettyServerCnxnFactory.java
@@ -259,14 +259,20 @@ public class NettyServerCnxnFactory extends 
ServerCnxnFactory {
                 allChannels.add(ctx.channel());
                 addCnxn(cnxn);
             }
+
             if (ctx.channel().pipeline().get(SslHandler.class) == null) {
-                SocketAddress remoteAddress = 
cnxn.getChannel().remoteAddress();
-                if (remoteAddress != null
-                        && !((InetSocketAddress) 
remoteAddress).getAddress().isLoopbackAddress()) {
-                    LOG.trace("NettyChannelHandler channelActive: remote={} 
local={}", remoteAddress, cnxn.getChannel().localAddress());
-                    zkServer.serverStats().incrementNonMTLSRemoteConnCount();
+                if (zkServer != null) {
+                    SocketAddress remoteAddress = 
cnxn.getChannel().remoteAddress();
+                    if (remoteAddress != null
+                            && !((InetSocketAddress) 
remoteAddress).getAddress().isLoopbackAddress()) {
+                        LOG.trace("NettyChannelHandler channelActive: 
remote={} local={}", remoteAddress, cnxn.getChannel().localAddress());
+                        
zkServer.serverStats().incrementNonMTLSRemoteConnCount();
+                    } else {
+                        
zkServer.serverStats().incrementNonMTLSLocalConnCount();
+                    }
                 } else {
-                    zkServer.serverStats().incrementNonMTLSLocalConnCount();
+                    LOG.trace("Opened non-TLS connection from {} but zkServer 
is not running",
+                            cnxn.getChannel().remoteAddress());
                 }
             }
         }
diff --git 
a/zookeeper-server/src/test/java/org/apache/zookeeper/server/NettyServerCnxnTest.java
 
b/zookeeper-server/src/test/java/org/apache/zookeeper/server/NettyServerCnxnTest.java
index 576b018..a02d5d7 100644
--- 
a/zookeeper-server/src/test/java/org/apache/zookeeper/server/NettyServerCnxnTest.java
+++ 
b/zookeeper-server/src/test/java/org/apache/zookeeper/server/NettyServerCnxnTest.java
@@ -174,9 +174,22 @@ public class NettyServerCnxnTest extends ClientBase {
         }
     }
 
-    @SuppressWarnings("unchecked")
     @Test
     public void testNonMTLSRemoteConn() throws Exception {
+        LeaderZooKeeperServer zks = mock(LeaderZooKeeperServer.class);
+        when(zks.isRunning()).thenReturn(true);
+        ServerStats.Provider providerMock = mock(ServerStats.Provider.class);
+        when(zks.serverStats()).thenReturn(new ServerStats(providerMock));
+        testNonMTLSRemoteConn(zks);
+    }
+
+    @Test
+    public void testNonMTLSRemoteConnZookKeeperServerNotReady() throws 
Exception {
+        testNonMTLSRemoteConn(null);
+    }
+
+    @SuppressWarnings("unchecked")
+    private void testNonMTLSRemoteConn(ZooKeeperServer zks) throws Exception {
         Channel channel = mock(Channel.class);
         ChannelId id = mock(ChannelId.class);
         ChannelFuture success = mock(ChannelFuture.class);
@@ -192,23 +205,18 @@ public class NettyServerCnxnTest extends ClientBase {
         when(channel.remoteAddress()).thenReturn(address);
         when(channel.id()).thenReturn(id);
         NettyServerCnxnFactory factory = new NettyServerCnxnFactory();
-        LeaderZooKeeperServer zks = mock(LeaderZooKeeperServer.class);
         factory.setZooKeeperServer(zks);
         Attribute atr = mock(Attribute.class);
         Mockito.doReturn(atr).when(channel).attr(
                 Mockito.any()
         );
         doNothing().when(atr).set(Mockito.any());
-
-        when(zks.isRunning()).thenReturn(true);
-
-        ServerStats.Provider providerMock = mock(ServerStats.Provider.class);
-        when(zks.serverStats()).thenReturn(new ServerStats(providerMock));
-
         factory.channelHandler.channelActive(context);
 
-        assertEquals(0, zks.serverStats().getNonMTLSLocalConnCount());
-        assertEquals(1, zks.serverStats().getNonMTLSRemoteConnCount());
+        if (zks != null) {
+            assertEquals(0, zks.serverStats().getNonMTLSLocalConnCount());
+            assertEquals(1, zks.serverStats().getNonMTLSRemoteConnCount());
+        }
     }
 
     @Test

Reply via email to