fix: add more check in cluster node parsing (#2538)

apache · Sep 22, 2024 · f2bc224 · f2bc224
1 parent 8c42aa0
commit f2bc224
Show file tree

Hide file tree

Showing 2 changed files with 27 additions and 1 deletion.
diff --git a/src/cluster/cluster.cc b/src/cluster/cluster.cc
@@ -773,9 +773,14 @@ Status Cluster::parseClusterNodes(const std::string &nodes_str, ClusterNodes *no
 
     // 6) slot info
     auto valid_range = NumericRange<int>{0, kClusterSlots - 1};
+    const std::regex node_id_regex(R"(\b[a-fA-F0-9]{40}\b)");
     for (unsigned i = 5; i < fields.size(); i++) {
       std::vector<std::string> ranges = util::Split(fields[i], "-");
       if (ranges.size() == 1) {
+        if (std::regex_match(fields[i], node_id_regex)) {
+          return {Status::ClusterInvalidInfo, "Invalid nodes definition: Missing newline between node entries."};
+        }
+
         auto parse_start = ParseInt<int>(ranges[0], valid_range, 10);
         if (!parse_start) {
           return {Status::ClusterInvalidInfo, errSlotOutOfRange};

diff --git a/tests/gocase/integration/cluster/cluster_test.go b/tests/gocase/integration/cluster/cluster_test.go
@@ -126,10 +126,31 @@ func TestClusterNodes(t *testing.T) {
 		require.ErrorContains(t, rdb.Do(ctx, "cluster", "nodes", "a").Err(), "CLUSTER")
 		require.ErrorContains(t, rdb.Do(ctx, "clusterx", "setnodeid", "a").Err(), "CLUSTER")
 		require.ErrorContains(t, rdb.Do(ctx, "clusterx", "setnodes", "a").Err(), "CLUSTER")
-		require.ErrorContains(t, rdb.Do(ctx, "clusterx", "setnodes", "a", -1).Err(), "Invalid cluster version")
+		require.ErrorContains(t, rdb.Do(ctx, "clusterx", "setnodes", "a", -1).Err(), "ERR Invalid cluster version")
 		require.ErrorContains(t, rdb.Do(ctx, "clusterx", "setslot", "16384", "07c37dfeb235213a872192d90877d0cd55635b91", 1).Err(), "CLUSTER")
 		require.ErrorContains(t, rdb.Do(ctx, "clusterx", "setslot", "16384", "a", 1).Err(), "CLUSTER")
 	})
+
+	t.Run("command line simulation with missing newlines", func(t *testing.T) {
+		clusterNodes := fmt.Sprintf("%s %s %d master - 0-100 %s %s %d slave %s",
+			nodeID, srv.Host(), srv.Port(),
+			"07c37dfeb235213a872192d90877d0cd55635b92", srv.Host(), srv.Port()+1, nodeID)
+
+		// set cluster nodes without newlines
+		err := rdb.Do(ctx, "clusterx", "SETNODES", clusterNodes, "2").Err()
+		require.ErrorContains(t, err, "Invalid nodes definition: Missing newline between node entries.")
+
+		// add the missing newline to correct the definition
+		clusterNodesWithNewline := fmt.Sprintf("%s %s %d master - 0-100\n%s %s %d slave %s",
+			nodeID, srv.Host(), srv.Port(),
+			"07c37dfeb235213a872192d90877d0cd55635b92", srv.Host(), srv.Port()+1, nodeID)
+
+		err = rdb.Do(ctx, "clusterx", "SETNODES", clusterNodesWithNewline, "2").Err()
+		require.NoError(t, err)
+		nodes := rdb.ClusterNodes(ctx).Val()
+		require.Contains(t, nodes, "0-100")
+		require.Contains(t, nodes, "slave")
+	})
 }
 
 func TestClusterReplicas(t *testing.T) {