Added handling to "?" and NULL hostnames in CLUSTER SLOTS

redis/src/cluster_topology.rs

@@ -103,14 +103,20 @@ pub(crate) fn parse_and_count_slots(
  if node.len() < 2 {
  return None;
  }
-
+ // According to the CLUSTER SLOTS documentation:
+ // If the received hostname is an empty string or NULL, clients should utilize the hostname of the responding node.
+ // However, if the received hostname is "?", it should be regarded as an indication of an unknown node.
  let hostname = if let Value::BulkString(ref ip) = node[0] {
  let hostname = String::from_utf8_lossy(ip);
  if hostname.is_empty() {
  addr_of_answering_node.into()
+ } else if hostname == "?" {
+ return None;
  } else {
  hostname
  }
+ } else if let Value::Nil = node[0] {
+ addr_of_answering_node.into()
  } else {
  return None;
  };
@@ -141,6 +147,13 @@ pub(crate) fn parse_and_count_slots(
  slots.push(Slot::new(start, end, nodes.pop().unwrap(), replicas));
  }
  }
+ if slots.is_empty() {
+ return Err(RedisError::from((
+ ErrorKind::ResponseError,
+ "Error parsing slots: No healthy node found",
+ format!("Raw slot map response: {:?}", raw_slot_resp),
+ )));
+ }
  // we sort the slots, because different nodes in a cluster might return the same slot view
  // in different orders, which might cause the views to be considered evaluated as not equal.
  slots.sort_unstable_by(|first, second| match first.start().cmp(&second.start()) {

redis/tests/test_cluster.rs

@@ -294,6 +294,101 @@ fn test_cluster_can_connect_to_server_that_sends_cluster_slots_without_host_name
  assert_eq!(value, Ok(Value::Nil));
 }
 
+#[test]
+fn test_cluster_can_connect_to_server_that_sends_cluster_slots_with_null_host_name() {
+ let name = "test_cluster_can_connect_to_server_that_sends_cluster_slots_with_null_host_name";
+
+ let MockEnv { mut connection, .. } = MockEnv::new(name, move |cmd: &[u8], _| {
+ if contains_slice(cmd, b"PING") {
+ Err(Ok(Value::SimpleString("OK".into())))
+ } else if contains_slice(cmd, b"CLUSTER") && contains_slice(cmd, b"SLOTS") {
+ Err(Ok(Value::Array(vec![Value::Array(vec![
+ Value::Int(0),
+ Value::Int(16383),
+ Value::Array(vec![Value::Nil, Value::Int(6379)]),
+ ])])))
+ } else {
+ Err(Ok(Value::Nil))
+ }
+ });
+
+ let value = cmd("GET").arg("test").query::<Value>(&mut connection);
+
+ assert_eq!(value, Ok(Value::Nil));
+}
+
+#[test]
+fn test_cluster_cannot_connect_to_server_with_unknown_host_name() {
+ let name = "test_cluster_cannot_connect_to_server_with_unknown_host_name";
+ let handler = move |cmd: &[u8], _| {
+ if contains_slice(cmd, b"PING") {
+ Err(Ok(Value::SimpleString("OK".into())))
+ } else if contains_slice(cmd, b"CLUSTER") && contains_slice(cmd, b"SLOTS") {
+ Err(Ok(Value::Array(vec![Value::Array(vec![
+ Value::Int(0),
+ Value::Int(16383),
+ Value::Array(vec![
+ Value::BulkString("?".as_bytes().to_vec()),
+ Value::Int(6379),
+ ]),
+ ])])))
+ } else {
+ Err(Ok(Value::Nil))
+ }
+ };
+ let client_builder = ClusterClient::builder(vec![&*format!("redis://{name}")]);
+ let client = client_builder.build().unwrap();
+ MOCK_CONN_UTILS.write().unwrap().insert(
+ name.to_string(),
+ MockConnectionUtils {
+ handler: Some(Arc::new(handler)),
+ ..Default::default()
+ },
+ );
+ let connection = client.get_generic_connection::<MockConnection>();
+ assert!(connection.is_err());
+ let err = connection.err().unwrap();
+ assert!(err
+ .to_string()
+ .contains("Error parsing slots: No healthy node found"))
+}
+
+#[test]
+fn test_cluster_can_connect_to_server_that_sends_cluster_slots_with_partial_nodes_with_unknown_host_name(
+) {
+ let name = "test_cluster_can_connect_to_server_that_sends_cluster_slots_with_partial_nodes_with_unknown_host_name";
+
+ let MockEnv { mut connection, .. } = MockEnv::new(name, move |cmd: &[u8], _| {
+ if contains_slice(cmd, b"PING") {
+ Err(Ok(Value::SimpleString("OK".into())))
+ } else if contains_slice(cmd, b"CLUSTER") && contains_slice(cmd, b"SLOTS") {
+ Err(Ok(Value::Array(vec![
+ Value::Array(vec![
+ Value::Int(0),
+ Value::Int(7000),
+ Value::Array(vec![
+ Value::BulkString(name.as_bytes().to_vec()),
+ Value::Int(6379),
+ ]),
+ ]),
+ Value::Array(vec![
+ Value::Int(7001),
+ Value::Int(16383),
+ Value::Array(vec![
+ Value::BulkString("?".as_bytes().to_vec()),
+ Value::Int(6380),
+ ]),
+ ]),
+ ])))
+ } else {
+ Err(Ok(Value::Nil))
+ }
+ });
+
+ let value = cmd("GET").arg("test").query::<Value>(&mut connection);
+ assert_eq!(value, Ok(Value::Nil));
+}
+
 #[test]
 fn test_cluster_pipeline_command_ordering() {
  let cluster = TestClusterContext::new(3, 0);