SOLR-14671: Parsing dynamic ZK config sometimes cause NuberFormatException (#1701)

solr/CHANGES.txt

@@ -167,6 +167,13 @@ Other Changes
 
 * SOLR-11868: Deprecate CloudSolrClient.setIdField, use information from Zookeeper (Erick Erickson)
 
+==================  8.6.1 ==================
+
+Bug Fixes
+---------------------
+
+* SOLR-14671: Parsing dynamic ZK config sometimes cause NuberFormatException (janhoy)
+
 ==================  8.6.0 ==================
 
 Consult the lucene/CHANGES.txt file for additional, low level, changes in this release.

solr/core/src/java/org/apache/solr/handler/admin/ZookeeperStatusHandler.java

@@ -112,7 +112,8 @@ public class ZookeeperStatusHandler extends RequestHandlerBase {
           .map(h -> h.resolveClientPortAddress() + ":" + h.clientPort)
           .sorted().collect(Collectors.toList());
       List<String> dynamicHosts = zkDynamicConfig.getServers().stream()
-          .map(h -> h.resolveClientPortAddress() + ":" + h.clientPort)
+          .map(h -> h.resolveClientPortAddress() + ":" +
+                  (h.clientPort != null ? h.clientPort : hostsFromConnectionString.getServers().get(0).clientPort))
           .sorted().collect(Collectors.toList());
       if (!connStringHosts.containsAll(dynamicHosts)) {
         errors.add("Your ZK connection string (" + connStringHosts.size() + " hosts) is different from the " +
@@ -120,7 +121,13 @@ public class ZookeeperStatusHandler extends RequestHandlerBase {
                 "dynamic reconfiguration and will only be able to connect to the zk hosts in your connection string.");
         status = STATUS_YELLOW;
       }
-      zookeepers = zkDynamicConfig; // Clone input
+      if (zkDynamicConfig.getServers().get(0).clientPort != null) {
+        // If we have dynamic config with client ports, use this list to iterate servers
+        zookeepers = zkDynamicConfig;
+      } else {
+        // Use list from connection string since client port is missing on dynamic config from ZK
+        zookeepers = hostsFromConnectionString;
+      }
     }
     final Map<String, Object> zkStatus = new HashMap<>();
     final List<Object> details = new ArrayList<>();

solr/core/src/test/org/apache/solr/common/cloud/ZkDynamicConfigTest.java

@@ -29,8 +29,9 @@ public class ZkDynamicConfigTest extends SolrTestCaseJ4 {
             "server.1=zoo1:2780:2783:participant;0.0.0.0:2181\n" +
             "server.2=zoo2:2781:2784:participant|zoo3:2783;2181\n" +
             "server.3=zoo3:2782:2785;zoo3-client:2181\n" +
+            "server.4=zoo4:2783:2786:participant\n" + // this assumes clientPort specified in static config
             "version=400000003");
-    assertEquals(3, parsed.size());
+    assertEquals(4, parsed.size());
 
     assertEquals("zoo1", parsed.getServers().get(0).address);
     assertEquals(Integer.valueOf(2780), parsed.getServers().get(0).leaderPort);
@@ -50,6 +51,12 @@ public class ZkDynamicConfigTest extends SolrTestCaseJ4 {
     assertNull(parsed.getServers().get(2).role);
     assertEquals("zoo3-client", parsed.getServers().get(2).clientPortAddress);
     assertEquals("zoo3-client", parsed.getServers().get(2).resolveClientPortAddress());
+
+    // client address/port optional if clientPort specified in static config file (back-compat mode)
+    assertEquals("participant", parsed.getServers().get(3).role);
+    assertEquals(null, parsed.getServers().get(3).clientPortAddress);
+    assertEquals("zoo4", parsed.getServers().get(3).resolveClientPortAddress());
+    assertEquals(null, parsed.getServers().get(3).clientPort);
   }
 
   @Test(expected = SolrException.class)

solr/solrj/src/java/org/apache/solr/common/cloud/ZkDynamicConfig.java

@@ -130,6 +130,7 @@ public class ZkDynamicConfig {
       if (!m.matches()) {
         throw new SolrException(SolrException.ErrorCode.BAD_REQUEST, "Could not parse dynamic zk config line: " + line);
       }
+      String clientPortStr = m.group("clientPort");
       return new Server(
           Integer.parseInt(m.group("serverId")),
           m.group("address"),
@@ -137,7 +138,7 @@ public class ZkDynamicConfig {
           Integer.parseInt(m.group("leaderElectionPort")),
           m.group("role"),
           m.group("clientPortAddress"),
-          Integer.parseInt(m.group("clientPort"))
+          clientPortStr != null ? Integer.parseInt(clientPortStr) : null
       );
     }
   }