Applied.
On Sun, May 6, 2012 at 1:59 PM, Matt Simerson <m...@tnpi.net> wrote:
> 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
>
>
