From 462e974d1e3511c3f8b975ef9139d5e7163769bf Mon Sep 17 00:00:00 2001 From: Jared Johnson Date: Fri, 7 Nov 2014 15:58:12 -0600 Subject: [PATCH 01/10] leave register() if neither geoip module loads --- plugins/ident/geoip | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/plugins/ident/geoip b/plugins/ident/geoip index df66dd1..04ba319 100644 --- a/plugins/ident/geoip +++ b/plugins/ident/geoip @@ -129,8 +129,12 @@ sub register { $self->{_args} = {@args}; $self->{_args}{db_dir} ||= '/usr/local/share/GeoIP'; - $self->load_geoip2() and return; - $self->load_geoip1(); + $self->load_geoip() or return; +} + +sub load_geoip { + $self->load_geoip2() and return 1; + $self->load_geoip1() and return 1; } sub load_geoip1 { @@ -152,6 +156,7 @@ sub load_geoip1 { $self->init_my_country_code(); $self->register_hook('connect', 'geoip_lookup'); + return 1; } sub load_geoip2 { From da3ed5ebbf713ab16817680b638cc2422fb31c36 Mon Sep 17 00:00:00 2001 From: Jared Johnson Date: Fri, 7 Nov 2014 16:22:49 -0600 Subject: [PATCH 02/10] Fix missing $self --- plugins/ident/geoip | 1 + 1 file changed, 1 insertion(+) diff --git a/plugins/ident/geoip b/plugins/ident/geoip index 04ba319..58f5d56 100644 --- a/plugins/ident/geoip +++ b/plugins/ident/geoip @@ -133,6 +133,7 @@ sub register { } sub load_geoip { + my ( $self ) = @_; $self->load_geoip2() and return 1; $self->load_geoip1() and return 1; } From d22396c29856d81e877e17ff4785bcc7474449fb Mon Sep 17 00:00:00 2001 From: Jared Johnson Date: Fri, 7 Nov 2014 16:30:54 -0600 Subject: [PATCH 03/10] Add headers with GeoIP data --- plugins/ident/geoip | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/plugins/ident/geoip b/plugins/ident/geoip index 58f5d56..5765b5a 100644 --- a/plugins/ident/geoip +++ b/plugins/ident/geoip @@ -130,6 +130,7 @@ sub register { $self->{_args}{db_dir} ||= '/usr/local/share/GeoIP'; $self->load_geoip() or return; + $self->register_hook( data_post => 'add_headers' ); } sub load_geoip { @@ -197,6 +198,17 @@ sub load_geoip2 { return; } +sub add_headers { + my ( $self, $txn ) = @_; + for my $h (qw( Country Country-Name Continent City ASN )) { + my $note = lc "geoip_$h"; + $h =~ s/-/_/; + next if ! $self->connection->notes($note); + $txn->header->delete("X-GeoIP-$h"); + $txn->header->add( "X-GeoIP-$h", $self->connection->notes($note), 0 ); + } +} + sub geoip2_lookup { my $self = shift; From cf0d87610c00a40d5a6632a8589dfe5424b1b477 Mon Sep 17 00:00:00 2001 From: Jared Johnson Date: Fri, 7 Nov 2014 16:32:44 -0600 Subject: [PATCH 04/10] Make GeoIP headers optional --- config.sample/add_geoip_headers | 1 + plugins/ident/geoip | 1 + t/config/add_geoip_headers | 1 + 3 files changed, 3 insertions(+) create mode 100644 config.sample/add_geoip_headers create mode 100644 t/config/add_geoip_headers diff --git a/config.sample/add_geoip_headers b/config.sample/add_geoip_headers new file mode 100644 index 0000000..d00491f --- /dev/null +++ b/config.sample/add_geoip_headers @@ -0,0 +1 @@ +1 diff --git a/plugins/ident/geoip b/plugins/ident/geoip index 5765b5a..517f8bc 100644 --- a/plugins/ident/geoip +++ b/plugins/ident/geoip @@ -130,6 +130,7 @@ sub register { $self->{_args}{db_dir} ||= '/usr/local/share/GeoIP'; $self->load_geoip() or return; + return if ! $self->qp->config('add_geoip_headers'); $self->register_hook( data_post => 'add_headers' ); } diff --git a/t/config/add_geoip_headers b/t/config/add_geoip_headers new file mode 100644 index 0000000..d00491f --- /dev/null +++ b/t/config/add_geoip_headers @@ -0,0 +1 @@ +1 From 0f0495f09ea73397aafd4025351d3e3d054490ef Mon Sep 17 00:00:00 2001 From: Jared Johnson Date: Fri, 7 Nov 2014 16:34:51 -0600 Subject: [PATCH 05/10] Explicit return --- plugins/ident/geoip | 1 + 1 file changed, 1 insertion(+) diff --git a/plugins/ident/geoip b/plugins/ident/geoip index 517f8bc..984de7a 100644 --- a/plugins/ident/geoip +++ b/plugins/ident/geoip @@ -138,6 +138,7 @@ sub load_geoip { my ( $self ) = @_; $self->load_geoip2() and return 1; $self->load_geoip1() and return 1; + return 0; } sub load_geoip1 { From 943b1fcaf37a2d544907aa096c17cc77f5d61128 Mon Sep 17 00:00:00 2001 From: Jared Johnson Date: Fri, 7 Nov 2014 16:39:28 -0600 Subject: [PATCH 06/10] Remove country_name header which is redundant --- plugins/ident/geoip | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/plugins/ident/geoip b/plugins/ident/geoip index 984de7a..931c1b0 100644 --- a/plugins/ident/geoip +++ b/plugins/ident/geoip @@ -202,9 +202,8 @@ sub load_geoip2 { sub add_headers { my ( $self, $txn ) = @_; - for my $h (qw( Country Country-Name Continent City ASN )) { + for my $h (qw( Country Continent City ASN )) { my $note = lc "geoip_$h"; - $h =~ s/-/_/; next if ! $self->connection->notes($note); $txn->header->delete("X-GeoIP-$h"); $txn->header->add( "X-GeoIP-$h", $self->connection->notes($note), 0 ); From 575dc73cdf812e03c975aca1d6739aa3e66d7b13 Mon Sep 17 00:00:00 2001 From: Jared Johnson Date: Thu, 13 Nov 2014 13:40:04 -0600 Subject: [PATCH 07/10] Use 'command line' instead of config() call It's awful, but consistent --- plugins/ident/geoip | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/plugins/ident/geoip b/plugins/ident/geoip index 931c1b0..8a3b4b1 100644 --- a/plugins/ident/geoip +++ b/plugins/ident/geoip @@ -73,6 +73,14 @@ The path to the GeoIP database directory. Default: /usr/local/share/GeoIP +=head2 add_headers + +Add message headers with GeoIP data + + ident/geoip [ add_headers (true|false) ] + +Default: true + =head1 LIMITATIONS The distance calculations are more concerned with being fast than accurate. @@ -130,7 +138,7 @@ sub register { $self->{_args}{db_dir} ||= '/usr/local/share/GeoIP'; $self->load_geoip() or return; - return if ! $self->qp->config('add_geoip_headers'); + return if $self->{_args}{add_headers} !~ /false/i; $self->register_hook( data_post => 'add_headers' ); } From 3b2b720ed536e94ab9976e000cb1340815749876 Mon Sep 17 00:00:00 2001 From: Jared Johnson Date: Thu, 13 Nov 2014 13:52:49 -0600 Subject: [PATCH 08/10] Add tests for header addition --- t/plugin_tests/ident/geoip | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/t/plugin_tests/ident/geoip b/t/plugin_tests/ident/geoip index a1ac6dd..d830627 100644 --- a/t/plugin_tests/ident/geoip +++ b/t/plugin_tests/ident/geoip @@ -27,6 +27,7 @@ sub register_tests { $self->register_test('test_set_continent'); $self->register_test('test_set_distance'); $self->register_test('test_set_asn'); + $self->register_test('test_add_headers'); } } @@ -46,6 +47,30 @@ sub test_geoip2_lookup { cmp_ok( $self->connection->notes('geoip_city'), 'eq', 'Deer Park', "24.24.24.24 is in city of Deer Park"); } +sub test_add_headers { + my ( $self ) = @_; + my @notes = qw( geoip_country geoip_continent geoip_city geoip_asn ); + $self->connection->notes( $_ => "test $_" ) for @notes; + my $header = $self->transaction->header( Mail::Header->new ); + my @tags = (qw( X-GeoIP-Country X-GeoIP-Continent X-GeoIP-City X-GeoIP-ASN )); + $header->add( $_ => 'DELETETHIS' ) for @tags; + $self->add_headers($self->transaction); + is( $self->all_headers('X-GeoIP-Country'), 'test geoip_country', + 'X-GeoIP-Country header added' ); + is( $self->all_headers('X-GeoIP-Continent'), 'test geoip_continent', + 'X-GeoIP-Continent header added' ); + is( $self->all_headers('X-GeoIP-City'), 'test geoip_city', + 'X-GeoIP-City header added' ); + is( $self->all_headers('X-GeoIP-ASN'), 'test geoip_asn', + 'X-GeoIP-ASN header added' ); +} + +sub all_headers { + # Return all instances of a given message header + my ( $self, $tag ) = @_; + return join " | ", map { chomp $_; $_ } $self->transaction->header->get($tag); +} + sub test_geoip_lookup { my $self = shift; From 0975b225822b8bfdc869b2f0a387c892ff07c112 Mon Sep 17 00:00:00 2001 From: Jared Johnson Date: Thu, 13 Nov 2014 14:19:34 -0600 Subject: [PATCH 09/10] Get rid of a warning --- plugins/ident/geoip | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugins/ident/geoip b/plugins/ident/geoip index 8a3b4b1..a816eb8 100644 --- a/plugins/ident/geoip +++ b/plugins/ident/geoip @@ -138,7 +138,7 @@ sub register { $self->{_args}{db_dir} ||= '/usr/local/share/GeoIP'; $self->load_geoip() or return; - return if $self->{_args}{add_headers} !~ /false/i; + return if defined $self->{_args}{add_headers} and $self->{_args}{add_headers} !~ /false/i; $self->register_hook( data_post => 'add_headers' ); } From a3ae6c51f45e27f942a8ac1d49f521718db44ffe Mon Sep 17 00:00:00 2001 From: Jared Johnson Date: Thu, 13 Nov 2014 14:55:17 -0600 Subject: [PATCH 10/10] Shorter lines and more explicit default --- plugins/ident/geoip | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/plugins/ident/geoip b/plugins/ident/geoip index a816eb8..c1c30c9 100644 --- a/plugins/ident/geoip +++ b/plugins/ident/geoip @@ -138,7 +138,9 @@ sub register { $self->{_args}{db_dir} ||= '/usr/local/share/GeoIP'; $self->load_geoip() or return; - return if defined $self->{_args}{add_headers} and $self->{_args}{add_headers} !~ /false/i; + my $enabled = $self->{_args}{add_headers}; + $enabled = 'true' if ! defined $enabled; + return if $enabled =~ /false/i; $self->register_hook( data_post => 'add_headers' ); }