Page MenuHomePhorge

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.
Tags
None
Referenced Files
Unknown Object (File)
Sun, Mar 24, 4:48 AM
Unknown Object (File)
Fri, Mar 15, 1:20 PM
Unknown Object (File)
Wed, Mar 13, 10:05 PM
Unknown Object (File)
Thu, Mar 7, 1:25 PM
Unknown Object (File)
Wed, Mar 6, 8:44 PM
Unknown Object (File)
Feb 27 2024, 9:46 PM
Unknown Object (File)
Feb 20 2024, 7:22 AM
Unknown Object (File)
Feb 15 2024, 9:39 AM
Subscribers
Restricted Project

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
Branch
master
Lint
No Lint Coverage
Unit
No Test Coverage
Build Status
Buildable 30949
Build 11530: arc lint + arc unit

Event Timeline

vanmeeuwen created this revision.
  • Probably also update existing nets
machniak subscribed.
machniak added inline comments.
src/app/Console/Commands/UpdateIP4NetsCommand.php
8

UpdateIp4NetsComand -> UpdateIp4Nets, here and in filename.

15

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
  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
23

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
src/app/Console/Commands/UpdateIP4NetsCommand.php
57

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
23

It's by design.

  • From update:ip4nets to data:ip4nets
  • Capture no networks case
  • Better error handling
  • Remove superfluous commentary
  • use sizeof on array
  • Log information about urls being retrieved
This revision is now accepted and ready to land.Sep 24 2020, 12:35 PM
  • Cleanup dependencies, fix phpstan errors
  • Add ipv6, optimize import
This revision was automatically updated to reflect the committed changes.