-
Type:
Bug
-
Resolution: Unresolved
-
Priority:
Minor - P4
-
None
-
Affects Version/s: None
-
Component/s: Connection String
The ConnString type has functions to validate auth/ssl/etc, but this validation would not be done if ClientOptions were used instead of a URI. I wrote up a minimal example at https://gist.github.com/divjotarora/e4f8c9f14bfc154c857f9a1fb5a73f99. The clientFromURI function sees an error from NewClient because SCRAM-SHA-1 requires a username. The clientFromOpts function sees no error.
One solution is to merge connstring and ClientOptions validation.
Reproduction
// The ConnString type has functions to validate auth/ssl/etc, but this // validation would not be done if ClientOptions were used instead of a URI. func TestMGD_MergeConnStringValidation_GODRIVER1714(t *testing.T) { t.Run("with URI", func(t *testing.T) { uri := "mongodb://localhost:27017/?authMechanism=SCRAM-SHA-1" opts := options.Client().ApplyURI(uri) _, err := mongo.Connect(opts) require.ErrorContains(t, err, "username required") }) t.Run("with ClientOptions", func(t *testing.T) { opts := options.Client().SetAuth(options.Credential{AuthMechanism: "SCRAM-SHA-1"}) _, err := mongo.Connect(opts) require.ErrorContains(t, err, "username required") }) }
Definition of Done
Given that the rules shared between connstring and ClientOptions each need only a handful of arguments, we should extend internal/optionsutil with Validate* functions that can be shared between ClientOptions.Validate() and ConnString.Validate() without changing the latter’s contract:
func ValidateCredential(mechanism, username, password string, props map[string]string) error { panic("todo") }
func ValidateDirect(direct *bool, hosts []string, isSRV bool) error { panic("todo") }
func ValidateLoadBalanced(loadBalanced *bool, hosts []string, replicaSet string, direct *bool) error { panic("todo") }
func ValidateSRVMaxHosts(srvMaxHosts *int, replicaSet string, loadBalanced *bool) error { panic("todo") }
func ValidateTLS(insecureSkipVerify, disableOCSP *bool) error { panic("todo") }