"NA" ASNs cause panic - Githubissues

dacamp commented 6 years ago

There are times when Cymru incorrectly returns "NA" as the ASN and the ipisp package returns the error. I've only seen these types of errors when submitting a large number of IPs. I ran into the error below when querying some 10k ips.

For example: NA | 193.32.71.189 | NA | UA | ripencc | 2018-04-26 | NA

And the resulting error (backtrace provided by the errors package on the client side):

➜  example ./main
2018/10/08 11:26:14 There was an error: strconv.Atoi: parsing "NA": invalid syntax
failed to conv into to string
github.com/ammario/ipisp.ParseASN
    /Users/dcam/golang/src/github.com/ammario/ipisp/asn.go:23
github.com/ammario/ipisp.parseASNs
    /Users/dcam/golang/src/github.com/ammario/ipisp/client.go:32
github.com/ammario/ipisp.(*whoisClient).LookupIPs
    /Users/dcam/golang/src/github.com/ammario/ipisp/whois_client.go:133
main.main
    /Users/dcam/example/main.go:49
runtime.main
    /usr/local/Cellar/go/1.11.1/libexec/src/runtime/proc.go:201
runtime.goexit
    /usr/local/Cellar/go/1.11.1/libexec/src/runtime/asm_amd64.s:1333
failed to parse asn
github.com/ammario/ipisp.parseASNs
    /Users/dcam/golang/src/github.com/ammario/ipisp/client.go:34
github.com/ammario/ipisp.(*whoisClient).LookupIPs
    /Users/dcam/golang/src/github.com/ammario/ipisp/whois_client.go:133
main.main
    /Users/dcam/example/main.go:49
runtime.main
    /usr/local/Cellar/go/1.11.1/libexec/src/runtime/proc.go:201
runtime.goexit
    /usr/local/Cellar/go/1.11.1/libexec/src/runtime/asm_amd64.s:1333
failed to parse asn list NA
github.com/ammario/ipisp.(*whoisClient).LookupIPs
    /Users/dcam/golang/src/github.com/ammario/ipisp/whois_client.go:135
main.main
    /Users/dcam/example/main.go:49
runtime.main
    /usr/local/Cellar/go/1.11.1/libexec/src/runtime/proc.go:201
runtime.goexit
    /usr/local/Cellar/go/1.11.1/libexec/src/runtime/asm_amd64.s:1333

There are times when Cymru correct responds with the right data (that IP has ASN AS205127) and nothing panics. The ideas I've come up with to handle the case would be to A. Drop the incorrect request completely, returning no information about the IP to the user or returning a non-fatal error. B. Retry the IP a limited number of times. On success, return information to the user or on failure return a non-fatal error. C. Do nothing to the package and expect users to manage the error client side.

I'm willing to help with the patch, I'm just curious on your intended outcome first.

Let me know if I can clarify anything and thank you for the package, it's really useful!

dacamp commented 6 years ago

I should note this is using the whoisClient and the LookupIPs function. You probably gathered that from the stack trace, but just in case :)

ammario commented 6 years ago

This is very unfortunate behavior from Team Cymru... Regarding B. It's unclear how long the rate limit lasts, but more importantly, because the API doesn't accept a context, we should prefer to return as quickly as possible. Regarding A. I think this is the best solution w/o breaking the API

ammario / ipisp

"NA" ASNs cause panic #10