g-andrade / locus

MMDB reader for geolocation and ASN lookup of IP addresses
https://hexdocs.pm/locus/
MIT License
109 stars 14 forks source link

java and python implementations from maxmind are very different #39

Closed nickjacob closed 5 months ago

nickjacob commented 5 months ago

yes.. to be fair the java & python implementation of this are more legit:

https://github.com/maxmind/MaxMind-DB-Reader-python/blob/main/maxminddb/reader.py#L128

(java the same).

I don't feel like it's fair to actually upstream this.. unless I implemented it that way & added tests. See https://maxmind.github.io/MaxMind-DB/#ipv4-addresses-in-an-ipv6-tree this spec sounds more like what locus did.. but what maxmind does is what the writer does.. so...

tested this on a few DBs that our code created & it's the same each time. Maybe I need to implement the offset thing.

With this patch/version.. we can produce a hybrid ipv4 & ipv6 DB from one input file from IP2location

g-andrade commented 5 months ago

Hi,

I haven’t looked at the IPv4 root node computation in such a long time that it’s almost like someone else’s code to me at this point!

From what I can gather out of your PR, the spec says one thing but implementations out there (official ones) sometimes (always?) do another. Are the two compatible by chance? I haven’t encountered any issues with this but I’ve only used MaxMind databases so far.

Thanks for the heads up, ping me again if a fix is needed.

nickjacob commented 5 months ago

Hi Guilherme thank you for looking at this! My PR needs a ton of work—that’s why I closed it. I’m going to test more with some of the databases I have and I’ll re-open if I can submit something that more closely matches the maxmind implementations

Get Outlook for iOShttps://aka.ms/o0ukef


From: Guilherme Andrade @.> Sent: Monday, February 5, 2024 9:58:11 PM To: g-andrade/locus @.> Cc: Nick Jacob @.>; State change @.> Subject: Re: [g-andrade/locus] java and python implementations from maxmind are very different (PR #39)

Hi,

I haven’t looked at the IPv4 root node computation in such a long time that it’s almost like someone else’s code to me at this point!

From what I can gather out of your PR, the spec says one thing but implementations out there (official ones) sometimes (always?) do another. Are the two compatible by chance? I haven’t encountered any issues with this but I’ve only used MaxMind databases so far.

Thanks for the heads up, ping me again if a fix is needed.

— Reply to this email directly, view it on GitHubhttps://github.com/g-andrade/locus/pull/39#issuecomment-1928693283, or unsubscribehttps://github.com/notifications/unsubscribe-auth/AAJDCSYYYLWAXW2BGSBUQFDYSGL4HAVCNFSM6AAAAABC3FGT7GVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTSMRYGY4TGMRYGM. You are receiving this because you modified the open/close state.Message ID: @.***>

g-andrade commented 2 months ago

There was indeed an issue with the IPv4 root node, #44, which I ran into when testing IPinfo databases.

Fixed in 75e38f3, release 2.3.8 (pushed to Hex).