basicheaders: added log messages, tests, named args

added log messages at each exit point
added tests
added reject_type option (defer -vs- deny)
added named argument parsing

plugins/check_basicheaders

@@ -15,13 +15,30 @@ some number of the days in the past or future.
 
 =head1 CONFIGURATION
 
-Takes one optional parameter, the number of days in the future or past
+The following optional parameters exist:
-beyond which to reject messages. (The default is to not reject messages
+
-based on the date.)
+=head2 days
+
+The number of days in the future or past beyond which to reject messages. When
+unset, messages are not rejected based on the date.
+
+  check_basicheaders [ days 3 ]
+
+=head2 reject_type
+
+Whether to issue a permanent or temporary rejection. The default is permanent.
+
+  check_basicheaders reject_type [ temp | perm ]
+
+Switching to a temporary rejection is most useful when testing the plugin. It
+allows an administrator to watch for a test period and make sure no valid mail
+is getting rejected.
 
 =head1 AUTHOR
 
-Written by Jim Winstead Jr.
+ 2004 - Written by Jim Winstead Jr.
+
+ 2012 - added logging, named arguments, reject_type, tests - Matt Simerson
 
 =head1 LICENSE
 
@@ -34,38 +51,64 @@ use Date::Parse qw(str2time);
 sub register {
     my ($self, $qp, @args) = @_;
 
-  if (@args > 0) {
+    if ( @args == 1 ) {
-    $self->{_days} = $args[0];
+        $self->log(LOGWARN, "deprecated arguments. Update your arguments to this plugin");
-    $self->log(LOGWARN, "WARNING: Ignoring additional arguments.") if (@args > 1);
+        $self->{_args}{days} = $args[0];
     }
+    elsif ( @args % 2 ) {
+        $self->log(LOGWARN, "invalid arguments");
+    }
+    else {
+        $self->{_args} = { @args };
+    };
 }
 
 sub hook_data_post {
     my ($self, $transaction) = @_;
 
-  return (DENY, "You have to send some data first")
+    my $deny = $self->{_args}{reject_type} eq 'temp' ? DENYSOFT : DENY;
-    if $transaction->data_size == 0;
 
-  my $header = $transaction->header;
+    if ( $transaction->data_size == 0 ) {
-  return (DENY, "Mail with no From header not accepted here")
+        $self->log(LOGINFO, "fail: no data");
-    unless $header && $header->get('From');
+        return ($deny, "You have to send some data first");
+    };
 
-  my $date = $header->get('Date');
+    my $header = $transaction->header or do {
+        $self->log(LOGINFO, "fail: no headers");
+        return ($deny, "missing header");
+    };
 
-  return (DENY, "Mail with no Date header not accepted here")
+    if ( ! $header->get('From') ) {
-    unless $date;
+        $self->log(LOGINFO, "fail: no from");
+        return ($deny, "We require a valid From header")
+    };
 
-  return (DECLINED) unless defined $self->{_days};
+    my $date = $header->get('Date') or do {
+        $self->log(LOGINFO, "fail: no date");
+        return ($deny, "We require a valid Date header");
+    };
 
-  my $ts = str2time($date);
+    my $days = $self->{_args}{days};
+    if ( ! defined $days ) {
+        $self->log(LOGINFO, "pass: no days arg");
+        return (DECLINED);
+    };
 
-  return (DECLINED) unless $ts;
+    my $ts = str2time($date) or do {
+        $self->log(LOGINFO, "skip: date not parseable ($date)");
+        return (DECLINED);
+    };
 
-  return (DENY, "The Date in the header was too far in the past")
+    if ( $ts < time - ($days*24*3600) ) {
-    if $ts < time - ($self->{_days}*24*3600);
+        $self->log(LOGINFO, "fail: date too old ($date)");
+        return ($deny, "The Date in the header is too far in the past")
+    };
 
-  return (DENY, "The Date in the header was too far in the future")
+    if ( $ts > time + ($days*24*3600) ) {
-    if $ts > time + ($self->{_days}*24*3600);
+        $self->log(LOGINFO, "fail: date in future ($date)");
+        return ($deny, "The Date in the header is too far in the future")
+    };
 
+    $self->log(LOGINFO, "pass");
     return (DECLINED);
 }

t/plugin_tests/check_basicheaders

@@ -0,0 +1,63 @@
+#!perl -w
+
+use strict;
+use Data::Dumper;
+
+use Qpsmtpd::Address;
+use Qpsmtpd::Constants;
+
+sub register_tests {
+    my $self = shift;
+
+    $self->register_test("test_hook_data_post", 5);
+}
+
+sub test_hook_data_post {
+    my $self = shift;
+
+    my $transaction = $self->qp->transaction;
+    my $test_email = 'matt@example.com';
+    my $address = Qpsmtpd::Address->new( "<$test_email>" );
+    my $header  = Mail::Header->new(Modify => 0, MailFrom => "COERCE");
+    my $now    = `date`;
+    my $future = `date -v +6d`;
+    my $past   = `date -v -6d`;
+    $self->{_args}{days} = 5;
+
+    $transaction->sender($address);
+    $transaction->header($header);
+    $transaction->header->add('From', "<$test_email>");
+    $transaction->header->add('Date', $now );
+    $transaction->body_write( "test message body " );
+
+    my ($code, $mess) = $self->hook_data_post( $transaction );
+    cmp_ok( DECLINED, '==', $code, "okay" );
+
+    $transaction->header->delete('Date');
+    ($code, $mess) = $self->hook_data_post( $transaction );
+    cmp_ok( DENY, '==', $code, "missing date ( $mess )" );
+
+    $transaction->header->delete('From');
+    $transaction->header->add('Date', $now );
+    ($code, $mess) = $self->hook_data_post( $transaction );
+    cmp_ok( DENY, '==', $code, "missing from ( $mess )" );
+
+    if ( $future ) {
+        $transaction->header->replace('Date', $future );
+        ($code, $mess) = $self->hook_data_post( $transaction );
+        cmp_ok( DENY, '==', $code, "too new ( $mess )" );
+
+        $transaction->header->replace('Date', $past );
+        ($code, $mess) = $self->hook_data_post( $transaction );
+        cmp_ok( DENY, '==', $code, "too old ( $mess )" );
+
+    }
+    else {
+        ok( 1, "skip: unable to use 'date' output");
+        ok( 1, "skip: unable to use 'date' output");
+    }
+
+    $self->{_args}{reject_type} = 'temp';
+    ($code, $mess) = $self->hook_data_post( $transaction );
+    cmp_ok( DENYSOFT, '==', $code, "defer, not deny ( $mess )" );
+};