Re: [PATCH] added logging and tests to check_badmailfrom

Applied.

On Sun, May 6, 2012 at 1:59 PM, Matt Simerson <[email protected]> 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 = '[email protected]';
> +    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, ['[email protected]'] ),
> "is_immune, null sender");
> +
> +    $address = Qpsmtpd::Address->new( '<matt@>' );
> +    $transaction->sender($address);
> +    ok( $self->is_immune( $transaction->sender, ['[email protected]'] ),
> "is_immune, missing host");
> +
> +    $address = Qpsmtpd::Address->new( '<@example.com>' );
> +    $transaction->sender($address);
> +    ok( $self->is_immune( $transaction->sender, ['[email protected]'] ),
> "is_immune, missing user");
> +
> +    $address = Qpsmtpd::Address->new( '<[email protected]>' );
> +    $transaction->sender($address);
> +    ok( ! $self->is_immune( $transaction->sender, ['[email protected]'] ),
> "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
>
>