Add query parameter parsing to ParseURL()

Before this change, ParseURL would only accept a very restricted
set of URLs (it returned an error, if it encountered any parameter).

This commit introduces the ability to process URLs like

	redis://localhost/1?dial_timeout=10s

and similar.

Go programs which were providing a configuration tunable (e.g.
CLI flag, config entry or environment variable) to configure
the Redis connection now don't need to perform this task
themselves.

Author: dmke

example_test.go

@@ -39,20 +39,22 @@ func ExampleNewClient() {
 }
 
 func ExampleParseURL() {
-	opt, err := redis.ParseURL("redis://:qwerty@localhost:6379/1")
+	opt, err := redis.ParseURL("redis://:qwerty@localhost:6379/1?dial_timeout=5s")
 	if err != nil {
 		panic(err)
 	}
 	fmt.Println("addr is", opt.Addr)
 	fmt.Println("db is", opt.DB)
 	fmt.Println("password is", opt.Password)
+	fmt.Println("dial timeout is", opt.DialTimeout)
 
 	// Create client as usually.
 	_ = redis.NewClient(opt)
 
 	// Output: addr is localhost:6379
 	// db is 1
 	// password is qwerty
+	// dial timeout is 5s
 }
 
 func ExampleNewFailoverClient() {

options.go

@@ -8,6 +8,7 @@ import (
 	"net"
 	"net/url"
 	"runtime"
+	"sort"
 	"strconv"
 	"strings"
 	"time"
@@ -192,9 +193,31 @@ func (opt *Options) clone() *Options {
 // Scheme is required.
 // There are two connection types: by tcp socket and by unix socket.
 // Tcp connection:
-// 		redis://<user>:<password>@<host>:<port>/<db_number>
+//		redis://<user>:<password>@<host>:<port>/<db_number>
 // Unix connection:
 //		unix://<user>:<password>@</path/to/redis.sock>?db=<db_number>
+// Most Option fields can be set using query parameters, with the following restrictions:
+//	- field names are mapped using snake-case conversion: to set MaxRetries, use max_retries
+//	- query parameters "network", "addr", "username" and "password" are ignored
+//	  (these are covered by other URL attributes)
+//	- only scalar type fields are supported (bool, int, duration)
+//	- for duration fields, values must be a valid input for time.ParseDuration();
+//	  additionally a plain integer as value (i.e. without unit) is intepreted as seconds
+//	- to disable a duration field, use value 0; to use the default value, leave the value blank
+//	  or remove the parameter
+//	- only the last value is interpreted if a parameter is given multiple times
+//	- unknown parameter names will result in an error
+// Examples:
+//		redis://user:password@localhost:6789/3?dial_timeout=3&db=1&read_timeout=6s&max_retries=2
+//		is equivalent to:
+//		&Options{
+//			Network:     "tcp",
+//			Addr:        "localhost:6789",
+//			DB:          1,               // path "/3" was overridden by "&db=1"
+//			DialTimeout: 3 * time.Second, // no time unit = seconds
+//			ReadTimeout: 6 * time.Second,
+//			MaxRetries:  2,
+//		}
 func ParseURL(redisURL string) (*Options, error) {
 	u, err := url.Parse(redisURL)
 	if err != nil {
@@ -216,10 +239,6 @@ func setupTCPConn(u *url.URL) (*Options, error) {
 
 	o.Username, o.Password = getUserPassword(u)
 
-	if len(u.Query()) > 0 {
-		return nil, errors.New("redis: no options supported")
-	}
-
 	h, p, err := net.SplitHostPort(u.Host)
 	if err != nil {
 		h = u.Host
@@ -250,7 +269,7 @@ func setupTCPConn(u *url.URL) (*Options, error) {
 		o.TLSConfig = &tls.Config{ServerName: h}
 	}
 
-	return o, nil
+	return setupConnParams(u, o)
 }
 
 func setupUnixConn(u *url.URL) (*Options, error) {
@@ -262,19 +281,122 @@ func setupUnixConn(u *url.URL) (*Options, error) {
 		return nil, errors.New("redis: empty unix socket path")
 	}
 	o.Addr = u.Path
-
 	o.Username, o.Password = getUserPassword(u)
+	return setupConnParams(u, o)
+}
 
-	dbStr := u.Query().Get("db")
-	if dbStr == "" {
-		return o, nil // if database is not set, connect to 0 db.
+type queryOptions struct {
+	q   url.Values
+	err error
+}
+
+func (o *queryOptions) string(name string) string {
+	vs := o.q[name]
+	if len(vs) == 0 {
+		return ""
 	}
+	delete(o.q, name) // enable detection of unknown parameters
+	return vs[len(vs)-1]
+}
 
-	db, err := strconv.Atoi(dbStr)
-	if err != nil {
-		return nil, fmt.Errorf("redis: invalid database number: %w", err)
+func (o *queryOptions) int(name string) int {
+	s := o.string(name)
+	if s == "" {
+		return 0
+	}
+	i, err := strconv.Atoi(s)
+	if err == nil {
+		return i
+	}
+	if o.err == nil {
+		o.err = fmt.Errorf("redis: invalid %s number: %s", name, err)
+	}
+	return 0
+}
+
+func (o *queryOptions) duration(name string) time.Duration {
+	s := o.string(name)
+	if s == "" {
+		return 0
+	}
+	// try plain number first
+	if i, err := strconv.Atoi(s); err == nil {
+		if i <= 0 {
+			// disable timeouts
+			return -1
+		}
+		return time.Duration(i) * time.Second
+	}
+	dur, err := time.ParseDuration(s)
+	if err == nil {
+		return dur
+	}
+	if o.err == nil {
+		o.err = fmt.Errorf("redis: invalid %s duration: %w", name, err)
+	}
+	return 0
+}
+
+func (o *queryOptions) bool(name string) bool {
+	switch s := o.string(name); s {
+	case "true", "1":
+		return true
+	case "false", "0", "":
+		return false
+	default:
+		if o.err == nil {
+			o.err = fmt.Errorf("redis: invalid %s boolean: expected true/false/1/0 or an empty string, got %q", name, s)
+		}
+		return false
+	}
+}
+
+func (o *queryOptions) remaining() []string {
+	if len(o.q) == 0 {
+		return nil
+	}
+	keys := make([]string, 0, len(o.q))
+	for k := range o.q {
+		keys = append(keys, k)
+	}
+	sort.Strings(keys)
+	return keys
+}
+
+// setupConnParams converts query parameters in u to option value in o.
+func setupConnParams(u *url.URL, o *Options) (*Options, error) {
+	q := queryOptions{q: u.Query()}
+
+	// compat: a future major release may use q.int("db")
+	if tmp := q.string("db"); tmp != "" {
+		db, err := strconv.Atoi(tmp)
+		if err != nil {
+			return nil, fmt.Errorf("redis: invalid database number: %w", err)
+		}
+		o.DB = db
+	}
+
+	o.MaxRetries = q.int("max_retries")
+	o.MinRetryBackoff = q.duration("min_retry_backoff")
+	o.MaxRetryBackoff = q.duration("max_retry_backoff")
+	o.DialTimeout = q.duration("dial_timeout")
+	o.ReadTimeout = q.duration("read_timeout")
+	o.WriteTimeout = q.duration("write_timeout")
+	o.PoolFIFO = q.bool("pool_fifo")
+	o.PoolSize = q.int("pool_size")
+	o.MinIdleConns = q.int("min_idle_conns")
+	o.MaxConnAge = q.duration("max_conn_age")
+	o.PoolTimeout = q.duration("pool_timeout")
+	o.IdleTimeout = q.duration("idle_timeout")
+	o.IdleCheckFrequency = q.duration("idle_check_frequency")
+	if q.err != nil {
+		return nil, q.err
+	}
+
+	// any parameters left?
+	if r := q.remaining(); len(r) > 0 {
+		return nil, fmt.Errorf("redis: unexpected option: %s", strings.Join(r, ", "))
 	}
-	o.DB = db
 
 	return o, nil
 }

options_test.go

@@ -40,6 +40,25 @@ func TestParseURL(t *testing.T) {
 		}, {
 			url: "redis://foo:bar@localhost:123",
 			o:   &Options{Addr: "localhost:123", Username: "foo", Password: "bar"},
+		}, {
+			// multiple params
+			url: "redis://localhost:123/?db=2&read_timeout=2&pool_fifo=true",
+			o:   &Options{Addr: "localhost:123", DB: 2, ReadTimeout: 2 * time.Second, PoolFIFO: true},
+		}, {
+			// special case handling for disabled timeouts
+			url: "redis://localhost:123/?db=2&idle_timeout=0",
+			o:   &Options{Addr: "localhost:123", DB: 2, IdleTimeout: -1},
+		}, {
+			// negative values disable timeouts as well
+			url: "redis://localhost:123/?db=2&idle_timeout=-1",
+			o:   &Options{Addr: "localhost:123", DB: 2, IdleTimeout: -1},
+		}, {
+			// absent timeout values will use defaults
+			url: "redis://localhost:123/?db=2&idle_timeout=",
+			o:   &Options{Addr: "localhost:123", DB: 2, IdleTimeout: 0},
+		}, {
+			url: "redis://localhost:123/?db=2&idle_timeout", // missing "=" at the end
+			o:   &Options{Addr: "localhost:123", DB: 2, IdleTimeout: 0},
 		}, {
 			url: "unix:///tmp/redis.sock",
 			o:   &Options{Addr: "/tmp/redis.sock"},
@@ -50,11 +69,27 @@ func TestParseURL(t *testing.T) {
 			url: "unix://foo:bar@/tmp/redis.sock?db=3",
 			o:   &Options{Addr: "/tmp/redis.sock", Username: "foo", Password: "bar", DB: 3},
 		}, {
+			// invalid db format
 			url: "unix://foo:bar@/tmp/redis.sock?db=test",
 			err: errors.New(`redis: invalid database number: strconv.Atoi: parsing "test": invalid syntax`),
+		}, {
+			// invalid int value
+			url: "redis://localhost/?pool_size=five",
+			err: errors.New(`redis: invalid pool_size number: strconv.Atoi: parsing "five": invalid syntax`),
+		}, {
+			// invalid bool value
+			url: "redis://localhost/?pool_fifo=yes",
+			err: errors.New(`redis: invalid pool_fifo boolean: expected true/false/1/0 or an empty string, got "yes"`),
+		}, {
+			// it returns first error
+			url: "redis://localhost/?db=foo&pool_size=five",
+			err: errors.New(`redis: invalid database number: strconv.Atoi: parsing "foo": invalid syntax`),
 		}, {
 			url: "redis://localhost/?abc=123",
-			err: errors.New("redis: no options supported"),
+			err: errors.New("redis: unexpected option: abc"),
+		}, {
+			url: "redis://localhost/?wrte_timout=10s&abc=123",
+			err: errors.New("redis: unexpected option: abc, wrte_timout"),
 		}, {
 			url: "http://google.com",
 			err: errors.New("redis: invalid URL scheme: http"),
@@ -98,7 +133,7 @@ func comprareOptions(t *testing.T, actual, expected *Options) {
 		t.Errorf("got %q, want %q", actual.Addr, expected.Addr)
 	}
 	if actual.DB != expected.DB {
-		t.Errorf("got %q, expected %q", actual.DB, expected.DB)
+		t.Errorf("DB: got %q, expected %q", actual.DB, expected.DB)
 	}
 	if actual.TLSConfig == nil && expected.TLSConfig != nil {
 		t.Errorf("got nil TLSConfig, expected a TLSConfig")
@@ -107,10 +142,49 @@ func comprareOptions(t *testing.T, actual, expected *Options) {
 		t.Errorf("got TLSConfig, expected no TLSConfig")
 	}
 	if actual.Username != expected.Username {
-		t.Errorf("got %q, expected %q", actual.Username, expected.Username)
+		t.Errorf("Username: got %q, expected %q", actual.Username, expected.Username)
 	}
 	if actual.Password != expected.Password {
-		t.Errorf("got %q, expected %q", actual.Password, expected.Password)
+		t.Errorf("Password: got %q, expected %q", actual.Password, expected.Password)
+	}
+	if actual.MaxRetries != expected.MaxRetries {
+		t.Errorf("MaxRetries: got %v, expected %v", actual.MaxRetries, expected.MaxRetries)
+	}
+	if actual.MinRetryBackoff != expected.MinRetryBackoff {
+		t.Errorf("MinRetryBackoff: got %v, expected %v", actual.MinRetryBackoff, expected.MinRetryBackoff)
+	}
+	if actual.MaxRetryBackoff != expected.MaxRetryBackoff {
+		t.Errorf("MaxRetryBackoff: got %v, expected %v", actual.MaxRetryBackoff, expected.MaxRetryBackoff)
+	}
+	if actual.DialTimeout != expected.DialTimeout {
+		t.Errorf("DialTimeout: got %v, expected %v", actual.DialTimeout, expected.DialTimeout)
+	}
+	if actual.ReadTimeout != expected.ReadTimeout {
+		t.Errorf("ReadTimeout: got %v, expected %v", actual.ReadTimeout, expected.ReadTimeout)
+	}
+	if actual.WriteTimeout != expected.WriteTimeout {
+		t.Errorf("WriteTimeout: got %v, expected %v", actual.WriteTimeout, expected.WriteTimeout)
+	}
+	if actual.PoolFIFO != expected.PoolFIFO {
+		t.Errorf("PoolFIFO: got %v, expected %v", actual.PoolFIFO, expected.PoolFIFO)
+	}
+	if actual.PoolSize != expected.PoolSize {
+		t.Errorf("PoolSize: got %v, expected %v", actual.PoolSize, expected.PoolSize)
+	}
+	if actual.MinIdleConns != expected.MinIdleConns {
+		t.Errorf("MinIdleConns: got %v, expected %v", actual.MinIdleConns, expected.MinIdleConns)
+	}
+	if actual.MaxConnAge != expected.MaxConnAge {
+		t.Errorf("MaxConnAge: got %v, expected %v", actual.MaxConnAge, expected.MaxConnAge)
+	}
+	if actual.PoolTimeout != expected.PoolTimeout {
+		t.Errorf("PoolTimeout: got %v, expected %v", actual.PoolTimeout, expected.PoolTimeout)
+	}
+	if actual.IdleTimeout != expected.IdleTimeout {
+		t.Errorf("IdleTimeout: got %v, expected %v", actual.IdleTimeout, expected.IdleTimeout)
+	}
+	if actual.IdleCheckFrequency != expected.IdleCheckFrequency {
+		t.Errorf("IdleCheckFrequency: got %v, expected %v", actual.IdleCheckFrequency, expected.IdleCheckFrequency)
 	}
 }