From e734cea45ac2177974f93f907df26068bb3a23eb Mon Sep 17 00:00:00 2001 From: Enrico Olivelli Date: Thu, 20 Jan 2022 12:00:29 +0100 Subject: [PATCH 1/3] ZOOKEEPER-3988 rg.apache.zookeeper.server.NettyServerCnxn.receiveMessage throws NullPointerException --- .../server/NettyServerCnxnFactory.java | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 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 0891021f35d..6b271326984 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 void channelActive(ChannelHandlerContext ctx) throws Exception { 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()); } } } From d6fee8db185ad2b9c632857a13de63f72adc91d9 Mon Sep 17 00:00:00 2001 From: Enrico Olivelli Date: Thu, 20 Jan 2022 13:59:12 +0100 Subject: [PATCH 2/3] Add tests --- .../zookeeper/server/NettyServerCnxnTest.java | 26 ++++++++++++------- 1 file changed, 17 insertions(+), 9 deletions(-) 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 576b0185163..ba589cea69c 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 @@ -177,6 +177,19 @@ public void testNonMTLSLocalConn() throws IOException, InterruptedException, Kee @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); + } + + 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 void testNonMTLSRemoteConn() throws Exception { 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 From cea1a4a8eccb2596d6694b34b3a336aa5df5c7d8 Mon Sep 17 00:00:00 2001 From: Enrico Olivelli Date: Thu, 20 Jan 2022 14:01:33 +0100 Subject: [PATCH 3/3] fix build --- .../java/org/apache/zookeeper/server/NettyServerCnxnTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 ba589cea69c..a02d5d75586 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,7 +174,6 @@ public void testNonMTLSLocalConn() throws IOException, InterruptedException, Kee } } - @SuppressWarnings("unchecked") @Test public void testNonMTLSRemoteConn() throws Exception { LeaderZooKeeperServer zks = mock(LeaderZooKeeperServer.class); @@ -189,6 +188,7 @@ 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);