fix(conn): validate URL retry and buffer values (#4144)

* fix(conn): validate URL retry and buffer values

Reject negative URL settings so a mistyped positive value cannot silently
become a default retry delay or an unintended buffer size. An explicit
zero retry timeout selects no delay; omitted values keep their defaults.

Closes #4132
Refs #4063

* docs(conn): clarify URL retry timeout rules

* docs(conn): note URL validation compatibility

RELEASE-NOTES.md

@@ -1,5 +1,9 @@
 # Release Notes
 
+## Unreleased
+
+- **URL configuration validation:** `ParseURL`, `ParseClusterURL`, and `ParseFailoverURL` now reject negative `dialer_retry_timeout`, `read_buffer_size`, and `write_buffer_size` values. Cluster and failover URLs that previously accepted negative retry timeouts now return an error; replace those values with `0` for no delay, or omit the parameter for the default 100 ms delay. An explicit `dialer_retry_timeout=0` now disables the retry delay ([#4144](https://github.com/redis/go-redis/pull/4144)).
+
 # 9.23.0 (2026-10-05)
 
 This is a minor release. It contains everything from 9.23.0-beta.1, so these notes cover the full 9.22.0 → 9.23.0 upgrade. The release adds:

options.go

@@ -144,6 +144,9 @@ type Options struct {
 	DialerRetries int
 
 	// DialerRetryTimeout is the backoff duration between retry attempts.
+	// Values <= 0 use the default; use DialerRetryBackoff to retry without delay.
+	// A URL dialer_retry_timeout=0 sets DialerRetryBackoff, which takes precedence
+	// over this field, including when this field is changed after parsing.
 	//
 	// default: 100 milliseconds
 	DialerRetryTimeout time.Duration
@@ -912,6 +915,11 @@ func NewDialer(opt *Options) func(context.Context, string, string) (net.Conn, er
 //     additionally a plain integer as value (i.e. without unit) is interpreted as seconds
 //   - to disable a duration field, use value less than or equal to 0; to use the default
 //     value, leave the value blank or remove the parameter
+//   - dialer_retry_timeout is an exception: 0 means no retry delay, while negative
+//     values are rejected. Explicit zero sets DialerRetryBackoff to return 0.
+//   - read_buffer_size and write_buffer_size must be non-negative; 0 uses the default.
+//     Invalid negative values are rejected so a configuration typo is not silently
+//     replaced with an unintended default.
 //   - only the last value is interpreted if a parameter is given multiple times
 //   - fields "network", "addr", "username" and "password" can only be set using other
 //     URL attributes (scheme, host, userinfo, resp.), query parameters using these
@@ -1051,6 +1059,45 @@ func (o *queryOptions) int(name string) int {
 	return 0
 }
 
+func (o *queryOptions) nonNegativeInt(name string) int {
+	i := o.int(name)
+	if i < 0 && o.err == nil {
+		o.err = fmt.Errorf("redis: invalid %s number: must be non-negative", name)
+	}
+	return i
+}
+
+func (o *queryOptions) dialerRetryTimeout() (time.Duration, func(int) time.Duration) {
+	const name = "dialer_retry_timeout"
+	s := o.string(name)
+	if s == "" {
+		return 0, nil
+	}
+	// Like duration, accept plain integers as seconds. ParseDuration also
+	// rejects overflow rather than letting multiplication wrap to a negative.
+	if _, err := strconv.Atoi(s); err == nil {
+		s += "s"
+	}
+	d, err := time.ParseDuration(s)
+	// Check the sign too: ParseDuration rounds negative fractions of a
+	// nanosecond to zero, but those are still invalid configuration values.
+	if err == nil && strings.HasPrefix(s, "-") {
+		err = errors.New("must be non-negative")
+	}
+	if err != nil {
+		if o.err == nil {
+			o.err = fmt.Errorf("redis: invalid %s duration: %w", name, err)
+		}
+		return 0, nil
+	}
+	if d == 0 {
+		// A zero Options field means unset. Use the backoff callback to retain
+		// an explicit URL zero through client initialization and pool defaults.
+		return 0, func(int) time.Duration { return 0 }
+	}
+	return d, nil
+}
+
 func (o *queryOptions) duration(name string) time.Duration {
 	s := o.string(name)
 	if s == "" {
@@ -1121,12 +1168,12 @@ func setupConnParams(u *url.URL, o *Options) (*Options, error) {
 	o.MaxRetryBackoff = q.duration("max_retry_backoff")
 	o.DialTimeout = q.duration("dial_timeout")
 	o.DialerRetries = q.int("dialer_retries")
-	o.DialerRetryTimeout = q.duration("dialer_retry_timeout")
+	o.DialerRetryTimeout, o.DialerRetryBackoff = q.dialerRetryTimeout()
 	o.ReadTimeout = q.duration("read_timeout")
 	o.WriteTimeout = q.duration("write_timeout")
 	o.ContextTimeoutEnabled = q.bool("context_timeout_enabled")
-	o.ReadBufferSize = q.int("read_buffer_size")
-	o.WriteBufferSize = q.int("write_buffer_size")
+	o.ReadBufferSize = q.nonNegativeInt("read_buffer_size")
+	o.WriteBufferSize = q.nonNegativeInt("write_buffer_size")
 	o.DisableIdentity = q.bool("disable_identity")
 	o.IdentitySuffix = q.string("identity_suffix")
 	o.PoolFIFO = q.bool("pool_fifo")

options_test.go

@@ -4,6 +4,7 @@ import (
 	"crypto/tls"
 	"errors"
 	"net"
+	"strings"
 	"sync"
 	"testing"
 	"time"
@@ -12,6 +13,105 @@ import (
 	"github.com/redis/go-redis/v9/maintnotifications"
 )
 
+func TestParseURLConnectionValidation(t *testing.T) {
+	parsers := []struct {
+		name  string
+		parse func(string) (*Options, error)
+	}{
+		{"standalone", func(query string) (*Options, error) {
+			return ParseURL("redis://localhost:6379?" + query)
+		}},
+		{"cluster", func(query string) (*Options, error) {
+			o, err := ParseClusterURL("redis://localhost:6379?" + query)
+			if err != nil {
+				return nil, err
+			}
+			o.init()
+			return o.clientOptions(), nil
+		}},
+		{"failover", func(query string) (*Options, error) {
+			o, err := ParseFailoverURL("redis://localhost:6379?master_name=mymaster&" + query)
+			if err != nil {
+				return nil, err
+			}
+			return o.clientOptions(), nil
+		}},
+	}
+	for _, parser := range parsers {
+		t.Run(parser.name, func(t *testing.T) {
+			t.Run("omitted retry delay", func(t *testing.T) {
+				o, err := parser.parse("")
+				if err != nil {
+					t.Fatal(err)
+				}
+				o.init()
+				if o.DialerRetryTimeout != 100*time.Millisecond || o.DialerRetryBackoff != nil {
+					t.Fatal("omitted retry delay must retain the default backoff")
+				}
+			})
+			for _, value := range []string{"", "0", "0s", "1", "250ms", "-1", "-1s", "-1ns", "-0.1ns", "bad", "9223372036854775807"} {
+				t.Run("retry/"+value, func(t *testing.T) {
+					o, err := parser.parse("dialer_retry_timeout=" + value)
+					if strings.HasPrefix(value, "-") || value == "bad" || value == "9223372036854775807" {
+						if err == nil || !strings.Contains(err.Error(), "dialer_retry_timeout") {
+							t.Fatalf("expected dialer_retry_timeout error, got %v", err)
+						}
+						return
+					}
+					if err != nil {
+						t.Fatal(err)
+					}
+					o.init()
+					want := 100 * time.Millisecond
+					switch value {
+					case "0", "0s":
+						want = 0
+					case "1":
+						want = time.Second
+					case "250ms":
+						want = 250 * time.Millisecond
+					}
+					got := o.DialerRetryTimeout
+					if o.DialerRetryBackoff != nil {
+						got = o.DialerRetryBackoff(0)
+					}
+					if got != want {
+						t.Fatalf("retry delay = %v, want %v", got, want)
+					}
+				})
+			}
+			for _, field := range []string{"read_buffer_size", "write_buffer_size"} {
+				for _, value := range []string{"-1", "bad", "0", "65536"} {
+					t.Run(field+"/"+value, func(t *testing.T) {
+						o, err := parser.parse(field + "=" + value)
+						if value == "-1" || value == "bad" {
+							if err == nil || !strings.Contains(err.Error(), field) {
+								t.Fatalf("expected %s error, got %v", field, err)
+							}
+							return
+						}
+						if err != nil {
+							t.Fatal(err)
+						}
+						o.init()
+						want := 32768
+						if value == "65536" {
+							want = 65536
+						}
+						got := o.ReadBufferSize
+						if field == "write_buffer_size" {
+							got = o.WriteBufferSize
+						}
+						if got != want {
+							t.Fatalf("%s = %d, want %d", field, got, want)
+						}
+					})
+				}
+			}
+		})
+	}
+}
+
 func TestParseURL(t *testing.T) {
 	cases := []struct {
 		url string

osscluster.go

@@ -113,6 +113,9 @@ type ClusterOptions struct {
 	DialerRetries int
 
 	// DialerRetryTimeout is the backoff duration between retry attempts.
+	// Values <= 0 use the default; use DialerRetryBackoff to retry without delay.
+	// A URL dialer_retry_timeout=0 sets DialerRetryBackoff, which takes precedence
+	// over this field, including when this field is changed after parsing.
 	//
 	// default: 100 milliseconds
 	DialerRetryTimeout time.Duration
@@ -330,6 +333,8 @@ func (opt *ClusterOptions) init() {
 //     additionally a plain integer as value (i.e. without unit) is interpreted as seconds
 //   - to disable a duration field, use value less than or equal to 0; to use the default
 //     value, leave the value blank or remove the parameter
+//   - dialer_retry_timeout is an exception: 0 means no retry delay, while negative
+//     values are rejected. Explicit zero sets DialerRetryBackoff to return 0.
 //   - only the last value is interpreted if a parameter is given multiple times
 //   - fields "network", "addr", "username" and "password" can only be set using other
 //     URL attributes (scheme, host, userinfo, resp.), query parameters using these
@@ -405,12 +410,12 @@ func setupClusterQueryParams(u *url.URL, o *ClusterOptions) (*ClusterOptions, er
 	o.MaxRetryBackoff = q.duration("max_retry_backoff")
 	o.DialTimeout = q.duration("dial_timeout")
 	o.DialerRetries = q.int("dialer_retries")
-	o.DialerRetryTimeout = q.duration("dialer_retry_timeout")
+	o.DialerRetryTimeout, o.DialerRetryBackoff = q.dialerRetryTimeout()
 	o.ReadTimeout = q.duration("read_timeout")
 	o.WriteTimeout = q.duration("write_timeout")
 	o.ContextTimeoutEnabled = q.bool("context_timeout_enabled")
-	o.ReadBufferSize = q.int("read_buffer_size")
-	o.WriteBufferSize = q.int("write_buffer_size")
+	o.ReadBufferSize = q.nonNegativeInt("read_buffer_size")
+	o.WriteBufferSize = q.nonNegativeInt("write_buffer_size")
 	o.DisableIdentity = q.bool("disable_identity")
 	o.IdentitySuffix = q.string("identity_suffix")
 	o.PoolFIFO = q.bool("pool_fifo")

sentinel.go

@@ -101,6 +101,9 @@ type FailoverOptions struct {
 	DialerRetries int
 
 	// DialerRetryTimeout is the backoff duration between retry attempts.
+	// Values <= 0 use the default; use DialerRetryBackoff to retry without delay.
+	// A URL dialer_retry_timeout=0 sets DialerRetryBackoff, which takes precedence
+	// over this field, including when this field is changed after parsing.
 	//
 	// default: 100 milliseconds
 	DialerRetryTimeout time.Duration
@@ -402,6 +405,8 @@ func (opt *FailoverOptions) clusterOptions() *ClusterOptions {
 //     additionally a plain integer as value (i.e. without unit) is interpreted as seconds
 //   - to disable a duration field, use value less than or equal to 0; to use the default
 //     value, leave the value blank or remove the parameter
+//   - dialer_retry_timeout is an exception: 0 means no retry delay, while negative
+//     values are rejected. Explicit zero sets DialerRetryBackoff to return 0.
 //   - only the last value is interpreted if a parameter is given multiple times
 //   - fields "network", "addr", "sentinel_username" and "sentinel_password" can only be set using other
 //     URL attributes (scheme, host, userinfo, resp.), query parameters using these
@@ -480,12 +485,12 @@ func setupFailoverConnParams(u *url.URL, o *FailoverOptions) (*FailoverOptions,
 	o.MaxRetryBackoff = q.duration("max_retry_backoff")
 	o.DialTimeout = q.duration("dial_timeout")
 	o.DialerRetries = q.int("dialer_retries")
-	o.DialerRetryTimeout = q.duration("dialer_retry_timeout")
+	o.DialerRetryTimeout, o.DialerRetryBackoff = q.dialerRetryTimeout()
 	o.ReadTimeout = q.duration("read_timeout")
 	o.WriteTimeout = q.duration("write_timeout")
 	o.ContextTimeoutEnabled = q.bool("context_timeout_enabled")
-	o.ReadBufferSize = q.int("read_buffer_size")
-	o.WriteBufferSize = q.int("write_buffer_size")
+	o.ReadBufferSize = q.nonNegativeInt("read_buffer_size")
+	o.WriteBufferSize = q.nonNegativeInt("write_buffer_size")
 	o.PoolFIFO = q.bool("pool_fifo")
 	o.PoolSize = q.int("pool_size")
 	o.MaxConcurrentDials = q.int("max_concurrent_dials")