[Rt-commit] rt branch, 4.0/warn-on-non-ASCII-email-headers, updated. rt-4.0.6-253-g2bc2f25
Alex Vandiver
alexmv at bestpractical.com
Mon Dec 17 16:45:03 EST 2012
The branch, 4.0/warn-on-non-ASCII-email-headers has been updated
via 2bc2f255d0e1bd44ccb95541a1d5b3763d7e996e (commit)
via 788d68771ea369c7ca1ca4f5a17b00a381d704d9 (commit)
from 1ba0f1a96e4fb7f01bd896ebd1b56d840c2e2116 (commit)
Summary of changes:
lib/RT/Interface/Email.pm | 21 +++++++++------------
lib/RT/Interface/Email/Auth/MailFrom.pm | 5 +++--
t/mail/header-characters.t | 13 +++----------
3 files changed, 15 insertions(+), 24 deletions(-)
- Log -----------------------------------------------------------------
commit 788d68771ea369c7ca1ca4f5a17b00a381d704d9
Author: Alex Vandiver <alexmv at bestpractical.com>
Date: Mon Dec 17 16:38:38 2012 -0500
Make the caller of ParseSenderAddressFromHead responsible for parse errors
There are two callsites for ParseSenderAddressFromHead; one in
RT::Interface::Email::Auth::MailFrom->GetCurrentUser, and one in
RT::Interface::Email->CheckForSuspiciousSender. The former already
warns if there are no addresses; and in the latter, the lack of
addresses is not a failure.
This reduces the number of warnings generated when there are parse
failures.
diff --git a/lib/RT/Interface/Email.pm b/lib/RT/Interface/Email.pm
index b4d13af..f98c48b 100644
--- a/lib/RT/Interface/Email.pm
+++ b/lib/RT/Interface/Email.pm
@@ -1098,9 +1098,6 @@ sub ParseSenderAddressFromHead {
$error_msg .= "$header: $addr_line";
}
- $RT::Logger->warning("Couldn't find a sender in "
- . join(', ', @sender_headers) );
-
return (undef, undef, $error_msg);
}
diff --git a/t/mail/header-characters.t b/t/mail/header-characters.t
index ece9c5d..c0b6db0 100644
--- a/t/mail/header-characters.t
+++ b/t/mail/header-characters.t
@@ -25,9 +25,7 @@ here's some content
my ($status, $id);
warnings_like { ( $status, $id ) = RT::Test->send_via_mailgate($mail) }
- [qr/Couldn't find a sender in/,
- qr/Couldn't find a sender in/,
- qr/Failed to parse sender using/,
+ [qr/Failed to parse sender using/,
qr/Couldn't parse or find sender's address/
],
'Got parse error for non-ASCII in From';
@@ -55,9 +53,7 @@ here's some content
my ($status, $id);
warnings_like { ( $status, $id ) = RT::Test->send_via_mailgate($mail) }
- [qr/Couldn't find a sender in/,
- qr/Couldn't find a sender in/,
- qr/Failed to parse sender using/,
+ [qr/Failed to parse sender using/,
qr/Couldn't parse or find sender's address/
],
'Got parse error for iso-8859-1 in From';
@@ -80,10 +76,7 @@ here's some content
my ($status, $id);
warnings_like { ( $status, $id ) = RT::Test->send_via_mailgate($mail) }
- [qr/Couldn't find a sender in/,
- qr/Couldn't find a sender in/,
- qr/Couldn't parse or find sender's address/
- ],
+ [qr/Couldn't parse or find sender's address/],
'Got parse error with no sender fields';
is( $status >> 8, 0, "The mail gateway exited normally" );
ok( !$id, "No ticket created" );
commit 2bc2f255d0e1bd44ccb95541a1d5b3763d7e996e
Author: Alex Vandiver <alexmv at bestpractical.com>
Date: Mon Dec 17 16:44:59 2012 -0500
For formatting, return a list of parse failures, and join in the caller
diff --git a/lib/RT/Interface/Email.pm b/lib/RT/Interface/Email.pm
index f98c48b..2051338 100644
--- a/lib/RT/Interface/Email.pm
+++ b/lib/RT/Interface/Email.pm
@@ -1071,34 +1071,34 @@ sub ParseCcAddressesFromHead {
=head2 ParseSenderAddressFromHead HEAD
-Takes a MIME::Header object. Returns (user at host, friendly name, error msg)
+Takes a MIME::Header object. Returns (user at host, friendly name, errors)
where the first two values are the From (evaluated in order of
Reply-To:, From:, Sender).
-An error msg may be returned even when a Sender value is found, since it
-could be a parse error for another sender field. In this case, the
-error isn't fatal, but may be useful to investigate the parse failure.
+A list of error messages may be returned even when a Sender value is
+found, since it could be a parse error for another (checked earlier)
+sender field. In this case, the errors aren't fatal, but may be useful
+to investigate the parse failure.
=cut
sub ParseSenderAddressFromHead {
my $head = shift;
my @sender_headers = ('Reply-To', 'From', 'Sender');
- my $error_msg; # Accumulate any errors
+ my @errors; # Accumulate any errors
#Figure out who's sending this message.
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, $error_msg) if $addr;
+ return ($addr, $name, @errors) if $addr;
chomp $addr_line;
- $error_msg = "Failed to parse sender using " if not $error_msg;
- $error_msg .= "$header: $addr_line";
+ push @errors, "$header: $addr_line";
}
- return (undef, undef, $error_msg);
+ return (undef, undef, @errors);
}
=head2 ParseErrorsToAddressFromHead HEAD
diff --git a/lib/RT/Interface/Email/Auth/MailFrom.pm b/lib/RT/Interface/Email/Auth/MailFrom.pm
index e7fcd97..84ed7d5 100644
--- a/lib/RT/Interface/Email/Auth/MailFrom.pm
+++ b/lib/RT/Interface/Email/Auth/MailFrom.pm
@@ -66,8 +66,9 @@ sub GetCurrentUser {
# We don't need to do any external lookups
- my ( $Address, $Name, $Error_msg ) = ParseSenderAddressFromHead( $args{'Message'}->head );
- $RT::Logger->warning($Error_msg) if $Error_msg;
+ my ( $Address, $Name, @errors ) = ParseSenderAddressFromHead( $args{'Message'}->head );
+ $RT::Logger->warning("Failed to parse ".join(', ', @errors))
+ if @errors;
unless ( $Address ) {
$RT::Logger->error("Couldn't parse or find sender's address");
diff --git a/t/mail/header-characters.t b/t/mail/header-characters.t
index c0b6db0..fe06d1a 100644
--- a/t/mail/header-characters.t
+++ b/t/mail/header-characters.t
@@ -25,7 +25,7 @@ here's some content
my ($status, $id);
warnings_like { ( $status, $id ) = RT::Test->send_via_mailgate($mail) }
- [qr/Failed to parse sender using/,
+ [qr/Failed to parse Reply-To:.*, From:/,
qr/Couldn't parse or find sender's address/
],
'Got parse error for non-ASCII in From';
@@ -53,7 +53,7 @@ here's some content
my ($status, $id);
warnings_like { ( $status, $id ) = RT::Test->send_via_mailgate($mail) }
- [qr/Failed to parse sender using/,
+ [qr/Failed to parse Reply-To:.*, From:/,
qr/Couldn't parse or find sender's address/
],
'Got parse error for iso-8859-1 in From';
-----------------------------------------------------------------------
More information about the Rt-commit
mailing list