Exception handler cannot make up its mind.
Ironholds opened this issue · 13 comments
Using the netspeeds database (specifically "Binary format for APIs with Cellular") we get:
test.netspeed_by_addr("2600:1006:b025:fad7:b945:d20a:9451:85d")Traceback (most recent call last):
File "", line 1, in
File "/home/ironholds/.local/lib/python2.7/site-packages/pygeoip-0.3.1-py2.7.egg/pygeoip/init.py", line 456, in netspeed_by_addr
return const.NETSPEED_NAMES[self.id_by_addr(addr)]
File "/home/ironholds/.local/lib/python2.7/site-packages/pygeoip-0.3.1-py2.7.egg/pygeoip/init.py", line 415, in id_by_addr
raise GeoIPError('Invalid database type; expected IPv4 address')
pygeoip.GeoIPError: Invalid database type; expected IPv4 address
test.netspeed_by_addr("203.0.113.30")
Traceback (most recent call last):
File "", line 1, in
File "/home/ironholds/.local/lib/python2.7/site-packages/pygeoip-0.3.1-py2.7.egg/pygeoip/init.py", line 456, in netspeed_by_addr
return const.NETSPEED_NAMES[self.id_by_addr(addr)]
File "/home/ironholds/.local/lib/python2.7/site-packages/pygeoip-0.3.1-py2.7.egg/pygeoip/init.py", line 413, in id_by_addr
raise GeoIPError('Invalid database type; expected IPv6 address')
pygeoip.GeoIPError: Invalid database type; expected IPv6 address
...so it wants IPv4 except when you provide one. Is this expected behaviour/an unsupporteddataset,or..?
Oh, I've never seen a IPv6 netspeed database, pygeoip are missing support for that. You can see supported types here: http://pygeoip.readthedocs.org/en/v0.3.1/supported-databases.html
This is probably a super easy fix, but since I've never had one in my hand I can't test this.
If you have time you could try tweak some here:
https://github.com/appliedsec/pygeoip/blob/master/pygeoip/__init__.py#L414
I guess your database might be detected as:
https://github.com/appliedsec/pygeoip/blob/master/pygeoip/const.py#L414
As far as I know, we don't have an IPv6 GeoIP Legacy Netspeed database. @Ironholds, what is the product number on your database?
It's 177. To be clear, I know there's no IPv6; the problem is that inputting an IPv4 address produces the error "I'm expecting an IPv6!" while inputting an IPv6 produces "I'm expecting an IPv4!"
From @tiwilliam's comments it looks like the problem is that the exception handlers don't elegantly deal with "a database that is not recognised by the ip=4 check or the ip=6 check". I can try and think of some ways to handle exceptions around database types in a more granular fashion if patches are welcome?
Okay, looks like the database type is just not supported at all, but the error handling doesn't check for the availability of /any/ support before checking for specific availability. I'll submit a patch.
That database type should work, possibly with a minor code change. The basic format is the same as the ISP/Org databases. It is a bit confusing as there is an older Netspeed type that uses a different format with fixed values in the API. The GeoIPNetspeedCell test database at https://github.com/maxmind/geoip-api-python/tree/master/tests/data should be in exactly the same format as this database.
Yeah, I'm going to do some tests at my end to check that 32 actually works. If it does, I'll throw in a patch allowing for it.
Some changes are necessary (how minor I can't really tell. I write R, not Python!) - I've tested "commenting out the exception handlers, loading the db and watching what happens" and the result is "it tries to scan through and then concludes the db is corrupt".
What happens if you use the org_by_addr method after commenting out the database check?
"raise GeoIPError(message) pygeoip.GeoIPError: Invalid database type, expected Org, ISP or ASNum"
Exception should be better and NetSpeedCell is now supported in master.