qbittorrent / qBittorrent

qBittorrent BitTorrent client
https://www.qbittorrent.org
Other
28.22k stars 3.97k forks source link

[Wishlist] Update GeoIP #2837

Closed ngosang closed 9 years ago

ngosang commented 9 years ago

The src/gui/geoip/GeoIP.dat database included in the master branch has a size of 1.1 MiB and the last version is 776 KiB. The size has changed a lot so I think it's outdated.

In the same download page there is other db for IPv6. I don't know how is the IPv6 support in qBittorrent but we are not resolving these IPs. Related #1833.

Looking at old issues I found this #1281. The user asked to relocate the db to make it easier to upgrade by hand.

Download page: http://dev.maxmind.com/geoip/legacy/geolite/

Chocobo1 commented 9 years ago

Just a quick thought, would it be better if we let qBt auto-update it every month or whatever period? (database is stored in its own file of course)

Also, ship a default not-too-old database with the installer in case the update won't work.

sledgehammer999 commented 9 years ago

Apparently @glassez inadvertently included his local copy with commit ff9a281b72ac46f5d2d90e84c380488f6342daf8. In the previous commit that file didn't exist. I am glad you noticed this.

Up until now, each time I do an official windows release I use the latest version of that file from maxmind.com. In fact, the build system errors out on Windows if that file isn't present reminding you to download it. Hence the readme file.

As for the IPv6 (and the v2 version) I can't something. We load the db into libtorrent and it provides a country code for each ip. The docs about libtorrent::session::load_country_db() don't mention something about the extra dbs. It seems that you can load only one db into libtorrent.

sledgehammer999 commented 9 years ago

Marking for v3.2.0 to remember to delete GeoIP.dat

sledgehammer999 commented 9 years ago

For anyone reading: I asked in the libtorrent mailing list. The api that handles geoip (aka session::load_country_db()) is considered deprecated now. The author says that this should be handled in the client programs instead. It is a simple function of querying the db for the IP. He also indicated that libtorrent handles one db at a time. So ipv4 and ipv6 cannot be loaded simultaneously.

glassez commented 9 years ago

The author says that this should be handled in the client programs instead.

@sledgehammer999 I can implement this. I only have a few questions. What version of MaxMind API should we use? We need to include GeoIP code in qBittorrent or to link with the library?

sledgehammer999 commented 9 years ago

@glassez Definitely GeoIP2. Here and here only the file format is documented. I cannot find specs on what is the minimum info/structures that a DB file contains. Anyway here is a ready C lib that handles the format. IMO this code should be included as 3rd party code into our repo, like we do with eg qtsingleapplication, qjson etc.

On the update cycle I have the following suggestions:

  1. We can implement our own update cycle, which will be independent of the OS db updates (linux distros update the v1 db centrally). It will also not be static, ie the db will not be frozen to the one the packager included.
  2. Use the API to find our local copy's build date (build_epoch). Check the system's date. Calculate if a newer db is likely to be available my Maxmind. If yes, download and decompress it.
  3. The calculation should take into account the fact that the free db is updated only once per month. Also it seems to get updated the first Tuesday of each month. I saw the info about the Tuesday in StackOverlow answer. I don't a link to it now.
  4. If the update mechanism is implemented, we could build qbt without needing to bundle a db with the binary. Or we could still include it to have faster bootstrap times...
sledgehammer999 commented 9 years ago

But more licensing woes. We cannot use that lib. From here:

Despite our best efforts, the FSF has never considered the Apache License to be compatible with GPL version 2, citing the patent termination and indemnification provisions as restrictions not present in the older GPL license. The Apache Software Foundation believes that you should always try to obey the constraints expressed by the copyright holder when redistributing their work.

sledgehammer999 commented 9 years ago

Also GNU has the same reaction:

Please note that this license is not compatible with GPL version 2, because it has some requirements that are not in that GPL version. These include certain patent termination and indemnification provisions. The patent termination provision is a good thing, which is why we recommend the Apache 2.0 license for substantial programs over other lax permissive licenses.

glassez commented 9 years ago

But more licensing woes. We cannot use that lib.

This means that we must design our code to work with this database?

sledgehammer999 commented 9 years ago

If we want to use v2 we must write our db parser based on the specs I linked. Unless someone else has done it before us. Or if the core issue is to have qbt update itself the db, maybe we could do the same with the v1 databases, which uses different format and parser code IIRC.

glassez commented 9 years ago

Or if the core issue is to have qbt update itself the db, maybe we could do the same with the v1 databases

I think that if it is declared as deprecated in libtorrent, we have to get our own equivalent. But if we still have to implement it, then we need to use the new database format.

sledgehammer999 commented 9 years ago

Leaving this here for anyone interested: Boost.Multiprecision seems to have an 128bit integer type: http://www.boost.org/doc/libs/1_58_0/libs/multiprecision/doc/html/boost_multiprecision/tut/ints/cpp_int.html IMO, if anyone tries to implement a C++ parser for the db, he should use Boost as much as possible. Unless the particular boost lib isn't header only... The above isn't mandatory. If you feel more comfortable with Qt or any other framework, just go ahead.

glassez commented 9 years ago

@sledgehammer999 I've implemented new GeoIP Manager on top of my #2433 PR. So, as soon as you finish the current review, I will update it to include this. Or, in another case, I can publish a new PR after #2433 is merged.

sledgehammer999 commented 9 years ago

I prefer after, so I can tell the other guys to rebase their multitude of the PRs waiting. I am curious, did you implement a parser on the new spec? If yes, you should consider publishing it as a separate project/lib too. And also decide on a different license than qbt (eg BSD) if you want. Just saying...

glassez commented 9 years ago

I am curious, did you implement a parser on the new spec?

Actually, Yes and no. I have created a parser with constraints. Format MaxMindDB quite extensive (apparently it's made with stock). But this is not the database itself, just a container for some of the database associated with the IP addresses. But we don't need it all. So I implemented only what we need for support GeoLite2 Country database.

ngosang commented 9 years ago

Will be fixed in #3186.

sledgehammer999 commented 9 years ago

Implemented in #3186.