[Rt-commit] r11983 - in rt/branches/3.8-TESTING: .

[jesse at bestpractical.com]

Author: jesse
Date: Wed Apr 30 16:09:26 2008
New Revision: 11983

Modified:
   rt/branches/3.8-TESTING/   (props changed)
   rt/branches/3.8-TESTING/lib/RT/Action/SendEmail.pm

Log:
 r30280 at 31b:  jesse | 2008-04-30 15:32:21 -0400
 * Refactor mail squelching to be able to warn as it removes 'wrong' recipients.
 * Remove duplicated code


Modified: rt/branches/3.8-TESTING/lib/RT/Action/SendEmail.pm
==============================================================================
--- rt/branches/3.8-TESTING/lib/RT/Action/SendEmail.pm	(original)
+++ rt/branches/3.8-TESTING/lib/RT/Action/SendEmail.pm	Wed Apr 30 16:09:26 2008
@@ -60,6 +60,8 @@
 use RT::EmailParser;
 use RT::Interface::Email;
 use Mail::Address;
+our @EMAIL_RECIPIENT_HEADERS = qw(To Cc Bcc);
+
 
 =head1 NAME
 
@@ -167,7 +169,7 @@
     $self->RemoveInappropriateRecipients();
 
     my %seen;
-    foreach my $type qw(To Cc Bcc) {
+    foreach my $type (@EMAIL_RECIPIENT_HEADERS) {
         @{ $self->{$type} }
             = grep defined && length && !$seen{ lc $_ }++,
             @{ $self->{$type} };
@@ -178,19 +180,13 @@
 
 # TODO: We should be pulling the recipients out of the template and shove them into To, Cc and Bcc
 
-    $self->SetHeader( 'To', join( ', ', @{ $self->{'To'} } ) )
-        if ( !$MIMEObj->head->get('To')
-        && $self->{'To'}
-        && @{ $self->{'To'} } );
-    $self->SetHeader( 'Cc', join( ', ', @{ $self->{'Cc'} } ) )
-        if ( !$MIMEObj->head->get('Cc')
-        && $self->{'Cc'}
-        && @{ $self->{'Cc'} } );
-    $self->SetHeader( 'Bcc', join( ', ', @{ $self->{'Bcc'} } ) )
-        if ( !$MIMEObj->head->get('Bcc')
-        && $self->{'Bcc'}
-        && @{ $self->{'Bcc'} } );
+    for my $header (@EMAIL_RECIPIENT_HEADERS) {
 
+    $self->SetHeader( $header, join( ', ', @{ $self->{$header} } ) )
+        if ( !$MIMEObj->head->get($header)
+        && $self->{$header}
+        && @{ $self->{$header} } );
+}
     # PseudoTo	(fake to headers) shouldn't get matched for message recipients.
     # If we don't have any 'To' header (but do have other recipients), drop in
     # the pseudo-to header.
@@ -315,7 +311,7 @@
     return $status unless $status > 0;
 
     my $success = $msgid . " sent ";
-    foreach (qw(To Cc Bcc)) {
+    foreach (@EMAIL_RECIPIENT_HEADERS) {
         my $recipients = $MIMEObj->head->get($_);
         $success .= " $_: " . $recipients if $recipients;
     }
@@ -645,17 +641,11 @@
 
     my @blacklist = ();
 
-    my @types = qw/To Cc Bcc/;
-
-    # Weed out any RT addresses. We really don't want to talk to ourselves!
-    @{ $self->{$_} } = RT::EmailParser::CullRTAddresses( "", @{ $self->{$_} } )
-        for (@types);
-
     # If there are no recipients, don't try to send the message.
     # If the transaction has content and has the header RT-Squelch-Replies-To
 
+    my $msgid = $self->TemplateObj->MIMEObj->head->get('Message-Id');
     if ( my $attachment = $self->TransactionObj->Attachments->First ) {
-        my $msgid = $self->TemplateObj->MIMEObj->head->get('Message-Id');
 
         if ( $attachment->GetHeader('RT-DetectedAutoGenerated') ) {
 
@@ -666,7 +656,7 @@
             if ( !RT->Config->Get('RedistributeAutoGeneratedMessages') ) {
 
                 # Don't send to any watchers.
-                @{ $self->{$_} } = () for (@types);
+                @{ $self->{$_} } = () for (@EMAIL_RECIPIENT_HEADERS);
                 $RT::Logger->info( $msgid
                         . " The incoming message was autogenerated. "
                         . "Not redistributing this message based on site configuration.\n"
@@ -676,7 +666,7 @@
             {
 
                 # Only send to "privileged" watchers.
-                foreach my $type (@types) {
+                foreach my $type (@EMAIL_RECIPIENT_HEADERS) {
                     foreach my $addr ( @{ $self->{$type} } ) {
                         my $user = RT::User->new($RT::SystemUser);
                         $user->LoadByEmail($addr);
@@ -702,12 +692,25 @@
     # Cycle through the people we're sending to and pull out anyone on the
     # system blacklist
 
-    foreach my $person_to_yank (@blacklist) {
-        $person_to_yank =~ s/\s//g;
-        foreach my $type (@types) {
-            @{ $self->{$type} } = grep !/^\Q$person_to_yank\E$/,
-                @{ $self->{$type} };
+    # Trim leading and trailing spaces. # Todo - we should really be canonicalizing all addresses
+    @blacklist = map { s/\s//g; } @blacklist;
+    foreach my $type (@EMAIL_RECIPIENT_HEADERS) {
+        my @addrs;
+        foreach my $addr ( @{ $self->{$type} } ) {
+
+         # Weed out any RT addresses. We really don't want to talk to ourselves!
+         # If we get a reply back, that means it's not an RT address
+            if ( !RT::EmailParser->CullRTAddresses($addr) ) {
+                $RT::Logger->info( $msgid . "$addr appears to point to this RT instance. Skipping" );
+                next;
+            }
+            if ( grep /^\Q$addr\E$/, @blacklist ) {
+                $RT::Logger->info( $msgid . "$addr was blacklisted for outbound mail on this transaction. Skipping");
+                next;
+            }
+            push @addrs, $addr;
         }
+        @{ $self->{$type} } = @addrs;
     }
 }