[Rt-commit] rt branch, 4.2/mail-sending, created. rt-4.1.5-47-g4481d83

Alex Vandiver alexmv at bestpractical.com
Fri Dec 14 18:30:18 EST 2012


The branch, 4.2/mail-sending has been created
        at  4481d8365f14ab39c63c918f0d28c7d5d05a5378 (commit)

- Log -----------------------------------------------------------------
commit 768c1a453c92ef774091dc02a2200d26b864855a
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 c164d15..8c28275 100755
--- a/etc/RT_Config.pm.in
+++ b/etc/RT_Config.pm.in
@@ -465,11 +465,9 @@ Set($ExtractSubjectTagNoMatch, ( ${RT::EmailSubjectTagRegex}
 
 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
@@ -726,39 +724,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
@@ -766,7 +731,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 1c7f9b7..cc4fd60 100644
--- a/lib/RT/Interface/Email.pm
+++ b/lib/RT/Interface/Email.pm
@@ -491,51 +491,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 837a9b9..782e022 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,
@@ -344,10 +342,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 c24af6f2d68d5c4e504f61b83b096121754968c0
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 3f52d10..23c6cdd 100644
--- a/lib/RT/Config.pm
+++ b/lib/RT/Config.pm
@@ -591,6 +591,15 @@ our %META = (
             $self->Set( DisableGD => 1 );
         },
     },
+    MailCommand => {
+        Type    => 'SCALAR',
+        PostLoadCheck => sub {
+            my $self = shift;
+            my $value = $self->Get('MailCommand');
+            $RT::Logger->error("Unknown outgoing mail configuration: $value")
+                unless $value =~/^(sendmail|sendmailpipe|qmail|testfile)$/;
+        },
+    },
     MailPlugins  => { Type => 'ARRAY' },
     GnuPG        => { Type => 'HASH' },
     GnuPGOptions => { Type => 'HASH',

commit 7aa1db9d7f55cd305e76a896e48c438a249be8d2
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 cc4fd60..9c9e76f 100644
--- a/lib/RT/Interface/Email.pm
+++ b/lib/RT/Interface/Email.pm
@@ -412,11 +412,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' );
 
@@ -498,8 +493,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 b643accb7edebb1153f5173f045214b363f6ade6
Author: Alex Vandiver <alexmv at bestpractical.com>
Date:   Mon Nov 5 19:11:42 2012 -0500

    Add the necessary "-t" ourselves if it is not there

diff --git a/etc/RT_Config.pm.in b/etc/RT_Config.pm.in
index 8c28275..bda8c9e 100755
--- a/etc/RT_Config.pm.in
+++ b/etc/RT_Config.pm.in
@@ -465,9 +465,8 @@ Set($ExtractSubjectTagNoMatch, ( ${RT::EmailSubjectTagRegex}
 
 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
@@ -691,16 +690,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 9c9e76f..907c3a4 100644
--- a/lib/RT/Interface/Email.pm
+++ b/lib/RT/Interface/Email.pm
@@ -418,6 +418,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'} ) {

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

    Note removal of option in UPGRADING-4.2

diff --git a/docs/UPGRADING-4.2 b/docs/UPGRADING-4.2
index 43a9305..cf22144 100644
--- a/docs/UPGRADING-4.2
+++ b/docs/UPGRADING-4.2
@@ -22,3 +22,7 @@ UPGRADING FROM RT 4.0.0 and greater
 * MakeClicky handlers added via a callback are now passed an "object" key in
   the parameter hash instead of "ticket".  The object may be any RT::Record
   subclass.
+
+* The 'smtp' $MailCommand has been removed, as it did not guarantee
+  delivery.  Use a local MTA for outgoing mail, via the 'sendmail'
+  setting.

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


More information about the Rt-commit mailing list