Skip to content

Commit

Permalink
Exclude '.' and ':' from isValidAuxChar's banned charset (#963)
Browse files Browse the repository at this point in the history
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 <pingxie@google.com>
  • Loading branch information
PingXie authored and madolson committed Sep 2, 2024
1 parent a33008d commit a02c532
Show file tree
Hide file tree
Showing 2 changed files with 24 additions and 1 deletion.
11 changes: 10 additions & 1 deletion src/cluster.c
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
14 changes: 14 additions & 0 deletions tests/unit/cluster/announce-client-ip.tcl
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
}

0 comments on commit a02c532

Please sign in to comment.