From 99aa304bdbfb1283f5e4455f11a5350bc04fd20e Mon Sep 17 00:00:00 2001 From: Mate Szalay-Beko Date: Fri, 29 Nov 2019 09:13:18 +0100 Subject: [PATCH] ZOOKEEPER-3633: AdminServer commands throw NPE when only secure client port is used When only secureClientPort is defined in the config and there is no regular clientPort, then both the stat and the conf commands result in 500 Server Error caused by NullPointerExceptions. The problem is that no serverCnxFactory is defined in the ZooKeeperServer in this case, we have only secureServerCnxnFactory. In the fix we return info about both the secure and unsecure connections. Example of the stat command output for secure-only configuration: ``` { "version" : "3.6.0-SNAPSHOT-8e8905069f4bff670c0492fe9e28ced0f86bca00, built on 11/29/2019 08:04 GMT", "read_only" : false, "server_stats" : { "packets_sent" : 1, "packets_received" : 1, "fsync_threshold_exceed_count" : 0, "client_response_stats" : { "last_buffer_size" : -1, "min_buffer_size" : -1, "max_buffer_size" : -1 }, "data_dir_size" : 671094270, "log_dir_size" : 671094270, "last_processed_zxid" : 20, "outstanding_requests" : 0, "server_state" : "standalone", "avg_latency" : 5.0, "max_latency" : 5, "min_latency" : 5, "num_alive_client_connections" : 1, "provider_null" : false, "uptime" : 15020 }, "client_response" : { "last_buffer_size" : -1, "min_buffer_size" : -1, "max_buffer_size" : -1 }, "node_count" : 6, "connections" : [ ], "secure_connections" : [ { "remote_socket_address" : "127.0.0.1:57276", "interest_ops" : 1, "outstanding_requests" : 0, "packets_received" : 1, "packets_sent" : 1 } ], "command" : "stats", "error" : null } ``` --- .../zookeeper/server/ZooKeeperServer.java | 2 +- .../zookeeper/server/admin/Commands.java | 17 +++++++++- .../zookeeper/server/admin/CommandsTest.java | 33 ++++++++++++++++++- 3 files changed, 49 insertions(+), 3 deletions(-) diff --git a/zookeeper-server/src/main/java/org/apache/zookeeper/server/ZooKeeperServer.java b/zookeeper-server/src/main/java/org/apache/zookeeper/server/ZooKeeperServer.java index 65be888380e..1c9dda77256 100644 --- a/zookeeper-server/src/main/java/org/apache/zookeeper/server/ZooKeeperServer.java +++ b/zookeeper-server/src/main/java/org/apache/zookeeper/server/ZooKeeperServer.java @@ -414,7 +414,7 @@ public ZooKeeperServerConf getConf() { zkDb.snapLog.getSnapDir().getAbsolutePath(), zkDb.snapLog.getDataDir().getAbsolutePath(), getTickTime(), - serverCnxnFactory.getMaxClientCnxnsPerHost(), + getMaxClientCnxnsPerHost(), getMinSessionTimeout(), getMaxSessionTimeout(), getServerId(), diff --git a/zookeeper-server/src/main/java/org/apache/zookeeper/server/admin/Commands.java b/zookeeper-server/src/main/java/org/apache/zookeeper/server/admin/Commands.java index d4de57af339..2679c96f757 100644 --- a/zookeeper-server/src/main/java/org/apache/zookeeper/server/admin/Commands.java +++ b/zookeeper-server/src/main/java/org/apache/zookeeper/server/admin/Commands.java @@ -573,7 +573,22 @@ public StatCommand() { @Override public CommandResponse run(ZooKeeperServer zkServer, Map kwargs) { CommandResponse response = super.run(zkServer, kwargs); - response.put("connections", zkServer.getServerCnxnFactory().getAllConnectionInfo(true)); + + final Iterable> connections; + if (zkServer.getServerCnxnFactory() != null) { + connections = zkServer.getServerCnxnFactory().getAllConnectionInfo(true); + } else { + connections = Collections.emptyList(); + } + response.put("connections", connections); + + final Iterable> secureConnections; + if (zkServer.getSecureServerCnxnFactory() != null) { + secureConnections = zkServer.getSecureServerCnxnFactory().getAllConnectionInfo(true); + } else { + secureConnections = Collections.emptyList(); + } + response.put("secure_connections", secureConnections); return response; } diff --git a/zookeeper-server/src/test/java/org/apache/zookeeper/server/admin/CommandsTest.java b/zookeeper-server/src/test/java/org/apache/zookeeper/server/admin/CommandsTest.java index 9f62a3fd547..4f55d08cc7a 100644 --- a/zookeeper-server/src/test/java/org/apache/zookeeper/server/admin/CommandsTest.java +++ b/zookeeper-server/src/test/java/org/apache/zookeeper/server/admin/CommandsTest.java @@ -33,6 +33,7 @@ import org.apache.zookeeper.metrics.MetricsUtils; import org.apache.zookeeper.server.ServerCnxnFactory; import org.apache.zookeeper.server.ServerStats; +import org.apache.zookeeper.server.ZKDatabase; import org.apache.zookeeper.server.ZooKeeperServer; import org.apache.zookeeper.server.quorum.BufferStats; import org.apache.zookeeper.test.ClientBase; @@ -219,7 +220,14 @@ public void testSetTraceMask() throws IOException, InterruptedException { @Test public void testStat() throws IOException, InterruptedException { - testCommand("stats", new Field("version", String.class), new Field("read_only", Boolean.class), new Field("server_stats", ServerStats.class), new Field("node_count", Integer.class), new Field("connections", Iterable.class), new Field("client_response", BufferStats.class)); + testCommand("stats", + new Field("version", String.class), + new Field("read_only", Boolean.class), + new Field("server_stats", ServerStats.class), + new Field("node_count", Integer.class), + new Field("connections", Iterable.class), + new Field("secure_connections", Iterable.class), + new Field("client_response", BufferStats.class)); } @Test @@ -258,4 +266,27 @@ public void testConsCommandSecureOnly() { assertThat(response.toMap().containsKey("secure_connections"), is(true)); } + /** + * testing Stat command, when only SecureClientPort is defined by the user and there is no + * regular (non-SSL port) open. In this case zkServer.getServerCnxnFactory === null + * see: ZOOKEEPER-3633 + */ + @Test + public void testStatCommandSecureOnly() { + Commands.StatCommand cmd = new Commands.StatCommand(); + ZooKeeperServer zkServer = mock(ZooKeeperServer.class); + ServerCnxnFactory cnxnFactory = mock(ServerCnxnFactory.class); + ServerStats serverStats = mock(ServerStats.class); + ZKDatabase zkDatabase = mock(ZKDatabase.class); + when(zkServer.getSecureServerCnxnFactory()).thenReturn(cnxnFactory); + when(zkServer.serverStats()).thenReturn(serverStats); + when(zkServer.getZKDatabase()).thenReturn(zkDatabase); + when(zkDatabase.getNodeCount()).thenReturn(0); + + CommandResponse response = cmd.run(zkServer, null); + + assertThat(response.toMap().containsKey("connections"), is(true)); + assertThat(response.toMap().containsKey("secure_connections"), is(true)); + } + }