This is now fixed.
Tested on php53 and php56 with and without the GeoIP PECL extension enabled.
Manually tested with ISP and ORG GeoIP databases.
Hi @joostdekeijzer Thank you for following up and testing on 5.3 and 5.6 with and without PECL. I will merge the PR shortly and hopefully it will work now. We have a couple weeks left until the stable release so hopefully enough time to catch any issue :+1:
Great! Let me know if there are any issues so I can try to fix those.
Any documentation on how to use ipv6 maxmind DB?
I assume you want to use the "new" maxmind databases (.mmdb files) ?
The current implementation uses the legacy Maxmind code and only supports the legacy database (.dat) files.
The currently used datafile (GeoLiteCity.dat) includes IPv6 information.
I tried this functionality today, but as far as I can tell,
GeoLiteCity.dat does not include IPv6 information. For that, one needs
I did a simple test using the script at: https://gist.github.com/olavmrk/1cc21f890ffc97998b84
That script gives the following output:
GeoLiteCity(22.214.171.124): 'Norway' GeoLiteCity(0:0:0:0:0:ffff:9e26:3e00): NULL GeoLiteCity(2001:700:1::): 'United States' GeoLiteCityv6(126.96.36.199): NULL GeoLiteCityv6(0:0:0:0:0:ffff:9e26:3e00): 'Norway' GeoLiteCityv6(2001:700:1::): 'Norway'
The correct country is Norway, so we can see that using
GeoLiteCity.dat gives the incorrect country. Meanwhile,
GeoLiteCityv6.dat is unable to look up IPv4-addresses directly.
What appears to work is looking up addresses in GeoLiteCityv6 using IPv6 addresses and IPv6-mapped IPv4 addresses.
When I test your IP addresses using the Piwik API,
0:0:0:0:0:ffff:9e26:3e00 does return Norway for me (using http://geolite.maxmind.com/download/geoip/database/GeoLiteCity.dat.gz ; updated 2015-03-04)
But that's not to say that the database is good. I find many IPv6 addresses to be reported in China when I'm quite sure they shouldn't.
Eg. a reported IPv6 for google.com using
dig <a class='mention' href='https://github.com/8'>@8</a>.8.8.8 aaaa google.com is
2a00:1450:4013:c00::8a which is reported to be in China.
I believe Piwik transforms a IPv6-mapped IPv4 address into a IPv4 address before passing it to the GeoIP module. I only included the lines with the IPv6-mapped IPv4 address since it indicates a possible workaround.
My problem isn't IPv4 addresses. My problem is that the
GeoLiteCity.dat does not work for IPv6 addresses, while the
GeoLiteCityv6.dat file does not (directly) work for IPv4 addresses.
GeoLiteCity.dat returns a result for IPv6-addresses, but the result appears to be incorrect. Meanwhile the result from
GeoLiteCityv6.dat is correct, so the problem isn't the MaxMind database. It is the use of
For example, looking up
GeoLiteCityv6.dat yields Ireland instead of China.
Another IPv6-address belonging in Norway is
GeoLiteCityv6.dat get correct while
GeoLiteCity.dat places it in Hong Kong.
I had an idea about the cause of the strange behavior of IPv6-addresses: It appears that when you try to look up an IPv6 address in
GeoLiteCity.dat, it simply casts it to a byte array and looks up the first four bytes as an IPv4 address.
That's good research!
I think I've found the base of the issue in the code.
Based on your findings, I guess there will also be the need to periodicly update the
I'm seeing three options for fixing this:
plugins/UserCountry/LocationProvider/GeoIp/Php.phpto load and maintain both
GeoLiteCityv6.dat, using one for IPv4 addresses and the other for IPv6 addresses.
plugins/UserCountry/LocationProvider/GeoIp/Php.phpto load and maintain
GeoLiteCityv6.dat, and transform IPv4 addresses into IPv6-mapped IPv4 addresses before passing it to the lookup function. Some simple tests indicate that this should work.
Piwik\Plugins\UserCountry\LocationProvider\GeoIp2location provider using the new
I guess option 2 is the one requiring the least amount of code changes, but some tricks would probably have to be employed in order to be backwards-compatible with existing installations. I thing it should be possible to look at the database type and switch the method for looking up IPv4 addresses depending on whether it is the
Option 3 looks like a better long term solution, but I don't know how tied Piwik is to the existing GeoIp location provider.
Loading and maintaining both GeoLiteCity and GeoLiteCityv6 is quite possible, see the modifications here: https://github.com/piwik/piwik/compare/master...joostdekeijzer:geoip-legacy-IPv6-datafiles?expand=1
I'm afraid the code did not get any prettier... Moving to GeoIP2 would be nice but is a lot of work, also because the output is different from GeoIP1(different spelling of names, etc.).
I think it needs some review and I haven't run any tests yet.
@mattab should this be in a separate ticket or shall I just create a PR with above changes and continue the discussion there?
To me that looks like a lot of changes -- I think option 2 would have been prettier :)
Looking at the code -- isn't the same changes required for the other database types (i.e. isp and org)? Or do they have a different structure combining IPv4 and IPv6 addresses?
Another thing I see is that you dropped in
2a02:fe0:c610:9f50:: in some test code. Since that address is an IPv6 address most likely in use by a single subscriber on a residential ISP, I don't think that address should be in that test.
I did not try option 2 because I was afraid of side-effects with backward-compatibility and the existing visting archives.
Also, I don't see any IPv6 reference for the ISP and ORG databases in the GeoIP code.
I just tried your IPv6 address (the one in the testcase https://github.com/piwik/piwik/blob/master/tests/PHPUnit/Fixtures/ManyVisitsWithGeoIP.php#L37 doesn't actually have any GeoIP info). Maybe use the Google IPv6 address instead?
Just tested: the ISP and ORG databases handle IPv6 addresses the same as GeoLiteCity.dat.
Entering the Google IPv6 addres will return:
So this will need to be handled...
Yes, backwards compatibility can be a challenge. I think it would be best solved by changing the defaults so that new installations would end up using
GeoLiteCityv6.dat, while not changing anything for existing installations.
Regarding the IPv6 address that should be tested -- I just don't think the address of a random subscriber on a random ISP is the best approach.
2001:700:1:: would be slightly better -- that is the main employee network at my workplace, but I still cannot promise that it is persistent in any way.
I'd like some lead developer feedback on this.
About your backward compatibility solution: that should indeed work but my feeling is it should be done when migrating to GeoIP2 (I've done some tests with GeoIP2 and the output is really different)
Also agree on your IPv6 test address comment. Maybe Piwik has an address themselves?
GeoIP2 ticket: https://github.com/piwik/piwik/issues/4487
I'd like some lead developer feedback on this.
for sure I would like to help here, but this issue is closed. In general it is helpful for us if we discuss in issues that are still opened, so that we will not forget about them. Would you mind creating an issue to describe the specific problem as above? if there is more than one problem, it is best to create more than one issue :+1: hope it works...