Exclude '.' and ':' from isValidAuxChar's banned charset

[PingXie]

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

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 70.29%. Comparing base (25dd943) to head (9ff7e5f).
Report is 19 commits behind head on unstable.

Additional details and impacted files

@@             Coverage Diff              @@
##           unstable     #963      +/-   ##
============================================
- Coverage     70.41%   70.29%   -0.13%     
============================================
  Files           114      114              
  Lines         61744    61729      -15     
============================================
- Hits          43480    43394      -86     
- Misses        18264    18335      +71     

Files with missing lines	Coverage Δ
src/cluster.c	88.38% <100.00%> (-0.12%)	⬇️

... and 12 files with indirect coverage changes

[zuiderkwast]

Thanks for fixing this! It'd be embarrassing for me to release with this bug.

I thought the chars were the valid ones and added dot and colon to it, but it's actually the banned chars.

I assumed this check was used in the cluster bus, so I assumed it was covered by the tests already. My bad. And thanks again for spotting it!

[zuiderkwast]

This non-IPv6 test shouldn't bind on ::1.

We skip IPv6 tests only when TCL doesn't support it, so it's no harm, but in general it still involves IPv6. Maybe the node will not start if the machine doesn't have IPv6.

[enjoy-binbin]

i try it work in tclsh 8.5 (this env does not support ipv6 so we can see ipv6 is skip)

Testing unit/cluster/announce-client-ip
[ok]: Set cluster announced IPv4 to invalid IP (1 ms)
[ok]: Set cluster announced IPv4 and check that it propagates (115 ms)
[ok]: Clear announced client IPv4 and check that it propagates (214 ms)
[ok]: Check for memory leaks (pid 5151) (392 ms)
[ok]: Check for memory leaks (pid 5140) (286 ms)
[ok]: Check for memory leaks (pid 5129) (288 ms)
[ok]: Check for memory leaks (pid 5111) (268 ms)
[ignore]: TCL version is too low and does not support this
[ok]: Check for memory leaks (pid 5522) (290 ms)
[ok]: Load cluster announced IPv4 config on server start (555 ms)
[ok]: Check for memory leaks (pid 5611) (290 ms)
[ignore]: TCL version is too low and does not support this
[1/1 done]: unit/cluster/announce-client-ip (17 seconds)

                   The End

Execution time of different units:
  17 seconds - unit/cluster/announce-client-ip

\o/ All tests passed without errors!

Cleanup: may take some time... OK
➜  valkey git:(unstable) tclsh

WARNING: This version of tcl is included in macOS for compatibility with legacy software.
In future versions of macOS the tcl runtime will not be available by
default, and may require you to install an additional package.

% info patchlevel
8.5.9

[enjoy-binbin]

tcl 8.6 in mac (support ipv6)

❯ ./runtest --single unit/cluster/announce-client-ip --clients 1
Cleanup: may take some time... OK
Starting test server at port 21079
[ready]: 82590
Testing unit/cluster/announce-client-ip
[ok]: Set cluster announced IPv4 to invalid IP (0 ms)
[ok]: Set cluster announced IPv4 and check that it propagates (121 ms)
[ok]: Clear announced client IPv4 and check that it propagates (220 ms)
[ok]: Check for memory leaks (pid 82678) (286 ms)
[ok]: Check for memory leaks (pid 82652) (265 ms)
[ok]: Check for memory leaks (pid 82627) (293 ms)
[ok]: Check for memory leaks (pid 82594) (288 ms)
[ok]: Set cluster announced IPv6 to invalid IP (0 ms)
[ok]: Set cluster announced IPv6 and check that it propagates (216 ms)
[ok]: Clear announced client IPv6 and check that it propagates (121 ms)
[ok]: Check for memory leaks (pid 82927) (294 ms)
[ok]: Check for memory leaks (pid 82916) (265 ms)
[ok]: Check for memory leaks (pid 82882) (277 ms)
[ok]: Check for memory leaks (pid 82870) (264 ms)
[ok]: Check for memory leaks (pid 83207) (310 ms)
[ok]: Load cluster announced IPv4 config on server start (562 ms)
[ok]: Check for memory leaks (pid 83269) (275 ms)
[ok]: Check for memory leaks (pid 83303) (289 ms)
[ok]: Load cluster announced IPv6 config on server start (548 ms)
[ok]: Check for memory leaks (pid 83460) (282 ms)
[1/1 done]: unit/cluster/announce-client-ip (32 seconds)

                   The End

Execution time of different units:
  32 seconds - unit/cluster/announce-client-ip

\o/ All tests passed without errors!

Cleanup: may take some time... OK
❯ tclsh
% info patchlevel
8.6.14

[zuiderkwast]

Yes, as I said, it works for us because TCL is not using IPv6 in this test. Only the valkey node is listening to IPv6-localhost ::1, but no clients connect to it over IPv6.

If you try to run this in a machine or container that doesn't have IPv6 at all, maybe the Valkey node fails to start if it can't listen to ::1?

[zuiderkwast]

At least this looks better: 😀

Suggested change

	start_cluster 1 0 {tags {external:skip cluster} overrides {cluster-replica-no-failover yes bind {127.0.0.1 ::1}}} {
	start_cluster 1 0 {tags {external:skip cluster} overrides {cluster-replica-no-failover yes}} {

It's probably not important though.