choksi2212 commented on code in PR #4497: URL: https://github.com/apache/iggy/pull/4497#discussion_r4258599698
########## foreign/go/client/connection_string.go: ########## @@ -0,0 +1,460 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +package client + +import ( + "fmt" + "math/big" + "net" + "strconv" + "strings" + + "github.com/apache/iggy/foreign/go/client/tcp" + ierror "github.com/apache/iggy/foreign/go/errors" +) + +const ( + defaultProtocol = "iggy" + protocolPrefix = "iggy+" + tcpProtocol = "tcp" +) + +// ParseConnectionString parses an Iggy TCP connection string into the TCP +// client options it describes, so the result can be passed to WithTcp: +// +// client.NewIggyClient(client.WithTcp(opts...)) +// +// with opts from client.ParseConnectionString("iggy://iggy:iggy@localhost:8090"). +// +// The accepted shape is: +// +// iggy://[username:password@ | personal_access_token@]host:port[?options] +// iggy+tcp://[username:password@ | personal_access_token@]host:port[?options] +// +// The scheme iggy:// means TCP, the same as the explicit iggy+tcp://. +// Other iggy+<transport> schemes such as iggy+quic:// are rejected, +// because the Go SDK implements the TCP transport only, which covers both +// plain TCP and TLS. Credentials are username:password or a single +// personal access token before the @; either way auto-login is configured, +// matching what the Rust and Node SDKs do with a connection string. The +// host may be a bracketed IPv6 literal, as in [::1]:8090. +// +// Query options follow the grammar the other SDKs share: +// +// tls=true|false enable TLS +// tls_domain=<name> TLS server name indication (SNI) +// tls_ca_file=<path> CA certificate file for verification +// reconnection_retries=<n|unlimited> +// reconnection_interval=<non-zero duration> +// reestablish_after=<duration> +// heartbeat_interval=<non-zero duration> +// nodelay=true|false +// +// Durations take the units the other SDKs share, from nanos to years, +// including the hr, hrs, wk, wks and month spellings, with optional +// fractions and parts run together or spaced, as in 1h30m or 1h 30m. +// The reconnection_interval and heartbeat_interval spellings must be +// non-zero, because the Rust SDK types them as non-zero durations; +// reestablish_after takes the zero spellings, which Rust accepts there. +// The Go client keeps its own reconnect and heartbeat pacing, so those +// three spellings are validated for compatibility and then ignored, and +// a value the other SDKs would reject is still rejected here, so one +// connection string is either valid everywhere or nowhere. The nodelay +// spelling is applied, because the Go client carries a matching +// TCP_NODELAY option. +func ParseConnectionString(connectionString string) ([]tcp.Option, error) { + protocolParts := strings.Split(connectionString, "://") + if len(protocolParts) != 2 { + return nil, ierror.InvalidConnectionString{} + } + + protocol, err := parseProtocol(protocolParts[0]) + if err != nil { + return nil, err + } + if protocol != tcpProtocol { + // Only a plain transport name is echoed, because a malformed + // scheme may carry credentials, as the C# SDK notes on the same + // check. + if !isASCIILetters(protocol) { + return nil, ierror.InvalidConnectionString{} + } + return nil, fmt.Errorf("%w: unsupported transport %q, Go SDK supports tcp only", + ierror.InvalidConnectionString{}, protocol) + } + + parts := strings.Split(protocolParts[1], "@") + if len(parts) != 2 { + return nil, ierror.InvalidConnectionString{} + } + + credentials, err := parseCredentials(parts[0]) + if err != nil { + return nil, err + } + + serverAndOptions := strings.Split(parts[1], "?") + if len(serverAndOptions) > 2 { + return nil, ierror.InvalidConnectionString{} + } + + var tlsConfig parsedTLS + tlsConfig.noDelay = true + if len(serverAndOptions) == 2 { + tlsConfig, err = parseConnectionOptions(serverAndOptions[1]) + if err != nil { + return nil, err + } + } + + serverAddress := serverAndOptions[0] + if serverAddress == "" { + return nil, ierror.InvalidConnectionString{} + } + host, portStr, err := net.SplitHostPort(serverAddress) + if err != nil || host == "" { + return nil, ierror.InvalidConnectionString{} + } + // Node and Rust accept any u16 port, 0 included; the dial fails later + // instead of the parse. A bad port reports the same error as every other + // parse failure, which is what the Rust SDK returns here. + if _, err := strconv.ParseUint(portStr, 10, 16); err != nil { + return nil, ierror.InvalidConnectionString{} + } + + options := []tcp.Option{tcp.WithServerAddress(serverAddress)} + options = append(options, tcp.WithAutoLogin(credentials)) + if tlsConfig.enabled { + var tlsOpts []tcp.TLSOption + if tlsConfig.domain != "" { + tlsOpts = append(tlsOpts, tcp.WithTLSDomain(tlsConfig.domain)) + } + if tlsConfig.caFile != "" { + tlsOpts = append(tlsOpts, tcp.WithTLSCAFile(tlsConfig.caFile)) + } + options = append(options, tcp.WithTLS(tlsOpts...)) + } + options = append(options, tcp.WithNoDelay(tlsConfig.noDelay)) + return options, nil +} + +// parseProtocol extracts the transport from the scheme, which is either the +// bare default protocol or an explicit iggy+<transport> spelling. +func parseProtocol(scheme string) (string, error) { + if scheme == defaultProtocol { + return tcpProtocol, nil + } + if !strings.HasPrefix(scheme, protocolPrefix) { + return "", ierror.InvalidConnectionString{} + } + return strings.TrimPrefix(scheme, protocolPrefix), nil +} + +// parseCredentials turns the userinfo before the @ into auto-login +// credentials: username:password or a single personal access token. +func parseCredentials(userinfo string) (tcp.Credentials, error) { + username, password, hasSeparator := strings.Cut(userinfo, ":") + if !hasSeparator { + if username == "" { + return tcp.Credentials{}, ierror.InvalidConnectionString{} + } + return tcp.NewPersonalAccessTokenCredentials(username), nil + } + if username == "" || password == "" { + return tcp.Credentials{}, ierror.InvalidConnectionString{} + } + return tcp.NewUsernamePasswordCredentials(username, password), nil +} + +type parsedTLS struct { + enabled bool + domain string + caFile string + noDelay bool +} + +// parseConnectionOptions validates the shared connection-string grammar, +// keeping the tls options and ignoring the reconnect spellings the Go +// client does not take from a connection string. +func parseConnectionOptions(optionsString string) (parsedTLS, error) { + parsed := parsedTLS{} Review Comment: Good catch — the fresh struct was resetting the default whenever a query carried other options. parsedTLS now starts with noDelay: true, so ?tls=true alone keeps Nagle off and nodelay=false stays the only way to turn it back on. Pinned in the NoDelay test with a query that leaves the spelling out. ########## foreign/go/client/connection_string.go: ########## @@ -0,0 +1,460 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +package client + +import ( + "fmt" + "math/big" + "net" + "strconv" + "strings" + + "github.com/apache/iggy/foreign/go/client/tcp" + ierror "github.com/apache/iggy/foreign/go/errors" +) + +const ( + defaultProtocol = "iggy" + protocolPrefix = "iggy+" + tcpProtocol = "tcp" +) + +// ParseConnectionString parses an Iggy TCP connection string into the TCP +// client options it describes, so the result can be passed to WithTcp: +// +// client.NewIggyClient(client.WithTcp(opts...)) +// +// with opts from client.ParseConnectionString("iggy://iggy:iggy@localhost:8090"). +// +// The accepted shape is: +// +// iggy://[username:password@ | personal_access_token@]host:port[?options] +// iggy+tcp://[username:password@ | personal_access_token@]host:port[?options] +// +// The scheme iggy:// means TCP, the same as the explicit iggy+tcp://. +// Other iggy+<transport> schemes such as iggy+quic:// are rejected, +// because the Go SDK implements the TCP transport only, which covers both +// plain TCP and TLS. Credentials are username:password or a single +// personal access token before the @; either way auto-login is configured, +// matching what the Rust and Node SDKs do with a connection string. The +// host may be a bracketed IPv6 literal, as in [::1]:8090. +// +// Query options follow the grammar the other SDKs share: +// +// tls=true|false enable TLS +// tls_domain=<name> TLS server name indication (SNI) +// tls_ca_file=<path> CA certificate file for verification +// reconnection_retries=<n|unlimited> +// reconnection_interval=<non-zero duration> +// reestablish_after=<duration> +// heartbeat_interval=<non-zero duration> +// nodelay=true|false +// +// Durations take the units the other SDKs share, from nanos to years, +// including the hr, hrs, wk, wks and month spellings, with optional +// fractions and parts run together or spaced, as in 1h30m or 1h 30m. +// The reconnection_interval and heartbeat_interval spellings must be +// non-zero, because the Rust SDK types them as non-zero durations; +// reestablish_after takes the zero spellings, which Rust accepts there. +// The Go client keeps its own reconnect and heartbeat pacing, so those +// three spellings are validated for compatibility and then ignored, and +// a value the other SDKs would reject is still rejected here, so one +// connection string is either valid everywhere or nowhere. The nodelay +// spelling is applied, because the Go client carries a matching +// TCP_NODELAY option. +func ParseConnectionString(connectionString string) ([]tcp.Option, error) { + protocolParts := strings.Split(connectionString, "://") + if len(protocolParts) != 2 { + return nil, ierror.InvalidConnectionString{} + } + + protocol, err := parseProtocol(protocolParts[0]) + if err != nil { + return nil, err + } + if protocol != tcpProtocol { + // Only a plain transport name is echoed, because a malformed + // scheme may carry credentials, as the C# SDK notes on the same + // check. + if !isASCIILetters(protocol) { + return nil, ierror.InvalidConnectionString{} + } + return nil, fmt.Errorf("%w: unsupported transport %q, Go SDK supports tcp only", + ierror.InvalidConnectionString{}, protocol) + } + + parts := strings.Split(protocolParts[1], "@") + if len(parts) != 2 { + return nil, ierror.InvalidConnectionString{} + } + + credentials, err := parseCredentials(parts[0]) + if err != nil { + return nil, err + } + + serverAndOptions := strings.Split(parts[1], "?") + if len(serverAndOptions) > 2 { + return nil, ierror.InvalidConnectionString{} + } + + var tlsConfig parsedTLS + tlsConfig.noDelay = true + if len(serverAndOptions) == 2 { + tlsConfig, err = parseConnectionOptions(serverAndOptions[1]) + if err != nil { + return nil, err + } + } + + serverAddress := serverAndOptions[0] + if serverAddress == "" { + return nil, ierror.InvalidConnectionString{} + } + host, portStr, err := net.SplitHostPort(serverAddress) + if err != nil || host == "" { + return nil, ierror.InvalidConnectionString{} + } + // Node and Rust accept any u16 port, 0 included; the dial fails later + // instead of the parse. A bad port reports the same error as every other + // parse failure, which is what the Rust SDK returns here. + if _, err := strconv.ParseUint(portStr, 10, 16); err != nil { + return nil, ierror.InvalidConnectionString{} + } + + options := []tcp.Option{tcp.WithServerAddress(serverAddress)} + options = append(options, tcp.WithAutoLogin(credentials)) + if tlsConfig.enabled { + var tlsOpts []tcp.TLSOption + if tlsConfig.domain != "" { + tlsOpts = append(tlsOpts, tcp.WithTLSDomain(tlsConfig.domain)) + } + if tlsConfig.caFile != "" { + tlsOpts = append(tlsOpts, tcp.WithTLSCAFile(tlsConfig.caFile)) + } + options = append(options, tcp.WithTLS(tlsOpts...)) + } + options = append(options, tcp.WithNoDelay(tlsConfig.noDelay)) + return options, nil +} + +// parseProtocol extracts the transport from the scheme, which is either the +// bare default protocol or an explicit iggy+<transport> spelling. +func parseProtocol(scheme string) (string, error) { + if scheme == defaultProtocol { + return tcpProtocol, nil + } + if !strings.HasPrefix(scheme, protocolPrefix) { + return "", ierror.InvalidConnectionString{} + } + return strings.TrimPrefix(scheme, protocolPrefix), nil +} + +// parseCredentials turns the userinfo before the @ into auto-login +// credentials: username:password or a single personal access token. +func parseCredentials(userinfo string) (tcp.Credentials, error) { + username, password, hasSeparator := strings.Cut(userinfo, ":") + if !hasSeparator { + if username == "" { + return tcp.Credentials{}, ierror.InvalidConnectionString{} + } + return tcp.NewPersonalAccessTokenCredentials(username), nil + } + if username == "" || password == "" { + return tcp.Credentials{}, ierror.InvalidConnectionString{} + } + return tcp.NewUsernamePasswordCredentials(username, password), nil +} + +type parsedTLS struct { + enabled bool + domain string + caFile string + noDelay bool +} + +// parseConnectionOptions validates the shared connection-string grammar, +// keeping the tls options and ignoring the reconnect spellings the Go +// client does not take from a connection string. +func parseConnectionOptions(optionsString string) (parsedTLS, error) { + parsed := parsedTLS{} + if optionsString == "" { + return parsed, ierror.InvalidConnectionString{} + } + for _, option := range strings.Split(optionsString, "&") { + name, value, found := strings.Cut(option, "=") + if !found { + return parsed, ierror.InvalidConnectionString{} + } + switch name { + case "tls": + if value != "true" && value != "false" { + return parsed, ierror.InvalidConnectionString{} + } + parsed.enabled = value == "true" + case "tls_domain": + parsed.domain = value + case "tls_ca_file": + parsed.caFile = value + case "reconnection_retries": + if value != "unlimited" { + if _, err := strconv.ParseUint(value, 10, 32); err != nil { + return parsed, ierror.InvalidNumberValue{} + } + } + case "reconnection_interval", "heartbeat_interval": + // Rust types both as non-zero durations, so the zero spellings + // and any zero-valued duration are rejected. + if nanos, err := parseIggyDuration(value); err != nil || nanos == 0 { Review Comment: Traced it: NonZeroIggyDuration also refuses anything below Duration::from_micros(1) because the wire format carries whole microseconds, and 1ns was slipping through here. Both options now treat nanos < 1_000 as invalid; the test refuses 1ns and 999ns and accepts 1000ns, 1µs and 1us, so the boundary is pinned from both sides. ########## foreign/go/client/connection_string.go: ########## @@ -0,0 +1,460 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +package client + +import ( + "fmt" + "math/big" + "net" + "strconv" + "strings" + + "github.com/apache/iggy/foreign/go/client/tcp" + ierror "github.com/apache/iggy/foreign/go/errors" +) + +const ( + defaultProtocol = "iggy" + protocolPrefix = "iggy+" + tcpProtocol = "tcp" +) + +// ParseConnectionString parses an Iggy TCP connection string into the TCP +// client options it describes, so the result can be passed to WithTcp: +// +// client.NewIggyClient(client.WithTcp(opts...)) +// +// with opts from client.ParseConnectionString("iggy://iggy:iggy@localhost:8090"). +// +// The accepted shape is: +// +// iggy://[username:password@ | personal_access_token@]host:port[?options] +// iggy+tcp://[username:password@ | personal_access_token@]host:port[?options] +// +// The scheme iggy:// means TCP, the same as the explicit iggy+tcp://. +// Other iggy+<transport> schemes such as iggy+quic:// are rejected, +// because the Go SDK implements the TCP transport only, which covers both +// plain TCP and TLS. Credentials are username:password or a single +// personal access token before the @; either way auto-login is configured, +// matching what the Rust and Node SDKs do with a connection string. The +// host may be a bracketed IPv6 literal, as in [::1]:8090. +// +// Query options follow the grammar the other SDKs share: +// +// tls=true|false enable TLS +// tls_domain=<name> TLS server name indication (SNI) +// tls_ca_file=<path> CA certificate file for verification +// reconnection_retries=<n|unlimited> +// reconnection_interval=<non-zero duration> +// reestablish_after=<duration> +// heartbeat_interval=<non-zero duration> +// nodelay=true|false +// +// Durations take the units the other SDKs share, from nanos to years, +// including the hr, hrs, wk, wks and month spellings, with optional +// fractions and parts run together or spaced, as in 1h30m or 1h 30m. +// The reconnection_interval and heartbeat_interval spellings must be +// non-zero, because the Rust SDK types them as non-zero durations; +// reestablish_after takes the zero spellings, which Rust accepts there. +// The Go client keeps its own reconnect and heartbeat pacing, so those +// three spellings are validated for compatibility and then ignored, and +// a value the other SDKs would reject is still rejected here, so one +// connection string is either valid everywhere or nowhere. The nodelay +// spelling is applied, because the Go client carries a matching +// TCP_NODELAY option. +func ParseConnectionString(connectionString string) ([]tcp.Option, error) { + protocolParts := strings.Split(connectionString, "://") + if len(protocolParts) != 2 { + return nil, ierror.InvalidConnectionString{} + } + + protocol, err := parseProtocol(protocolParts[0]) + if err != nil { + return nil, err + } + if protocol != tcpProtocol { + // Only a plain transport name is echoed, because a malformed + // scheme may carry credentials, as the C# SDK notes on the same + // check. + if !isASCIILetters(protocol) { + return nil, ierror.InvalidConnectionString{} + } + return nil, fmt.Errorf("%w: unsupported transport %q, Go SDK supports tcp only", + ierror.InvalidConnectionString{}, protocol) + } + + parts := strings.Split(protocolParts[1], "@") + if len(parts) != 2 { + return nil, ierror.InvalidConnectionString{} + } + + credentials, err := parseCredentials(parts[0]) + if err != nil { + return nil, err + } + + serverAndOptions := strings.Split(parts[1], "?") + if len(serverAndOptions) > 2 { + return nil, ierror.InvalidConnectionString{} + } + + var tlsConfig parsedTLS + tlsConfig.noDelay = true + if len(serverAndOptions) == 2 { + tlsConfig, err = parseConnectionOptions(serverAndOptions[1]) + if err != nil { + return nil, err + } + } + + serverAddress := serverAndOptions[0] + if serverAddress == "" { + return nil, ierror.InvalidConnectionString{} + } + host, portStr, err := net.SplitHostPort(serverAddress) + if err != nil || host == "" { + return nil, ierror.InvalidConnectionString{} + } + // Node and Rust accept any u16 port, 0 included; the dial fails later + // instead of the parse. A bad port reports the same error as every other + // parse failure, which is what the Rust SDK returns here. + if _, err := strconv.ParseUint(portStr, 10, 16); err != nil { + return nil, ierror.InvalidConnectionString{} + } + + options := []tcp.Option{tcp.WithServerAddress(serverAddress)} + options = append(options, tcp.WithAutoLogin(credentials)) + if tlsConfig.enabled { + var tlsOpts []tcp.TLSOption + if tlsConfig.domain != "" { + tlsOpts = append(tlsOpts, tcp.WithTLSDomain(tlsConfig.domain)) + } + if tlsConfig.caFile != "" { + tlsOpts = append(tlsOpts, tcp.WithTLSCAFile(tlsConfig.caFile)) + } + options = append(options, tcp.WithTLS(tlsOpts...)) + } + options = append(options, tcp.WithNoDelay(tlsConfig.noDelay)) + return options, nil +} + +// parseProtocol extracts the transport from the scheme, which is either the +// bare default protocol or an explicit iggy+<transport> spelling. +func parseProtocol(scheme string) (string, error) { + if scheme == defaultProtocol { + return tcpProtocol, nil + } + if !strings.HasPrefix(scheme, protocolPrefix) { + return "", ierror.InvalidConnectionString{} + } + return strings.TrimPrefix(scheme, protocolPrefix), nil +} + +// parseCredentials turns the userinfo before the @ into auto-login +// credentials: username:password or a single personal access token. +func parseCredentials(userinfo string) (tcp.Credentials, error) { + username, password, hasSeparator := strings.Cut(userinfo, ":") + if !hasSeparator { + if username == "" { + return tcp.Credentials{}, ierror.InvalidConnectionString{} + } + return tcp.NewPersonalAccessTokenCredentials(username), nil + } + if username == "" || password == "" { + return tcp.Credentials{}, ierror.InvalidConnectionString{} + } + return tcp.NewUsernamePasswordCredentials(username, password), nil +} + +type parsedTLS struct { + enabled bool + domain string + caFile string + noDelay bool +} + +// parseConnectionOptions validates the shared connection-string grammar, +// keeping the tls options and ignoring the reconnect spellings the Go +// client does not take from a connection string. +func parseConnectionOptions(optionsString string) (parsedTLS, error) { + parsed := parsedTLS{} + if optionsString == "" { + return parsed, ierror.InvalidConnectionString{} + } + for _, option := range strings.Split(optionsString, "&") { + name, value, found := strings.Cut(option, "=") + if !found { + return parsed, ierror.InvalidConnectionString{} + } + switch name { + case "tls": + if value != "true" && value != "false" { + return parsed, ierror.InvalidConnectionString{} + } + parsed.enabled = value == "true" + case "tls_domain": + parsed.domain = value + case "tls_ca_file": + parsed.caFile = value + case "reconnection_retries": + if value != "unlimited" { + if _, err := strconv.ParseUint(value, 10, 32); err != nil { + return parsed, ierror.InvalidNumberValue{} + } + } + case "reconnection_interval", "heartbeat_interval": + // Rust types both as non-zero durations, so the zero spellings + // and any zero-valued duration are rejected. + if nanos, err := parseIggyDuration(value); err != nil || nanos == 0 { + return parsed, ierror.InvalidConnectionString{} + } + case "reestablish_after": + // Rust takes this one as a plain duration, where the zero + // spellings parse. + if _, err := parseIggyDuration(value); err != nil { + return parsed, ierror.InvalidConnectionString{} + } + case "nodelay": + if value != "true" && value != "false" { + return parsed, ierror.InvalidConnectionString{} + } + parsed.noDelay = value == "true" + default: + return parsed, ierror.InvalidConnectionString{} + } + } + return parsed, nil +} + +// isASCIILetters reports whether s is non-empty and ASCII letters only, so +// an unsupported-transport error can echo it without ever echoing the rest +// of a malformed scheme, which may carry credentials. +func isASCIILetters(s string) bool { + if s == "" { + return false + } + for i := 0; i < len(s); i++ { + if (s[i] < 'a' || s[i] > 'z') && (s[i] < 'A' || s[i] > 'Z') { + return false + } + } + return true +} + +// parseIggyDuration parses a duration the Rust SDK accepts, returning the +// total nanoseconds. It mirrors IggyDuration::from_str: the value is +// lowercased, the zero spellings become a zero duration, and everything +// else goes through a humantime-style grammar of amounts and units, with +// optional fractions and parts run together as in 1h30m or spaced as in +// 1h 30m. Fractions must land on a whole nanosecond, as in the Rust and +// Node parsers, so 1.5us parses while 0.5ns does not. +func parseIggyDuration(value string) (uint64, error) { + lower := strings.ToLower(value) + switch lower { + case "0", "unlimited", "disabled", "none": + return 0, nil + } + remainder := strings.TrimLeft(lower, " ") + if remainder == "" { + return 0, errInvalidDuration(value) + } + + var seconds, nanos big.Int + for remainder != "" { + wholePart, fracPart, fracScale, rest, ok := cutLeadingAmount(remainder) + if !ok { + return 0, errInvalidDuration(value) + } + remainder = rest + + index, rest, ok := cutDurationUnit(remainder) Review Comment: Done — the amount and its unit may now be separated by whitespace, and µs joined the unit table beside us, so 1 s and 1µs parse for every option the way humantime parses them. Both are in the grammar test. ########## foreign/go/client/connection_string_test.go: ########## @@ -0,0 +1,381 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +package client + +import ( + "context" + "crypto/ecdsa" + "crypto/elliptic" + "crypto/rand" + "crypto/tls" + "crypto/x509" + "crypto/x509/pkix" + "encoding/pem" + "math/big" + "net" + "os" + "path/filepath" + "testing" + "time" + + "github.com/apache/iggy/foreign/go/client/tcp" + "github.com/apache/iggy/foreign/go/internal/command" + . "github.com/apache/iggy/foreign/go/internal/fakevsr" + "github.com/apache/iggy/foreign/go/internal/vsr" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// applyOptions runs options the way NewIggyClient does, and returns the +// resulting TCP client so a test can observe what the parser produced. +func applyOptions(t *testing.T, options []tcp.Option) *tcp.IggyTcpClient { + t.Helper() + return tcp.NewIggyTcpClient(nil, options...) +} + +func TestParseConnectionString_ParsesTheDefaultSchemeWithPasswordCredentials(t *testing.T) { + opts, err := ParseConnectionString("iggy://iggy:[email protected]:8090") + require.NoError(t, err) + client := applyOptions(t, opts) + + assert.Equal(t, "127.0.0.1:8090", client.GetConnectionInfo().ServerAddress) +} + +func TestParseConnectionString_ParsesTheExplicitTcpSchemeWithAPersonalAccessToken(t *testing.T) { + opts, err := ParseConnectionString("iggy+tcp://iggypat-1234567890abcdef@localhost:8090") + require.NoError(t, err) + client := applyOptions(t, opts) + + assert.Equal(t, "localhost:8090", client.GetConnectionInfo().ServerAddress) +} + +func TestParseConnectionString_ParsesBracketedIPv6Host(t *testing.T) { + opts, err := ParseConnectionString("iggy://iggy:secret@[::1]:8090") + require.NoError(t, err) + client := applyOptions(t, opts) + + assert.Equal(t, "[::1]:8090", client.GetConnectionInfo().ServerAddress) +} + +// TestParseConnectionString_NoDelayOptionLandsOnTheClient pins the nodelay +// rule: the spelling validates and, unlike a dropped validation, the +// parsed value reaches the client, so ?nodelay=false really turns Nagle +// back on and the default stays with Nagle off. +func TestParseConnectionString_NoDelayOptionLandsOnTheClient(t *testing.T) { + withoutOption, err := ParseConnectionString("iggy://iggy:secret@localhost:8090") + require.NoError(t, err) + assert.True(t, applyOptions(t, withoutOption).GetNoDelay(), + "the default must keep Nagle disabled") + + enabled, err := ParseConnectionString("iggy://iggy:secret@localhost:8090?nodelay=true") + require.NoError(t, err) + assert.True(t, applyOptions(t, enabled).GetNoDelay()) + + disabled, err := ParseConnectionString("iggy://iggy:secret@localhost:8090?nodelay=false") + require.NoError(t, err) + assert.False(t, applyOptions(t, disabled).GetNoDelay(), + "?nodelay=false must reach the dial instead of being dropped") +} + +// TestParseConnectionString_NonZeroDurationOptionsRejectZeroSpellings +// pins the typing the Rust SDK gives each option: +// reconnection_interval and heartbeat_interval are non-zero durations, so +// the zero spellings and any zero-valued duration are refused, while +// reestablish_after is a plain duration where they parse. +func TestParseConnectionString_NonZeroDurationOptionsRejectZeroSpellings(t *testing.T) { + for _, option := range []string{"reconnection_interval", "heartbeat_interval"} { + for _, value := range []string{"0", "none", "disabled", "unlimited", "0s", "0ms"} { + invalid := "iggy://iggy:secret@localhost:8090?" + option + "=" + value + t.Run(invalid, func(t *testing.T) { + opts, err := ParseConnectionString(invalid) + require.Error(t, err, + "%s is a non-zero duration in the Rust SDK", option) + assert.Empty(t, opts) + }) + } + } + for _, value := range []string{"0", "none", "disabled", "unlimited", "0s", "0ms"} { + valid := "iggy://iggy:secret@localhost:8090?reestablish_after=" + value + t.Run(valid, func(t *testing.T) { + opts, err := ParseConnectionString(valid) + require.NoError(t, err, + "reestablish_after is a plain duration in the Rust SDK") + assert.NotEmpty(t, opts) + }) + } +} + +// TestParseConnectionString_AcceptsTheExtendedDurationUnits pins the +// spellings the Node and C# SDKs parse and the shared grammar carries: +// hr, hrs, wk, wks and month, plus the millis spelling they use instead +// of milliseconds. +func TestParseConnectionString_AcceptsTheExtendedDurationUnits(t *testing.T) { + for _, options := range []string{ + "reconnection_interval=1hr", + "reconnection_interval=2hrs", + "reconnection_interval=1wk", + "reconnection_interval=2wks", + "reconnection_interval=1month", + "reconnection_interval=2months", + "reconnection_interval=1hr 30m", + "reconnection_interval=250millis", + "heartbeat_interval=1hr", + "reestablish_after=1month", + } { + t.Run(options, func(t *testing.T) { + opts, err := ParseConnectionString("iggy://iggy:secret@localhost:8090?" + options) + require.NoError(t, err) + assert.NotEmpty(t, opts) + }) + } + // The long spelling stays outside the grammar, as it is in the Node + // and C# SDKs. + opts, err := ParseConnectionString("iggy://iggy:secret@localhost:8090?reconnection_interval=1milliseconds") + require.Error(t, err) + assert.Empty(t, opts) +} + +// TestParseConnectionString_RejectsUnsupportedTransports checks the scheme +// rule and pins the credential rule together: unsupported transports are +// rejected with an error naming the transport, and a malformed scheme +// never leaks the credentials embedded in it. +func TestParseConnectionString_RejectsUnsupportedTransportsAndNeverEchoesCredentials(t *testing.T) { + for _, value := range []string{ + "iggy+quic://iggy:secret@localhost:8090", + "iggy+ws://iggy:secret@localhost:8090", + "http://iggy:secret@localhost:8090";, + } { + t.Run(value, func(t *testing.T) { + opts, err := ParseConnectionString(value) + require.Error(t, err) + assert.Empty(t, opts) + assert.NotContains(t, err.Error(), "secret", + "the transport error must not echo the connection string") + }) + } + // A scheme that embeds credentials after iggy+ must not surface them + // in the error, only the ASCII-letter transport name is echoed. + leaking := "iggy+user:secret@host:8090://host:8090" + t.Run(leaking, func(t *testing.T) { + opts, err := ParseConnectionString(leaking) + require.Error(t, err) + assert.Empty(t, opts) + assert.NotContains(t, err.Error(), "secret") + assert.NotContains(t, err.Error(), "user:secret@host") + }) +} + +// TestParseConnectionString_ConnectsAndLogsInAgainstAServer drives the +// full connect-and-login flow from the issue acceptance criteria: opts +// from ParseConnectionString, wired into NewIggyClient via WithTcp, +// connecting and signing in against a running (fake) server. +func TestParseConnectionString_ConnectsAndLogsInAgainstAServer(t *testing.T) { + server := Listen(t, nil) + address := server.Listener().Addr().String() + + opts, err := ParseConnectionString("iggy://iggy:iggy@" + address) + require.NoError(t, err) + cli, err := NewIggyClient(WithTcp(opts...)) + require.NoError(t, err) + t.Cleanup(func() { _ = cli.Close() }) + + require.NoError(t, cli.Connect(context.Background())) + + recorded := server.Recorded() + require.Len(t, recorded, 2) + // The roster read is auth-gated, so the sign-in comes first. + assert.Equal(t, vsr.OperationRegister, recorded[0].Operation()) + assert.Contains(t, string(recorded[0].Payload), "iggy") + assert.Equal(t, uint32(command.GetClusterMetadataCode), recorded[1].Code()) +} + +// TestParseConnectionString_TLSEnabledByQueryString drives the same flow +// with tls=true and a TLS-only fake server: a client that ignored the +// option would send its register frame in plaintext and the TLS handshake +// would fail, so a successful connect-and-login proves the option landed. +func TestParseConnectionString_TLSEnabledByQueryString(t *testing.T) { + certificate, caPath := selfSignedCert(t) + + server := Listen(t, func(conn net.Conn) net.Conn { + return tls.Server(conn, &tls.Config{Certificates: []tls.Certificate{certificate}}) + }) + address := server.Listener().Addr().String() + host, port, err := net.SplitHostPort(address) + require.NoError(t, err) + + connectionString := "iggy://iggy:iggy@" + host + ":" + port + + "?tls=true&tls_domain=localhost&tls_ca_file=" + caPath + opts, err := ParseConnectionString(connectionString) + require.NoError(t, err) + cli, err := NewIggyClient(WithTcp(opts...)) + require.NoError(t, err) + t.Cleanup(func() { _ = cli.Close() }) + + require.NoError(t, cli.Connect(context.Background())) + + recorded := server.Recorded() + require.Len(t, recorded, 2) + assert.Equal(t, vsr.OperationRegister, recorded[0].Operation()) + assert.Equal(t, uint32(command.GetClusterMetadataCode), recorded[1].Code()) +} + +// selfSignedCert mints a throwaway server certificate for the loopback +// listener, so the test carries no expiry or checkout-layout dependency. +func selfSignedCert(t *testing.T) (tls.Certificate, string) { Review Comment: Moved to fakevsr.SelfSignedCert and all three suites use it — the failover tests turned out to be a third caller, so they moved to it as well. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
