Repository: denisenkom/go-mssqldb
Pull request #707 (2021-12-14 00:22:11)

Fix an issue where Host is named instance name
For Microsoft SQL Host could take form [server]\\[instance]. Without this fix LookupIP will fail in this scenario.

srdan-bozovic-msft: @kardianos could you help review?
Thx!
kardianos: @srdan-bozovic-msft This seems wrong to me. `msdsn.Config` contains separate `Host` and `Instance` fields. If you manually populate that struct, does it work? If so, then any change must happen in the msdsn package.

What configuration DSN format do you use? If you are using the URL based DSN format, does it work? For instance, if you use: `sqlserver://<user>:<pass>@<host>[:port]/<instance?database=<db-name>` will it work?

Parsing a host from instance within the tds.go isn't correct.

shueybubbles: oh and there should be a test included.
srdan-bozovic-msft: Thx for the guidance @kardianos.

The problem I was trying to solve has been exposed in routing scenario, where both Host and Instance were assigned to host.

I made another commit to address this.

Is this proper way to fix the issue?
kardianos: @srdan-bozovic-msft So if I understand correctly, the DNS entry for the host is "blah\Foo"? I need a concrete example to work from first. Please fill in the following details of a hypothetical example:

 1. DNS A Record that resolves to the host:
 2. Instance Name:
 3. Value of `msdsn.Config.Host`
 4. Value of `msdsn.Config.Instance`

Once I have these four values, I might begin to understand.

srdan-bozovic-msft: @kardianos 
Driver receives from the server routing information [code pointer](https://github.com/denisenkom/go-mssqldb/blob/b7d2437c0aa77f318d484b54c5f50763c2cfbb98/tds.go#L1252) "audit-test-server.database.windows.net\f7716e6f65ef"

Currently all of that is copied to a Host while it should be split btw Host and Instance

msdsn.Config.Host = audit-test-server.database.windows.net
msdsn.Config.Instance = _f7716e6f65ef_

Patch:
diff --git a/error.go b/error.go
index 59a7a1a..09964b8 100644
--- a/error.go
+++ b/error.go
@@ -57,23 +57,19 @@ func (e Error) SQLErrorLineNo() int32 {
 }
 
 type StreamError struct {
-	Message string
+	InnerError error
 }
 
 func (e StreamError) Error() string {
-	return e.Message
-}
-
-func streamErrorf(format string, v ...interface{}) StreamError {
-	return StreamError{"Invalid TDS stream: " + fmt.Sprintf(format, v...)}
+	return "Invalid TDS stream: " + e.InnerError.Error()
 }
 
 func badStreamPanic(err error) {
-	panic(streamErrorf("%v", err))
+	panic(StreamError{InnerError: err})
 }
 
 func badStreamPanicf(format string, v ...interface{}) {
-	panic(streamErrorf(format, v...))
+	panic(fmt.Errorf(format, v...))
 }
 
 // ServerError is returned when the server got a fatal error
diff --git a/msdsn/conn_str.go b/msdsn/conn_str.go
index e9ec63a..40bedb0 100644
--- a/msdsn/conn_str.go
+++ b/msdsn/conn_str.go
@@ -77,7 +77,8 @@ type Config struct {
 
 func SetupTLS(certificate string, insecureSkipVerify bool, hostInCertificate string) (*tls.Config, error) {
 	config := tls.Config{
-		ServerName: hostInCertificate,
+		ServerName:         hostInCertificate,
+		InsecureSkipVerify: insecureSkipVerify,
 
 		// fix for https://github.com/denisenkom/go-mssqldb/issues/166
 		// Go implementation of TLS payload size heuristic algorithm splits single TDS package to multiple TCP segments,
@@ -101,9 +102,6 @@ func SetupTLS(certificate string, insecureSkipVerify bool, hostInCertificate str
 	certs := x509.NewCertPool()
 	certs.AppendCertsFromPEM(pem)
 	config.RootCAs = certs
-	if insecureSkipVerify {
-		config.InsecureSkipVerify = true
-	}
 	return &config, nil
 }
 
diff --git a/tds.go b/tds.go
index dbc9b21..a4e2c9d 100644
--- a/tds.go
+++ b/tds.go
@@ -1250,11 +1250,16 @@ initiate_connection:
 
 	if sess.routedServer != "" {
 		toconn.Close()
-		p.Host = sess.routedServer
+		// Need to handle case when routedServer is in "host\instance" format.
+		routedParts := strings.SplitN(sess.routedServer, "\\", 2)
+		p.Host = routedParts[0]
+		if len(routedParts) == 2 {
+			p.Instance = routedParts[1]
+		}
 		p.Port = uint64(sess.routedPort)
 		if !p.HostInCertificateProvided && p.TLSConfig != nil {
 			p.TLSConfig = p.TLSConfig.Clone()
-			p.TLSConfig.ServerName = sess.routedServer
+			p.TLSConfig.ServerName = p.Host
 		}
 		goto initiate_connection
 	}


Test patch:
diff --git a/log_test.go b/log_test.go
index 78900f3..9ac86d7 100644
--- a/log_test.go
+++ b/log_test.go
@@ -66,7 +66,7 @@ func TestLogger(t *testing.T) {
 	// Set up a retryable error and the test cases that will exercise it
 	errMsg := "Retryable Test Error"
 	retryPrefix := "RETRY: "
-	inErr := StreamError{Message: errMsg}
+	inErr := StreamError{InnerError: fmt.Errorf(errMsg)}
 
 	testcases := [...]struct {
 		name        string
@@ -98,7 +98,7 @@ func TestLogger(t *testing.T) {
 			logger:      bufLogger{&captureBuf},
 			ctxLogger:   nil,
 			flags:       msdsn.LogRetries,
-			expectedMsg: retryPrefix + errMsg,
+			expectedMsg: retryPrefix + "Invalid TDS stream: " + errMsg,
 		},
 		{
 			name:        "sqlserver with Logger logging",
@@ -106,7 +106,7 @@ func TestLogger(t *testing.T) {
 			logger:      bufLogger{&captureBuf},
 			ctxLogger:   nil,
 			flags:       msdsn.LogRetries,
-			expectedMsg: retryPrefix + errMsg,
+			expectedMsg: retryPrefix + "Invalid TDS stream: " + errMsg,
 		},
 		{
 			name:        "mssql with ContextLogger logging",
@@ -114,7 +114,7 @@ func TestLogger(t *testing.T) {
 			logger:      nil,
 			ctxLogger:   bufContextLogger{&captureBuf},
 			flags:       msdsn.LogRetries,
-			expectedMsg: errMsg,
+			expectedMsg: "Invalid TDS stream: " + errMsg,
 		},
 		{
 			name:        "sqlserver with ContextLogger logging",
@@ -122,7 +122,7 @@ func TestLogger(t *testing.T) {
 			logger:      nil,
 			ctxLogger:   bufContextLogger{&captureBuf},
 			flags:       msdsn.LogRetries,
-			expectedMsg: errMsg,
+			expectedMsg: "Invalid TDS stream: " + errMsg,
 		},
 		{
 			name:        "mssql with Logger logging but no flags set",
diff --git a/mssql_test.go b/mssql_test.go
index ebf7c32..d1bab1b 100644
--- a/mssql_test.go
+++ b/mssql_test.go
@@ -108,7 +108,7 @@ func equalErrors(e1 error, e2 error) bool {
 func TestCheckBadConn(t *testing.T) {
 
 	netErr := &net.OpError{Err: fmt.Errorf("fake net.Error")}
-	streamErr := StreamError{Message: "fake StreamError"}
+	streamErr := StreamError{InnerError: fmt.Errorf("fake StreamError")}
 	serverErr := ServerError{sqlError: Error{Message: "fake ServerError"}}
 	goodConnErr := fmt.Errorf("fake error that leaves connection good")
 
diff --git a/queries_test.go b/queries_test.go
index 7387301..e654524 100644
--- a/queries_test.go
+++ b/queries_test.go
@@ -5,6 +5,7 @@ import (
 	"context"
 	"database/sql"
 	"database/sql/driver"
+	"errors"
 	"fmt"
 	"io"
 	"log"
@@ -1403,26 +1404,26 @@ func TestProcessQueryNextErrors(t *testing.T) {
 		t.Fatal("Next should not return EOF, instead should fail when it encounters closed connection")
 	}
 	err = rows.Next([]driver.Value{&val})
-	if err != driver.ErrBadConn {
-		t.Fatal("Connection should be bad")
+	if !errors.Is(err, driver.ErrBadConn) {
+		t.Fatalf("Connection should be bad A, is: %[1]T %[1]v", err)
 	}
 	err = conn.Ping(context.Background())
-	if err != driver.ErrBadConn {
-		t.Fatal("Connection should be bad")
+	if !errors.Is(err, driver.ErrBadConn) {
+		t.Fatalf("Connection should be bad B, is: %[1]T %[1]v", err)
 	}
 	_, err = conn.BeginTx(
 		context.Background(),
 		driver.TxOptions{})
-	if err != driver.ErrBadConn {
-		t.Fatal("Connection should be bad")
+	if !errors.Is(err, driver.ErrBadConn) {
+		t.Fatalf("Connection should be bad C, is: %[1]T %[1]v", err)
 	}
 	_, err = stmt.QueryContext(context.Background(), []driver.NamedValue{})
-	if err != driver.ErrBadConn {
-		t.Fatal("Connection should be bad")
+	if !errors.Is(err, driver.ErrBadConn) {
+		t.Fatalf("Connection should be bad D, is: %[1]T %[1]v", err)
 	}
 	_, err = stmt.ExecContext(context.Background(), []driver.NamedValue{})
-	if err != driver.ErrBadConn {
-		t.Fatal("Connection should be bad")
+	if !errors.Is(err, driver.ErrBadConn) {
+		t.Fatalf("Connection should be bad E, is: %[1]T %[1]v", err)
 	}
 }