From 6a176d498b3fd4e81724bc2dbdc55611453f880c Mon Sep 17 00:00:00 2001 From: Felipe Gasper Date: Tue, 15 Jul 2025 10:57:51 -0400 Subject: [PATCH 1/5] Fix segfault --- common/db/db.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/common/db/db.go b/common/db/db.go index 8d57c4af3..285f2a347 100644 --- a/common/db/db.go +++ b/common/db/db.go @@ -123,7 +123,7 @@ func NewSessionProvider(opts options.ToolOptions) (*SessionProvider, error) { } err = client.Ping(context.Background(), nil) if err != nil { - return nil, fmt.Errorf("failed to connect to %s: %v", opts.URI.ParsedConnString(), err) + return nil, fmt.Errorf("failed to connect to server (opts: %s): %v", opts, err) } // create the provider From 0b3c9dbcaf73f8912ea4edf9aaaf263ecdf39e4f Mon Sep 17 00:00:00 2001 From: Felipe Gasper Date: Tue, 15 Jul 2025 11:09:45 -0400 Subject: [PATCH 2/5] struct --- common/db/db.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/common/db/db.go b/common/db/db.go index 285f2a347..f9b7cce73 100644 --- a/common/db/db.go +++ b/common/db/db.go @@ -123,7 +123,7 @@ func NewSessionProvider(opts options.ToolOptions) (*SessionProvider, error) { } err = client.Ping(context.Background(), nil) if err != nil { - return nil, fmt.Errorf("failed to connect to server (opts: %s): %v", opts, err) + return nil, fmt.Errorf("failed to connect to server (opts: %+v): %v", opts, err) } // create the provider From 3d73064d20278a6bbad74518a7bccafd3c6a56f7 Mon Sep 17 00:00:00 2001 From: Felipe Gasper Date: Tue, 15 Jul 2025 11:25:19 -0400 Subject: [PATCH 3/5] add test --- common/db/db.go | 2 +- common/db/db_test.go | 77 ++++++++++++++++++++++++++------------------ 2 files changed, 47 insertions(+), 32 deletions(-) diff --git a/common/db/db.go b/common/db/db.go index f9b7cce73..8d57c4af3 100644 --- a/common/db/db.go +++ b/common/db/db.go @@ -123,7 +123,7 @@ func NewSessionProvider(opts options.ToolOptions) (*SessionProvider, error) { } err = client.Ping(context.Background(), nil) if err != nil { - return nil, fmt.Errorf("failed to connect to server (opts: %+v): %v", opts, err) + return nil, fmt.Errorf("failed to connect to %s: %v", opts.URI.ParsedConnString(), err) } // create the provider diff --git a/common/db/db_test.go b/common/db/db_test.go index 481c3bee0..4a116e285 100644 --- a/common/db/db_test.go +++ b/common/db/db_test.go @@ -74,43 +74,58 @@ func DBGetConnString() *options.URI { func TestNewSessionProvider(t *testing.T) { testtype.SkipUnlessTestType(t, testtype.IntegrationTestType) - auth := DBGetAuthOptions() - ssl := DBGetSSLOptions() + //auth := DBGetAuthOptions() + //ssl := DBGetSSLOptions() Convey("When initializing a session provider", t, func() { - Convey("with the standard options, a provider with a standard"+ - " connector should be returned", func() { - opts := options.ToolOptions{ - Connection: &options.Connection{ - Port: DefaultTestPort, - }, - URI: DBGetConnString(), - SSL: &ssl, - Auth: &auth, - } - provider, err := NewSessionProvider(opts) - So(err, ShouldBeNil) + /* + Convey("with the standard options, a provider with a standard"+ + " connector should be returned", func() { + opts := options.ToolOptions{ + Connection: &options.Connection{ + Port: DefaultTestPort, + }, + URI: DBGetConnString(), + SSL: &ssl, + Auth: &auth, + } + provider, err := NewSessionProvider(opts) + So(err, ShouldBeNil) - Convey("and should be closeable", func() { - provider.Close() + Convey("and should be closeable", func() { + provider.Close() + }) }) - }) - Convey("the master session should be successfully "+ - " initialized", func() { - opts := options.ToolOptions{ - Connection: &options.Connection{ - Port: DefaultTestPort, - }, - URI: DBGetConnString(), - SSL: &ssl, - Auth: &auth, - } - provider, err := NewSessionProvider(opts) - So(err, ShouldBeNil) - So(provider.client.Ping(context.Background(), nil), ShouldBeNil) - }) + Convey("the master session should be successfully "+ + " initialized", func() { + opts := options.ToolOptions{ + Connection: &options.Connection{ + Port: DefaultTestPort, + }, + URI: DBGetConnString(), + SSL: &ssl, + Auth: &auth, + } + provider, err := NewSessionProvider(opts) + So(err, ShouldBeNil) + So(provider.client.Ping(context.Background(), nil), ShouldBeNil) + }) + */ + + Convey( + "the error should return when there’s only a host", + func() { + opts := options.ToolOptions{ + Connection: &options.Connection{ + Host: "localhost:12345", + }, + } + _, err := NewSessionProvider(opts) + So(err, ShouldNotBeNil) + }, + ) }) } From 18f35dfc700704efd36cb20809e6b5f8801c9b1c Mon Sep 17 00:00:00 2001 From: Felipe Gasper Date: Tue, 15 Jul 2025 11:45:57 -0400 Subject: [PATCH 4/5] undo some --- common/db/db.go | 2 +- common/db/db_test.go | 65 +++++++++++++++++++-------------------- common/options/options.go | 8 +++++ 3 files changed, 40 insertions(+), 35 deletions(-) diff --git a/common/db/db.go b/common/db/db.go index 8d57c4af3..f9b7cce73 100644 --- a/common/db/db.go +++ b/common/db/db.go @@ -123,7 +123,7 @@ func NewSessionProvider(opts options.ToolOptions) (*SessionProvider, error) { } err = client.Ping(context.Background(), nil) if err != nil { - return nil, fmt.Errorf("failed to connect to %s: %v", opts.URI.ParsedConnString(), err) + return nil, fmt.Errorf("failed to connect to server (opts: %+v): %v", opts, err) } // create the provider diff --git a/common/db/db_test.go b/common/db/db_test.go index 4a116e285..3063dcf39 100644 --- a/common/db/db_test.go +++ b/common/db/db_test.go @@ -74,44 +74,41 @@ func DBGetConnString() *options.URI { func TestNewSessionProvider(t *testing.T) { testtype.SkipUnlessTestType(t, testtype.IntegrationTestType) - //auth := DBGetAuthOptions() - //ssl := DBGetSSLOptions() + auth := DBGetAuthOptions() + ssl := DBGetSSLOptions() Convey("When initializing a session provider", t, func() { + Convey("with the standard options, a provider with a standard"+ + " connector should be returned", func() { + opts := options.ToolOptions{ + Connection: &options.Connection{ + Port: DefaultTestPort, + }, + URI: DBGetConnString(), + SSL: &ssl, + Auth: &auth, + } + provider, err := NewSessionProvider(opts) + So(err, ShouldBeNil) - /* - Convey("with the standard options, a provider with a standard"+ - " connector should be returned", func() { - opts := options.ToolOptions{ - Connection: &options.Connection{ - Port: DefaultTestPort, - }, - URI: DBGetConnString(), - SSL: &ssl, - Auth: &auth, - } - provider, err := NewSessionProvider(opts) - So(err, ShouldBeNil) - - Convey("and should be closeable", func() { - provider.Close() - }) + Convey("and should be closeable", func() { + provider.Close() }) + }) - Convey("the master session should be successfully "+ - " initialized", func() { - opts := options.ToolOptions{ - Connection: &options.Connection{ - Port: DefaultTestPort, - }, - URI: DBGetConnString(), - SSL: &ssl, - Auth: &auth, - } - provider, err := NewSessionProvider(opts) - So(err, ShouldBeNil) - So(provider.client.Ping(context.Background(), nil), ShouldBeNil) - }) - */ + Convey("the master session should be successfully "+ + " initialized", func() { + opts := options.ToolOptions{ + Connection: &options.Connection{ + Port: DefaultTestPort, + }, + URI: DBGetConnString(), + SSL: &ssl, + Auth: &auth, + } + provider, err := NewSessionProvider(opts) + So(err, ShouldBeNil) + So(provider.client.Ping(context.Background(), nil), ShouldBeNil) + }) Convey( "the error should return when there’s only a host", diff --git a/common/options/options.go b/common/options/options.go index d99cc18a8..89e64d957 100644 --- a/common/options/options.go +++ b/common/options/options.go @@ -695,6 +695,10 @@ func (opts *ToolOptions) NormalizeOptionsAndURI() error { return err } + if opts.Auth == nil { + opts.Auth = &Auth{} + } + // finalize auth options, filling in missing passwords if opts.Auth.ShouldAskForPassword() { pass, err := password.Prompt("mongo user") @@ -1069,6 +1073,10 @@ func (opts *ToolOptions) setOptionsFromURI(cs *connstring.ConnString) error { } } + if opts.SSL == nil { + opts.SSL = &SSL{} + } + // ignore opts.UseSSL being false due to zero-value problem (TOOLS-2459 PR for details) // Ignore: opts.UseSSL = false, cs.SSL = true (have cs take precedence) // Treat as conflict: opts.UseSSL = true, cs.SSL = false From 33b00de4b733fe5d30133846502bbcf03fa2a906 Mon Sep 17 00:00:00 2001 From: Felipe Gasper Date: Tue, 15 Jul 2025 11:52:51 -0400 Subject: [PATCH 5/5] move around; more conservative --- common/db/db_test.go | 29 +++++++++++++++-------------- common/options/options.go | 10 +--------- 2 files changed, 16 insertions(+), 23 deletions(-) diff --git a/common/db/db_test.go b/common/db/db_test.go index 3063dcf39..b053a8428 100644 --- a/common/db/db_test.go +++ b/common/db/db_test.go @@ -77,6 +77,21 @@ func TestNewSessionProvider(t *testing.T) { auth := DBGetAuthOptions() ssl := DBGetSSLOptions() Convey("When initializing a session provider", t, func() { + Convey( + "the error should return when there’s only a host", + func() { + opts := options.ToolOptions{ + Connection: &options.Connection{ + Host: "localhost:12345", + }, + SSL: &ssl, + } + + _, err := NewSessionProvider(opts) + So(err, ShouldNotBeNil) + }, + ) + Convey("with the standard options, a provider with a standard"+ " connector should be returned", func() { opts := options.ToolOptions{ @@ -109,20 +124,6 @@ func TestNewSessionProvider(t *testing.T) { So(err, ShouldBeNil) So(provider.client.Ping(context.Background(), nil), ShouldBeNil) }) - - Convey( - "the error should return when there’s only a host", - func() { - opts := options.ToolOptions{ - Connection: &options.Connection{ - Host: "localhost:12345", - }, - } - - _, err := NewSessionProvider(opts) - So(err, ShouldNotBeNil) - }, - ) }) } diff --git a/common/options/options.go b/common/options/options.go index 89e64d957..c8e8fc1ca 100644 --- a/common/options/options.go +++ b/common/options/options.go @@ -695,12 +695,8 @@ func (opts *ToolOptions) NormalizeOptionsAndURI() error { return err } - if opts.Auth == nil { - opts.Auth = &Auth{} - } - // finalize auth options, filling in missing passwords - if opts.Auth.ShouldAskForPassword() { + if opts.Auth != nil && opts.Auth.ShouldAskForPassword() { pass, err := password.Prompt("mongo user") if err != nil { return fmt.Errorf("error reading password: %v", err) @@ -1073,10 +1069,6 @@ func (opts *ToolOptions) setOptionsFromURI(cs *connstring.ConnString) error { } } - if opts.SSL == nil { - opts.SSL = &SSL{} - } - // ignore opts.UseSSL being false due to zero-value problem (TOOLS-2459 PR for details) // Ignore: opts.UseSSL = false, cs.SSL = true (have cs take precedence) // Treat as conflict: opts.UseSSL = true, cs.SSL = false