From 4aa583b6f8bab0387497bddced7702e6cdb8abc6 Mon Sep 17 00:00:00 2001
From: Edward Muller <edwardam@interlix.com>
Date: Tue, 15 Nov 2016 10:27:20 -0800
Subject: [PATCH] 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 904f2f8d..9d8e917f 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 465bf005..effebd5a 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")
 			}
 		})
 	}