search

nsqio / go-nsq

The official Go package for NSQ
MIT License
2.59k stars 444 forks source link

panic in Consumer.nextLookupdEndpoint #316

Closed martin-sucha closed 3 years ago

martin-sucha commented 3 years ago

Hi,

we have seen a panic like this when an invalid address is provided to ConnectToNSQLookupd with go-nsq@v1.0.8:

panic: parse "http:// myhostname:4161": invalid character " " in host name
goroutine 1870 [running]:
github.com/nsqio/go-nsq.(*Consumer).nextLookupdEndpoint(0xc00034ab00, 0x1, 0x139bd85)
/go/pkg/mod/github.com/nsqio/go-nsq@v1.0.8/consumer.go:432 +0x477
github.com/nsqio/go-nsq.(*Consumer).queryLookupd(0xc00034ab00)
/go/pkg/mod/github.com/nsqio/go-nsq@v1.0.8/consumer.go:467 +0x96
github.com/nsqio/go-nsq.(*Consumer).lookupdLoop(0xc00034ab00)
/go/pkg/mod/github.com/nsqio/go-nsq@v1.0.8/consumer.go:397 +0x34e
created by github.com/nsqio/go-nsq.(*Consumer).ConnectToNSQLookupd
/go/pkg/mod/github.com/nsqio/go-nsq@v1.0.8/consumer.go:340 +0x1c7

I would expect ConnectToNSQLookupd to return error in this case instead of the code panicking later.

There is some validation in validatedLookupAddr but the code is different than the one in nextLookupdEndpoint.

I guess the best fix would be to parse the address just once in ConnectToNSQLookupd and store the parsed URL into r.lookupdHTTPAddrs

mreiferson commented 3 years ago

Agreed, we should error earlier than the connect loop / goroutine. Would you be interested in submitting a fix for this?