[Rt-commit] rt branch, 4.0/warn-on-non-ASCII-email-headers, created. rt-4.0.6-251-gdf404f3

[Jim Brandt]

The branch, 4.0/warn-on-non-ASCII-email-headers has been created
        at  df404f3fa193a81b3e465c044b83eaed70694ba4 (commit)

- Log -----------------------------------------------------------------
commit 220063a15cb1bcb25bee21aac709fa46869dd5a7
Author: Jim Brandt <jbrandt at bestpractical.com>
Date:   Fri Jul 13 09:43:28 2012 -0400

    Avoid warnings with undef $From

diff --git a/lib/RT/Interface/Email.pm b/lib/RT/Interface/Email.pm
index 2594e99..43b501d 100644
--- a/lib/RT/Interface/Email.pm
+++ b/lib/RT/Interface/Email.pm
@@ -149,6 +149,9 @@ sub CheckForSuspiciousSender {
 
     my ( $From, $junk ) = ParseSenderAddressFromHead($head);
 
+    # If unparseable (non-ASCII), $From can come back undef
+    return undef if not defined $From;
+
     if (   ( $From =~ /^mailer-daemon\@/i )
         or ( $From =~ /^postmaster\@/i )
         or ( $From eq "" ))

commit df404f3fa193a81b3e465c044b83eaed70694ba4
Author: Jim Brandt <jbrandt at bestpractical.com>
Date:   Fri Jul 13 13:49:40 2012 -0400

    Improve logging for failed sender parsing in email
    
    Update the existing error in MailFrom.pm's GetCurrentUser to indicate
    that a parsing failure can also prevent finding a valid sender.
    
    Add an info-level message to ParseSenderAddressFromHead containing
    the headers and content that failed so admins can see the problem
    with elevated logging. Also indicate if it was a parse failure or
    missing headers.
    
    Note that this info message will appear twice for a normal rt-mailgate
    run because the method is called from two different places.

diff --git a/lib/RT/Interface/Email.pm b/lib/RT/Interface/Email.pm
index 43b501d..4203aa9 100644
--- a/lib/RT/Interface/Email.pm
+++ b/lib/RT/Interface/Email.pm
@@ -1078,13 +1078,27 @@ of the From (evaluated in order of Reply-To:, From:, Sender)
 
 sub ParseSenderAddressFromHead {
     my $head = shift;
+    my @unparsed;  # Track unparsed emails
+    my @sender_headers = ('Reply-To', 'From', 'Sender');
 
     #Figure out who's sending this message.
-    foreach my $header ('Reply-To', 'From', 'Sender') {
+    foreach my $header ( @sender_headers ) {
         my $addr_line = $head->get($header) || next;
         my ($addr, $name) = ParseAddressFromHeader( $addr_line );
         # only return if the address is not empty
         return ($addr, $name) if $addr;
+
+        chomp $addr_line;
+        push @unparsed, "$header: $addr_line";
+    }
+
+    if( (scalar @unparsed) == 0 ){
+        $RT::Logger->info("Couldn't find a sender in "
+                          . join(', ', @sender_headers) );
+    }
+    else{
+        $RT::Logger->info("Couldn't parse sender using "
+                          . join(', ', @unparsed) );
     }
 
     return (undef, undef);
diff --git a/lib/RT/Interface/Email/Auth/MailFrom.pm b/lib/RT/Interface/Email/Auth/MailFrom.pm
index e733bda..87c6a9c 100644
--- a/lib/RT/Interface/Email/Auth/MailFrom.pm
+++ b/lib/RT/Interface/Email/Auth/MailFrom.pm
@@ -68,7 +68,7 @@ sub GetCurrentUser {
     # We don't need to do any external lookups
     my ( $Address, $Name ) = ParseSenderAddressFromHead( $args{'Message'}->head );
     unless ( $Address ) {
-        $RT::Logger->error("Couldn't find sender's address");
+        $RT::Logger->error("Couldn't parse or find sender's address");
         return ( $args{'CurrentUser'}, -1 );
     }
 
diff --git a/t/mail/header-characters.t b/t/mail/header-characters.t
new file mode 100644
index 0000000..dd20bb7
--- /dev/null
+++ b/t/mail/header-characters.t
@@ -0,0 +1,70 @@
+use strict;
+use warnings;
+
+use RT::Test tests => 12;
+use Test::Warn;
+
+my ($baseurl, $m) = RT::Test->started_ok;
+
+diag "Testing non-ASCII in From: header";
+{
+    my $mail = <<'.';
+From: René@example.com
+Reply-To: =?iso-8859-1?Q?Ren=E9?= <Ren<E9>@example.com>
+Subject: testing non-ASCII From
+Content-Type: text/plain; charset=iso-8859-1
+
+here's some content
+.
+
+    my ($status, $id);
+    warning_like { ( $status, $id ) = RT::Test->send_via_mailgate($mail) }
+      qr/Couldn't parse or find sender's address/,
+        'Got parse error';
+    is( $status >> 8, 0, "The mail gateway exited normally" );
+    TODO: {
+          local $TODO = "Currently don't handle non-ASCII for sender";
+          ok( $id, "Created ticket" );
+      }
+
+}
+
+diag "Testing iso-8859-1 encoded non-ASCII in From: header";
+{
+    my $mail = <<'.';
+From: =?iso-8859-1?Q?Ren=E9?= <Ren<E9>@example.com>
+Reply-To: =?iso-8859-1?Q?Ren=E9?= <Ren<E9>@example.com>
+Subject: testing non-ASCII From
+Content-Type: text/plain; charset=iso-8859-1
+
+here's some content
+.
+
+    my ($status, $id);
+    warning_like { ( $status, $id ) = RT::Test->send_via_mailgate($mail) }
+      qr/Couldn't parse or find sender's address/,
+        'Got parse error';
+    is( $status >> 8, 0, "The mail gateway exited normally" );
+    TODO: {
+          local $TODO = "Currently don't handle non-ASCII in sender";
+          ok( $id, "Created ticket" );
+      }
+}
+
+diag "No sender";
+{
+    my $mail = <<'.';
+To: rt at example.com
+Subject: testing non-ASCII From
+Content-Type: text/plain; charset=iso-8859-1
+
+here's some content
+.
+
+    my ($status, $id);
+    warning_like { ( $status, $id ) = RT::Test->send_via_mailgate($mail) }
+      qr/Couldn't parse or find sender's address/,
+        'Got parse error';
+    is( $status >> 8, 0, "The mail gateway exited normally" );
+    ok( !$id, "No ticket created" );
+}

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