Skip to content

Commit 53709f0

Browse files
marcingrzejszczakmp911de
marcingrzejszczak
authored and
mp911de
committed
Improve Cluster Nodes parsing.
Without this fix there's a problem with parsing of the cluster nodes command output (e.g. a trailing comma after cport is making the parsing fail) with this change we're converting regexp parsing to code parsing which also includes support for trailing commas after cport Closes spring-projects#2862 Original Pull Request: spring-projects#3000
1 parent 8906eb5 commit 53709f0

File tree

2 files changed

+115
-17
lines changed

2 files changed

+115
-17
lines changed

‎src/main/java/org/springframework/data/redis/connection/convert/Converters.java‎

Lines changed: 73 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -20,11 +20,10 @@
2020
import java.time.Duration;
2121
import java.util.*;
2222
import java.util.concurrent.TimeUnit;
23-
import java.util.regex.Matcher;
24-
import java.util.regex.Pattern;
2523

2624
import org.apache.commons.logging.Log;
2725
import org.apache.commons.logging.LogFactory;
26+
2827
import org.springframework.core.convert.converter.Converter;
2928
import org.springframework.data.geo.Distance;
3029
import org.springframework.data.geo.GeoResult;
@@ -45,6 +44,7 @@
4544
import org.springframework.data.redis.connection.zset.Tuple;
4645
import org.springframework.data.redis.serializer.RedisSerializer;
4746
import org.springframework.data.redis.util.ByteUtils;
47+
import org.springframework.lang.NonNull;
4848
import org.springframework.lang.Nullable;
4949
import org.springframework.util.Assert;
5050
import org.springframework.util.ClassUtils;
@@ -545,10 +545,15 @@ enum ClusterNodesConverter implements Converter<String, RedisClusterNode> {
545545
* <li>{@code %s:%i} (Redis 3)</li>
546546
* <li>{@code %s:%i@%i} (Redis 4, with bus port)</li>
547547
* <li>{@code %s:%i@%i,%s} (Redis 7, with announced hostname)</li>
548+
*
549+
* The output of the {@code CLUSTER NODES } command is just a space-separated CSV string, where each
550+
* line represents a node in the cluster. The following is an example of output on Redis 7.2.0.
551+
* You can check the latest <a href="https://redis.io/docs/latest/commands/cluster-nodes/">here</a>.
552+
*
553+
* {@code <id> <ip:port@cport[,hostname]> <flags> <master> <ping-sent> <pong-recv> <config-epoch> <link-state> <slot> <slot> ... <slot>}
554+
*
548555
* </ul>
549556
*/
550-
static final Pattern clusterEndpointPattern = Pattern
551-
.compile("\\[?([0-9a-zA-Z\\-_\\.:]*)\\]?:([0-9]+)(?:@[0-9]+(?:,([^,].*))?)?");
552557
private static final Map<String, Flag> flagLookupMap;
553558

554559
static {
@@ -567,18 +572,75 @@ enum ClusterNodesConverter implements Converter<String, RedisClusterNode> {
567572
static final int LINK_STATE_INDEX = 7;
568573
static final int SLOTS_INDEX = 8;
569574

575+
record AddressPortHostname(String addressPart, String portPart, @Nullable String hostnamePart) {
576+
577+
static AddressPortHostname of(String[] args) {
578+
Assert.isTrue(args.length >= HOST_PORT_INDEX + 1, "ClusterNode information does not define host and port");
579+
// <ip:port@cport[,hostname]>
580+
String hostPort = args[HOST_PORT_INDEX];
581+
int lastColon = hostPort.lastIndexOf(":");
582+
Assert.isTrue(lastColon != -1, "ClusterNode information does not define host and port");
583+
String addressPart = getAddressPart(hostPort, lastColon);
584+
// Everything to the right of port
585+
int indexOfColon = hostPort.indexOf(",");
586+
boolean hasColon = indexOfColon != -1;
587+
String hostnamePart = getHostnamePart(hasColon, hostPort, indexOfColon);
588+
String portPart = getPortPart(hostPort, lastColon, hasColon, indexOfColon);
589+
return new AddressPortHostname(addressPart, portPart, hostnamePart);
590+
}
591+
592+
@NonNull private static String getAddressPart(String hostPort, int lastColon) {
593+
// Everything to the left of port
594+
// 127.0.0.1:6380
595+
// 127.0.0.1:6380@6381
596+
// :6380
597+
// :6380@6381
598+
// 2a02:6b8:c67:9c:0:6d8b:33da:5a2c:6380
599+
// 2a02:6b8:c67:9c:0:6d8b:33da:5a2c:6380@6381
600+
// 127.0.0.1:6380,hostname1
601+
// 127.0.0.1:6380@6381,hostname1
602+
// :6380,hostname1
603+
// :6380@6381,hostname1
604+
// 2a02:6b8:c67:9c:0:6d8b:33da:5a2c:6380,hostname1
605+
// 2a02:6b8:c67:9c:0:6d8b:33da:5a2c:6380@6381,hostname1
606+
String addressPart = hostPort.substring(0, lastColon);
607+
// [2a02:6b8:c67:9c:0:6d8b:33da:5a2c]:6380
608+
// [2a02:6b8:c67:9c:0:6d8b:33da:5a2c]:6380@6381
609+
// [2a02:6b8:c67:9c:0:6d8b:33da:5a2c]:6380,hostname1
610+
// [2a02:6b8:c67:9c:0:6d8b:33da:5a2c]:6380@6381,hostname1
611+
if (addressPart.startsWith("[") && addressPart.endsWith("]")) {
612+
addressPart = addressPart.substring(1, addressPart.length() - 1);
613+
}
614+
return addressPart;
615+
}
616+
617+
@Nullable
618+
private static String getHostnamePart(boolean hasColon, String hostPort, int indexOfColon) {
619+
// Everything to the right starting from comma
620+
String hostnamePart = hasColon ? hostPort.substring(indexOfColon + 1) : null;
621+
return StringUtils.hasText(hostnamePart) ? hostnamePart : null;
622+
}
623+
624+
@NonNull private static String getPortPart(String hostPort, int lastColon, boolean hasColon, int indexOfColon) {
625+
String portPart = hostPort.substring(lastColon + 1);
626+
if (portPart.contains("@")) {
627+
portPart = portPart.substring(0, portPart.indexOf("@"));
628+
} else if (hasColon) {
629+
portPart = portPart.substring(0, indexOfColon);
630+
}
631+
return portPart;
632+
}
633+
}
634+
570635
@Override
571636
public RedisClusterNode convert(String source) {
572637

573638
String[] args = source.split(" ");
574639

575-
Matcher matcher = clusterEndpointPattern.matcher(args[HOST_PORT_INDEX]);
576-
577-
Assert.isTrue(matcher.matches(), "ClusterNode information does not define host and port");
578-
579-
String addressPart = matcher.group(1);
580-
String portPart = matcher.group(2);
581-
String hostnamePart = matcher.group(3);
640+
AddressPortHostname addressPortHostname = AddressPortHostname.of(args);
641+
String addressPart = addressPortHostname.addressPart;
642+
String portPart = addressPortHostname.portPart;
643+
String hostnamePart = addressPortHostname.hostnamePart;
582644

583645
SlotRange range = parseSlotRange(args);
584646
Set<Flag> flags = parseFlags(args);

‎src/test/java/org/springframework/data/redis/connection/convert/ConvertersUnitTests.java‎

Lines changed: 42 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -18,9 +18,9 @@
1818
import static org.assertj.core.api.Assertions.*;
1919

2020
import java.util.Iterator;
21-
import java.util.regex.Matcher;
2221
import java.util.stream.Stream;
2322

23+
import org.assertj.core.api.SoftAssertions;
2424
import org.junit.jupiter.api.Test;
2525
import org.junit.jupiter.params.ParameterizedTest;
2626
import org.junit.jupiter.params.provider.Arguments;
@@ -72,6 +72,10 @@ class ConvertersUnitTests {
7272

7373
private static final String CLUSTER_NODE_WITH_SINGLE_INVALID_IPV6_HOST = "67adfe3df1058896e3cb49d2863e0f70e7e159fa 2a02:6b8:c67:9c:0:6d8b:33da:5a2c: master,nofailover - 0 1692108412315 1 connected 0-5460";
7474

75+
private static final String CLUSTER_NODE_WITH_SINGLE_IPV4_EMPTY_HOSTNAME = "3765733728631672640db35fd2f04743c03119c6 10.180.0.33:11003@16379, master - 0 1708041426947 2 connected 0-5460";
76+
77+
private static final String CLUSTER_NODE_WITH_SINGLE_IPV4_HOSTNAME = "3765733728631672640db35fd2f04743c03119c6 10.180.0.33:11003@16379,hostname1 master - 0 1708041426947 2 connected 0-5460";
78+
7579
@Test // DATAREDIS-315
7680
void toSetOfRedis30ClusterNodesShouldConvertSingleStringNodesResponseCorrectly() {
7781

@@ -248,6 +252,39 @@ void toClusterNodeWithIPv6Hostname() {
248252
assertThat(node.getSlotRange().getSlots().size()).isEqualTo(5461);
249253
}
250254

255+
@Test // https://github.com/spring-projects/spring-data-redis/issues/2862
256+
void toClusterNodeWithIPv4EmptyHostname() {
257+
RedisClusterNode node = Converters.toClusterNode(CLUSTER_NODE_WITH_SINGLE_IPV4_EMPTY_HOSTNAME);
258+
259+
SoftAssertions.assertSoftly(softAssertions -> {
260+
softAssertions.assertThat(node.getId()).isEqualTo("3765733728631672640db35fd2f04743c03119c6");
261+
softAssertions.assertThat(node.getHost()).isEqualTo("10.180.0.33");
262+
softAssertions.assertThat(node.hasValidHost()).isTrue();
263+
softAssertions.assertThat(node.getPort()).isEqualTo(11003);
264+
softAssertions.assertThat(node.getType()).isEqualTo(NodeType.MASTER);
265+
softAssertions.assertThat(node.getFlags()).contains(Flag.MASTER);
266+
softAssertions.assertThat(node.getLinkState()).isEqualTo(LinkState.CONNECTED);
267+
softAssertions.assertThat(node.getSlotRange().getSlots().size()).isEqualTo(5461);
268+
});
269+
}
270+
271+
@Test // https://github.com/spring-projects/spring-data-redis/issues/2862
272+
void toClusterNodeWithIPv4Hostname() {
273+
RedisClusterNode node = Converters.toClusterNode(CLUSTER_NODE_WITH_SINGLE_IPV4_HOSTNAME);
274+
275+
SoftAssertions.assertSoftly(softAssertions -> {
276+
softAssertions.assertThat(node.getId()).isEqualTo("3765733728631672640db35fd2f04743c03119c6");
277+
softAssertions.assertThat(node.getHost()).isEqualTo("10.180.0.33");
278+
softAssertions.assertThat(node.getName()).isEqualTo("hostname1");
279+
softAssertions.assertThat(node.hasValidHost()).isTrue();
280+
softAssertions.assertThat(node.getPort()).isEqualTo(11003);
281+
softAssertions.assertThat(node.getType()).isEqualTo(NodeType.MASTER);
282+
softAssertions.assertThat(node.getFlags()).contains(Flag.MASTER);
283+
softAssertions.assertThat(node.getLinkState()).isEqualTo(LinkState.CONNECTED);
284+
softAssertions.assertThat(node.getSlotRange().getSlots().size()).isEqualTo(5461);
285+
});
286+
}
287+
251288
@Test // GH-2678
252289
void toClusterNodeWithIPv6HostnameSquareBrackets() {
253290

@@ -273,12 +310,11 @@ void toClusterNodeWithInvalidIPv6Hostname() {
273310
@MethodSource("clusterNodesEndpoints")
274311
void shouldAcceptHostPatterns(String endpoint, String expectedAddress, String expectedPort, String expectedHostname) {
275312

276-
Matcher matcher = ClusterNodesConverter.clusterEndpointPattern.matcher(endpoint);
277-
assertThat(matcher.matches()).isTrue();
313+
ClusterNodesConverter.AddressPortHostname addressPortHostname = ClusterNodesConverter.AddressPortHostname.of(new String[] { "id", endpoint });
278314

279-
assertThat(matcher.group(1)).isEqualTo(expectedAddress);
280-
assertThat(matcher.group(2)).isEqualTo(expectedPort);
281-
assertThat(matcher.group(3)).isEqualTo(expectedHostname);
315+
assertThat(addressPortHostname.addressPart()).isEqualTo(expectedAddress);
316+
assertThat(addressPortHostname.portPart()).isEqualTo(expectedPort);
317+
assertThat(addressPortHostname.hostnamePart()).isEqualTo(expectedHostname);
282318
}
283319

284320
static Stream<Arguments> clusterNodesEndpoints() {

0 commit comments

Comments
 (0)