consumer: fix panic in nextLookupdEndpoint #321
Conversation
|
Note that I have not tested the code yet as I don't have a nsq instance on the machine I'm currently on. |
|
I'm confused, it looks like |
|
If you use |
|
I've updated the commit message so that it is clearer. |
There was a problem hiding this comment.
Thanks @martin-sucha, I think we can simplify this a bit so the diff isn't as noisy.
consumer.go
Outdated
| @@ -126,7 +126,7 @@ type Consumer struct { | |||
|
|
|||
| // used at connection close to force a possible reconnect | |||
| lookupdRecheckChan chan int | |||
| lookupdHTTPAddrs []string | |||
| lookupdHTTPAddrs []lookupdHTTPAddr | |||
There was a problem hiding this comment.
I don't see any benefit of introducing a type here, let's keep this a string and then a few of the other changes here won't be necessary.
There was a problem hiding this comment.
Ok, I removed the type.
If you use " myhostname:4161" as NSQd HTTP address
then the code panics in nextLookupdEndpoint:
panic: parse "http:// myhostname:4161": invalid character " " in host name
This is caused different validation of the input when configured
in ConnectToNSQLookupd and when used in nextLookupdEndpoint.
Moving preparation of the URL to the beginning so that we don't
need to do any input validation in nextLookupdEndpoint.
Fixes nsqio#316
The code can panic if invalid lookupd address is provided:
This is caused different validation of the input when configured
in ConnectToNSQLookupd and when used in nextLookupdEndpoint.
Moving preparation of the URL to the beginning so that we don't
need to do any input validation in nextLookupEndpoint.
Fixes #316