Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Bug fix and improvement 2021 Nov #442

Merged
merged 7 commits into from
Nov 19, 2021
Merged

Bug fix and improvement 2021 Nov #442

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
227a2ae
Fix a serious bug in code for setting the values of "deliverystatus" …
azumakuniyuki Nov 19, 2021
820a300
Fix a comment in Sisimai::Reason->PolicyViolation->true method
azumakuniyuki Nov 19, 2021
2401c9b
Tiny code improvement and strictly checks the value of "smtpcommand"
azumakuniyuki Nov 19, 2021
2fadc6e
Tiny code improvement and strictly checks the value of "smtpcommand"
azumakuniyuki Nov 19, 2021
cfe9b0f
Tiny code improvement
azumakuniyuki Nov 19, 2021
e977bf7
Strictly checks the value of "smtpcommand"
azumakuniyuki Nov 19, 2021
177b3c5
These changes will be back ported to `master` and `4-stable` branches
azumakuniyuki Nov 19, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions ChangeLog.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,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".
- Fix a serious bugs:
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fix serious bugs:

- Values of `deliverystatus` and `replycode` detected from the message body
did not used at `Sisimai::Lhost::Exim`.
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

did not use

- `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
--------------------------------------------------------------------------------
Expand Down
4 changes: 2 additions & 2 deletions lib/Sisimai/Lhost/Exim.pm
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -434,8 +434,8 @@ sub inquire {
#
# 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;
Expand Down
17 changes: 9 additions & 8 deletions lib/Sisimai/Reason/Filtered.pm
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,18 +48,19 @@ sub true {
return 0 if $tempreason eq 'suspend';

require Sisimai::Reason::UserUnknown;
my $alterclass = 'Sisimai::Reason::UserUnknown';
my $commandtxt = $argvs->{'smtpcommand'} // '';
my $diagnostic = lc $argvs->{'diagnosticcode'} // '';

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 Sisimai::Reason::UserUnknown->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);
return 1 if Sisimai::Reason::UserUnknown->match($diagnostic);
}
return 0;
}
Expand Down
11 changes: 3 additions & 8 deletions lib/Sisimai/Reason/NoRelaying.pm
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,14 +48,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 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;
}

Expand Down
2 changes: 1 addition & 1 deletion lib/Sisimai/Reason/NotAccept.pm
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,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;
}
Expand Down
2 changes: 1 addition & 1 deletion lib/Sisimai/Reason/PolicyViolation.pm
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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::Fact] argvs Object to be detected the reason
# @return [Integer] 1: is policy violation
# 0: is not policyviolation
Expand Down
5 changes: 5 additions & 0 deletions lib/Sisimai/Reason/SpamDetected.pm
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down
13 changes: 11 additions & 2 deletions lib/Sisimai/Reason/VirusDetected.pm
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,13 +27,22 @@ sub match {
}

sub true {
# The bounce reason is security error or not
# The bounce reason is "virusdetected" or not
# @param [Sisimai::Fact] argvs Object to be detected the reason
# @return [Integer] 1: virus detected
# 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;
Expand Down
4 changes: 2 additions & 2 deletions t/653-lhost-exim.t
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ my $isexpected = {
'06' => [['4.0.947', '', 'expired', 0]],
'07' => [['4.0.922', '', 'mailboxfull', 0]],
'08' => [['4.0.947', '', 'expired', 0]],
'29' => [['5.0.901', '550', 'blocked', 0]],
'29' => [['5.0.0', '550', 'blocked', 0]],
'30' => [['5.7.1', '554', 'userunknown', 1]],
'31' => [['5.0.912', '', 'hostunknown', 1]],
'32' => [['5.0.971', '571', 'blocked', 0]],
Expand All @@ -37,7 +37,7 @@ my $isexpected = {
'46' => [['5.7.1', '554', 'blocked', 0]],
'47' => [['5.0.971', '550', 'blocked', 0]],
'48' => [['5.7.1', '550', 'rejected', 0]],
'49' => [['5.0.971', '550', 'blocked', 0]],
'49' => [['5.0.0', '550', 'blocked', 0]],
'50' => [['5.1.7', '550', 'rejected', 0]],
'51' => [['5.1.0', '553', 'rejected', 0]],
'52' => [['5.0.902', '', 'syntaxerror', 0]],
Expand Down
28 changes: 14 additions & 14 deletions xt/653-lhost-exim.t
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -89,13 +89,13 @@ my $isexpected = {
'01081' => [['5.0.0', '', 'hostunknown', 1]],
'01082' => [['5.0.901', '', 'onhold', 0]],
'01083' => [['5.0.0', '', 'onhold', 0]],
'01084' => [['5.0.930', '550', 'systemerror', 0]],
'01085' => [['5.0.971', '550', 'blocked', 0],
'01084' => [['5.0.0', '550', 'systemerror', 0]],
'01085' => [['5.0.0', '550', 'blocked', 0],
['5.0.971', '550', 'blocked', 0]],
'01086' => [['5.0.0', '', 'onhold', 0]],
'01087' => [['5.0.901', '550', 'onhold', 0]],
'01087' => [['5.0.0', '550', 'onhold', 0]],
'01088' => [['5.0.901', '550', 'onhold', 0],
['5.0.901', '550', 'onhold', 0]],
['5.0.0', '550', 'onhold', 0]],
'01089' => [['5.0.0', '', 'mailererror', 0]],
'01090' => [['5.0.0', '', 'onhold', 0]],
'01091' => [['5.0.0', '', 'onhold', 0]],
Expand Down Expand Up @@ -135,7 +135,7 @@ my $isexpected = {
'01125' => [['5.7.1', '554', 'blocked', 0]],
'01126' => [['5.0.971', '550', 'blocked', 0]],
'01127' => [['5.7.1', '550', 'rejected', 0]],
'01128' => [['5.0.971', '550', 'blocked', 0]],
'01128' => [['5.0.0', '550', 'blocked', 0]],
'01129' => [['5.1.7', '550', 'rejected', 0]],
'01130' => [['5.1.0', '553', 'rejected', 0]],
'01131' => [['5.0.902', '', 'syntaxerror', 0]],
Expand All @@ -157,12 +157,12 @@ my $isexpected = {
'01148' => [['5.0.980', '550', 'spamdetected', 0]],
'01149' => [['5.0.901', '550', 'rejected', 0]],
'01150' => [['5.7.1', '553', 'blocked', 0]],
'01151' => [['5.0.921', '550', 'suspend', 0]],
'01152' => [['5.0.971', '550', 'blocked', 0]],
'01153' => [['5.0.971', '550', 'blocked', 0]],
'01151' => [['5.0.0', '550', 'suspend', 0]],
'01152' => [['5.0.0', '550', 'blocked', 0]],
'01153' => [['5.0.0', '550', 'blocked', 0]],
'01154' => [['5.7.1', '553', 'blocked', 0]],
'01155' => [['5.0.971', '550', 'blocked', 0]],
'01156' => [['5.0.971', '550', 'blocked', 0]],
'01155' => [['5.0.0', '550', 'blocked', 0]],
'01156' => [['5.0.0', '550', 'blocked', 0]],
'01157' => [['5.0.0', '', 'spamdetected', 0]],
'01158' => [['5.0.0', '', 'filtered', 0]],
'01159' => [['5.0.0', '', 'spamdetected', 0]],
Expand All @@ -181,7 +181,7 @@ my $isexpected = {
'01172' => [['5.0.0', '', 'hostunknown', 1]],
'01173' => [['5.0.0', '', 'networkerror', 0]],
'01175' => [['5.0.0', '', 'expired', 0]],
'01176' => [['5.0.911', '550', 'userunknown', 1]],
'01176' => [['5.0.0', '550', 'userunknown', 1]],
'01177' => [['5.0.0', '', 'filtered', 0]],
'01178' => [['4.0.947', '', 'expired', 0]],
'01179' => [['5.0.0', '', 'mailererror', 0]],
Expand All @@ -190,13 +190,13 @@ my $isexpected = {
'01182' => [['5.1.1', '550', 'userunknown', 1]],
'01183' => [['5.0.0', '', 'mailboxfull', 0]],
'01184' => [['5.1.1', '550', 'userunknown', 1]],
'01185' => [['5.0.921', '554', 'suspend', 0]],
'01186' => [['5.0.911', '550', 'userunknown', 1]],
'01185' => [['5.0.0', '554', 'suspend', 0]],
'01186' => [['5.0.0', '550', 'userunknown', 1]],
'01187' => [['5.0.0', '', 'hostunknown', 1]],
'01188' => [['5.2.0', '550', 'spamdetected', 0]],
'01189' => [['5.0.0', '', 'expired', 0]],
'01190' => [['5.0.0', '', 'hostunknown', 1]],
'01191' => [['5.0.921', '550', 'suspend', 0]],
'01191' => [['5.0.0', '550', 'suspend', 0]],
};

plan 'skip_all', sprintf("%s not found", $samplepath) unless -d $samplepath;
Expand Down
2 changes: 1 addition & 1 deletion xt/654-lhost-ezweb.t
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,7 @@ my $isexpected = {
'01105' => [['5.0.910', '', 'filtered', 0]],
'01106' => [['5.0.911', '550', 'userunknown', 1]],
'01107' => [['5.0.910', '', 'filtered', 0]],
'01108' => [['5.7.1', '553', 'userunknown', 1]],
'01108' => [['5.7.1', '553', 'norelaying', 0]],
'01109' => [['5.7.1', '553', 'userunknown', 1]],
'01110' => [['5.0.910', '', 'filtered', 0]],
'01111' => [['5.0.0', '', 'suspend', 0]],
Expand Down