Improve RT::Interface::Email::Filter::SpamAssassin

I had a look at the Filter::SpamAssassin mail plugin, and have a
question and some suggested improvements.

When reading the code, I’m unable to understand how $item can contain
a reference to the message. The argument to GetCurrentUser() is a
hash of arguments, not a reference to the message. Can anyone explain
how the current code work?

The current code wipe out the CurrentUser variable, and must be called
first in the chain of plugins. There is no good reason why it need to
do this, so I propose to change it to keep the CurrentUser value.

Here is a patch to change the argument handling, to pass
$args{‘Message’} to spamassasin, instead of the strange $item value.
The patch also make sure CurrentUser and AuthLevel is passed through
when no change is wanted.

If the patch is correct, please include it in a future version of RT.

— lib/RT/Interface/Email/Filter/SpamAssassin.pm 2004-04-30 00:26:36.000000000 +0200
+++ local/lib/RT/Interface/Email/Filter/SpamAssassinPere.pm 2004-08-26 10:46:34.000000000 +0200
@@ -27,15 +27,20 @@
my $spamtest = Mail::SpamAssassin->new();

sub GetCurrentUser {

  • my %args = ( Message => undef,
  •             CurrentUser => undef,
    
  •             AuthLevel   => undef,
    
  •             @_ );
    
    my $item = shift;
  • my $status = $spamtest->check ($item);
  • return (undef, 0) unless $status->is_spam();
  • my $status = $spamtest->check ($args{‘Message’});
  • return ($args{‘CurrentUser’}, $args{‘AuthLevel’})
  •   unless $status->is_spam();
    
    eval { $status->rewrite_mail() };
    if ($status->get_hits > $status->get_required_hits()*1.5) {
    # Spammy indeed
  •    return (undef, -1);
    
  •    return ($args{'CurrentUser'}, -1);
    
    }
  • return (undef, 0);
  • return ($args{‘CurrentUser’}, $args{‘AuthLevel’});
    }

=head1 NAME

[Petter Reinholdtsen]

I had a look at the Filter::SpamAssassin mail plugin, and have a
question and some suggested improvements.

On a related note, what about extending this filter to place suspected
spam messages into a configurable queue, making sure incorrectly
classified mails aren’t lost.

Something like this would make this configurable. Is this a usable
extention?

— lib/RT/Interface/Email/Filter/SpamAssassin.pm 2004-04-30 00:26:36.000000000 +0200
+++ lib/RT/Interface/Email/Filter/SpamAssassin.pm 2004-08-29 23:05:50.000000000 +0200
@@ -33,7 +33,14 @@
eval { $status->rewrite_mail() };
if ($status->get_hits > $status->get_required_hits()*1.5) {
# Spammy indeed

  •    return (undef, -1);
    
  •   if ($RT::SpamAssassinQueue) {
    
  •       # Move to spam queue
    
  •       $args{'Queue'}->Load( $RT::SpamAssassinQueue );
    
  •   } else {
    
  •       # tell Gateway() to drop the mail
    
  •       $RT::Logger->info("SpamHeader: Dropping spam message!");
    
  •       return ($args{CurrentUser}, -1);
    
  •   }
    
    }
    return (undef, 0);
    }

One could also add several spam levels with associated actions/queues,
making it possible to throw away mail with very high spam score, and
queue in a special spam queue the mail with lower spam score.