[PATCH] added logging and tests to check_badmailfrom

[Matt Simerson]

added logging and tests to check_badmailfrom

refactored several checks out of hook_mail and added LOGDEBUG

added tests for is_immune method
---
plugins/check_badmailfrom        |   27 +++++++++++++++++++++++----
t/plugin_tests/check_badmailfrom |   29 ++++++++++++++++++++++++++++-
2 files changed, 51 insertions(+), 5 deletions(-)

diff --git a/plugins/check_badmailfrom b/plugins/check_badmailfrom
index 0a92f23..975cecc 100644
--- a/plugins/check_badmailfrom
+++ b/plugins/check_badmailfrom
@@ -58,15 +58,13 @@ sub hook_mail {
        @badmailfrom = @{$self->{_badmailfrom_config}};
    };

-    return DECLINED if ! scalar @badmailfrom;
-    return DECLINED if $sender->format eq '<>';
-    return DECLINED if ! $sender->host || ! $sender->user;
+    return DECLINED if $self->is_immune( $sender, \@badmailfrom );

    my $host = lc $sender->host;
    my $from = lc($sender->user) . '@' . $host;

    for my $config (@badmailfrom) {
-        $config =~ s/^\s+//g;   # trim any leading whitespace
+        $config =~ s/^\s+//g;   # trim leading whitespace
        my ($bad, $reason) = split /\s+/, $config, 2;
        next unless $bad;
        next unless $self->is_match( $from, $bad, $host );
@@ -105,3 +103,24 @@ sub hook_rcpt {
    $self->log(LOGINFO, $note);
    return (DENY, $note);
}
+
+sub is_immune {
+    my ($self, $sender, $badmf ) = @_;
+
+    if ( ! scalar @$badmf ) {
+        $self->log(LOGDEBUG, 'skip: empty list');
+        return 1;
+    };
+
+    if ( ! $sender || $sender->format eq '<>' ) {
+        $self->log(LOGDEBUG, 'skip: null sender');
+        return 1;
+    };
+
+    if ( ! $sender->host || ! $sender->user ) {
+        $self->log(LOGDEBUG, 'skip: missing user or host');
+        return 1;
+    };
+
+    return;
+};
diff --git a/t/plugin_tests/check_badmailfrom b/t/plugin_tests/check_badmailfrom
index e183003..60610fe 100644
--- a/t/plugin_tests/check_badmailfrom
+++ b/t/plugin_tests/check_badmailfrom
@@ -1,3 +1,4 @@
+#!perl -w

use strict;
use Data::Dumper;
@@ -7,11 +8,38 @@ use Qpsmtpd::Address;
sub register_tests {
    my $self = shift;

+    $self->register_test("test_badmailfrom_is_immune", 5);
    $self->register_test("test_badmailfrom_match", 7);
    $self->register_test("test_badmailfrom_hook_mail", 4);
    $self->register_test("test_badmailfrom_hook_rcpt", 2);
}

+sub test_badmailfrom_is_immune {
+    my $self = shift;
+
+    my $transaction = $self->qp->transaction;
+    my $test_email = 'm...@test.com';
+    my $address = Qpsmtpd::Address->new( "<$test_email>" );
+    $transaction->sender($address);
+    ok( $self->is_immune( $transaction->sender, [] ), "is_immune, empty list");
+
+    $address = Qpsmtpd::Address->new( '<>' );
+    $transaction->sender($address);
+    ok( $self->is_immune( $transaction->sender, ['b...@example.com'] ), 
"is_immune, null sender");
+
+    $address = Qpsmtpd::Address->new( '<matt@>' );
+    $transaction->sender($address);
+    ok( $self->is_immune( $transaction->sender, ['b...@example.com'] ), 
"is_immune, missing host");
+
+    $address = Qpsmtpd::Address->new( '<@example.com>' );
+    $transaction->sender($address);
+    ok( $self->is_immune( $transaction->sender, ['b...@example.com'] ), 
"is_immune, missing user");
+
+    $address = Qpsmtpd::Address->new( '<m...@example.com>' );
+    $transaction->sender($address);
+    ok( ! $self->is_immune( $transaction->sender, ['b...@example.com'] ), 
"is_immune, false");
+};
+
sub test_badmailfrom_hook_mail {
    my $self = shift;

@@ -77,4 +105,3 @@ sub test_badmailfrom_match {
        "check_badmailfrom pattern non-match");
};

-
-- 
1.7.9.6