Go: Fix Timeout Casting in time.Duration

go/config/config.go

@@ -4,9 +4,11 @@ package config
 
 import (
 	"errors"
+	"fmt"
 	"time"
 
 	"github.com/valkey-io/valkey-glide/go/v2/internal/protobuf"
+	"github.com/valkey-io/valkey-glide/go/v2/internal/utils"
 )
 
 const (
@@ -119,7 +121,11 @@ func (config *baseClientConfiguration) toProtobuf() (*protobuf.ConnectionRequest
 
 	request.ReadFrom = mapReadFrom(config.readFrom)
 	if config.requestTimeout != 0 {
-		request.RequestTimeout = uint32(config.requestTimeout)
+		requestTimeout, err := utils.DurationToMilliseconds(config.requestTimeout)
+		if err != nil {
+			return nil, fmt.Errorf("setting request timeout returned an error: %w", err)
+		}
+		request.RequestTimeout = requestTimeout
 	}
 
 	if config.clientName != "" {
@@ -228,7 +234,11 @@ func (config *ClientConfiguration) ToProtobuf() (*protobuf.ConnectionRequest, er
 	}
 
 	if config.AdvancedClientConfiguration.connectionTimeout != 0 {
-		request.ConnectionTimeout = uint32(config.AdvancedClientConfiguration.connectionTimeout)
+		connectionTimeout, err := utils.DurationToMilliseconds(config.AdvancedClientConfiguration.connectionTimeout)
+		if err != nil {
+			return nil, fmt.Errorf("setting connection timeout returned an error: %w", err)
+		}
+		request.ConnectionTimeout = connectionTimeout
 	}
 
 	return request, nil
@@ -276,6 +286,9 @@ func (config *ClientConfiguration) WithReadFrom(readFrom ReadFrom) *ClientConfig
 // encompasses sending the request, awaiting for a response from the server, and any required reconnections or retries. If the
 // specified timeout is exceeded for a pending request, it will result in a timeout error. If not set, a default value will be
 // used.
+//
+// Using a negative value or a value that exceeds the max duration of 2^32 - 1 milliseconds will lead to an invalid
+// configuration.
 func (config *ClientConfiguration) WithRequestTimeout(requestTimeout time.Duration) *ClientConfiguration {
 	config.requestTimeout = requestTimeout
 	return config
@@ -360,7 +373,11 @@ func (config *ClusterClientConfiguration) ToProtobuf() (*protobuf.ConnectionRequ
 
 	request.ClusterModeEnabled = true
 	if (config.AdvancedClusterClientConfiguration.connectionTimeout) != 0 {
-		request.ConnectionTimeout = uint32(config.AdvancedClusterClientConfiguration.connectionTimeout)
+		connectionTimeout, err := utils.DurationToMilliseconds(config.AdvancedClusterClientConfiguration.connectionTimeout)
+		if err != nil {
+			return nil, fmt.Errorf("setting connection timeout returned an error: %w", err)
+		}
+		request.ConnectionTimeout = connectionTimeout
 	}
 	if config.subscriptionConfig != nil && len(config.subscriptionConfig.subscriptions) > 0 {
 		request.PubsubSubscriptions = config.subscriptionConfig.toProtobuf()
@@ -412,6 +429,9 @@ func (config *ClusterClientConfiguration) WithReadFrom(readFrom ReadFrom) *Clust
 // encompasses sending the request, awaiting a response from the server, and any required reconnections or retries. If the
 // specified timeout is exceeded for a pending request, it will result in a timeout error. If not set, a default value will be
 // used.
+//
+// Using a negative value or a value that exceeds the max duration of 2^32 - 1 milliseconds will lead to an invalid
+// configuration.
 func (config *ClusterClientConfiguration) WithRequestTimeout(requestTimeout time.Duration) *ClusterClientConfiguration {
 	config.requestTimeout = requestTimeout
 	return config
@@ -471,8 +491,8 @@ type AdvancedClientConfiguration struct {
 	connectionTimeout time.Duration
 }
 
-// NewAdvancedGlideClientConfiguration returns a new [AdvancedClientConfiguration] with default settings.
-func NewAdvancedGlideClientConfiguration() *AdvancedClientConfiguration {
+// NewAdvancedClientConfiguration returns a new [AdvancedClientConfiguration] with default settings.
+func NewAdvancedClientConfiguration() *AdvancedClientConfiguration {
 	return &AdvancedClientConfiguration{}
 }
 
@@ -481,6 +501,9 @@ func NewAdvancedGlideClientConfiguration() *AdvancedClientConfiguration {
 // during initial client creation and any reconnections that may occur during request processing.
 // Note: A high connection timeout may lead to prolonged blocking of the entire command
 // pipeline. If not explicitly set, a default value of 250 milliseconds will be used.
+//
+// Using a negative value or a value that exceeds the max duration of 2^32 - 1 milliseconds will lead to an invalid
+// configuration.
 func (config *AdvancedClientConfiguration) WithConnectionTimeout(
 	connectionTimeout time.Duration,
 ) *AdvancedClientConfiguration {
@@ -500,6 +523,9 @@ func NewAdvancedClusterClientConfiguration() *AdvancedClusterClientConfiguration
 }
 
 // WithConnectionTimeout sets the duration to wait for a TCP/TLS connection to complete.
+//
+// Using a negative value or a value that exceeds the max duration of 2^32 - 1 milliseconds will lead to an invalid
+// configuration.
 func (config *AdvancedClusterClientConfiguration) WithConnectionTimeout(
 	connectionTimeout time.Duration,
 ) *AdvancedClusterClientConfiguration {

go/config/config_test.go

@@ -8,6 +8,7 @@ import (
 	"time"
 
 	"github.com/stretchr/testify/assert"
+	"github.com/valkey-io/valkey-glide/go/v2/internal/errors"
 	"github.com/valkey-io/valkey-glide/go/v2/internal/protobuf"
 )
 
@@ -67,7 +68,7 @@ func TestConfig_allFieldsSet(t *testing.T) {
 		ReadFrom:           protobuf.ReadFrom_PreferReplica,
 		ClusterModeEnabled: false,
 		AuthenticationInfo: &protobuf.AuthenticationInfo{Username: username, Password: password},
-		RequestTimeout:     uint32(timeout),
+		RequestTimeout:     uint32(timeout.Milliseconds()),
 		ClientName:         clientName,
 		ConnectionRetryStrategy: &protobuf.ConnectionRetryStrategy{
 			NumberOfRetries: uint32(retries),
@@ -241,3 +242,75 @@ func TestConfig_AzAffinity(t *testing.T) {
 
 	assert.Equal(t, expected, result)
 }
+
+func TestConfig_InvalidRequestAndConnectionTimeouts(t *testing.T) {
+	var errorType *errors.ConfigurationError
+
+	// RequestTimeout Negative duration
+	config := NewClientConfiguration().
+		WithRequestTimeout(-1 * time.Hour)
+
+	_, err := config.ToProtobuf()
+	assert.Error(t, err)
+	assert.ErrorAs(t, err, &errorType)
+	assert.Contains(t, err.Error(), "invalid duration was specified")
+
+	config2 := NewClusterClientConfiguration().
+		WithRequestTimeout(-1 * time.Hour)
+
+	_, err2 := config2.ToProtobuf()
+	assert.Error(t, err2)
+	assert.ErrorAs(t, err2, &errorType)
+	assert.Contains(t, err2.Error(), "invalid duration was specified")
+
+	// RequestTimeout 50 days
+	config3 := NewClientConfiguration().
+		WithRequestTimeout(1200 * time.Hour)
+
+	_, err3 := config3.ToProtobuf()
+	assert.Error(t, err3)
+	assert.ErrorAs(t, err3, &errorType)
+	assert.Contains(t, err3.Error(), "invalid duration was specified")
+
+	config4 := NewClusterClientConfiguration().
+		WithRequestTimeout(1200 * time.Hour)
+
+	_, err4 := config4.ToProtobuf()
+	assert.Error(t, err4)
+	assert.ErrorAs(t, err4, &errorType)
+	assert.Contains(t, err4.Error(), "invalid duration was specified")
+
+	// ConnectionTimeout Negative duration
+	config5 := NewClientConfiguration().
+		WithAdvancedConfiguration(NewAdvancedClientConfiguration().WithConnectionTimeout(-1 * time.Hour))
+
+	_, err5 := config5.ToProtobuf()
+	assert.Error(t, err5)
+	assert.ErrorAs(t, err5, &errorType)
+	assert.Contains(t, err5.Error(), "invalid duration was specified")
+
+	config6 := NewClusterClientConfiguration().
+		WithAdvancedConfiguration(NewAdvancedClusterClientConfiguration().WithConnectionTimeout(-1 * time.Hour))
+
+	_, err6 := config6.ToProtobuf()
+	assert.Error(t, err6)
+	assert.ErrorAs(t, err6, &errorType)
+	assert.Contains(t, err6.Error(), "invalid duration was specified")
+
+	// ConnectionTimeout 50 days
+	config7 := NewClientConfiguration().
+		WithAdvancedConfiguration(NewAdvancedClientConfiguration().WithConnectionTimeout(1200 * time.Hour))
+
+	_, err7 := config7.ToProtobuf()
+	assert.Error(t, err7)
+	assert.ErrorAs(t, err7, &errorType)
+	assert.Contains(t, err7.Error(), "invalid duration was specified")
+
+	config8 := NewClusterClientConfiguration().
+		WithAdvancedConfiguration(NewAdvancedClusterClientConfiguration().WithConnectionTimeout(1200 * time.Hour))
+
+	_, err8 := config8.ToProtobuf()
+	assert.Error(t, err8)
+	assert.ErrorAs(t, err8, &errorType)
+	assert.Contains(t, err8.Error(), "invalid duration was specified")
+}

go/integTest/connection_test.go

@@ -120,9 +120,9 @@ func (suite *GlideTestSuite) TestConnectionTimeout() {
 			var timeoutClient interfaces.BaseClientCommands
 			var err error
 			if clusterMode {
-				timeoutClient, err = suite.createConnectionTimeoutClusterClient(10000, 250)
+				timeoutClient, err = suite.createConnectionTimeoutClusterClient(10*time.Second, 250*time.Millisecond)
 			} else {
-				timeoutClient, err = suite.createConnectionTimeoutClient(10000, 250, backoffStrategy)
+				timeoutClient, err = suite.createConnectionTimeoutClient(10*time.Second, 250*time.Millisecond, backoffStrategy)
 			}
 			assert.NoError(suite.T(), err)
 			if timeoutClient != nil {

go/integTest/glide_test_suite_test.go

@@ -374,7 +374,7 @@ func (suite *GlideTestSuite) createConnectionTimeoutClient(
 		WithRequestTimeout(requestTimeout).
 		WithReconnectStrategy(backoffStrategy).
 		WithAdvancedConfiguration(
-			config.NewAdvancedGlideClientConfiguration().WithConnectionTimeout(connectTimeout))
+			config.NewAdvancedClientConfiguration().WithConnectionTimeout(connectTimeout))
 	return glide.NewClient(clientConfig)
 }
 

go/internal/utils/transform_utils.go

@@ -4,8 +4,12 @@ package utils
 
 import (
 	"fmt"
+	"math"
 	"strconv"
+	"time"
 	"unsafe"
+
+	"github.com/valkey-io/valkey-glide/go/v2/internal/errors"
 )
 
 // Convert `s` of type `string` into `[]byte`
@@ -101,3 +105,12 @@ func ToString(v any) (string, bool) {
 		return fmt.Sprintf("%v", val), true
 	}
 }
+
+// Convert to and perform bound checks for uint32 representation for milliseconds
+func DurationToMilliseconds(d time.Duration) (uint32, error) {
+	milliseconds := d.Milliseconds()
+	if milliseconds < 0 || milliseconds > math.MaxUint32 {
+		return 0, &errors.ConfigurationError{Msg: "invalid duration was specified"}
+	}
+	return uint32(milliseconds), nil
+}