You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@spamassassin.apache.org by mm...@apache.org on 2012/10/23 21:12:18 UTC
svn commit: r1401393 - in /spamassassin/trunk/lib/Mail/SpamAssassin:
Conf/Parser.pm Message.pm Plugin/MIMEEval.pm Plugin/VBounce.pm Util.pm
Author: mmartinec
Date: Tue Oct 23 19:12:17 2012
New Revision: 1401393
URL: http://svn.apache.org/viewvc?rev=1401393&view=rev
Log:
Bug 6854: Optimizations, profiling
Modified:
spamassassin/trunk/lib/Mail/SpamAssassin/Conf/Parser.pm
spamassassin/trunk/lib/Mail/SpamAssassin/Message.pm
spamassassin/trunk/lib/Mail/SpamAssassin/Plugin/MIMEEval.pm
spamassassin/trunk/lib/Mail/SpamAssassin/Plugin/VBounce.pm
spamassassin/trunk/lib/Mail/SpamAssassin/Util.pm
Modified: spamassassin/trunk/lib/Mail/SpamAssassin/Conf/Parser.pm
URL: http://svn.apache.org/viewvc/spamassassin/trunk/lib/Mail/SpamAssassin/Conf/Parser.pm?rev=1401393&r1=1401392&r2=1401393&view=diff
==============================================================================
--- spamassassin/trunk/lib/Mail/SpamAssassin/Conf/Parser.pm (original)
+++ spamassassin/trunk/lib/Mail/SpamAssassin/Conf/Parser.pm Tue Oct 23 19:12:17 2012
@@ -944,11 +944,13 @@ sub _meta_deps_recurse {
my @tokens = ($rule =~ m/$lexer/g);
# Go through each token in the meta rule
+ my $conf_tests = $conf->{tests};
foreach my $token (@tokens) {
# has to be an alpha+numeric token
- next if ($token =~ /^(?:\W+|[+-]?\d+(?:\.\d+)?)$/);
+ # next if $token =~ /^(?:\W+|[+-]?\d+(?:\.\d+)?)$/;
+ next if $token !~ /^_*[A-Za-z][A-Za-z0-9_]*\z/s; # faster
# and has to be a rule name
- next unless exists $conf->{tests}->{$token};
+ next unless exists $conf_tests->{$token};
# add and recurse
push(@{$deps}, untaint_var($token));
Modified: spamassassin/trunk/lib/Mail/SpamAssassin/Message.pm
URL: http://svn.apache.org/viewvc/spamassassin/trunk/lib/Mail/SpamAssassin/Message.pm?rev=1401393&r1=1401392&r2=1401393&view=diff
==============================================================================
--- spamassassin/trunk/lib/Mail/SpamAssassin/Message.pm (original)
+++ spamassassin/trunk/lib/Mail/SpamAssassin/Message.pm Tue Oct 23 19:12:17 2012
@@ -208,8 +208,8 @@ sub new {
# bug 4363
# Check to see if we should do CRLF instead of just LF
- # For now, just check the first header and do whatever it does
- if (@message && $message[0] =~ /\015\012/) {
+ # For now, just check the first and last line and do whatever it does
+ if (@message && ($message[0] =~ /\015\012/ || $message[-1] =~ /\015\012/)) {
$self->{line_ending} = "\015\012";
dbg("message: line ending changed to CRLF");
}
@@ -306,15 +306,17 @@ sub new {
$self->{'pristine_body_length'} = length($self->{'pristine_body'});
}
- # CRLF -> LF
+ # CRLF -> LF (but avoid expensive operation unless necessary)
# also merge multiple blank lines into a single one
+ my $squash_crlf = $self->{line_ending} eq "\015\012";
my $start;
# iterate over lines in reverse order
for (my $cnt=$#message; $cnt>=0; $cnt--) {
- $message[$cnt] =~ s/\015\012/\012/;
+ $message[$cnt] =~ s/\015\012/\012/ if $squash_crlf;
# line is blank
- if ($message[$cnt] !~ /\S/) {
+ if ($message[$cnt] =~ /^\s*$/) {
+ # /^\s*$/ is about 5% faster then !/\S/, but still expensive here
if (!defined $start) {
$start=$cnt;
}
@@ -809,9 +811,10 @@ sub _parse_multipart {
my $line;
my $tmp_line = @{$body};
for ($line=0; $line < $tmp_line; $line++) {
-# dbg("message: multipart line $line: \"" . $body->[$line] . "\"");
+# dbg("message: multipart line $line: \"" . $body->[$line] . "\"");
# specifically look for an opening boundary
- if ($body->[$line] =~ /^--\Q$boundary\E\s*$/) {
+ if (substr($body->[$line],0,2) eq '--' # triage
+ && $body->[$line] =~ /^--\Q$boundary\E\s*$/) {
# Make note that we found the opening boundary
$self->{mime_boundary_state}->{$boundary} = 1;
@@ -843,8 +846,10 @@ sub _parse_multipart {
my $line_count = @{$body};
foreach ( @{$body} ) {
# if we're on the last body line, or we find any boundary marker,
- # deal with the mime part
- $found_end_boundary = defined $boundary && /^--\Q$boundary\E(?:--)?\s*$/;
+ # deal with the mime part;
+ # a triage before an unlikely-to-match regexp avoids a CPU hotspot
+ $found_end_boundary = defined $boundary && substr($_,0,2) eq '--'
+ && /^--\Q$boundary\E(?:--)?\s*$/;
if ( --$line_count == 0 || $found_end_boundary ) {
my $line = $_; # remember the last line
Modified: spamassassin/trunk/lib/Mail/SpamAssassin/Plugin/MIMEEval.pm
URL: http://svn.apache.org/viewvc/spamassassin/trunk/lib/Mail/SpamAssassin/Plugin/MIMEEval.pm?rev=1401393&r1=1401392&r2=1401393&view=diff
==============================================================================
--- spamassassin/trunk/lib/Mail/SpamAssassin/Plugin/MIMEEval.pm (original)
+++ spamassassin/trunk/lib/Mail/SpamAssassin/Plugin/MIMEEval.pm Tue Oct 23 19:12:17 2012
@@ -227,7 +227,7 @@ sub _check_attachments {
$pms->{mime_checked_attachments} = 1;
# results
- $pms->{mime_base64_blanks} = 0;
+# $pms->{mime_base64_blanks} = 0; # expensive to determine, no longer avail
$pms->{mime_base64_count} = 0;
$pms->{mime_base64_encoded_text} = 0;
# $pms->{mime_base64_illegal} = 0;
@@ -275,17 +275,19 @@ sub _check_attachments {
$part_type[$part] = $ctype;
$part_bytes[$part] = 0 if $cd !~ /attachment/;
+ my $cte_is_base64 = $cte =~ /base64/i;
my $previous = '';
foreach (@{$p->raw()}) {
- if ($cte =~ /base64/i) {
- if ($previous =~ /^\s*$/ && /^\s*$/) {
- $pms->{mime_base64_blanks} = 1;
- }
- # MIME_BASE64_ILLEGAL: now a zero-hitter
- # if (m@[^A-Za-z0-9+/=\n]@ || /=[^=\s]/) {
- # $pms->{mime_base64_illegal} = 1;
- # }
- }
+
+ # if ($cte_is_base64) {
+ # if ($previous =~ /^\s*$/ && /^\s*$/) { # expensive, avoid!
+ # $pms->{mime_base64_blanks} = 1; # never used, don't bother
+ # }
+ # # MIME_BASE64_ILLEGAL: now a zero-hitter
+ # # if (m@[^A-Za-z0-9+/=\n]@ || /=[^=\s]/) {
+ # # $pms->{mime_base64_illegal} = 1;
+ # # }
+ # }
# if ($pms->{mime_html_no_charset} && $ctype eq 'text/html' && defined $charset) {
# $pms->{mime_html_no_charset} = 0;
@@ -510,8 +512,7 @@ sub _check_base64_length {
my $cte = lc($p->get_header('content-transfer-encoding') || '');
next if ($cte !~ /^base64$/);
foreach my $l ( @{$p->raw()} ) {
- my $len = length $l;
- $result = $len if ($len > $result);
+ $result = length $l if length $l > $result;
}
}
Modified: spamassassin/trunk/lib/Mail/SpamAssassin/Plugin/VBounce.pm
URL: http://svn.apache.org/viewvc/spamassassin/trunk/lib/Mail/SpamAssassin/Plugin/VBounce.pm?rev=1401393&r1=1401392&r2=1401393&view=diff
==============================================================================
--- spamassassin/trunk/lib/Mail/SpamAssassin/Plugin/VBounce.pm (original)
+++ spamassassin/trunk/lib/Mail/SpamAssassin/Plugin/VBounce.pm Tue Oct 23 19:12:17 2012
@@ -92,13 +92,15 @@ C<whitelist_bounce_relays> lines are als
sub have_any_bounce_relays {
my ($self, $pms) = @_;
- return (defined $pms->{conf}->{whitelist_bounce_relays} &&
- (scalar values %{$pms->{conf}->{whitelist_bounce_relays}} != 0));
+ return $pms->{conf}->{whitelist_bounce_relays} &&
+ %{$pms->{conf}->{whitelist_bounce_relays}};
}
sub check_whitelist_bounce_relays {
my ($self, $pms) = @_;
+ return 0 if !$self->have_any_bounce_relays($pms);
+
my $body = $pms->get_decoded_stripped_body_text_array();
my $res;
@@ -118,6 +120,10 @@ sub check_whitelist_bounce_relays {
# fixed, otherwise we'll miss some messages due to their MIME structure
my $pristine = $pms->{msg}->get_pristine_body();
+
+ # triage, avoids expensive loop through large mail with attachments
+ return 0 if $pristine !~ /Received:/i;
+
my $found_received = 0;
my $fullhdr = '';
foreach my $line ($pristine =~ /^(.*)$/gm) {
Modified: spamassassin/trunk/lib/Mail/SpamAssassin/Util.pm
URL: http://svn.apache.org/viewvc/spamassassin/trunk/lib/Mail/SpamAssassin/Util.pm?rev=1401393&r1=1401392&r2=1401393&view=diff
==============================================================================
--- spamassassin/trunk/lib/Mail/SpamAssassin/Util.pm (original)
+++ spamassassin/trunk/lib/Mail/SpamAssassin/Util.pm Tue Oct 23 19:12:17 2012
@@ -280,36 +280,43 @@ sub untaint_hostname {
# untaint_var(\%ENV);
#
sub untaint_var {
- no re 'taint'; # override a "use re 'taint'" from outer scope
- local ($_) = @_;
- return unless defined;
-
- unless (ref) {
+# my $arg = $_[0]; # avoid copying unnecessarily
+ my $r = ref $_[0];
+ if (!$r) {
+ no re 'taint'; # override a "use re 'taint'" from outer scope
+ return if !defined $_[0];
local($1); # avoid Perl taint bug: tainted global $1 propagates taintedness
- /^(.*)\z/s;
+ $_[0] =~ /^(.*)\z/s;
return $1;
}
- elsif (ref eq 'ARRAY') {
- $_ = untaint_var($_) for @{$_};
- return @{$_} if wantarray;
- }
- elsif (ref eq 'HASH') {
- while (my ($k, $v) = each %{$_}) {
- if (!defined $v && $_ == \%ENV) {
- delete ${$_}{$k};
- next;
+ elsif ($r eq 'ARRAY') {
+ my $arg = $_[0];
+ $_ = untaint_var($_) for @{$arg};
+ return @{$arg} if wantarray;
+ }
+ elsif ($r eq 'HASH') {
+ my $arg = $_[0];
+ if ($arg == \%ENV) { # purge undefs from %ENV, untaint the rest
+ while (my($k, $v) = each %{$arg}) {
+ # It is safe to delete the item most recently returned by each()
+ if (!defined $v) { delete ${$arg}{$k}; next }
+ ${$arg}{untaint_var($k)} = untaint_var($v);
+ }
+ } else {
+ while (my($k, $v) = each %{$arg}) {
+ ${$arg}{untaint_var($k)} = untaint_var($v);
}
- ${$_}{untaint_var($k)} = untaint_var($v);
}
- return %{$_} if wantarray;
+ return %{$arg} if wantarray;
}
- elsif (ref eq 'SCALAR' or ref eq 'REF') {
- ${$_} = untaint_var(${$_});
+ elsif ($r eq 'SCALAR' || $r eq 'REF') {
+ my $arg = $_[0];
+ ${$arg} = untaint_var(${$arg});
}
else {
- warn "util: can't untaint a " . ref($_) . "!\n";
+ warn "util: can't untaint a $r !\n";
}
- return $_;
+ return $_[0];
}
###########################################################################