From 019ff6eb38ef232dbf116aeb08780588be207417 Mon Sep 17 00:00:00 2001 From: Edward Muller Date: Fri, 11 Nov 2016 15:47:49 -0800 Subject: [PATCH 1/4] Add support for parsing redis:// and rediss:// URLs This includes setting up a default dialer that handles the ssl handshake. --- options.go | 72 ++++++++++++++++++++++++++++++++++++++ options_test.go | 92 +++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 164 insertions(+) create mode 100644 options_test.go diff --git a/options.go b/options.go index 376fa6b..904f2f8 100644 --- a/options.go +++ b/options.go @@ -1,7 +1,12 @@ package redis import ( + "crypto/tls" + "errors" "net" + "net/url" + "strconv" + "strings" "time" "gopkg.in/redis.v5/internal/pool" @@ -58,6 +63,9 @@ type Options struct { // Enables read only queries on slave nodes. ReadOnly bool + + // Config to use when connecting via TLS + TLSConfig *tls.Config } func (opt *Options) init() { @@ -92,6 +100,70 @@ func (opt *Options) init() { } } +// ParseURL parses a redis URL into options that can be used to connect to redis +func ParseURL(redisURL string) (*Options, error) { + o := &Options{Network: "tcp"} + u, err := url.Parse(redisURL) + if err != nil { + return nil, err + } + + if u.Scheme != "redis" && u.Scheme != "rediss" { + return nil, errors.New("invalid redis URL scheme: " + u.Scheme) + } + + if u.User != nil { + if p, ok := u.User.Password(); ok { + o.Password = p + } + } + + if len(u.Query()) > 0 { + return nil, errors.New("no options supported") + } + + h, p, err := net.SplitHostPort(u.Host) + if err != nil { + h = u.Host + } + if h == "" { + h = "localhost" + } + if p == "" { + p = "6379" + } + o.Addr = net.JoinHostPort(h, p) + + f := strings.FieldsFunc(u.Path, func(r rune) bool { + return r == '/' + }) + switch len(f) { + case 0: + o.DB = 0 + case 1: + if o.DB, err = strconv.Atoi(f[0]); err != nil { + return nil, errors.New("Invalid redis database number: " + err.Error()) + } + default: + return nil, errors.New("invalid redis URL path: " + u.Path) + } + + if u.Scheme == "rediss" { + o.Dialer = func() (net.Conn, error) { + conn, err := net.DialTimeout(o.Network, o.Addr, o.DialTimeout) + if err != nil { + return nil, err + } + if o.TLSConfig == nil { + o.TLSConfig = &tls.Config{InsecureSkipVerify: true} + } + t := tls.Client(conn, o.TLSConfig) + return t, t.Handshake() + } + } + return o, nil +} + func newConnPool(opt *Options) *pool.ConnPool { return pool.NewConnPool( opt.Dialer, diff --git a/options_test.go b/options_test.go new file mode 100644 index 0000000..c551811 --- /dev/null +++ b/options_test.go @@ -0,0 +1,92 @@ +package redis + +import ( + "errors" + "testing" +) + +func TestParseURL(t *testing.T) { + cases := []struct { + u string + addr string + db int + dialer bool + err error + }{ + { + "redis://localhost:123/1", + "localhost:123", + 1, false, nil, + }, + { + "redis://localhost:123", + "localhost:123", + 0, false, nil, + }, + { + "redis://localhost/1", + "localhost:6379", + 1, false, nil, + }, + { + "redis://12345", + "12345:6379", + 0, false, nil, + }, + { + "rediss://localhost:123", + "localhost:123", + 0, true, nil, + }, + { + "redis://localhost/?abc=123", + "", + 0, false, errors.New("no options supported"), + }, + { + "http://google.com", + "", + 0, false, errors.New("invalid redis URL scheme: http"), + }, + { + "redis://localhost/1/2/3/4", + "", + 0, false, errors.New("invalid redis URL path: /1/2/3/4"), + }, + { + "12345", + "", + 0, false, errors.New("invalid redis URL scheme: "), + }, + { + "redis://localhost/iamadatabase", + "", + 0, false, errors.New("Invalid redis database number: strconv.ParseInt: parsing \"iamadatabase\": invalid syntax"), + }, + } + + for _, c := range cases { + t.Run(c.u, func(t *testing.T) { + o, err := ParseURL(c.u) + if c.err == nil && err != nil { + t.Fatalf("Expected err to be nil, but got: '%q'", err) + return + } + if c.err != nil && err != nil { + if c.err.Error() != err.Error() { + t.Fatalf("Expected err to be '%q', but got '%q'", c.err, err) + } + return + } + if o.Addr != c.addr { + t.Errorf("Expected Addr to be '%s', but got '%s'", c.addr, o.Addr) + } + if o.DB != c.db { + t.Errorf("Expecdted DB to be '%d', but got '%d'", c.db, o.DB) + } + if c.dialer && o.Dialer == nil { + t.Errorf("Expected a Dialer to be set, but isn't") + } + }) + } +} From 70eddf606d56a43f710b4e8caf26b5805d4739b7 Mon Sep 17 00:00:00 2001 From: Edward Muller Date: Mon, 14 Nov 2016 09:40:12 -0800 Subject: [PATCH 2/4] Place these tests behind a build tag Sub test support doesn't work for go < 1.7 --- options_test.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/options_test.go b/options_test.go index c551811..465bf00 100644 --- a/options_test.go +++ b/options_test.go @@ -1,3 +1,5 @@ +// +build go1.7 + package redis import ( From 4aa583b6f8bab0387497bddced7702e6cdb8abc6 Mon Sep 17 00:00:00 2001 From: Edward Muller Date: Tue, 15 Nov 2016 10:27:20 -0800 Subject: [PATCH 3/4] Updates based on PR feedback --- options.go | 23 +++++++++-------------- options_test.go | 24 ++++++++++++------------ 2 files changed, 21 insertions(+), 26 deletions(-) diff --git a/options.go b/options.go index 904f2f8..9d8e917 100644 --- a/options.go +++ b/options.go @@ -64,7 +64,7 @@ type Options struct { // Enables read only queries on slave nodes. ReadOnly bool - // Config to use when connecting via TLS + // TLS Config to use. When set TLS will be negotiated. TLSConfig *tls.Config } @@ -74,7 +74,12 @@ func (opt *Options) init() { } if opt.Dialer == nil { opt.Dialer = func() (net.Conn, error) { - return net.DialTimeout(opt.Network, opt.Addr, opt.DialTimeout) + conn, err := net.DialTimeout(opt.Network, opt.Addr, opt.DialTimeout) + if opt.TLSConfig == nil || err != nil { + return conn, err + } + t := tls.Client(conn, opt.TLSConfig) + return t, t.Handshake() } } if opt.PoolSize == 0 { @@ -142,24 +147,14 @@ func ParseURL(redisURL string) (*Options, error) { o.DB = 0 case 1: if o.DB, err = strconv.Atoi(f[0]); err != nil { - return nil, errors.New("Invalid redis database number: " + err.Error()) + return nil, errors.New("invalid redis database number: " + err.Error()) } default: return nil, errors.New("invalid redis URL path: " + u.Path) } if u.Scheme == "rediss" { - o.Dialer = func() (net.Conn, error) { - conn, err := net.DialTimeout(o.Network, o.Addr, o.DialTimeout) - if err != nil { - return nil, err - } - if o.TLSConfig == nil { - o.TLSConfig = &tls.Config{InsecureSkipVerify: true} - } - t := tls.Client(conn, o.TLSConfig) - return t, t.Handshake() - } + o.TLSConfig = &tls.Config{InsecureSkipVerify: true} } return o, nil } diff --git a/options_test.go b/options_test.go index 465bf00..effebd5 100644 --- a/options_test.go +++ b/options_test.go @@ -9,11 +9,11 @@ import ( func TestParseURL(t *testing.T) { cases := []struct { - u string - addr string - db int - dialer bool - err error + u string + addr string + db int + tls bool + err error }{ { "redis://localhost:123/1", @@ -63,7 +63,7 @@ func TestParseURL(t *testing.T) { { "redis://localhost/iamadatabase", "", - 0, false, errors.New("Invalid redis database number: strconv.ParseInt: parsing \"iamadatabase\": invalid syntax"), + 0, false, errors.New("invalid redis database number: strconv.ParseInt: parsing \"iamadatabase\": invalid syntax"), }, } @@ -71,23 +71,23 @@ func TestParseURL(t *testing.T) { t.Run(c.u, func(t *testing.T) { o, err := ParseURL(c.u) if c.err == nil && err != nil { - t.Fatalf("Expected err to be nil, but got: '%q'", err) + t.Fatalf("unexpected error: '%q'", err) return } if c.err != nil && err != nil { if c.err.Error() != err.Error() { - t.Fatalf("Expected err to be '%q', but got '%q'", c.err, err) + t.Fatalf("got %q, expected %q", err, c.err) } return } if o.Addr != c.addr { - t.Errorf("Expected Addr to be '%s', but got '%s'", c.addr, o.Addr) + t.Errorf("got %q, want %q", o.Addr, c.addr) } if o.DB != c.db { - t.Errorf("Expecdted DB to be '%d', but got '%d'", c.db, o.DB) + t.Errorf("got %q, expected %q", o.DB, c.db) } - if c.dialer && o.Dialer == nil { - t.Errorf("Expected a Dialer to be set, but isn't") + if c.tls && o.TLSConfig == nil { + t.Errorf("got nil TLSConfig, expected a TLSConfig") } }) } From df2009821f8066d50a977f0a5df3a155aadcff5b Mon Sep 17 00:00:00 2001 From: Edward Muller Date: Wed, 16 Nov 2016 10:35:35 -0800 Subject: [PATCH 4/4] Default to secure I suspect that many people will need InsecureSkipVerify:true though, but that should be an explicit decision to expose security issues instead of papering over them. --- options.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/options.go b/options.go index 9d8e917..77ffbce 100644 --- a/options.go +++ b/options.go @@ -154,7 +154,7 @@ func ParseURL(redisURL string) (*Options, error) { } if u.Scheme == "rediss" { - o.TLSConfig = &tls.Config{InsecureSkipVerify: true} + o.TLSConfig = &tls.Config{ServerName: h} } return o, nil }