[Rt-commit] rt branch, 4.2/mail-sending, created. rt-4.1.5-158-g8b83a38

Alex Vandiver alexmv at bestpractical.com
Thu Dec 27 11:44:39 EST 2012


The branch, 4.2/mail-sending has been created
        at  8b83a3888bdb5b1150021c7f7ce6b3e32c19644e (commit)

- Log -----------------------------------------------------------------
commit 2300617fe5790fa6e2945f918273a487cce21307
Author: Alex Vandiver <alexmv at bestpractical.com>
Date:   Mon Nov 5 18:56:34 2012 -0500

    Remove SMTP delivery option, which can drop email
    
    The 'smtp' delivery mode, despite being supported, was a poor choice of
    delivery, as in the case where the chosen sever failed to respond, it
    would silently drop the message.  Remove the option, so that this does
    not bite users.

diff --git a/etc/RT_Config.pm.in b/etc/RT_Config.pm.in
index fb219ac..b8dfe29 100755
--- a/etc/RT_Config.pm.in
+++ b/etc/RT_Config.pm.in
@@ -485,11 +485,9 @@ Set( $CheckMoreMSMailHeaders, 0);
 
 C<$MailCommand> defines which method RT will use to try to send mail.
 We know that 'sendmailpipe' works fairly well.  If 'sendmailpipe'
-doesn't work well for you, try 'sendmail'.  Other options are 'smtp'
-or 'qmail'.
-
-Note that you should remove the '-t' from C<$SendmailArguments> if you
-use 'sendmail' rather than 'sendmailpipe'
+doesn't work well for you, try 'sendmail'.  Note that you should remove
+the '-t' from C<$SendmailArguments> if you use 'sendmail' rather than
+'sendmailpipe'.  'qmail' is also a supported value.
 
 For testing purposes, or to simply disable sending mail out into the
 world, you can set C<$MailCommand> to 'testfile' which writes all mail
@@ -746,39 +744,6 @@ Set($SendmailPath, "/usr/sbin/sendmail");
 
 =back
 
-=head2 SMTP configuration
-
-These options only take effect if C<$MailCommand> is 'smtp'
-
-=over 4
-
-=item C<$SMTPServer>
-
-C<$SMTPServer> should be set to the hostname of the SMTP server to use
-
-=cut
-
-Set($SMTPServer, undef);
-
-=item C<$SMTPFrom>
-
-C<$SMTPFrom> should be set to the 'From' address to use, if not the
-email's 'From'
-
-=cut
-
-Set($SMTPFrom, undef);
-
-=item C<$SMTPDebug>
-
-C<$SMTPDebug> should be set to 1 to debug SMTP mail sending
-
-=cut
-
-Set($SMTPDebug, 0);
-
-=back
-
 =head2 Other mailers
 
 =over 4
@@ -786,7 +751,7 @@ Set($SMTPDebug, 0);
 =item C<@MailParams>
 
 C<@MailParams> defines a list of options passed to $MailCommand if it
-is not 'sendmailpipe', 'sendmail', or 'smtp'
+is not 'sendmailpipe' or 'sendmail';
 
 =cut
 
diff --git a/lib/RT/Action/SendEmail.pm b/lib/RT/Action/SendEmail.pm
index e459377..f2308cd 100644
--- a/lib/RT/Action/SendEmail.pm
+++ b/lib/RT/Action/SendEmail.pm
@@ -1086,11 +1086,6 @@ sub SetHeaderAsEncoding {
 
     my $head = $self->TemplateObj->MIMEObj->head;
 
-    if ( lc($field) eq 'from' and RT->Config->Get('SMTPFrom') ) {
-        $head->replace( $field, RT->Config->Get('SMTPFrom') );
-        return;
-    }
-
     my $value = $head->get( $field );
     $value = $self->MIMEEncodeString( $value, $enc );
     $head->replace( $field, $value );
diff --git a/lib/RT/Interface/Email.pm b/lib/RT/Interface/Email.pm
index 35548a5..1455ea9 100644
--- a/lib/RT/Interface/Email.pm
+++ b/lib/RT/Interface/Email.pm
@@ -494,51 +494,6 @@ sub SendEmail {
             return 0;
         }
     }
-    elsif ( $mail_command eq 'smtp' ) {
-        require Net::SMTP;
-        my $smtp = do { local $@; eval { Net::SMTP->new(
-            Host  => RT->Config->Get('SMTPServer'),
-            Debug => RT->Config->Get('SMTPDebug'),
-        ) } };
-        unless ( $smtp ) {
-            $RT::Logger->crit( "Could not connect to SMTP server.");
-            if ($TicketObj) {
-                _RecordSendEmailFailure( $TicketObj );
-            }
-            return 0;
-        }
-
-        # duplicate head as we want drop Bcc field
-        my $head = $args{'Entity'}->head->dup;
-        my @recipients = map $_->address, map 
-            Email::Address->parse($head->get($_)), qw(To Cc Bcc);                       
-        $head->delete('Bcc');
-
-        my $sender = RT->Config->Get('SMTPFrom')
-            || $args{'Entity'}->head->get('From');
-        chomp $sender;
-
-        my $status = $smtp->mail( $sender )
-            && $smtp->recipient( @recipients );
-
-        if ( $status ) {
-            $smtp->data;
-            my $fh = $smtp->tied_fh;
-            $head->print( $fh );
-            print $fh "\n";
-            $args{'Entity'}->print_body( $fh );
-            $smtp->dataend;
-        }
-        $smtp->quit;
-
-        unless ( $status ) {
-            $RT::Logger->crit( "$msgid: Could not send mail via SMTP." );
-            if ( $TicketObj ) {
-                _RecordSendEmailFailure( $TicketObj );
-            }
-            return 0;
-        }
-    }
     else {
         local ($ENV{'MAILADDRESS'}, $ENV{'PERL_MAILERS'});
 
diff --git a/sbin/rt-test-dependencies.in b/sbin/rt-test-dependencies.in
index 86aa0bb..023f92d 100644
--- a/sbin/rt-test-dependencies.in
+++ b/sbin/rt-test-dependencies.in
@@ -70,7 +70,6 @@ GetOptions(
 
     'with-GPG',
     'with-ICAL',
-    'with-SMTP',
     'with-GRAPHVIZ',
     'with-GD',
     'with-DASHBOARDS',
@@ -100,7 +99,6 @@ my %default = (
     'with-DEV' => @RT_DEVEL_MODE@, 
     'with-GPG' => @RT_GPG@,
     'with-ICAL' => 1,
-    'with-SMTP' => 1,
     'with-GRAPHVIZ' => @RT_GRAPHVIZ@,
     'with-GD' => @RT_GD@,
     'with-DASHBOARDS' => 1,
@@ -345,10 +343,6 @@ $deps{'ICAL'} = [ text_to_hash( << '.') ];
 Data::ICal
 .
 
-$deps{'SMTP'} = [ text_to_hash( << '.') ];
-Net::SMTP
-.
-
 $deps{'DASHBOARDS'} = [ text_to_hash( << '.') ];
 HTML::RewriteAttributes 0.05
 MIME::Types

commit d65e44fd275c1f4946d5efcd9f77be6af9284ea3
Author: Alex Vandiver <alexmv at bestpractical.com>
Date:   Mon Nov 5 19:09:05 2012 -0500

    Check and warn at load-time if "smtp" was set as the $MailCommand

diff --git a/lib/RT/Config.pm b/lib/RT/Config.pm
index e54cecc..794fdc9 100644
--- a/lib/RT/Config.pm
+++ b/lib/RT/Config.pm
@@ -590,6 +590,16 @@ our %META = (
             $self->Set( DisableGD => 1 );
         },
     },
+    MailCommand => {
+        Type    => 'SCALAR',
+        PostLoadCheck => sub {
+            my $self = shift;
+            my $value = $self->Get('MailCommand');
+            $RT::Logger->error("Unknown value for \$MailCommand: $value")
+                unless ref($value) eq "CODE"
+                    or $value =~/^(sendmail|sendmailpipe|qmail|testfile)$/;
+        },
+    },
     MailPlugins  => { Type => 'ARRAY' },
     GnuPG        => { Type => 'HASH' },
     GnuPGOptions => { Type => 'HASH',

commit e37ce9e29d37dbf58af655f2dc104323689adc7b
Author: Alex Vandiver <alexmv at bestpractical.com>
Date:   Wed Dec 19 14:25:15 2012 -0500

    Fall back to sendmailpipe on unknown $MailConfig settings
    
    This provides the most sane default that we can; if sendmail or
    equivalent is not installed, outgoing mail will trigger RT to record
    the "cannot send outgoing mail" transaction.

diff --git a/lib/RT/Config.pm b/lib/RT/Config.pm
index 794fdc9..c0b3a44 100644
--- a/lib/RT/Config.pm
+++ b/lib/RT/Config.pm
@@ -595,9 +595,10 @@ our %META = (
         PostLoadCheck => sub {
             my $self = shift;
             my $value = $self->Get('MailCommand');
-            $RT::Logger->error("Unknown value for \$MailCommand: $value")
-                unless ref($value) eq "CODE"
-                    or $value =~/^(sendmail|sendmailpipe|qmail|testfile)$/;
+            return if ref($value) eq "CODE"
+                or $value =~/^(sendmail|sendmailpipe|qmail|testfile)$/;
+            $RT::Logger->error("Unknown value for \$MailCommand: $value; defaulting to sendmailpipe");
+            $self->Set( MailCommand => 'sendmailpipe' );
         },
     },
     MailPlugins  => { Type => 'ARRAY' },

commit f76a54f650a33a89f2bf635008047232f8c9956f
Author: Alex Vandiver <alexmv at bestpractical.com>
Date:   Mon Nov 5 19:19:28 2012 -0500

    Move "testfile" logic to where other custom Mail::Mailer types are configured

diff --git a/lib/RT/Interface/Email.pm b/lib/RT/Interface/Email.pm
index 1455ea9..9d01981 100644
--- a/lib/RT/Interface/Email.pm
+++ b/lib/RT/Interface/Email.pm
@@ -415,11 +415,6 @@ sub SendEmail {
 
     my $mail_command = RT->Config->Get('MailCommand');
 
-    if ($mail_command eq 'testfile' and not $Mail::Mailer::testfile::config{outfile}) {
-        $Mail::Mailer::testfile::config{outfile} = File::Temp->new;
-        $RT::Logger->info("Storing outgoing emails in $Mail::Mailer::testfile::config{outfile}");
-    }
-
     # if it is a sub routine, we just return it;
     return $mail_command->($args{'Entity'}) if UNIVERSAL::isa( $mail_command, 'CODE' );
 
@@ -501,8 +496,12 @@ sub SendEmail {
         if ( $mail_command eq 'sendmail' ) {
             $ENV{'PERL_MAILERS'} = RT->Config->Get('SendmailPath');
             push @mailer_args, split(/\s+/, RT->Config->Get('SendmailArguments'));
-        }
-        else {
+        } elsif ( $mail_command eq 'testfile' ) {
+            unless ($Mail::Mailer::testfile::config{outfile}) {
+                $Mail::Mailer::testfile::config{outfile} = File::Temp->new;
+                $RT::Logger->info("Storing outgoing emails in $Mail::Mailer::testfile::config{outfile}");
+            }
+        } else {
             push @mailer_args, RT->Config->Get('MailParams');
         }
 

commit 192cd4936b08c989970d890bf38345abc8ce4393
Author: Alex Vandiver <alexmv at bestpractical.com>
Date:   Mon Nov 5 19:11:42 2012 -0500

    Automatically add or remove "-t" from $SendmailArguments

diff --git a/etc/RT_Config.pm.in b/etc/RT_Config.pm.in
index b8dfe29..acfcd86 100755
--- a/etc/RT_Config.pm.in
+++ b/etc/RT_Config.pm.in
@@ -485,9 +485,8 @@ Set( $CheckMoreMSMailHeaders, 0);
 
 C<$MailCommand> defines which method RT will use to try to send mail.
 We know that 'sendmailpipe' works fairly well.  If 'sendmailpipe'
-doesn't work well for you, try 'sendmail'.  Note that you should remove
-the '-t' from C<$SendmailArguments> if you use 'sendmail' rather than
-'sendmailpipe'.  'qmail' is also a supported value.
+doesn't work well for you, try 'sendmail'.  'qmail' is also a supported
+value.
 
 For testing purposes, or to simply disable sending mail out into the
 world, you can set C<$MailCommand> to 'testfile' which writes all mail
@@ -711,16 +710,14 @@ These options only take effect if C<$MailCommand> is 'sendmail' or
 =item C<$SendmailArguments>
 
 C<$SendmailArguments> defines what flags to pass to C<$SendmailPath>
-If you picked 'sendmailpipe', you MUST add a -t flag to
-C<$SendmailArguments> These options are good for most sendmail
-wrappers and work-a-likes.
+These options are good for most sendmail wrappers and work-a-likes.
 
 These arguments are good for sendmail brand sendmail 8 and newer:
-C<Set($SendmailArguments,"-oi -t -ODeliveryMode=b -OErrorMode=m");>
+C<Set($SendmailArguments,"-oi -ODeliveryMode=b -OErrorMode=m");>
 
 =cut
 
-Set($SendmailArguments, "-oi -t");
+Set($SendmailArguments, "-oi");
 
 
 =item C<$SendmailBounceArguments>
diff --git a/lib/RT/Interface/Email.pm b/lib/RT/Interface/Email.pm
index 9d01981..8b82deb 100644
--- a/lib/RT/Interface/Email.pm
+++ b/lib/RT/Interface/Email.pm
@@ -421,6 +421,7 @@ sub SendEmail {
     if ( $mail_command eq 'sendmailpipe' ) {
         my $path = RT->Config->Get('SendmailPath');
         my @args = shellwords(RT->Config->Get('SendmailArguments'));
+        push @args, "-t" unless grep {$_ eq "-t"} @args;
 
         # SetOutgoingMailFrom and bounces conflict, since they both want -f
         if ( $args{'Bounce'} ) {
@@ -495,7 +496,8 @@ sub SendEmail {
         my @mailer_args = ($mail_command);
         if ( $mail_command eq 'sendmail' ) {
             $ENV{'PERL_MAILERS'} = RT->Config->Get('SendmailPath');
-            push @mailer_args, split(/\s+/, RT->Config->Get('SendmailArguments'));
+            push @mailer_args, grep {$_ ne "-t"}
+                split(/\s+/, RT->Config->Get('SendmailArguments'));
         } elsif ( $mail_command eq 'testfile' ) {
             unless ($Mail::Mailer::testfile::config{outfile}) {
                 $Mail::Mailer::testfile::config{outfile} = File::Temp->new;

commit 8b83a3888bdb5b1150021c7f7ce6b3e32c19644e
Author: Alex Vandiver <alexmv at bestpractical.com>
Date:   Mon Nov 26 15:27:22 2012 -0500

    Note removal of 'smtp' $MailCommand option in UPGRADING-4.2

diff --git a/docs/UPGRADING-4.2 b/docs/UPGRADING-4.2
index 52be27e..b1071e2 100644
--- a/docs/UPGRADING-4.2
+++ b/docs/UPGRADING-4.2
@@ -51,3 +51,8 @@ UPGRADING FROM RT 4.0.0 and greater
   users with invalid addresses will continue to work until the next time they
   are updated by an administrator on the modify user page.  If you prefer no
   address validation, set $ValidateUserEmailAddresses to 0.
+
+* The 'smtp' value for $MailCommand, along with the associated
+  $SMTPServer, $SMTPFrom, and $SMTPDebug options, has been removed
+  because it did not guarantee delivery.  Instead, use a local MTA for
+  outgoing mail, via the 'sendmailpipe' setting to $MailCommand.

-----------------------------------------------------------------------


More information about the Rt-commit mailing list