maxmind / GeoIP2-perl

Perl API for MaxMind's GeoIP2 web services and databases
https://metacpan.org/release/GeoIP2/
Other
18 stars 11 forks source link

$city->city() throws error when not populated #52

Closed erempel closed 6 years ago

erempel commented 6 years ago

When resolving a $city->city() on a GeoIP2city data file for an IP address that does not have a resolved city the error is:

HASH(0xe82558) is not a valid NameHashRef

Trace begun at /usr/local/share/perl5/GeoIP2/Types.pm line 249 GeoIP2::Types::_tc_fail('HASH(0xe82558)', 'NameHashRef') called at (eval 988) line 228 eval {...} at (eval 988) line 213 GeoIP2::Record::City::new(undef, 'locales', 'ARRAY(0x7a7d48)') called at /usr/local/share/perl5/GeoIP2/Role/Model/Location.pm line 107 GeoIP2::Role::Model::Location::_build_record('GeoIP2::Model::City=HASH(0x236b9d8)', 'city', '_raw_city') called at (eval 141) line 30

I am using the perl only module. Version are perl-MaxMind-DB-Reader-1.000013 from https://metacpan.org/pod/MaxMind::DB::Reader Redhat 6.9 perl 5.10.1

oschwald commented 6 years ago

I am unable to reproduce this, e.g.:

use v5.26;

use GeoIP2::Database::Reader;

my $reader = GeoIP2::Database::Reader->new( file => '/usr/local/share/GeoIP/GeoLite2-City.mmdb');

say $reader->city(ip => '4.4.4.4')->city->name // 'undef';

prints out undef as expected.

This is the record for 4.4.4.4:

  {
    "continent": 
      {
        "code": 
          "NA" <utf8_string>
        "geoname_id": 
          6255149 <uint32>
        "names": 
          {
            "de": 
              "Nordamerika" <utf8_string>
            "en": 
              "North America" <utf8_string>
            "es": 
              "Norteamérica" <utf8_string>
            "fr": 
              "Amérique du Nord" <utf8_string>
            "ja": 
              "北アメリカ" <utf8_string>
            "pt-BR": 
              "América do Norte" <utf8_string>
            "ru": 
              "Северная Америка" <utf8_string>
            "zh-CN": 
              "北美洲" <utf8_string>
          }
      }
    "country": 
      {
        "geoname_id": 
          6252001 <uint32>
        "iso_code": 
          "US" <utf8_string>
        "names": 
          {
            "de": 
              "USA" <utf8_string>
            "en": 
              "United States" <utf8_string>
            "es": 
              "Estados Unidos" <utf8_string>
            "fr": 
              "États-Unis" <utf8_string>
            "ja": 
              "アメリカ合衆国" <utf8_string>
            "pt-BR": 
              "Estados Unidos" <utf8_string>
            "ru": 
              "США" <utf8_string>
            "zh-CN": 
              "美国" <utf8_string>
          }
      }
    "location": 
      {
        "accuracy_radius": 
          1000 <uint16>
        "latitude": 
          37.751000 <double>
        "longitude": 
          -97.822000 <double>
      }
    "registered_country": 
      {
        "geoname_id": 
          6252001 <uint32>
        "iso_code": 
          "US" <utf8_string>
        "names": 
          {
            "de": 
              "USA" <utf8_string>
            "en": 
              "United States" <utf8_string>
            "es": 
              "Estados Unidos" <utf8_string>
            "fr": 
              "États-Unis" <utf8_string>
            "ja": 
              "アメリカ合衆国" <utf8_string>
            "pt-BR": 
              "Estados Unidos" <utf8_string>
            "ru": 
              "США" <utf8_string>
            "zh-CN": 
              "美国" <utf8_string>
          }
      }
  }

Could you provide a specific test case?

erempel commented 6 years ago

If I try the same thing it blows up.

$ perl use GeoIP2::Database::Reader; my $reader = GeoIP2::Database::Reader->new( file => '/home/xerr/login-tracker/GeoLite2-City.mmdb'); print $reader->city(ip => '4.4.4.4')->city->name // 'undef'; HASH(0x203ba68) is not a valid NameHashRef

Trace begun at /usr/local/share/perl5/GeoIP2/Types.pm line 249 GeoIP2::Types::_tc_fail('HASH(0x203ba68)', 'NameHashRef') called at (eval 982) line 228 eval {...} at (eval 982) line 213 GeoIP2::Record::City::new(undef, 'locales', 'ARRAY(0x32b93d0)') called at /usr/local/share/perl5/GeoIP2/Role/Model/Location.pm line 107 GeoIP2::Role::Model::Location::_build_record('GeoIP2::Model::City=HASH(0x2109460)', 'city', '_raw_city') called at (eval 141) line 30 GeoIP2::Model::City::city('GeoIP2::Model::City=HASH(0x2109460)') called at - line 3

here is the record

use GeoIP2::Database::Reader; my $reader = GeoIP2::Database::Reader->new( file => '/home/xerr/login-tracker/GeoLite2-City.mmdb'); $city = $reader->city(ip => '4.4.4.4'); use Data::Dumper; print Dumper($city);

$VAR1 = bless( { '_raw_postal' => {}, '_raw_location' => { 'longitude' => '-97.822', 'latitude' => '37.751', 'accuracy_radius' => 1000 }, '_raw_country' => { 'iso_code' => 'US', 'names' => { 'pt-BR' => 'Estados Unidos', 'es' => 'Estados Unidos', 'ru' => "\x{421}\x{428}\x{410}", 'en' => 'United States', 'zh-CN' => "\x{7f8e}\x{56fd}", 'fr' => "\x{c9}tats-Unis", 'de' => 'USA', 'ja' => "\x{30a2}\x{30e1}\x{30ea}\x{30ab}\x{5408}\x{8846}\x{56fd}" }, 'geoname_id' => 6252001 }, '_raw_registered_country' => { 'iso_code' => 'US', 'names' => { 'pt-BR' => 'Estados Unidos', 'es' => 'Estados Unidos', 'ru' => "\x{421}\x{428}\x{410}", 'en' => 'United States', 'zh-CN' => "\x{7f8e}\x{56fd}", 'fr' => "\x{c9}tats-Unis", 'de' => 'USA', 'ja' => "\x{30a2}\x{30e1}\x{30ea}\x{30ab}\x{5408}\x{8846}\x{56fd}" }, 'geoname_id' => 6252001 }, 'locales' => [ 'en' ], '_raw_maxmind' => {}, '_raw_continent' => { 'names' => { 'pt-BR' => "Am\x{e9}rica do Norte", 'es' => "Norteam\x{e9}rica", 'ru' => "\x{421}\x{435}\x{432}\x{435}\x{440}\x{43d}\x{430}\x{44f} \x{410}\x{43c}\x{435}\x{440}\x{438}\x{43a}\x{430}", 'en' => 'North America', 'zh-CN' => "\x{5317}\x{7f8e}\x{6d32}", 'fr' => "Am\x{e9}rique du Nord", 'de' => 'Nordamerika', 'ja' => "\x{5317}\x{30a2}\x{30e1}\x{30ea}\x{30ab}" }, 'geoname_id' => 6255149, 'code' => 'NA' }, '_raw_city' => {}, '_raw_represented_country' => {}, '_raw_traits' => { 'ip_address' => '4.4.4.4' }, 'raw' => { 'country' => $VAR1->{'_raw_country'}, 'location' => $VAR1->{'_raw_location'}, 'continent' => $VAR1->{'_raw_continent'}, 'locales' => $VAR1->{'locales'}, 'traits' => $VAR1->{'_raw_traits'}, 'registered_country' => $VAR1->{'_raw_registered_country'} } }, 'GeoIP2::Model::City' );

horgh commented 6 years ago

The trace seems to say that we're getting to this point and then saying the hash is not valid:

sub NameHashRef () {
    return quote_sub(
        q{ GeoIP2::Types::_tc_fail( $_[0], 'NameHashRef' )
               unless ref $_[0]
               && Scalar::Util::reftype( $_[0] ) eq 'HASH'
               && ! Scalar::Util::blessed( $_[0] )
               && &List::MoreUtils::all( sub { defined $_ && ! ref $_ }, values %{ $_[0] } ); }
    );
}

I'm not sure how that's possible.

Given your other open issue, I wonder if there could be a dependency problem. I'm wondering if a behaviour change in List::MoreUtils::all() (in an old version of that distribution) could be causing it.

Could you please tell me your version of these two distributions?

It might also be good to double check that you have all the dependencies installed, and all their dependencies. The ones for this distribution are listed in the cpanfile in the root directory. I'm not sure an easy way to do this on your system. Typically for testing I install with cpanm. You could try that, if you have a system for testing on.

erempel commented 6 years ago

Scalar::Util - 1.49 List::MoreUtils - 0.22

erempel commented 6 years ago

The test I did above was done on the same host that I built all of the modules on (we build RPMs for everything). All of the tests succeeded for all of the modules EXCEPT GeoIP2 ! === Configure Requires ===

 Module              Want Have
 ------------------- ---- ----
 ExtUtils::MakeMaker  any 7.30

=== Build Requires ===

 Module              Want Have
 ------------------- ---- ----
 ExtUtils::MakeMaker  any 7.30

=== Test Requires ===

 Module                Want     Have
 --------------------- ---- --------
 ExtUtils::MakeMaker    any     7.30
 File::Spec             any     3.30
 HTTP::Response         any    5.824
 HTTP::Status           any    5.817
 IO::Compress::Gzip     any    2.021
 MaxMind::DB::Metadata  any 0.040001
 Path::Class            any     0.37
 Test::Builder          any 1.302086
 Test::Fatal            any    0.008
 Test::More            0.96 1.302086
 Test::Number::Delta    any     1.06
 base                   any     2.14
 utf8                   any     1.07

=== Test Recommends ===

 Module         Want     Have
 ---------- -------- --------
 CPAN::Meta 2.120900 2.150010

=== Runtime Requires ===

 Module                   Want     Have
 -------------------- -------- --------
 B                         any     1.22
 Data::Dumper              any    2.124
 Data::Validate::IP       0.24     0.27
 Exporter                  any     5.63
 Getopt::Long              any     2.38
 HTTP::Headers             any    5.827
 HTTP::Request             any    5.827
 JSON::MaybeXS             any 1.003009
 LWP::Protocol::https      any    undef
 LWP::UserAgent            any    5.833
 List::MoreUtils           any     0.22
 List::Util                any     1.49
 MIME::Base64              any     3.08
 MaxMind::DB::Reader  1.000000 1.000013
 Moo                       any 2.003002
 Moo::Role                 any 2.003002
 Params::Validate          any     0.92
 Scalar::Util              any     1.49
 Sub::Quote                any 2.004000
 Throwable::Error          any 0.200013
 Try::Tiny                 any     0.11
 URI                       any     1.40
 lib                       any     0.62
 namespace::clean          any     0.27
 strict                    any     1.04
 warnings                  any     1.06

So all of those versions seem to be appropriate. Then the tests... t/00-report-prereqs.t ....................... ok
t/GeoIP2/Database/Reader-Anonymous-IP.t ..... ok
t/GeoIP2/Database/Reader-ASN.t .............. ok
t/GeoIP2/Database/Reader-Connection-Type.t .. ok
t/GeoIP2/Database/Reader-Domain.t ........... ok
t/GeoIP2/Database/Reader-Enterprise.t ....... ok
t/GeoIP2/Database/Reader-ISP.t .............. ok
t/GeoIP2/Database/Reader.t .................. 1/?

Failed test 'calling $model_obj->represented_country() does not throw an error - city model'

#   at t/GeoIP2/Database/Reader.t line 100.
#          got: 'HASH(0x2a14460) is not a valid NameHashRef
# 
# Trace begun at /usr/local/uvredhat/BUILD/GeoIP2-2.003005/blib/lib/GeoIP2/Types.pm line 249
# GeoIP2::Types::_tc_fail('HASH(0x2a14460)', 'NameHashRef') called at (eval 1020) line 260
# eval {...} at (eval 1020) line 245
# GeoIP2::Record::RepresentedCountry::new(undef, 'locales', 'ARRAY(0x2424850)') called at /usr/local/uvredhat/BUILD/GeoIP2-2.003005/blib/lib/GeoIP2/Role/Model/Location.pm line 107
# GeoIP2::Role::Model::Location::_build_record('GeoIP2::Model::City=HASH(0x2a7c170)', 'represented_country', '_raw_represented_country') called at (eval 156) line 30
# GeoIP2::Model::City::represented_country('GeoIP2::Model::City=HASH(0x2a7c170)') called at t/GeoIP2/Database/Reader.t line 98
# main::__ANON__ at /usr/local/share/perl5/Test/Fatal.pm line 23
# Test::Fatal::__ANON__ at /usr/local/share/perl5/Try/Tiny.pm line 71
# eval {...} at /usr/local/share/perl5/Try/Tiny.pm line 67
# Try::Tiny::try('CODE(0x408f660)', 'Try::Tiny::Catch=REF(0x408f7e0)') called at /usr/local/share/perl5/Test/Fatal.pm line 30
# Test::Fatal::exception('CODE(0x408f498)') called at t/GeoIP2/Database/Reader.t line 100
# main::__ANON__ at /usr/share/perl5/Test/Builder.pm line 310
# eval {...} at /usr/share/perl5/Test/Builder.pm line 310
# Test::Builder::subtest('Test::Builder=HASH(0x23c49a8)', 'GeoIP2-City', 'CODE(0x200cf70)') called at /usr/share/perl5/Test/More.pm line 807
# Test::More::subtest('GeoIP2-City', 'CODE(0x200cf70)') called at t/GeoIP2/Database/Reader.t line 114
# '
#     expected: undef
# Looks like you failed 1 test of 20.
horgh commented 6 years ago

Thanks for the info!

I'm suspicious of the List::MoreUtils version being at fault. Specifically, see this change in 0.27_04.

Could you try this program and see what it shows?

use strict;
use warnings;

use List::MoreUtils qw( all );

my @a;
my $x = all( sub { $_ }, @a );
print $x || 'falsey', "\n";

If the output is falsey, I think this is the problem. In that case, would it be possible for you to update List::MoreUtils?

We should probably update our dependency to include a version for this distribution if this is the problem.

erempel commented 6 years ago

The output is falsey so it looks like you may be right.

I will add a >= 0.27_04 version requirement to our RPM and get this in place. I'll update here when I have done.

erempel commented 6 years ago

Upgrading to the latest List::MoreUtils (0.426) the make test succeeds and the previous example now prints "undef" like you say it should. Now I just have to decide host to safely use this incompatible List::MoreUtils

horgh commented 6 years ago

Great to hear! Thanks for letting me know. I'll keep this issue open until we update our dependencies to reflect the need for a particular version of this.