From ad0ede302cc1363985e677d008f23ef2031536bc Mon Sep 17 00:00:00 2001 From: Ping Xie Date: Wed, 28 Aug 2024 23:35:31 -0700 Subject: [PATCH] Exclude '.' and ':' from `isValidAuxChar`'s banned charset (#963) Fix a bug in isValidAuxChar where valid characters '.' and ':' were incorrectly included in the banned charset. This issue affected the validation of auxiliary fields in the nodes.conf file used by Valkey in cluster mode, particularly when handling IPv4 and IPv6 addresses. The code now correctly allows '.' and ':' as valid characters, ensuring proper handling of these fields. Comments were added to clarify the use of the banned charset. Related to #736 --------- Signed-off-by: Ping Xie --- src/cluster.c | 11 ++++++++++- tests/unit/cluster/announce-client-ip.tcl | 14 ++++++++++++++ 2 files changed, 24 insertions(+), 1 deletion(-) diff --git a/src/cluster.c b/src/cluster.c index ced7668d65..566ac5a19e 100644 --- a/src/cluster.c +++ b/src/cluster.c @@ -748,7 +748,16 @@ int verifyClusterNodeId(const char *name, int length) { } int isValidAuxChar(int c) { - return isalnum(c) || (strchr("!#$%&()*+.:;<>?@[]^{|}~", c) == NULL); + /* Return true if the character is alphanumeric */ + if (isalnum(c)) { + return 1; + } + + /* List of invalid characters */ + static const char *invalid_charset = "!#$%&()*+;<>?@[]^{|}~"; + + /* Return true if the character is NOT in the invalid charset */ + return strchr(invalid_charset, c) == NULL; } int isValidAuxString(char *s, unsigned int length) { diff --git a/tests/unit/cluster/announce-client-ip.tcl b/tests/unit/cluster/announce-client-ip.tcl index f0aebd0807..68d13f821a 100644 --- a/tests/unit/cluster/announce-client-ip.tcl +++ b/tests/unit/cluster/announce-client-ip.tcl @@ -147,3 +147,17 @@ start_cluster 2 2 {tags {external:skip cluster ipv6} overrides {cluster-replica- [lindex $clients $j] close } } + +start_cluster 1 0 {tags {external:skip cluster} overrides {cluster-replica-no-failover yes bind {127.0.0.1 ::1}}} { + test "Load cluster announced IPv4 config on server start" { + R 0 config set cluster-announce-client-ipv4 "1.1.1.1" + restart_server 0 true false + } +} + +start_cluster 1 0 {tags {external:skip cluster ipv6} overrides {cluster-replica-no-failover yes bind {127.0.0.1 ::1}}} { + test "Load cluster announced IPv6 config on server start" { + R 0 config set cluster-announce-client-ipv6 "cafe:1234::0" + restart_server 0 true false + } +}