Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

"NA" ASNs cause panic #10

Closed
dacamp opened this issue Oct 8, 2018 · 2 comments
Closed

"NA" ASNs cause panic #10

dacamp opened this issue Oct 8, 2018 · 2 comments

Comments

@dacamp
Copy link
Contributor

dacamp commented Oct 8, 2018

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
Copy link
Contributor Author

dacamp commented Oct 8, 2018

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
Copy link
Owner

ammario commented Oct 8, 2018

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 added a commit that referenced this issue Oct 8, 2018
@ammario ammario closed this as completed in 65229eb Oct 8, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants