From 0c7ee4941b6b61f1170a468dc0448a8929b64665 Mon Sep 17 00:00:00 2001
From: Matt Simerson <matt@tnpi.net>
Date: Sat, 5 May 2012 00:27:16 -0400
Subject: [PATCH] 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 = 'matt@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, ['bad@example.com'] ), "is_immune, null sender");
+
+    $address = Qpsmtpd::Address->new( '<matt@>' );
+    $transaction->sender($address);
+    ok( $self->is_immune( $transaction->sender, ['bad@example.com'] ), "is_immune, missing host");
+
+    $address = Qpsmtpd::Address->new( '<@example.com>' );
+    $transaction->sender($address);
+    ok( $self->is_immune( $transaction->sender, ['bad@example.com'] ), "is_immune, missing user");
+
+    $address = Qpsmtpd::Address->new( '<matt@example.com>' );
+    $transaction->sender($address);
+    ok( ! $self->is_immune( $transaction->sender, ['bad@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");
 };
 
-