Skip to content

Commit 6f3c9d1

Browse files
tfogotfogo
authored
GODRIVER-1490 Split parsing and validation logic (#309)
1 parent 5ebbe78 commit 6f3c9d1

File tree

14 files changed

+82
-50
lines changed

14 files changed

+82
-50
lines changed

‎internal/testutil/config.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -236,7 +236,7 @@ func ConnString(t *testing.T) connstring.ConnString {
236236
mongodbURI = AddCompressorToUri(mongodbURI)
237237

238238
var err error
239-
connectionString, err = connstring.Parse(mongodbURI)
239+
connectionString, err = connstring.ParseAndValidate(mongodbURI)
240240
if err != nil {
241241
connectionStringErr = err
242242
}
@@ -256,7 +256,7 @@ func GetConnString() (connstring.ConnString, error) {
256256

257257
mongodbURI = AddTLSConfigToURI(mongodbURI)
258258

259-
cs, err := connstring.Parse(mongodbURI)
259+
cs, err := connstring.ParseAndValidate(mongodbURI)
260260
if err != nil {
261261
return connstring.ConnString{}, err
262262
}

‎mongo/integration/initial_dns_seedlist_discovery_test.go

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -67,10 +67,7 @@ func runSeedlistDiscoveryTest(mt *mtest.T, file string) {
6767
assert.NotNil(mt, err, "expected URI parsing error, got nil")
6868
return
6969
}
70-
// the resolved connstring may not have valid credentials
71-
if err != nil && err.Error() == "error parsing uri: authsource without username is invalid" {
72-
err = nil
73-
}
70+
7471
assert.Nil(mt, err, "Connect error: %v", err)
7572
assert.Equal(mt, connstring.SchemeMongoDBSRV, cs.Scheme,
7673
"expected scheme %v, got %v", connstring.SchemeMongoDBSRV, cs.Scheme)

‎mongo/integration/mtest/setup.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -203,7 +203,7 @@ func getConnString() (connstring.ConnString, error) {
203203
}
204204
uri = addTLSConfig(uri)
205205
uri = addCompressors(uri)
206-
return connstring.Parse(uri)
206+
return connstring.ParseAndValidate(uri)
207207
}
208208

209209
// compareVersions compares two version number strings (i.e. positive integers separated by

‎mongo/options/clientoptions.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -152,7 +152,7 @@ func (c *ClientOptions) ApplyURI(uri string) *ClientOptions {
152152
return c
153153
}
154154

155-
cs, err := connstring.Parse(uri)
155+
cs, err := connstring.ParseAndValidate(uri)
156156
if err != nil {
157157
c.err = err
158158
return c

‎mongo/options/clientoptions_test.go

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -240,7 +240,7 @@ func TestClientOptions(t *testing.T) {
240240
"AuthSourceNoUsername",
241241
"mongodb://localhost/?authSource=random-database-example",
242242
&ClientOptions{err: internal.WrapErrorf(
243-
errors.New("authsource without username is invalid"), "error parsing uri",
243+
errors.New("authsource without username is invalid"), "error validating uri",
244244
)},
245245
},
246246
{
@@ -414,15 +414,15 @@ func TestClientOptions(t *testing.T) {
414414
"mongodb://localhost/?tlsCertificateFile=testdata/nopass/cert.pem",
415415
&ClientOptions{err: internal.WrapErrorf(
416416
errors.New("the tlsPrivateKeyFile URI option must be provided if the tlsCertificateFile option is specified"),
417-
"error parsing uri",
417+
"error validating uri",
418418
)},
419419
},
420420
{
421421
"TLS only tlsPrivateKeyFile",
422422
"mongodb://localhost/?tlsPrivateKeyFile=testdata/nopass/key.pem",
423423
&ClientOptions{err: internal.WrapErrorf(
424424
errors.New("the tlsCertificateFile URI option must be provided if the tlsPrivateKeyFile option is specified"),
425-
"error parsing uri",
425+
"error validating uri",
426426
)},
427427
},
428428
{
@@ -431,7 +431,7 @@ func TestClientOptions(t *testing.T) {
431431
&ClientOptions{err: internal.WrapErrorf(
432432
errors.New("the sslClientCertificateKeyFile/tlsCertificateKeyFile URI option cannot be provided "+
433433
"along with tlsCertificateFile or tlsPrivateKeyFile"),
434-
"error parsing uri",
434+
"error validating uri",
435435
)},
436436
},
437437
}

‎mongo/read_write_concern_spec_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -95,7 +95,7 @@ func runConnectionStringTestFile(t *testing.T, filePath string) {
9595
}
9696

9797
func runConnectionStringTest(t *testing.T, test connectionStringTest) {
98-
cs, err := connstring.Parse(test.URI)
98+
cs, err := connstring.ParseAndValidate(test.URI)
9999
if !test.Valid {
100100
assert.NotNil(t, err, "expected Parse error, got nil")
101101
return

‎x/mongo/driver/connstring/connstring.go

Lines changed: 42 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,24 @@ import (
2121
"go.mongodb.org/mongo-driver/x/mongo/driver/wiremessage"
2222
)
2323

24-
// Parse parses the provided uri and returns a URI object.
24+
// ParseAndValidate parses the provided URI into a ConnString object.
25+
// It check that all values are valid.
26+
func ParseAndValidate(s string) (ConnString, error) {
27+
p := parser{dnsResolver: dns.DefaultResolver}
28+
err := p.parse(s)
29+
if err != nil {
30+
return p.ConnString, internal.WrapErrorf(err, "error parsing uri")
31+
}
32+
err = p.ConnString.Validate()
33+
if err != nil {
34+
return p.ConnString, internal.WrapErrorf(err, "error validating uri")
35+
}
36+
return p.ConnString, nil
37+
}
38+
39+
// Parse parses the provided URI into a ConnString object
40+
// but does not check that all values are valid. Use `ConnString.Validate()`
41+
// to run the validation checks separately.
2542
func Parse(s string) (ConnString, error) {
2643
p := parser{dnsResolver: dns.DefaultResolver}
2744
err := p.parse(s)
@@ -37,7 +54,9 @@ type ConnString struct {
3754
AppName string
3855
AuthMechanism string
3956
AuthMechanismProperties map[string]string
57+
AuthMechanismPropertiesSet bool
4058
AuthSource string
59+
AuthSourceSet bool
4160
Compressors []string
4261
Connect ConnectMode
4362
ConnectSet bool
@@ -109,6 +128,15 @@ func (u *ConnString) String() string {
109128
return u.Original
110129
}
111130

131+
// Validate checks that the Auth and SSL parameters are valid values.
132+
func (u *ConnString) Validate() error {
133+
p := parser{
134+
dnsResolver: dns.DefaultResolver,
135+
ConnString: *u,
136+
}
137+
return p.validate()
138+
}
139+
112140
// ConnectMode informs the driver on how to connect
113141
// to the server.
114142
type ConnectMode uint8
@@ -251,6 +279,17 @@ func (p *parser) parse(original string) error {
251279
return err
252280
}
253281

282+
// If WTimeout was set from manual options passed in, set WTImeoutSet to true.
283+
if p.WTimeoutSetFromOption {
284+
p.WTimeoutSet = true
285+
}
286+
287+
return nil
288+
}
289+
290+
func (p *parser) validate() error {
291+
var err error
292+
254293
err = p.validateAuth()
255294
if err != nil {
256295
return err
@@ -265,11 +304,6 @@ func (p *parser) parse(original string) error {
265304
return writeconcern.ErrInconsistent
266305
}
267306

268-
// If WTimeout was set from manual options passed in, set WTImeoutSet to true.
269-
if p.WTimeoutSetFromOption {
270-
p.WTimeoutSet = true
271-
}
272-
273307
return nil
274308
}
275309

@@ -473,8 +507,10 @@ func (p *parser) addOption(pair string) error {
473507
}
474508
p.AuthMechanismProperties[kv[0]] = kv[1]
475509
}
510+
p.AuthMechanismPropertiesSet = true
476511
case "authsource":
477512
p.AuthSource = value
513+
p.AuthSourceSet = true
478514
case "compressors":
479515
compressors := strings.Split(value, ",")
480516
if len(compressors) < 1 {

‎x/mongo/driver/connstring/connstring_spec_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -117,7 +117,7 @@ func runTest(t *testing.T, filename string, test *testCase, warningsError bool)
117117
if _, skip := skipTest[test.Description]; skip {
118118
t.Skip()
119119
}
120-
cs, err := connstring.Parse(test.URI)
120+
cs, err := connstring.ParseAndValidate(test.URI)
121121
// Since we don't have warnings in Go, we return warnings as errors.
122122
//
123123
// This is a bit unfortuante, but since we do raise warnings as errors with the newer

‎x/mongo/driver/connstring/connstring_test.go

Lines changed: 21 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ func TestAppName(t *testing.T) {
3030
for _, test := range tests {
3131
s := fmt.Sprintf("mongodb://localhost/?%s", test.s)
3232
t.Run(s, func(t *testing.T) {
33-
cs, err := connstring.Parse(s)
33+
cs, err := connstring.ParseAndValidate(s)
3434
if test.err {
3535
require.Error(t, err)
3636
} else {
@@ -56,7 +56,7 @@ func TestAuthMechanism(t *testing.T) {
5656
for _, test := range tests {
5757
s := fmt.Sprintf("mongodb://user:pass@localhost/?%s", test.s)
5858
t.Run(s, func(t *testing.T) {
59-
cs, err := connstring.Parse(s)
59+
cs, err := connstring.ParseAndValidate(s)
6060
if test.err {
6161
require.Error(t, err)
6262
} else {
@@ -81,7 +81,7 @@ func TestAuthSource(t *testing.T) {
8181
for _, test := range tests {
8282
s := fmt.Sprintf("mongodb://user:pass@localhost/%s", test.s)
8383
t.Run(s, func(t *testing.T) {
84-
cs, err := connstring.Parse(s)
84+
cs, err := connstring.ParseAndValidate(s)
8585
if test.err {
8686
require.Error(t, err)
8787
} else {
@@ -107,7 +107,7 @@ func TestConnect(t *testing.T) {
107107
for _, test := range tests {
108108
s := fmt.Sprintf("mongodb://localhost/?%s", test.s)
109109
t.Run(s, func(t *testing.T) {
110-
cs, err := connstring.Parse(s)
110+
cs, err := connstring.ParseAndValidate(s)
111111
if test.err {
112112
require.Error(t, err)
113113
} else {
@@ -133,7 +133,7 @@ func TestConnectTimeout(t *testing.T) {
133133
for _, test := range tests {
134134
s := fmt.Sprintf("mongodb://localhost/?%s", test.s)
135135
t.Run(s, func(t *testing.T) {
136-
cs, err := connstring.Parse(s)
136+
cs, err := connstring.ParseAndValidate(s)
137137
if test.err {
138138
require.Error(t, err)
139139
} else {
@@ -160,7 +160,7 @@ func TestHeartbeatInterval(t *testing.T) {
160160
for _, test := range tests {
161161
s := fmt.Sprintf("mongodb://localhost/?%s", test.s)
162162
t.Run(s, func(t *testing.T) {
163-
cs, err := connstring.Parse(s)
163+
cs, err := connstring.ParseAndValidate(s)
164164
if test.err {
165165
require.Error(t, err)
166166
} else {
@@ -187,7 +187,7 @@ func TestLocalThreshold(t *testing.T) {
187187
for _, test := range tests {
188188
s := fmt.Sprintf("mongodb://localhost/?%s", test.s)
189189
t.Run(s, func(t *testing.T) {
190-
cs, err := connstring.Parse(s)
190+
cs, err := connstring.ParseAndValidate(s)
191191
if test.err {
192192
require.Error(t, err)
193193
} else {
@@ -213,7 +213,7 @@ func TestMaxConnIdleTime(t *testing.T) {
213213
for _, test := range tests {
214214
s := fmt.Sprintf("mongodb://localhost/?%s", test.s)
215215
t.Run(s, func(t *testing.T) {
216-
cs, err := connstring.Parse(s)
216+
cs, err := connstring.ParseAndValidate(s)
217217
if test.err {
218218
require.Error(t, err)
219219
} else {
@@ -239,7 +239,7 @@ func TestMaxPoolSize(t *testing.T) {
239239
for _, test := range tests {
240240
s := fmt.Sprintf("mongodb://localhost/?%s", test.s)
241241
t.Run(s, func(t *testing.T) {
242-
cs, err := connstring.Parse(s)
242+
cs, err := connstring.ParseAndValidate(s)
243243
if test.err {
244244
require.Error(t, err)
245245
} else {
@@ -266,7 +266,7 @@ func TestMinPoolSize(t *testing.T) {
266266
for _, test := range tests {
267267
s := fmt.Sprintf("mongodb://localhost/?%s", test.s)
268268
t.Run(s, func(t *testing.T) {
269-
cs, err := connstring.Parse(s)
269+
cs, err := connstring.ParseAndValidate(s)
270270
if test.err {
271271
require.Error(t, err)
272272
} else {
@@ -292,7 +292,7 @@ func TestReadPreference(t *testing.T) {
292292
for _, test := range tests {
293293
s := fmt.Sprintf("mongodb://localhost/?%s", test.s)
294294
t.Run(s, func(t *testing.T) {
295-
cs, err := connstring.Parse(s)
295+
cs, err := connstring.ParseAndValidate(s)
296296
if test.err {
297297
require.Error(t, err)
298298
} else {
@@ -321,7 +321,7 @@ func TestReadPreferenceTags(t *testing.T) {
321321
for _, test := range tests {
322322
s := fmt.Sprintf("mongodb://localhost/?%s", test.s)
323323
t.Run(s, func(t *testing.T) {
324-
cs, err := connstring.Parse(s)
324+
cs, err := connstring.ParseAndValidate(s)
325325
if test.err {
326326
require.Error(t, err)
327327
} else {
@@ -346,7 +346,7 @@ func TestMaxStaleness(t *testing.T) {
346346
for _, test := range tests {
347347
s := fmt.Sprintf("mongodb://localhost/?%s", test.s)
348348
t.Run(s, func(t *testing.T) {
349-
cs, err := connstring.Parse(s)
349+
cs, err := connstring.ParseAndValidate(s)
350350
if test.err {
351351
require.Error(t, err)
352352
} else {
@@ -370,7 +370,7 @@ func TestReplicaSet(t *testing.T) {
370370
for _, test := range tests {
371371
s := fmt.Sprintf("mongodb://localhost/?%s", test.s)
372372
t.Run(s, func(t *testing.T) {
373-
cs, err := connstring.Parse(s)
373+
cs, err := connstring.ParseAndValidate(s)
374374
if test.err {
375375
require.Error(t, err)
376376
} else {
@@ -395,7 +395,7 @@ func TestRetryWrites(t *testing.T) {
395395
for _, test := range tests {
396396
s := fmt.Sprintf("mongodb://localhost/?%s", test.s)
397397
t.Run(s, func(t *testing.T) {
398-
cs, err := connstring.Parse(s)
398+
cs, err := connstring.ParseAndValidate(s)
399399
if test.err {
400400
require.Error(t, err)
401401
return
@@ -421,7 +421,7 @@ func TestRetryReads(t *testing.T) {
421421
for _, test := range tests {
422422
s := fmt.Sprintf("mongodb://localhost/?%s", test.s)
423423
t.Run(s, func(t *testing.T) {
424-
cs, err := connstring.Parse(s)
424+
cs, err := connstring.ParseAndValidate(s)
425425
if test.err {
426426
require.Error(t, err)
427427
return
@@ -436,7 +436,7 @@ func TestRetryReads(t *testing.T) {
436436
func TestScheme(t *testing.T) {
437437
// Can't unit test 'mongodb+srv' because that requires networking. Tested
438438
// in x/mongo/driver/topology/initial_dns_seedlist_discovery_test.go
439-
cs, err := connstring.Parse("mongodb://localhost/")
439+
cs, err := connstring.ParseAndValidate("mongodb://localhost/")
440440
require.NoError(t, err)
441441
require.Equal(t, cs.Scheme, "mongodb")
442442
require.Equal(t, cs.Scheme, connstring.SchemeMongoDB)
@@ -457,7 +457,7 @@ func TestServerSelectionTimeout(t *testing.T) {
457457
for _, test := range tests {
458458
s := fmt.Sprintf("mongodb://localhost/?%s", test.s)
459459
t.Run(s, func(t *testing.T) {
460-
cs, err := connstring.Parse(s)
460+
cs, err := connstring.ParseAndValidate(s)
461461
if test.err {
462462
require.Error(t, err)
463463
} else {
@@ -484,7 +484,7 @@ func TestSocketTimeout(t *testing.T) {
484484
for _, test := range tests {
485485
s := fmt.Sprintf("mongodb://localhost/?%s", test.s)
486486
t.Run(s, func(t *testing.T) {
487-
cs, err := connstring.Parse(s)
487+
cs, err := connstring.ParseAndValidate(s)
488488
if test.err {
489489
require.Error(t, err)
490490
} else {
@@ -511,7 +511,7 @@ func TestWTimeout(t *testing.T) {
511511
for _, test := range tests {
512512
s := fmt.Sprintf("mongodb://localhost/?%s", test.s)
513513
t.Run(s, func(t *testing.T) {
514-
cs, err := connstring.Parse(s)
514+
cs, err := connstring.ParseAndValidate(s)
515515
if test.err {
516516
require.Error(t, err)
517517
} else {
@@ -545,7 +545,7 @@ func TestCompressionOptions(t *testing.T) {
545545
for _, tc := range tests {
546546
uri := fmt.Sprintf("mongodb://localhost/?%s", tc.uriOptions)
547547
t.Run(tc.name, func(t *testing.T) {
548-
cs, err := connstring.Parse(uri)
548+
cs, err := connstring.ParseAndValidate(uri)
549549
if tc.err {
550550
assert.Error(t, err)
551551
} else {

‎x/mongo/driver/examples/count/main.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ func main() {
3131
log.Fatalf("uri flag must have a value")
3232
}
3333

34-
cs, err := connstring.Parse(*uri)
34+
cs, err := connstring.ParseAndValidate(*uri)
3535
if err != nil {
3636
log.Fatal(err)
3737
}

0 commit comments

Comments
 (0)