ZOOKEEPER-3633: AdminServer commands throw NPE when only secure client port is used

symat · nkalmar · commit 01e198aec9ce · 2019-12-03T10:01:22.000+01:00
When only secureClientPort is defined in the config and there is no regular clientPort, then both the stat and the conf commands on the AdminServer 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 } ``` Author: Mate Szalay-Beko <szalay.beko.mate@gmail.com> Reviewers: Andor Molnar <andor@apache.org>, Enrico Olivelli <eolivelli@apache.org>, Norbert Kalmar <nkalmar@apache.org> Closes #1161 from symat/ZOOKEEPER-3633
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
@@ -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
@@ -577,7 +577,22 @@ public StatCommand() {
         @Override
         public CommandResponse run(ZooKeeperServer zkServer, Map<String, String> kwargs) {
             CommandResponse response = super.run(zkServer, kwargs);
-            response.put("connections", zkServer.getServerCnxnFactory().getAllConnectionInfo(true));
+
+            final Iterable<Map<String, Object>> connections;
+            if (zkServer.getServerCnxnFactory() != null) {
+                connections = zkServer.getServerCnxnFactory().getAllConnectionInfo(true);
+            } else {
+                connections = Collections.emptyList();
+            }
+            response.put("connections", connections);
+
+            final Iterable<Map<String, Object>> 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
@@ -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
@@ -264,4 +272,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));
+    }
+
 }
