diff --git a/plugins/greylisting b/plugins/greylisting index 76b25bb..deaa044 100644 --- a/plugins/greylisting +++ b/plugins/greylisting @@ -213,7 +213,8 @@ sub register { $self->{_args} = $config; $self->init_db(); $self->register_hooks(); - $self->prune_db(); + eval { $self->prune_db(); }; + $self->log(LOGERROR, "Unable to prune greylist DB: $@") if $@; if ($self->{_args}{upgrade}) { $self->convert_db(); } @@ -383,16 +384,25 @@ sub greylist { my $key = $self->get_greylist_key($sender, $rcpt) or return DECLINED; + my @result; + eval { + $self->db->lock; + @result = $self->greylist_result($key, $config); + $self->db->unlock; + }; + return $self->errcode($@) if $@; + return @result; +} +sub greylist_result { + my ($self, $key, $config) = @_; my $fmt = "%s:%d:%d:%d"; - - $self->db->lock or return DECLINED; my $value = $self->db->get($key); if ( ! $value ) { # new IP or entry timed out - record new $self->db->set( $key, sprintf $fmt, $self->now, 1, 0, 0 ); $self->log(LOGWARN, "fail: initial DENYSOFT, unknown"); - return $self->cleanup_and_return(); + return $self->failcode(); } my ( $ts, $new, $black, $white ) = split /:/, $value; @@ -404,7 +414,7 @@ sub greylist { if ( $self->now - $ts < $config->{white_timeout} ) { $self->db->set( $key, sprintf $fmt, $self->now, $new, $black, ++$white ); $self->log(LOGINFO, "pass: white, $white deliveries"); - return $self->cleanup_and_return(DECLINED); + return DECLINED; } else { $self->log(LOGINFO, "key $key has timed out (white)"); @@ -416,26 +426,29 @@ sub greylist { $self->db->set( $key, sprintf $fmt, $ts, $new, ++$black, 0 ); $self->log(LOGWARN, "fail: black DENYSOFT - $black deferred connections"); - return $self->cleanup_and_return(); + return $self->failcode(); } $self->log(LOGWARN, "pass: timed out (grey)"); - return $self->cleanup_and_return(DECLINED); + return DECLINED; } # This exists purely to be overridden for testing sub now { time() } -sub cleanup_and_return { - my ($self, $return_val) = @_; - - $self->db->unlock; - return $return_val if defined $return_val; # explicit override +sub failcode { + my ($self) = @_; return DECLINED if defined $self->{_args}{reject} && !$self->{_args}{reject}; return DENYSOFT, $DENYMSG; } +sub errcode { + my ($self, $err) = @_; + $self->log(LOGERROR, "UNABLE TO OBTAIN GREYLIST RESULT:$err"); + return DECLINED; +} + sub get_greylist_key { my $self = shift; my $sender = shift || $self->qp->transaction->sender; diff --git a/t/plugin_tests/greylisting b/t/plugin_tests/greylisting index db96f73..e5b93a9 100644 --- a/t/plugin_tests/greylisting +++ b/t/plugin_tests/greylisting @@ -26,6 +26,8 @@ sub register_tests { $self->register_test('test_init_redis'); $self->register_test('test_init_dbm'); $self->register_test('test_parse_redis_server'); + $self->register_test('test_failcode'); + $self->register_test('test_errcode'); } sub test_load_exclude_files { @@ -365,3 +367,24 @@ sub test_parse_redis_server { 'parse_redis_server(): add default port' ); } +sub test_failcode { + my ($self) = @_; + $self->{_args}{reject} = 0; + is( $self->rc( $self->failcode() ), 'DECLINED', + 'failcode(): reject disabled' ); + delete $self->{_args}{reject}; + is( $self->rc( $self->failcode() ), 'DENYSOFT: This mail is temporarily denied', + 'failcode(): reject enabled' ); +} + +sub test_errcode { + my ($self) = @_; + delete $self->qp->{_logged}; + is( $self->rc( $self->errcode('test error') ), + 'DECLINED', + 'errcode(): proper return code' ); + is( join("\n", @{ $self->qp->{_logged} || [] }), + 'LOGERROR:greylistingUNABLE TO OBTAIN GREYLIST RESULT:test error', + 'errocode(): logged error' ); +} +