You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@lucene.apache.org by th...@apache.org on 2021/01/05 19:00:35 UTC

[lucene-solr] branch master updated: SOLR-15058: Enforce node_name contains colon and port and find first underscore after colon to parse context (#2178)

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

thelabdude pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/lucene-solr.git


The following commit(s) were added to refs/heads/master by this push:
     new 2fcaba1  SOLR-15058: Enforce node_name contains colon and port and find first underscore after colon to parse context (#2178)
2fcaba1 is described below

commit 2fcaba1ce24aeded56e264bfb1525f8d02a8b337
Author: Timothy Potter <th...@gmail.com>
AuthorDate: Tue Jan 5 12:00:14 2021 -0700

    SOLR-15058: Enforce node_name contains colon and port and find first underscore after colon to parse context (#2178)
---
 solr/CHANGES.txt                                   |  3 ++
 .../apache/solr/cloud/ClusterStateMockUtil.java    |  6 ++-
 .../solr/cloud/ClusterStateMockUtilTest.java       | 12 +++---
 .../org/apache/solr/cloud/NodeMutatorTest.java     | 12 +++---
 .../test/org/apache/solr/cloud/OverseerTest.java   | 44 +++++++++++-----------
 .../src/test/org/apache/solr/cloud/ZkCLITest.java  |  8 +++-
 .../org/apache/solr/cloud/ZkControllerTest.java    |  2 +
 .../test/org/apache/solr/core/CoreSorterTest.java  |  4 +-
 .../solr/handler/admin/HealthCheckHandlerTest.java |  2 +-
 .../handler/component/CloudReplicaSourceTest.java  | 44 +++++++++++-----------
 .../java/org/apache/solr/common/util/Utils.java    |  7 +++-
 .../solrj/routing/ReplicaListTransformerTest.java  |  2 +-
 .../ShufflingReplicaListTransformerTest.java       |  2 +-
 .../apache/solr/common/cloud/UrlSchemeTest.java    |  9 +++--
 14 files changed, 88 insertions(+), 69 deletions(-)

diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt
index 556d323..687ebf7 100644
--- a/solr/CHANGES.txt
+++ b/solr/CHANGES.txt
@@ -288,6 +288,9 @@ Bug Fixes
 * SOLR-15048: Fixed collapse parser behavior when dealing with docs boosted by QueryElevationComponent that are in the
   null group to treat them consistently regardless of collapse field type or group head selector. (hossman)
 
+* SOLR-15058: Enforce node_name contains colon and port and find first underscore after colon to parse context
+  when converting a node_name to a base URL. (Timothy Potter, Su Sasa)
+
 Other Changes
 ---------------------
 
diff --git a/solr/core/src/test/org/apache/solr/cloud/ClusterStateMockUtil.java b/solr/core/src/test/org/apache/solr/cloud/ClusterStateMockUtil.java
index 9019be7..1ffe0cc 100644
--- a/solr/core/src/test/org/apache/solr/cloud/ClusterStateMockUtil.java
+++ b/solr/core/src/test/org/apache/solr/cloud/ClusterStateMockUtil.java
@@ -21,6 +21,7 @@ import java.nio.charset.StandardCharsets;
 import java.util.Arrays;
 import java.util.HashMap;
 import java.util.HashSet;
+import java.util.Locale;
 import java.util.Map;
 import java.util.regex.Matcher;
 import java.util.regex.Pattern;
@@ -224,8 +225,9 @@ public class ClusterStateMockUtil {
     }
 
     Map<String,Object> replicaPropMap = new HashMap<>();
-    replicaPropMap.put(ZkStateReader.NODE_NAME_PROP, "baseUrl" + node + "_");
-    replicaPropMap.put(ZkStateReader.BASE_URL_PROP, "http://baseUrl" + node);
+    int port = 8982 + Integer.parseInt(node);
+    String nodeName = String.format(Locale.ROOT, "baseUrl%s:%d_", node, port);
+    replicaPropMap.put(ZkStateReader.NODE_NAME_PROP, nodeName);
     replicaPropMap.put(ZkStateReader.STATE_PROP, state.toString());
     replicaPropMap.put(ZkStateReader.CORE_NAME_PROP, sliceName + "_" + replicaName);
     replicaPropMap.put(ZkStateReader.REPLICA_TYPE, replicaType.name());
diff --git a/solr/core/src/test/org/apache/solr/cloud/ClusterStateMockUtilTest.java b/solr/core/src/test/org/apache/solr/cloud/ClusterStateMockUtilTest.java
index 16a1d1e..9a72d4b 100644
--- a/solr/core/src/test/org/apache/solr/cloud/ClusterStateMockUtilTest.java
+++ b/solr/core/src/test/org/apache/solr/cloud/ClusterStateMockUtilTest.java
@@ -34,7 +34,7 @@ public class ClusterStateMockUtilTest extends SolrTestCaseJ4 {
 
   @Test
   public void testBuildClusterState_Simple() {
-    try (ZkStateReader zkStateReader = ClusterStateMockUtil.buildClusterState("csr", "baseUrl1_")) {
+    try (ZkStateReader zkStateReader = ClusterStateMockUtil.buildClusterState("csr", "baseUrl1:8983_")) {
       ClusterState clusterState = zkStateReader.getClusterState();
       assertNotNull(clusterState);
       assertEquals(1, clusterState.getCollectionStates().size());
@@ -48,10 +48,10 @@ public class ClusterStateMockUtilTest extends SolrTestCaseJ4 {
       assertEquals(1, slice1.getReplicas().size());
       Replica replica1 = slice1.getReplica("replica1");
       assertNotNull(replica1);
-      assertEquals("baseUrl1_", replica1.getNodeName());
+      assertEquals("baseUrl1:8983_", replica1.getNodeName());
       assertEquals("slice1_replica1", replica1.getCoreName());
-      assertEquals("http://baseUrl1", replica1.getBaseUrl());
-      assertEquals("http://baseUrl1/slice1_replica1/", replica1.getCoreUrl());
+      assertEquals("http://baseUrl1:8983", replica1.getBaseUrl());
+      assertEquals("http://baseUrl1:8983/slice1_replica1/", replica1.getCoreUrl());
       assertEquals(Replica.State.ACTIVE, replica1.getState());
       assertEquals(Replica.Type.NRT, replica1.getType());
     }
@@ -59,7 +59,7 @@ public class ClusterStateMockUtilTest extends SolrTestCaseJ4 {
 
   @Test
   public void testBuildClusterState_ReplicaTypes() {
-    try (ZkStateReader zkStateReader = ClusterStateMockUtil.buildClusterState("csntp", "baseUrl1_")) {
+    try (ZkStateReader zkStateReader = ClusterStateMockUtil.buildClusterState("csntp", "baseUrl1:8983_")) {
       ClusterState clusterState = zkStateReader.getClusterState();
       assertNotNull(clusterState);
       assertEquals(1, clusterState.getCollectionStates().size());
@@ -79,7 +79,7 @@ public class ClusterStateMockUtilTest extends SolrTestCaseJ4 {
 
   @Test
   public void testBuildClusterState_ReplicaStateAndType() {
-    try (ZkStateReader zkStateReader = ClusterStateMockUtil.buildClusterState("csrStRpDnF", "baseUrl1_")) {
+    try (ZkStateReader zkStateReader = ClusterStateMockUtil.buildClusterState("csrStRpDnF", "baseUrl1:8983_")) {
       ClusterState clusterState = zkStateReader.getClusterState();
       assertNotNull(clusterState);
       assertEquals(1, clusterState.getCollectionStates().size());
diff --git a/solr/core/src/test/org/apache/solr/cloud/NodeMutatorTest.java b/solr/core/src/test/org/apache/solr/cloud/NodeMutatorTest.java
index b1acf6c..e9401d0 100644
--- a/solr/core/src/test/org/apache/solr/cloud/NodeMutatorTest.java
+++ b/solr/core/src/test/org/apache/solr/cloud/NodeMutatorTest.java
@@ -32,14 +32,14 @@ import org.junit.Test;
 @SolrTestCaseJ4.SuppressSSL // tests compare for http:
 public class NodeMutatorTest extends SolrTestCaseJ4Test {
 
-  private static final String NODE3 = "baseUrl3_";
-  private static final String NODE3_URL = "http://baseUrl3";
+  private static final String NODE3 = "baseUrl3:8985_";
+  private static final String NODE3_URL = "http://baseUrl3:8985";
 
-  private static final String NODE2 = "baseUrl2_";
-  private static final String NODE2_URL = "http://baseUrl2";
+  private static final String NODE2 = "baseUrl2:8984_";
+  private static final String NODE2_URL = "http://baseUrl2:8984";
 
-  private static final String NODE1 = "baseUrl1_";
-  private static final String NODE1_URL = "http://baseUrl1";
+  private static final String NODE1 = "baseUrl1:8983_";
+  private static final String NODE1_URL = "http://baseUrl1:8983";
 
   @Test
   public void downNodeReportsAllImpactedCollectionsAndNothingElse() throws IOException {
diff --git a/solr/core/src/test/org/apache/solr/cloud/OverseerTest.java b/solr/core/src/test/org/apache/solr/cloud/OverseerTest.java
index 91d0f95..1632617 100644
--- a/solr/core/src/test/org/apache/solr/cloud/OverseerTest.java
+++ b/solr/core/src/test/org/apache/solr/cloud/OverseerTest.java
@@ -405,7 +405,7 @@ public class OverseerTest extends SolrTestCaseJ4 {
       try (ZkStateReader reader = new ZkStateReader(zkClient)) {
         reader.createClusterStateWatchersAndUpdate();
 
-        mockController = new MockZKController(server.getZkAddress(), "127.0.0.1_solr", overseers);
+        mockController = new MockZKController(server.getZkAddress(), "127.0.0.1:8983_solr", overseers);
 
         final int numShards = 6; // this is not the number of shards in the collection
 
@@ -449,7 +449,7 @@ public class OverseerTest extends SolrTestCaseJ4 {
       try (ZkStateReader reader = new ZkStateReader(zkClient)) {
         reader.createClusterStateWatchersAndUpdate();
 
-        mockController = new MockZKController(server.getZkAddress(), "127.0.0.1_solr", overseers);
+        mockController = new MockZKController(server.getZkAddress(), "127.0.0.1:8983_solr", overseers);
 
         final int numShards = 3;
         mockController.createCollection(COLLECTION, 3);
@@ -516,7 +516,7 @@ public class OverseerTest extends SolrTestCaseJ4 {
       try (ZkStateReader reader = new ZkStateReader(zkClient)) {
         reader.createClusterStateWatchersAndUpdate();
 
-        mockController = new MockZKController(server.getZkAddress(), "127.0.0.1_solr", overseers);
+        mockController = new MockZKController(server.getZkAddress(), "127.0.0.1:8983_solr", overseers);
 
         try (ZkController zkController = createMockZkController(server.getZkAddress(), zkClient, reader)) {
 
@@ -527,7 +527,7 @@ public class OverseerTest extends SolrTestCaseJ4 {
           }
         }
         ZkNodeProps m = new ZkNodeProps(Overseer.QUEUE_OPERATION, OverseerAction.DOWNNODE.toLower(),
-            ZkStateReader.NODE_NAME_PROP, "127.0.0.1_solr");
+            ZkStateReader.NODE_NAME_PROP, "127.0.0.1:8983_solr");
         List<ZkWriteCommand> commands = new NodeMutator().downNode(reader.getClusterState(), m);
 
         ZkDistributedQueue q = overseers.get(0).getStateUpdateQueue();
@@ -591,7 +591,7 @@ public class OverseerTest extends SolrTestCaseJ4 {
       createCollection(COLLECTION, 1);
 
       ZkNodeProps m = new ZkNodeProps(Overseer.QUEUE_OPERATION, OverseerAction.STATE.toLower(),
-          ZkStateReader.NODE_NAME_PROP, "node1_",
+          ZkStateReader.NODE_NAME_PROP, "node1:8983_",
           ZkStateReader.COLLECTION_PROP, COLLECTION,
           ZkStateReader.SHARD_ID_PROP, "shard1",
           ZkStateReader.CORE_NAME_PROP, "core1",
@@ -606,7 +606,7 @@ public class OverseerTest extends SolrTestCaseJ4 {
 
       //publish node state (active)
       m = new ZkNodeProps(Overseer.QUEUE_OPERATION, OverseerAction.STATE.toLower(),
-          ZkStateReader.NODE_NAME_PROP, "node1_",
+          ZkStateReader.NODE_NAME_PROP, "node1:8983_",
           ZkStateReader.COLLECTION_PROP, COLLECTION,
           ZkStateReader.SHARD_ID_PROP, "shard1",
           ZkStateReader.CORE_NAME_PROP, "core1",
@@ -662,7 +662,7 @@ public class OverseerTest extends SolrTestCaseJ4 {
       reader = new ZkStateReader(zkClient);
       reader.createClusterStateWatchersAndUpdate();
 
-      mockController = new MockZKController(server.getZkAddress(), "127.0.0.1_solr", overseers);
+      mockController = new MockZKController(server.getZkAddress(), "127.0.0.1:8983_solr", overseers);
 
       overseerClient = electNewOverseer(server.getZkAddress());
 
@@ -724,7 +724,7 @@ public class OverseerTest extends SolrTestCaseJ4 {
       reader = new ZkStateReader(zkClient);
       reader.createClusterStateWatchersAndUpdate();
 
-      mockController = new MockZKController(server.getZkAddress(), "127.0.0.1_solr", overseers);
+      mockController = new MockZKController(server.getZkAddress(), "127.0.0.1:8983_solr", overseers);
 
       LeaderElector overseerElector = new LeaderElector(zkClient);
       if (overseers.size() > 0) {
@@ -903,7 +903,7 @@ public class OverseerTest extends SolrTestCaseJ4 {
       for (int i = 0; i < atLeast(4); i++) {
         killCounter.incrementAndGet(); // for each round allow 1 kill
 
-        mockController = new MockZKController(server.getZkAddress(), "node1_", overseers);
+        mockController = new MockZKController(server.getZkAddress(), "node1:8983_", overseers);
 
         TimeOut timeout = new TimeOut(10, TimeUnit.SECONDS, TimeSource.NANO_TIME);
         while (!timeout.hasTimedOut()) {
@@ -951,7 +951,7 @@ public class OverseerTest extends SolrTestCaseJ4 {
           }
         }
 
-        mockController2 = new MockZKController(server.getZkAddress(), "node2_", overseers);
+        mockController2 = new MockZKController(server.getZkAddress(), "node2:8984_", overseers);
 
        timeout = new TimeOut(10, TimeUnit.SECONDS, TimeSource.NANO_TIME);
         while (!timeout.hasTimedOut()) {
@@ -1035,7 +1035,7 @@ public class OverseerTest extends SolrTestCaseJ4 {
       reader = new ZkStateReader(zkClient);
       reader.createClusterStateWatchersAndUpdate();
 
-      mockController = new MockZKController(server.getZkAddress(), "127.0.0.1_solr", overseers);
+      mockController = new MockZKController(server.getZkAddress(), "127.0.0.1:8983_solr", overseers);
 
       overseerClient = electNewOverseer(server.getZkAddress());
 
@@ -1051,7 +1051,7 @@ public class OverseerTest extends SolrTestCaseJ4 {
 
       mockController.close();
 
-      mockController = new MockZKController(server.getZkAddress(), "127.0.0.1_solr", overseers);
+      mockController = new MockZKController(server.getZkAddress(), "127.0.0.1:8983_solr", overseers);
 
       mockController.publishState(COLLECTION, "core1", "core_node1","shard1", Replica.State.RECOVERING, 1, true, overseers.get(0));
 
@@ -1093,7 +1093,7 @@ public class OverseerTest extends SolrTestCaseJ4 {
       reader = new ZkStateReader(zkClient);
       reader.createClusterStateWatchersAndUpdate();
 
-      mockController = new MockZKController(server.getZkAddress(), "127.0.0.1_solr", overseers);
+      mockController = new MockZKController(server.getZkAddress(), "127.0.0.1:8983_solr", overseers);
 
       final int MAX_COLLECTIONS = 10, MAX_CORES = 10, MAX_STATE_CHANGES = 20000;
 
@@ -1209,7 +1209,7 @@ public class OverseerTest extends SolrTestCaseJ4 {
           "createNodeSet", "");
       queue.offer(Utils.toJSON(m));
       m = new ZkNodeProps(Overseer.QUEUE_OPERATION, OverseerAction.STATE.toLower(),
-          ZkStateReader.NODE_NAME_PROP, "127.0.0.1_solr",
+          ZkStateReader.NODE_NAME_PROP, "127.0.0.1:8983_solr",
           ZkStateReader.SHARD_ID_PROP, "shard1",
           ZkStateReader.COLLECTION_PROP, COLLECTION,
           ZkStateReader.CORE_NAME_PROP, "core1",
@@ -1217,7 +1217,7 @@ public class OverseerTest extends SolrTestCaseJ4 {
           ZkStateReader.STATE_PROP, Replica.State.RECOVERING.toString());
       queue.offer(Utils.toJSON(m));
       m = new ZkNodeProps(Overseer.QUEUE_OPERATION, "state",
-          ZkStateReader.NODE_NAME_PROP, "node1_",
+          ZkStateReader.NODE_NAME_PROP, "node1:8983_",
           ZkStateReader.SHARD_ID_PROP, "shard1",
           ZkStateReader.COLLECTION_PROP, COLLECTION,
           ZkStateReader.CORE_NAME_PROP, "core2",
@@ -1230,7 +1230,7 @@ public class OverseerTest extends SolrTestCaseJ4 {
       //submit to proper queue
       queue = overseers.get(0).getStateUpdateQueue();
       m = new ZkNodeProps(Overseer.QUEUE_OPERATION, OverseerAction.STATE.toLower(),
-          ZkStateReader.NODE_NAME_PROP, "127.0.0.1_solr",
+          ZkStateReader.NODE_NAME_PROP, "127.0.0.1:8983_solr",
           ZkStateReader.SHARD_ID_PROP, "shard1",
           ZkStateReader.COLLECTION_PROP, COLLECTION,
           ZkStateReader.CORE_NAME_PROP, "core3",
@@ -1271,7 +1271,7 @@ public class OverseerTest extends SolrTestCaseJ4 {
 
       ZkNodeProps m = new ZkNodeProps(Overseer.QUEUE_OPERATION, OverseerAction.STATE.toLower(),
           ZkStateReader.SHARD_ID_PROP, "shard1",
-          ZkStateReader.NODE_NAME_PROP, "127.0.0.1_solr",
+          ZkStateReader.NODE_NAME_PROP, "127.0.0.1:8983_solr",
           ZkStateReader.COLLECTION_PROP, "c1",
           ZkStateReader.CORE_NAME_PROP, "core1",
           ZkStateReader.CORE_NODE_NAME_PROP, "core_node1",
@@ -1285,7 +1285,7 @@ public class OverseerTest extends SolrTestCaseJ4 {
 
       m = new ZkNodeProps(Overseer.QUEUE_OPERATION, OverseerAction.STATE.toLower(),
           ZkStateReader.SHARD_ID_PROP, "shard1",
-          ZkStateReader.NODE_NAME_PROP, "127.0.0.1_solr",
+          ZkStateReader.NODE_NAME_PROP, "127.0.0.1:8983_solr",
           ZkStateReader.COLLECTION_PROP, "c1",
           ZkStateReader.CORE_NAME_PROP, "core1",
           ZkStateReader.ROLES_PROP, "",
@@ -1296,7 +1296,7 @@ public class OverseerTest extends SolrTestCaseJ4 {
 
       m = new ZkNodeProps(Overseer.QUEUE_OPERATION, OverseerAction.STATE.toLower(),
           ZkStateReader.SHARD_ID_PROP, "shard1",
-          ZkStateReader.NODE_NAME_PROP, "127.0.0.1_solr",
+          ZkStateReader.NODE_NAME_PROP, "127.0.0.1:8983_solr",
           ZkStateReader.COLLECTION_PROP, "c1",
           ZkStateReader.CORE_NAME_PROP, "core1",
           ZkStateReader.ROLES_PROP, "",
@@ -1332,7 +1332,7 @@ public class OverseerTest extends SolrTestCaseJ4 {
           "collection", testCollectionName,
           ZkStateReader.SHARD_ID_PROP, "x",
           ZkStateReader.CORE_NODE_NAME_PROP, "core_node1",
-          ZkStateReader.NODE_NAME_PROP, "127.0.0.1_solr",
+          ZkStateReader.NODE_NAME_PROP, "127.0.0.1:8983_solr",
           ZkStateReader.CORE_NAME_PROP, "core1",
           ZkStateReader.STATE_PROP, Replica.State.DOWN.toString()
       );
@@ -1484,7 +1484,7 @@ public class OverseerTest extends SolrTestCaseJ4 {
           final int N = (numReplicas-rr)*numShards + ss;
           ZkNodeProps m = new ZkNodeProps(Overseer.QUEUE_OPERATION, OverseerAction.STATE.toLower(),
               ZkStateReader.SHARD_ID_PROP, "shard"+ss,
-              ZkStateReader.NODE_NAME_PROP, "127.0.0.1_solr",
+              ZkStateReader.NODE_NAME_PROP, "127.0.0.1:8983_solr",
               ZkStateReader.COLLECTION_PROP, COLLECTION,
               ZkStateReader.CORE_NAME_PROP, "core"+N,
               ZkStateReader.CORE_NODE_NAME_PROP, "core_node"+N,
@@ -1508,7 +1508,7 @@ public class OverseerTest extends SolrTestCaseJ4 {
           final int N = (numReplicas-rr)*numShards + ss;
           ZkNodeProps m = new ZkNodeProps(Overseer.QUEUE_OPERATION, OverseerAction.STATE.toLower(),
               ZkStateReader.SHARD_ID_PROP, "shard"+ss,
-              ZkStateReader.NODE_NAME_PROP, "127.0.0.1_solr",
+              ZkStateReader.NODE_NAME_PROP, "127.0.0.1:8983_solr",
               ZkStateReader.COLLECTION_PROP, COLLECTION,
               ZkStateReader.CORE_NAME_PROP, "core"+N,
               ZkStateReader.ROLES_PROP, "",
diff --git a/solr/core/src/test/org/apache/solr/cloud/ZkCLITest.java b/solr/core/src/test/org/apache/solr/cloud/ZkCLITest.java
index b3a14da..9a54daf 100644
--- a/solr/core/src/test/org/apache/solr/cloud/ZkCLITest.java
+++ b/solr/core/src/test/org/apache/solr/cloud/ZkCLITest.java
@@ -386,8 +386,12 @@ public class ZkCLITest extends SolrTestCaseJ4 {
 
   @Override
   public void tearDown() throws Exception {
-    zkClient.close();
-    zkServer.shutdown();
+    if (zkClient != null) {
+      zkClient.close();
+    }
+    if (zkServer != null) {
+      zkServer.shutdown();
+    }
     super.tearDown();
   }
 
diff --git a/solr/core/src/test/org/apache/solr/cloud/ZkControllerTest.java b/solr/core/src/test/org/apache/solr/cloud/ZkControllerTest.java
index f92039a..8018e78 100644
--- a/solr/core/src/test/org/apache/solr/cloud/ZkControllerTest.java
+++ b/solr/core/src/test/org/apache/solr/cloud/ZkControllerTest.java
@@ -108,6 +108,8 @@ public class ZkControllerTest extends SolrTestCaseJ4 {
           // getBaseUrlForNodeName
           assertEquals("http://zzz.xxx:1234/solr",
               zkStateReader.getBaseUrlForNodeName("zzz.xxx:1234_solr"));
+          assertEquals("http://zzz_xxx:1234/solr",
+              zkStateReader.getBaseUrlForNodeName("zzz_xxx:1234_solr"));
           assertEquals("http://xxx:99",
               zkStateReader.getBaseUrlForNodeName("xxx:99_"));
           assertEquals("http://foo-bar.baz.org:9999/some_dir",
diff --git a/solr/core/src/test/org/apache/solr/core/CoreSorterTest.java b/solr/core/src/test/org/apache/solr/core/CoreSorterTest.java
index 7757606..fd7b5c4 100644
--- a/solr/core/src/test/org/apache/solr/core/CoreSorterTest.java
+++ b/solr/core/src/test/org/apache/solr/core/CoreSorterTest.java
@@ -90,11 +90,11 @@ public class CoreSorterTest extends SolrTestCaseJ4 {
     // compute nodes, some live, some down
     final int maxNodesOfAType = perShardCounts.stream() // not too important how many we have, but lets have plenty
         .mapToInt(c -> c.totalReplicasInLiveNodes + c.totalReplicasInDownNodes + c.myReplicas).max().getAsInt();
-    List<String> liveNodes = IntStream.range(0, maxNodesOfAType).mapToObj(i -> "192.168.0." + i + "_8983").collect(Collectors.toList());
+    List<String> liveNodes = IntStream.range(0, maxNodesOfAType).mapToObj(i -> "192.168.0." + i + ":8983_").collect(Collectors.toList());
     Collections.shuffle(liveNodes, random());
     String thisNode = liveNodes.get(0);
     List<String> otherLiveNodes = liveNodes.subList(1, liveNodes.size());
-    List<String> downNodes = IntStream.range(0, maxNodesOfAType).mapToObj(i -> "192.168.1." + i + "_8983").collect(Collectors.toList());
+    List<String> downNodes = IntStream.range(0, maxNodesOfAType).mapToObj(i -> "192.168.1." + i + ":8983_").collect(Collectors.toList());
 
     // divide into two collections
     int numCol1 = random().nextInt(perShardCounts.size());
diff --git a/solr/core/src/test/org/apache/solr/handler/admin/HealthCheckHandlerTest.java b/solr/core/src/test/org/apache/solr/handler/admin/HealthCheckHandlerTest.java
index f6252db..1ee868b 100644
--- a/solr/core/src/test/org/apache/solr/handler/admin/HealthCheckHandlerTest.java
+++ b/solr/core/src/test/org/apache/solr/handler/admin/HealthCheckHandlerTest.java
@@ -196,7 +196,7 @@ public class HealthCheckHandlerTest extends SolrCloudTestCase {
     //  node2: collection1 -> shard1: [ replica2 (active), replica4 (down) ]
     //         collection2 -> shard1: [ replica1 (active) ]
     try (ZkStateReader reader = ClusterStateMockUtil.buildClusterState(
-        "csrr2rDr2Dcsr2FrR", 1, "node1", "node2")) {
+        "csrr2rDr2Dcsr2FrR", 1, "baseUrl1:8983_", "baseUrl2:8984_")) {
       ClusterState clusterState = reader.getClusterState();
 
       // Node 1
diff --git a/solr/core/src/test/org/apache/solr/handler/component/CloudReplicaSourceTest.java b/solr/core/src/test/org/apache/solr/handler/component/CloudReplicaSourceTest.java
index aa8e887..d859de4 100644
--- a/solr/core/src/test/org/apache/solr/handler/component/CloudReplicaSourceTest.java
+++ b/solr/core/src/test/org/apache/solr/handler/component/CloudReplicaSourceTest.java
@@ -45,7 +45,7 @@ public class CloudReplicaSourceTest extends SolrTestCaseJ4 {
     HttpShardHandlerFactory.WhitelistHostChecker whitelistHostChecker = Mockito.mock(HttpShardHandlerFactory.WhitelistHostChecker.class);
     ModifiableSolrParams params = new ModifiableSolrParams();
     params.set("shards", "slice1,slice2");
-    try (ZkStateReader zkStateReader = ClusterStateMockUtil.buildClusterState("csr*sr2", "baseUrl1_", "baseUrl2_")) {
+    try (ZkStateReader zkStateReader = ClusterStateMockUtil.buildClusterState("csr*sr2", "baseUrl1:8983_", "baseUrl2:8984_")) {
       CloudReplicaSource cloudReplicaSource = new CloudReplicaSource.Builder()
           .collection("collection1")
           .onlyNrt(false)
@@ -57,9 +57,9 @@ public class CloudReplicaSourceTest extends SolrTestCaseJ4 {
       assertEquals(2, cloudReplicaSource.getSliceCount());
       assertEquals(2, cloudReplicaSource.getSliceNames().size());
       assertEquals(1, cloudReplicaSource.getReplicasBySlice(0).size());
-      assertEquals("http://baseUrl1/slice1_replica1/", cloudReplicaSource.getReplicasBySlice(0).get(0));
+      assertEquals("http://baseUrl1:8983/slice1_replica1/", cloudReplicaSource.getReplicasBySlice(0).get(0));
       assertEquals(1, cloudReplicaSource.getReplicasBySlice(1).size());
-      assertEquals("http://baseUrl2/slice2_replica2/", cloudReplicaSource.getReplicasBySlice(1).get(0));
+      assertEquals("http://baseUrl2:8984/slice2_replica2/", cloudReplicaSource.getReplicasBySlice(1).get(0));
     }
   }
 
@@ -70,7 +70,7 @@ public class CloudReplicaSourceTest extends SolrTestCaseJ4 {
     ModifiableSolrParams params = new ModifiableSolrParams();
     params.set("shards", "slice1,slice2");
     // here node2 is not live so there should be no replicas found for slice2
-    try (ZkStateReader zkStateReader = ClusterStateMockUtil.buildClusterState("csr*sr2", "baseUrl1_")) {
+    try (ZkStateReader zkStateReader = ClusterStateMockUtil.buildClusterState("csr*sr2", "baseUrl1:8983_")) {
       CloudReplicaSource cloudReplicaSource = new CloudReplicaSource.Builder()
           .collection("collection1")
           .onlyNrt(false)
@@ -82,7 +82,7 @@ public class CloudReplicaSourceTest extends SolrTestCaseJ4 {
       assertEquals(2, cloudReplicaSource.getSliceCount());
       assertEquals(2, cloudReplicaSource.getSliceNames().size());
       assertEquals(1, cloudReplicaSource.getReplicasBySlice(0).size());
-      assertEquals("http://baseUrl1/slice1_replica1/", cloudReplicaSource.getReplicasBySlice(0).get(0));
+      assertEquals("http://baseUrl1:8983/slice1_replica1/", cloudReplicaSource.getReplicasBySlice(0).get(0));
       assertEquals(0, cloudReplicaSource.getReplicasBySlice(1).size());
     }
   }
@@ -94,7 +94,7 @@ public class CloudReplicaSourceTest extends SolrTestCaseJ4 {
     ModifiableSolrParams params = new ModifiableSolrParams();
     params.set("shards", "slice1,slice2");
     // here replica3 is in DOWN state so only 1 replica should be returned for slice2
-    try (ZkStateReader zkStateReader = ClusterStateMockUtil.buildClusterState("csr*sr2r3D", "baseUrl1_", "baseUrl2_", "baseUrl3_")) {
+    try (ZkStateReader zkStateReader = ClusterStateMockUtil.buildClusterState("csr*sr2r3D", "baseUrl1:8983_", "baseUrl2:8984_", "baseUrl3:8985_")) {
       CloudReplicaSource cloudReplicaSource = new CloudReplicaSource.Builder()
           .collection("collection1")
           .onlyNrt(false)
@@ -106,10 +106,10 @@ public class CloudReplicaSourceTest extends SolrTestCaseJ4 {
       assertEquals(2, cloudReplicaSource.getSliceCount());
       assertEquals(2, cloudReplicaSource.getSliceNames().size());
       assertEquals(1, cloudReplicaSource.getReplicasBySlice(0).size());
-      assertEquals("http://baseUrl1/slice1_replica1/", cloudReplicaSource.getReplicasBySlice(0).get(0));
+      assertEquals("http://baseUrl1:8983/slice1_replica1/", cloudReplicaSource.getReplicasBySlice(0).get(0));
       assertEquals(1, cloudReplicaSource.getReplicasBySlice(1).size());
       assertEquals(1, cloudReplicaSource.getReplicasBySlice(1).size());
-      assertEquals("http://baseUrl2/slice2_replica2/", cloudReplicaSource.getReplicasBySlice(1).get(0));
+      assertEquals("http://baseUrl2:8984/slice2_replica2/", cloudReplicaSource.getReplicasBySlice(1).get(0));
     }
   }
 
@@ -119,7 +119,7 @@ public class CloudReplicaSourceTest extends SolrTestCaseJ4 {
     HttpShardHandlerFactory.WhitelistHostChecker whitelistHostChecker = Mockito.mock(HttpShardHandlerFactory.WhitelistHostChecker.class);
     ModifiableSolrParams params = new ModifiableSolrParams();
     params.set("collection", "collection1,collection2");
-    try (ZkStateReader zkStateReader = ClusterStateMockUtil.buildClusterState("csr*sr2csr*", "baseUrl1_", "baseUrl2_")) {
+    try (ZkStateReader zkStateReader = ClusterStateMockUtil.buildClusterState("csr*sr2csr*", "baseUrl1:8983_", "baseUrl2:8984_")) {
       CloudReplicaSource cloudReplicaSource = new CloudReplicaSource.Builder()
           .collection("collection1")
           .onlyNrt(false)
@@ -139,13 +139,13 @@ public class CloudReplicaSourceTest extends SolrTestCaseJ4 {
         // using the collection param can return slice names in any order
         switch (sliceName) {
           case "collection1_slice1":
-            assertEquals("http://baseUrl1/slice1_replica1/", cloudReplicaSource.getReplicasBySlice(i).get(0));
+            assertEquals("http://baseUrl1:8983/slice1_replica1/", cloudReplicaSource.getReplicasBySlice(i).get(0));
             break;
           case "collection1_slice2":
-            assertEquals("http://baseUrl2/slice2_replica2/", cloudReplicaSource.getReplicasBySlice(i).get(0));
+            assertEquals("http://baseUrl2:8984/slice2_replica2/", cloudReplicaSource.getReplicasBySlice(i).get(0));
             break;
           case "collection2_slice1":
-            assertEquals("http://baseUrl1/slice1_replica3/", cloudReplicaSource.getReplicasBySlice(i).get(0));
+            assertEquals("http://baseUrl1:8983/slice1_replica3/", cloudReplicaSource.getReplicasBySlice(i).get(0));
             break;
         }
       }
@@ -157,7 +157,7 @@ public class CloudReplicaSourceTest extends SolrTestCaseJ4 {
     ReplicaListTransformer replicaListTransformer = Mockito.mock(ReplicaListTransformer.class);
     HttpShardHandlerFactory.WhitelistHostChecker whitelistHostChecker = Mockito.mock(HttpShardHandlerFactory.WhitelistHostChecker.class);
     ModifiableSolrParams params = new ModifiableSolrParams();
-    try (ZkStateReader zkStateReader = ClusterStateMockUtil.buildClusterState("csr*sr2", "baseUrl1_", "baseUrl2_")) {
+    try (ZkStateReader zkStateReader = ClusterStateMockUtil.buildClusterState("csr*sr2", "baseUrl1:8983_", "baseUrl2:8984_")) {
       CloudReplicaSource cloudReplicaSource = new CloudReplicaSource.Builder()
           .collection("collection1")
           .onlyNrt(false)
@@ -176,10 +176,10 @@ public class CloudReplicaSourceTest extends SolrTestCaseJ4 {
         // need to switch because without a shards param, the order of slices is not deterministic
         switch (sliceName) {
           case "slice1":
-            assertEquals("http://baseUrl1/slice1_replica1/", cloudReplicaSource.getReplicasBySlice(i).get(0));
+            assertEquals("http://baseUrl1:8983/slice1_replica1/", cloudReplicaSource.getReplicasBySlice(i).get(0));
             break;
           case "slice2":
-            assertEquals("http://baseUrl2/slice2_replica2/", cloudReplicaSource.getReplicasBySlice(i).get(0));
+            assertEquals("http://baseUrl2:8984/slice2_replica2/", cloudReplicaSource.getReplicasBySlice(i).get(0));
             break;
         }
       }
@@ -192,7 +192,7 @@ public class CloudReplicaSourceTest extends SolrTestCaseJ4 {
     HttpShardHandlerFactory.WhitelistHostChecker whitelistHostChecker = Mockito.mock(HttpShardHandlerFactory.WhitelistHostChecker.class);
     ModifiableSolrParams params = new ModifiableSolrParams();
     // the cluster state will have slice2 with two tlog replicas out of which the first one will be the leader
-    try (ZkStateReader zkStateReader = ClusterStateMockUtil.buildClusterState("csrr*st2t2", "baseUrl1_", "baseUrl2_")) {
+    try (ZkStateReader zkStateReader = ClusterStateMockUtil.buildClusterState("csrr*st2t2", "baseUrl1:8983_", "baseUrl2:8984_")) {
       CloudReplicaSource cloudReplicaSource = new CloudReplicaSource.Builder()
           .collection("collection1")
           .onlyNrt(true) // enable only nrt mode
@@ -210,11 +210,11 @@ public class CloudReplicaSourceTest extends SolrTestCaseJ4 {
         switch (sliceName) {
           case "slice1":
             assertEquals(2, cloudReplicaSource.getReplicasBySlice(i).size());
-            assertEquals("http://baseUrl1/slice1_replica1/", cloudReplicaSource.getReplicasBySlice(i).get(0));
+            assertEquals("http://baseUrl1:8983/slice1_replica1/", cloudReplicaSource.getReplicasBySlice(i).get(0));
             break;
           case "slice2":
             assertEquals(1, cloudReplicaSource.getReplicasBySlice(i).size());
-            assertEquals("http://baseUrl2/slice2_replica3/", cloudReplicaSource.getReplicasBySlice(i).get(0));
+            assertEquals("http://baseUrl2:8984/slice2_replica3/", cloudReplicaSource.getReplicasBySlice(i).get(0));
             break;
         }
       }
@@ -229,7 +229,7 @@ public class CloudReplicaSourceTest extends SolrTestCaseJ4 {
     params.set("collection", "collection1,collection2");
     // the cluster state will have collection1 with slice2 with two tlog replicas out of which the first one will be the leader
     // and collection2 with just a single slice and a tlog replica that will be leader
-    try (ZkStateReader zkStateReader = ClusterStateMockUtil.buildClusterState("csrr*st2t2cst", "baseUrl1_", "baseUrl2_")) {
+    try (ZkStateReader zkStateReader = ClusterStateMockUtil.buildClusterState("csrr*st2t2cst", "baseUrl1:8983_", "baseUrl2:8984_")) {
       CloudReplicaSource cloudReplicaSource = new CloudReplicaSource.Builder()
           .collection("collection1")
           .onlyNrt(true) // enable only nrt mode
@@ -247,15 +247,15 @@ public class CloudReplicaSourceTest extends SolrTestCaseJ4 {
         switch (sliceName) {
           case "collection1_slice1":
             assertEquals(2, cloudReplicaSource.getReplicasBySlice(i).size());
-            assertEquals("http://baseUrl1/slice1_replica1/", cloudReplicaSource.getReplicasBySlice(i).get(0));
+            assertEquals("http://baseUrl1:8983/slice1_replica1/", cloudReplicaSource.getReplicasBySlice(i).get(0));
             break;
           case "collection1_slice2":
             assertEquals(1, cloudReplicaSource.getReplicasBySlice(i).size());
-            assertEquals("http://baseUrl2/slice2_replica3/", cloudReplicaSource.getReplicasBySlice(i).get(0));
+            assertEquals("http://baseUrl2:8984/slice2_replica3/", cloudReplicaSource.getReplicasBySlice(i).get(0));
             break;
           case "collection2_slice1":
             assertEquals(1, cloudReplicaSource.getReplicasBySlice(i).size());
-            assertEquals("http://baseUrl1/slice1_replica5/", cloudReplicaSource.getReplicasBySlice(i).get(0));
+            assertEquals("http://baseUrl1:8983/slice1_replica5/", cloudReplicaSource.getReplicasBySlice(i).get(0));
             break;
         }
       }
diff --git a/solr/solrj/src/java/org/apache/solr/common/util/Utils.java b/solr/solrj/src/java/org/apache/solr/common/util/Utils.java
index 86bef91..13e732d 100644
--- a/solr/solrj/src/java/org/apache/solr/common/util/Utils.java
+++ b/solr/solrj/src/java/org/apache/solr/common/util/Utils.java
@@ -759,7 +759,12 @@ public class Utils {
     return getBaseUrlForNodeName(nodeName, urlScheme, false);
   }
   public static String getBaseUrlForNodeName(final String nodeName, String urlScheme,  boolean isV2) {
-    final int _offset = nodeName.indexOf("_");
+    final int colonAt = nodeName.indexOf(':');
+    if (colonAt == -1) {
+      throw new IllegalArgumentException("nodeName does not contain expected ':' separator: " + nodeName);
+    }
+
+    final int _offset = nodeName.indexOf("_", colonAt);
     if (_offset < 0) {
       throw new IllegalArgumentException("nodeName does not contain expected '_' separator: " + nodeName);
     }
diff --git a/solr/solrj/src/test/org/apache/solr/client/solrj/routing/ReplicaListTransformerTest.java b/solr/solrj/src/test/org/apache/solr/client/solrj/routing/ReplicaListTransformerTest.java
index 5e90da2..0706985 100644
--- a/solr/solrj/src/test/org/apache/solr/client/solrj/routing/ReplicaListTransformerTest.java
+++ b/solr/solrj/src/test/org/apache/solr/client/solrj/routing/ReplicaListTransformerTest.java
@@ -137,7 +137,7 @@ public class ReplicaListTransformerTest extends SolrTestCase {
       final Map<String,Object> propMap = new HashMap<String,Object>();
       propMap.put("base_url", url);
       propMap.put("core", "test_core");
-      propMap.put("node_name", "test_node");
+      propMap.put("node_name", "test_node:80_");
       propMap.put("type", "NRT");
       // a skeleton replica, good enough for this test's purposes
       final Replica replica = new Replica(name, propMap,"c1","s1");
diff --git a/solr/solrj/src/test/org/apache/solr/client/solrj/routing/ShufflingReplicaListTransformerTest.java b/solr/solrj/src/test/org/apache/solr/client/solrj/routing/ShufflingReplicaListTransformerTest.java
index e9156c8..b629737 100644
--- a/solr/solrj/src/test/org/apache/solr/client/solrj/routing/ShufflingReplicaListTransformerTest.java
+++ b/solr/solrj/src/test/org/apache/solr/client/solrj/routing/ShufflingReplicaListTransformerTest.java
@@ -40,7 +40,7 @@ public class ShufflingReplicaListTransformerTest extends SolrTestCase {
       Map<String, Object> propMap = new HashMap<>();
       propMap.put("core", "core" + counter);
       propMap.put("type", "NRT");
-      propMap.put("node_name", "node" + counter + "_");
+      propMap.put("node_name", "node" + counter + ":8983_");
       counter++;
       replicas.add(new Replica(url, propMap, "c1", "s1"));
     }
diff --git a/solr/solrj/src/test/org/apache/solr/common/cloud/UrlSchemeTest.java b/solr/solrj/src/test/org/apache/solr/common/cloud/UrlSchemeTest.java
index 077f3ca..30f6c3f 100644
--- a/solr/solrj/src/test/org/apache/solr/common/cloud/UrlSchemeTest.java
+++ b/solr/solrj/src/test/org/apache/solr/common/cloud/UrlSchemeTest.java
@@ -34,12 +34,13 @@ public class UrlSchemeTest extends SolrTestCase {
     //  mock a SolrZkClient with some live nodes and cluster props set.
     String liveNode1 = "192.168.1.1:8983_solr";
     String liveNode2 = "127.0.0.1:8983_solr";
-    String liveNode3 = "127.0.0.1_";
+    String liveNode3 = "127.0.0.1:80_";
     String liveNode4 = "127.0.0.1:61631_l_%2Fig";
+    String liveNode5 = "some_weird_hostname-here:8983_solr%2Fx";
 
     assertEquals("https://192.168.1.1:8983/solr", t.getBaseUrlForNodeName(liveNode1));
     assertEquals("https://127.0.0.1:8983/solr", t.getBaseUrlForNodeName(liveNode2));
-    assertEquals("https://127.0.0.1", t.getBaseUrlForNodeName(liveNode3));
+    assertEquals("https://127.0.0.1:80", t.getBaseUrlForNodeName(liveNode3));
     assertEquals("https://127.0.0.1:61631/l_/ig", t.getBaseUrlForNodeName(liveNode4));
     // heal wrong scheme too
     assertEquals("https://127.0.0.1:8983/solr", t.applyUrlScheme("127.0.0.1:8983/solr"));
@@ -48,8 +49,10 @@ public class UrlSchemeTest extends SolrTestCase {
     t.setUrlScheme(HTTP);
     assertEquals("http://192.168.1.1:8983/solr", t.getBaseUrlForNodeName(liveNode1));
     assertEquals("http://127.0.0.1:8983/solr", t.getBaseUrlForNodeName(liveNode2));
-    assertEquals("http://127.0.0.1", t.getBaseUrlForNodeName(liveNode3));
+    assertEquals("http://127.0.0.1:80", t.getBaseUrlForNodeName(liveNode3));
     assertEquals("http://127.0.0.1:61631/l_/ig", t.getBaseUrlForNodeName(liveNode4));
+    assertEquals("http://some_weird_hostname-here:8983/solr/x", t.getBaseUrlForNodeName(liveNode5));
+
     // heal wrong scheme too
     assertEquals("http://127.0.0.1:8983/solr", t.applyUrlScheme("https://127.0.0.1:8983/solr"));
   }