Fully parse secure and skip_verify DSN params (#862)

The current code will interpret a DSN with `?secure=false` in its
options as a directive to turn TSL _on!_

Obviously re-setting what is the default value is redundant, but
deployment systems that template the DSN from e.g. environment variables
may need to do this.

The previous behavior (`?secure` without a value set turns secure on) is
preserved to minimize surprise.

Signed-off-by: Nathan J. Mehl <n@oden.io>

Signed-off-by: Nathan J. Mehl <n@oden.io>

clickhouse_options.go

@@ -21,13 +21,14 @@ import (
 	"context"
 	"crypto/tls"
 	"fmt"
-	"github.com/ClickHouse/ch-go/compress"
-	"github.com/pkg/errors"
 	"net"
 	"net/url"
 	"strconv"
 	"strings"
 	"time"
+
+	"github.com/ClickHouse/ch-go/compress"
+	"github.com/pkg/errors"
 )
 
 type CompressionMethod byte
@@ -229,9 +230,25 @@ func (o *Options) fromDSN(in string) error {
 			}
 			o.ReadTimeout = duration
 		case "secure":
-			secure = true
+			secureParam := params.Get(v)
+			if secureParam == "" {
+				secure = true
+			} else {
+				secure, err = strconv.ParseBool(secureParam)
+				if err != nil {
+					return fmt.Errorf("clickhouse [dsn parse]:secure: %s", err)
+				}
+			}
 		case "skip_verify":
-			skipVerify = true
+			skipVerifyParam := params.Get(v)
+			if skipVerifyParam == "" {
+				skipVerify = true
+			} else {
+				skipVerify, err = strconv.ParseBool(skipVerifyParam)
+				if err != nil {
+					return fmt.Errorf("clickhouse [dsn parse]:verify: %s", err)
+				}
+			}
 		case "connection_open_strategy":
 			switch params.Get(v) {
 			case "in_order":

clickhouse_options_test.go

@@ -18,8 +18,10 @@
 package clickhouse
 
 import (
-	"github.com/stretchr/testify/assert"
+	"crypto/tls"
 	"testing"
+
+	"github.com/stretchr/testify/assert"
 )
 
 // TestParseDSN does not implement all use cases yet
@@ -127,6 +129,86 @@ func TestParseDSN(t *testing.T) {
 			},
 			"",
 		},
+		{
+			"native protocol with secure",
+			"clickhouse://127.0.0.1/test_database?secure=true",
+			&Options{
+				Protocol: Native,
+				TLS: &tls.Config{
+					InsecureSkipVerify: false,
+				},
+				Addr:     []string{"127.0.0.1"},
+				Settings: Settings{},
+				Auth: Auth{
+					Database: "test_database",
+				},
+				scheme: "clickhouse",
+			},
+			"",
+		},
+		{
+			"native protocol with skip_verify",
+			"clickhouse://127.0.0.1/test_database?secure=true&skip_verify=true",
+			&Options{
+				Protocol: Native,
+				TLS: &tls.Config{
+					InsecureSkipVerify: true,
+				},
+				Addr:     []string{"127.0.0.1"},
+				Settings: Settings{},
+				Auth: Auth{
+					Database: "test_database",
+				},
+				scheme: "clickhouse",
+			},
+			"",
+		},
+		{
+			"native protocol with secure (legacy)",
+			"clickhouse://127.0.0.1/test_database?secure",
+			&Options{
+				Protocol: Native,
+				TLS: &tls.Config{
+					InsecureSkipVerify: false,
+				},
+				Addr:     []string{"127.0.0.1"},
+				Settings: Settings{},
+				Auth: Auth{
+					Database: "test_database",
+				},
+				scheme: "clickhouse",
+			},
+			"",
+		},
+		{
+			"native protocol with skip_verify (legacy)",
+			"clickhouse://127.0.0.1/test_database?secure&skip_verify",
+			&Options{
+				Protocol: Native,
+				TLS: &tls.Config{
+					InsecureSkipVerify: true,
+				},
+				Addr:     []string{"127.0.0.1"},
+				Settings: Settings{},
+				Auth: Auth{
+					Database: "test_database",
+				},
+				scheme: "clickhouse",
+			},
+			"",
+		},
+		{
+			"native protocol with secure (bad)",
+			"clickhouse://127.0.0.1/test_database?secure=ture",
+			nil,
+			"clickhouse [dsn parse]:secure: strconv.ParseBool: parsing \"ture\": invalid syntax",
+		},
+		{
+			"native protocol with skip_verify (bad)",
+			"clickhouse://127.0.0.1/test_database?secure&skip_verify=ture",
+			nil,
+			"clickhouse [dsn parse]:verify: strconv.ParseBool: parsing \"ture\": invalid syntax",
+		},
 		{
 			"native protocol with default lz4 compression",
 			"clickhouse://127.0.0.1/test_database?compress=true",