From 51e4fa75e1217f9a5a820fc9f6475901ef7f2654 Mon Sep 17 00:00:00 2001 From: Greta Sharoyan Date: Fri, 10 Jul 2026 14:28:03 -0700 Subject: [PATCH] check err before type assertion Signed-off-by: Greta Sharoyan --- CHANGELOG.md | 1 + internal/client/client.go | 6 +++--- internal/client/client_test.go | 29 +++++++++++++++++++++++++++++ 3 files changed, 33 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f90f9264..08533309 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,7 @@ # Release History ## Unreleased +- Fix panic in `InitThriftClient` when the endpoint URL is malformed: the thrift transport was type-asserted to `*thrift.THttpClient` before the error from `NewTHttpClientWithOptions` was checked, so a URL that fails to parse (nil transport) caused `interface conversion: thrift.TTransport is nil, not *thrift.THttpClient`; the error is now returned instead (databricks/databricks-sql-go#394) - Stop using `html/template` in the U2M OAuth callback page. Reachable use of `html/template` disabled the Go linker's dead-code elimination for the entire binary of any application importing this driver; the page is now rendered with plain string building and explicit HTML escaping, restoring full DCE (databricks/databricks-sql-go#343) - Fix DECIMAL precision loss inside complex types: DECIMAL values nested in STRUCT, ARRAY, and MAP columns are now rendered losslessly with their exact scale (e.g. `19.99` instead of `19.990000000000002`), matching the behavior of top-level DECIMAL columns (databricks/databricks-sql-go#253) diff --git a/internal/client/client.go b/internal/client/client.go index 8f1ca780..3563dbb0 100644 --- a/internal/client/client.go +++ b/internal/client/client.go @@ -317,6 +317,9 @@ func InitThriftClient(cfg *config.Config, httpclient *http.Client) (*ThriftServi } tTrans, err = thrift.NewTHttpClientWithOptions(endpoint, thrift.THttpClientOptions{Client: httpclient}) + if err != nil { + return nil, dbsqlerrint.NewRequestError(context.TODO(), dbsqlerr.ErrInvalidURL, err) + } thriftHttpClient := tTrans.(*thrift.THttpClient) thriftHttpClient.SetHeader("User-Agent", BuildUserAgent(cfg)) @@ -324,9 +327,6 @@ func InitThriftClient(cfg *config.Config, httpclient *http.Client) (*ThriftServi default: return nil, dbsqlerrint.NewDriverError(context.TODO(), fmt.Sprintf("unsupported transport `%s`", cfg.ThriftTransport), nil) } - if err != nil { - return nil, dbsqlerrint.NewRequestError(context.TODO(), dbsqlerr.ErrInvalidURL, err) - } if err = tTrans.Open(); err != nil { return nil, dbsqlerrint.NewRequestError(context.TODO(), fmt.Sprintf("failed to open http transport for endpoint %s", endpoint), err) } diff --git a/internal/client/client_test.go b/internal/client/client_test.go index 7a19568f..03278593 100644 --- a/internal/client/client_test.go +++ b/internal/client/client_test.go @@ -11,6 +11,7 @@ import ( "time" dbsqlerr "github.com/databricks/databricks-sql-go/errors" + "github.com/databricks/databricks-sql-go/internal/config" "github.com/pkg/errors" "github.com/stretchr/testify/require" ) @@ -325,3 +326,31 @@ func TestRetryPolicy(t *testing.T) { } }) } + +// TestInitThriftClientMalformedEndpointDoesNotPanic is a regression test: when the +// endpoint URL is malformed, thrift.NewTHttpClientWithOptions returns a nil +// TTransport and an error. InitThriftClient previously type-asserted that nil +// transport to *thrift.THttpClient before checking the error, panicking with +// "interface conversion: thrift.TTransport is nil, not *thrift.THttpClient". +// It must now surface the error instead. +func TestInitThriftClientMalformedEndpointDoesNotPanic(t *testing.T) { + cfg := &config.Config{ + UserConfig: config.UserConfig{ + Protocol: "https", + Host: "example.cloud.databricks.com", + Port: 443, + // A control character passes ToEndpointURL's non-empty check but + // makes url.Parse (inside NewTHttpClientWithOptions) fail. + HTTPPath: "/sql/1.0/warehouses/\x7f", + }, + ThriftProtocol: "binary", + ThriftTransport: "http", + } + + require.NotPanics(t, func() { + // Non-nil httpclient so we exercise the transport path, not the + // missing-authenticator early return. + _, err := InitThriftClient(cfg, &http.Client{}) + require.Error(t, err) + }) +}