mgo.DialWithInfo does not throw error given an empty connection string (i.e., func extractURL does not check for empty string)

Ouroboros commented 5 years ago

What version of MongoDB are you using (`mongod --version`)?

MongoDB shell version v4.0.0
git version: 3b07af3d4f471ae89e8186d33bbb1d5259597d51
OpenSSL version: OpenSSL 1.0.2g  1 Mar 2016
allocator: tcmalloc
modules: none
build environment:
    distmod: ubuntu1604
    distarch: x86_64
    target_arch: x86_64

What version of Go are you using (`go version`)?

go version go1.11 linux/amd64

What operating system and processor architecture are you using (`go env`)?

GOARCH="amd64"
GOOS="linux"

DISTRIB_ID=Ubuntu
DISTRIB_RELEASE=16.04
DISTRIB_CODENAME=xenial
DISTRIB_DESCRIPTION="Ubuntu 16.04.4 LTS"

What did you do?

Write a piece of code to pass an empty string to mgo.PraseURL():

logs.Debugf("url: %q", mgoURL)

dialInfo, err := mgo.ParseURL(mgoURL)
if err != nil {
    logs.Errorf("mgo.ParseURL err(%v)", err)
    return
}

logs.Debugf("info: %+v", dialInfo)
logs.Debugf("err: %v", err)

dialInfo.Timeout = time.Second * 20
sess, err := mgo.DialWithInfo(dialInfo)
if err != nil {
    logs.Errorf("mgo.DialWithInfo err(%v)", err)
    return
}

logs.Debugf("sess: %+v", sess)
logs.Debugf("err: %v", err)

Read the log:

[] 22:50:43 mgo.go:17 NewMgoClient  [DEBU:008] url: ""
[] 22:50:43 mgo.go:25 NewMgoClient  [DEBU:009] info: &{Addrs:[] Timeout:0s Database: ReplicaSetName: Source: Service: ServiceHost: Mechanism: Username: Password: PoolLimit:0 PoolTimeout:0s ReadTimeout:0s WriteTimeout:0s AppName: ReadPreference:0xc001bb4300 Safe:{W:0 WMode: RMode: WTimeout:0 FSync:false J:false} FailFast:false Direct:false MinPoolSize:0 MaxIdleTimeMS:0 DialServer:<nil> Dial:<nil>}
[] 22:50:43 mgo.go:26 NewMgoClient  [DEBU:00a] err: <nil>
[] 22:50:43 mgo.go:35 NewMgoClient  [DEBU:00b] sess: &{defaultdb:test sourcedb:admin syncTimeout:20000000000 consistency:2 creds:[] dialCred:<nil> safeOp:0xc001b6e690 mgoCluster:0xc001b30f00 slaveSocket:<nil> masterSocket:<nil> m:{w:{state:0 sema:0} writerSem:0 readerSem:0 readerCount:0 readerWait:0} queryConfig:{op:{query:<nil> collection: serverTags:[] selector:<nil> replyFunc:<nil> mode:0 skip:0 limit:0 options:{Query:<nil> OrderBy:<nil> Hint:<nil> Explain:false Snapshot:false ReadPreference:[] MaxScan:0 MaxTimeMS:0 Comment: Collation:<nil>} hasOptions:false flags:0 readConcern:} prefetch:0.25 limit:0} bypassValidation:false slaveOk:false dialInfo:0xc0019aeb40}
[] 22:50:43 mgo.go:36 NewMgoClient  [DEBU:00c] err: <nil>

Note that no error occoured. Howerver, there should be.

Note that this issue is OS-dependent. On Windows 10, the same code throws an error:

[] 23:02:12 mgo.go:17 NewMgoClient  [DEBU:008] url: ""
[] 23:02:12 mgo.go:25 NewMgoClient  [DEBU:009] info: &{Addrs:[] Timeout:0s Database: ReplicaSetName: Source: Service: ServiceHost: Mechanism: Username: Password: PoolLimit:0 PoolTimeout:0s ReadTimeout:0s WriteTimeout:0s AppName: ReadPreference:0xc0002e1d60 Safe:{W:0 WMode: RMode: WTimeout:0 FSync:false J:false} FailFast:false Direct:false MinPoolSize:0 MaxIdleTimeMS:0 DialServer:<nil> Dial:<nil>}
[] 23:02:12 mgo.go:26 NewMgoClient  [DEBU:00a] err: <nil>
[] 23:02:37 mgo.go:31 NewMgoClient  [ERRO:01b] mgo.DialWithInfo err(no reachable servers)
[] 23:02:37 db_mobile.go:17 NewDB  [CRIT:01c] dbutils.NewMgoClient() err(no reachable servers)

Possible solution:

You can check if string is empty in func extractURL: https://github.com/globalsign/mgo/blob/master/session.go#L790.

Can you reproduce the issue on the latest `development` branch?

Yes, it behaviours the same.

zheeeng commented 5 years ago

Have the same issue. And there is the SO question: https://stackoverflow.com/questions/52256008/golang-cannot-recover-from-panic-mgo-dialwithinfo/52257061

eminano commented 5 years ago

Hi @Ouroboros,

Sorry for the delay!

Passing an empty URL is not necessarily an error, in this case it means mongo uses the default URL to connect (mongodb://localhost:27017). From your tests, it seems the default URL behaviour is OS specific.

If you want to prevent empty URLs, you can just add a check on your end before passing it to mgo.

Making the driver flag it as an error would be a breaking change for people who are relying on that default URL to connect to mongo.

Hope this helps clarify,

Thanks, Esther

globalsign / mgo