[CN-922] Fix IPv6 address parsing error

[semihbkgr]

Fixes #963

[codecov-commenter]

Codecov Report

Merging #964 (fe8fd67) into master (7ef6939) will decrease coverage by 0.14%.
Report is 1 commits behind head on master.
The diff coverage is 76.21%.

@@            Coverage Diff             @@
##           master     #964      +/-   ##
==========================================
- Coverage   78.33%   78.20%   -0.14%     
==========================================
  Files         385      385              
  Lines       21184    21406     +222     
==========================================
+ Hits        16595    16741     +146     
- Misses       3566     3627      +61     
- Partials     1023     1038      +15     

Files Changed	Coverage Δ
client_internals_test_utils.go	69.23% <0.00%> (-2.77%)	⬇️
cluster/cloud_config.go	100.00% <ø> (ø)
internal/cluster/connection.go	37.50% <0.00%> (-0.18%)	⬇️
internal/cloud/cloud_discovery.go	53.42% <13.33%> (-14.76%)	⬇️
internal/rest/http_error.go	57.14% <37.50%> (-13.45%)	⬇️
internal/sql/driver/result.go	85.26% <50.00%> (-4.85%)	⬇️
internal/serialization/default_compact_writer.go	83.89% <66.19%> (-10.12%)	⬇️
internal/rest/http_client.go	82.08% <78.94%> (-2.92%)	⬇️
internal/sql/driver/sql_service.go	72.26% <83.33%> (-0.95%)	⬇️
internal/serialization/default_compact_reader.go	86.95% <99.03%> (+2.16%)	⬆️
... and 5 more

... and 3 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

[yuce]

Some ideas...

[yuce]

Here are some ideas...

[yuce]

Is this check necessary?

[semihbkgr]

It is necessary as we get i+1 as an index of addr at the rest

[yuce]

That condition makes the function return true for 192.168.1.1:. Is that what was intended?

[yuce]

Returning true for something like 192.168.1.1 is misleading, and may be an indicator that hasPort isn't suitable for our case. Maybe moving the code that splits host and port outside of ParseAddris better. How about something like:

func ParseAddr(addr string) (string, int, error) {
	addr = strings.TrimSpace(addr)
	if addr == "" {
		return defaultHost, 0, nil
	}
	host, port, err := splitHostPort(addr)
	if err != nil {
		return "", 0, ihzerrors.NewClientError("", err, hzerrors.ErrInvalidAddress)
	}
	if port < 0 || port > 65535 {
		return "", 0, ihzerrors.NewClientError("port not in valid range", err, hzerrors.ErrInvalidAddress)
	}
	return host, port, nil
}

func splitHostPort(addr string) (host string, port int, err error) {
	idx := strings.LastIndex(addr, ":")
	if idx == -1 {
		return addr, 0, nil
	}
	port, err = strconv.Atoi(addr[idx+1:])
	if err != nil {
		return "", 0, fmt.Errorf("invalid port")
	}
	host, _, err = net.SplitHostPort(addr)
	if err != nil {
		return "", 0, err
	}
	if host == "" {
		host = defaultHost
	}
	return host, port, nil
}

[yuce]

What's the panic you're getting?
The following works OK without panics:

func splitHostPort(addr string) (host string, port int, err error) {
	idx := strings.LastIndex(addr, ":")
	if idx == -1 {
		return addr, 0, nil
	}
	port, err = strconv.Atoi(addr[idx+1:])
	if err != nil {
		return "", 0, fmt.Errorf("invalid port")
	}
	host, _, err = net.SplitHostPort(addr)
	if err != nil {
		return "", 0, err
	}
	if host == "" {
		host = defaultHost
	}
	return host, port, nil
}

func main() {
	host, port, err := splitHostPort("192.168.1.1:")
	if err != nil {
		log.Fatal(err)
	}
	fmt.Println(host, port)
}

[semihbkgr]

It works well, and is better way than checking if address has port separately.