Page MenuHomekolab.org

Rely on our own IP4 network tables instead of any sort of GeoIP stuff
ClosedPublic

Authored by vanmeeuwen on Sep 23 2020, 5:02 PM.

Details

Summary

Replace dependency with implementation

Also sets the default user_setting for country to the derived country

Test Plan

None yet

Diff Detail

Repository
rK kolab
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

vanmeeuwen requested review of this revision.Sep 23 2020, 5:02 PM
vanmeeuwen created this revision.
vanmeeuwen updated this revision to Diff 4345.Sep 23 2020, 5:05 PM
  • Probably also update existing nets
vanmeeuwen updated this revision to Diff 4351.Sep 24 2020, 10:04 AM
  • Drop whois column
vanmeeuwen updated this revision to Diff 4357.Sep 24 2020, 10:07 AM
  • Also order by net_mask
machniak requested changes to this revision.Sep 24 2020, 10:12 AM
machniak added a subscriber: machniak.
machniak added inline comments.
src/app/Console/Commands/UpdateIP4NetsCommand.php
8 ↗(On Diff #4345)

UpdateIp4NetsComand -> UpdateIp4Nets, here and in filename.

15 ↗(On Diff #4345)

I'd like it to be "data:ip4nets", for consistency with "data:countries". Or make update:countries, if you really prefer "update:" prefix. Also, I'm not sure how useful this would be but we could have php artisan data:ip4nets {ip} to see/test the resolution.

57 ↗(On Diff #4345)
  1. A nicer error handling when the request fails. Right now the only error I see is "Unable to display the estimated time if the maximum number of steps is not set.". What's more, it creates an empty file, so you cannot update it later.
  2. In data:countries we don't use curl, file_get_contents/file_put_contents for consistency.

This should fix both:

if (!is_file($file)) {
    $content = file_get_contents($url); // This throws errors on failure
    if ($content) {
        file_put_contents($file, $content);
    }
}
src/app/Utils.php
53

What if there's no record found? What if country is unset? I don't see a fallback.

src/database/migrations/2020_06_04_140800_create_ip4nets_table.php
24

I think the net_ prefix is redundant for all of these columns. How about "network" instead of "net_number"? How about storing IPs as numbers? Our main select have to convert everything.

This revision now requires changes to proceed.Sep 24 2020, 10:12 AM
vanmeeuwen added inline comments.Sep 24 2020, 10:19 AM
src/app/Console/Commands/UpdateIP4NetsCommand.php
57 ↗(On Diff #4345)

I'm not sure how to capture curl errors to be honest.

I believe the curl version does not assign the contents of the file obtained to memory but instead passes through the downloaded information directly to the file descriptor.

The file size here is some 10 megabytes at times -- but could be up to 20 megabytes.

src/database/migrations/2020_06_04_140800_create_ip4nets_table.php
24

It's by design.

vanmeeuwen updated this revision to Diff 4363.Sep 24 2020, 10:29 AM
  • From update:ip4nets to data:ip4nets
  • Capture no networks case
  • Better error handling
vanmeeuwen updated this revision to Diff 4366.Sep 24 2020, 10:31 AM
  • Remove superfluous commentary
vanmeeuwen updated this revision to Diff 4372.Sep 24 2020, 10:37 AM
  • use sizeof on array
  • Log information about urls being retrieved
machniak accepted this revision.Sep 24 2020, 12:35 PM
This revision is now accepted and ready to land.Sep 24 2020, 12:35 PM
machniak updated this revision to Diff 4405.Sep 25 2020, 4:35 PM
  • Cleanup dependencies, fix phpstan errors
vanmeeuwen updated this revision to Diff 4810.Sun, Oct 18, 1:13 PM
  • Add ipv6, optimize import
This revision was automatically updated to reflect the committed changes.