Merge pull request #443 from sisimai/import-pr-442

Import Pull-Request #442

ChangeLog.md

@@ -16,6 +16,13 @@ v4.25.11p2
     Thanks to @vhenon
   - Add `rfc3464-41.eml` and `rfc3464-42.eml`
   - Remove all the HTML elements from the value of "diagnosticcode".
+  - #443 Fix serious bugs: imported from #442
+    - Values of `deliverystatus` and `replycode` detected from the message body
+      did not use at `Sisimai::Lhost::Exim`.
+    - `true()` method strictly checks the value of `smtpcommand` at some classes
+      in `Sisimai::Reason`. For example, when a detected reason is `spamdetected`
+      and `virusdetected` the value of `smtpcommand` should be `DATA` or an SMTP
+      command to be sent after `DATA`.
 
 v4.25.11
 --------------------------------------------------------------------------------

lib/Sisimai/Lhost/Exim.pm

@@ -435,8 +435,8 @@ sub make {
             #   Diagnostic-Code: smtp; 450 TEMPERROR: retry timeout exceeded
             # The value of "Status:" indicates permanent error but the value
             # of SMTP reply code in Diagnostic-Code: field is "TEMPERROR"!!!!
-            my $sv = Sisimai::SMTP::Status->find($e->{'diagnosis'}) || '';
-            my $rv = Sisimai::SMTP::Reply->find($e->{'diagnosis'})  || '';
+            my $sv = $e->{'status'}    || Sisimai::SMTP::Status->find($e->{'diagnosis'}) || '';
+            my $rv = $e->{'replycode'} || Sisimai::SMTP::Reply->find($e->{'diagnosis'})  || '';
             my $s1 = 0; # First character of Status as integer
             my $r1 = 0; # First character of SMTP reply code as integer
             my $v1 = 0;
@@ -524,7 +524,7 @@ azumakuniyuki
 
 =head1 COPYRIGHT
 
-Copyright (C) 2014-2020 azumakuniyuki, All rights reserved.
+Copyright (C) 2014-2021 azumakuniyuki, All rights reserved.
 
 =head1 LICENSE
 

lib/Sisimai/Reason/Filtered.pm

@@ -49,15 +49,18 @@ sub true {
 
     require Sisimai::Reason::UserUnknown;
     my $alterclass = 'Sisimai::Reason::UserUnknown';
-    my $commandtxt = $argvs->smtpcommand // '';
     my $diagnostic = lc $argvs->diagnosticcode // '';
 
     if( $tempreason eq 'filtered' ) {
         # Delivery status code points "filtered".
-        return 1 if( $alterclass->match($diagnostic) || __PACKAGE__->match($diagnostic) );
+        return 1 if $alterclass->match($diagnostic);
+        return 1 if __PACKAGE__->match($diagnostic);
 
-    } elsif( $commandtxt ne 'RCPT' && $commandtxt ne 'MAIL' ) {
-        # Check the value of Diagnostic-Code and the last SMTP command
+    } else {
+        # The value of "reason" isn't "filtered" when the value of "smtpcommand" is an SMTP command
+        # to be sent before the SMTP DATA command because all the MTAs read the headers and the
+        # entire message body after the DATA command.
+        return 0 if $argvs->{'smtpcommand'} =~ /\A(?:CONN|EHLO|HELO|MAIL|RCPT)\z/;
         return 1 if __PACKAGE__->match($diagnostic);
         return 1 if $alterclass->match($diagnostic);
     }

lib/Sisimai/Reason/NoRelaying.pm

@@ -47,14 +47,9 @@ sub true {
     my $class = shift;
     my $argvs = shift // return undef;
 
-    if( my $r = $argvs->reason // '' ) {
-        # Do not overwrite the reason
-        return 0 if( $r eq 'securityerror' || $r eq 'systemerror' || $r eq 'undefined' );
-    } else {
-        # Check the value of Diagnosic-Code: header with patterns
-        return 1 if __PACKAGE__->match(lc $argvs->diagnosticcode);
-    }
-    return 0;
+    return 0 if $argvs->{'reason'}      =~ /\A(?:securityerror|systemerror|undefined)\z/;
+    return 0 if $argvs->{'smtpcommand'} =~ /\A(?:CONN|EHLO|HELO)\z/;
+    return 1 if __PACKAGE__->match(lc $argvs->{'diagnosticcode'});return 0;
 }
 
 1;

lib/Sisimai/Reason/NotAccept.pm

@@ -40,7 +40,7 @@ sub true {
 
     # SMTP Reply Code is 521, 554 or 556
     return 1 if $argvs->replycode =~ /\A(?:521|554|556)\z/;
-    return 0 unless $argvs->smtpcommand eq 'MAIL';
+    return 0 if $argvs->smtpcommand ne 'MAIL';
     return 1 if __PACKAGE__->match(lc $argvs->diagnosticcode);
     return 0;
 }

lib/Sisimai/Reason/PolicyViolation.pm

@@ -44,7 +44,7 @@ sub match {
 }
 
 sub true {
-    # The bounce reason is security error or not
+    # The bounce reason is "policyviolation" or not
     # @param    [Sisimai::Data] argvs   Object to be detected the reason
     # @return   [Integer]               1: is policy violation
     #                                   0: is not policyviolation

lib/Sisimai/Reason/SpamDetected.pm

@@ -157,6 +157,11 @@ sub true {
     return undef unless $argvs->deliverystatus;
     return 1 if $argvs->reason eq 'spamdetected';
     return 1 if (Sisimai::SMTP::Status->name($argvs->deliverystatus) || '') eq 'spamdetected';
+
+    # The value of "reason" isn't "spamdetected" when the value of "smtpcommand" is an SMTP command
+    # to be sent before the SMTP DATA command because all the MTAs read the headers and the entire
+    # message body after the DATA command.
+    return 0 if $argvs->{'smtpcommand'} =~ /\A(?:CONN|EHLO|HELO|MAIL|RCPT)\z/;
     return 1 if __PACKAGE__->match(lc $argvs->diagnosticcode);
     return 0;
 }

lib/Sisimai/Reason/VirusDetected.pm

@@ -32,7 +32,16 @@ sub true {
     #                                   0: virus was not detected
     # @since v4.22.0
     # @see http://www.ietf.org/rfc/rfc2822.txt
-    return undef;
+    my $class = shift;
+    my $argvs = shift // return undef;
+
+    # The value of "reason" isn't "virusdetected" when the value of "smtpcommand" is an SMTP com-
+    # mand to be sent before the SMTP DATA command because all the MTAs read the headers and the
+    # entire message body after the DATA command.
+    return 1 if $argvs->{'reason'} eq 'virusdetected';
+    return 0 if $argvs->{'smtpcommand'} =~ /\A(?:CONN|EHLO|HELO|MAIL|RCPT)\z/;
+    return 1 if __PACKAGE__->match(lc $argvs->{'diagnosticcode'});
+    return 0;
 }
 
 1;
@@ -90,7 +99,7 @@ azumakuniyuki
 
 =head1 COPYRIGHT
 
-Copyright (C) 2017-2019 azumakuniyuki, All rights reserved.
+Copyright (C) 2017-2019,2021 azumakuniyuki, All rights reserved.
 
 =head1 LICENSE
 

t/618-lhost-exim.t

@@ -15,7 +15,7 @@ my $isexpected = [
     { 'n' => '06', 's' => qr/\A4[.]0[.]\d+\z/,    'r' => qr/expired/,        'b' => qr/\A1\z/ },
     { 'n' => '07', 's' => qr/\A4[.]0[.]\d+\z/,    'r' => qr/mailboxfull/,    'b' => qr/\A1\z/ },
     { 'n' => '08', 's' => qr/\A4[.]0[.]\d+\z/,    'r' => qr/expired/,        'b' => qr/\A1\z/ },
-    { 'n' => '29', 's' => qr/\A5[.]0[.]\d+\z/,    'r' => qr/blocked/,        'b' => qr/\A1\z/ },
+    { 'n' => '29', 's' => qr/\A5[.]0[.]0\z/,      'r' => qr/blocked/,        'b' => qr/\A1\z/ },
     { 'n' => '30', 's' => qr/\A5[.]7[.]1\z/,      'r' => qr/userunknown/,    'b' => qr/\A0\z/ },
     { 'n' => '31', 's' => qr/\A5[.]0[.]\d+\z/,    'r' => qr/hostunknown/,    'b' => qr/\A0\z/ },
     { 'n' => '32', 's' => qr/\A5[.]0[.]\d+\z/,    'r' => qr/blocked/,        'b' => qr/\A1\z/ },
@@ -35,7 +35,7 @@ my $isexpected = [
     { 'n' => '46', 's' => qr/\A5[.]7[.]1\z/,      'r' => qr/blocked/,        'b' => qr/\A1\z/ },
     { 'n' => '47', 's' => qr/\A5[.]0[.]\d+\z/,    'r' => qr/blocked/,        'b' => qr/\A1\z/ },
     { 'n' => '48', 's' => qr/\A5[.]7[.]1\z/,      'r' => qr/rejected/,       'b' => qr/\A1\z/ },
-    { 'n' => '49', 's' => qr/\A5[.]0[.]\d+\z/,    'r' => qr/blocked/,        'b' => qr/\A1\z/ },
+    { 'n' => '49', 's' => qr/\A5[.]0[.]0/,        'r' => qr/blocked/,        'b' => qr/\A1\z/ },
     { 'n' => '50', 's' => qr/\A5[.]1[.]7\z/,      'r' => qr/rejected/,       'b' => qr/\A1\z/ },
     { 'n' => '51', 's' => qr/\A5[.]1[.]0\z/,      'r' => qr/rejected/,       'b' => qr/\A1\z/ },
     { 'n' => '52', 's' => qr/\A5[.]0[.]\d+\z/,    'r' => qr/syntaxerror/,    'b' => qr/\A1\z/ },

xt/619-lhost-ezweb.t

@@ -114,7 +114,7 @@ my $isexpected = [
     { 'n' => '01105', 'r' => qr/filtered/    },
     { 'n' => '01106', 'r' => qr/userunknown/ },
     { 'n' => '01107', 'r' => qr/filtered/    },
-    { 'n' => '01108', 'r' => qr/userunknown/ },
+    { 'n' => '01108', 'r' => qr/norelaying/  },
     { 'n' => '01109', 'r' => qr/userunknown/ },
     { 'n' => '01110', 'r' => qr/filtered/    },
     { 'n' => '01111', 'r' => qr/suspend/     },