You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@spamassassin.apache.org by he...@apache.org on 2022/11/01 20:11:59 UTC
svn commit: r1904981 - in /spamassassin/trunk: MANIFEST UPGRADE lib/Mail/SpamAssassin/Conf/Parser.pm lib/Mail/SpamAssassin/Plugin/Check.pm t/basic_meta2.t t/basic_meta_net.t t/dcc.t t/dnsbl.t t/shortcircuit.t
Author: hege
Date: Tue Nov 1 20:11:58 2022
New Revision: 1904981
URL: http://svn.apache.org/viewvc?rev=1904981&view=rev
Log:
Bug 7735 - Meta rules need to handle missing/unrun dependencies
- revert to old logic
Removed:
spamassassin/trunk/t/basic_meta_net.t
Modified:
spamassassin/trunk/MANIFEST
spamassassin/trunk/UPGRADE
spamassassin/trunk/lib/Mail/SpamAssassin/Conf/Parser.pm
spamassassin/trunk/lib/Mail/SpamAssassin/Plugin/Check.pm
spamassassin/trunk/t/basic_meta2.t
spamassassin/trunk/t/dcc.t
spamassassin/trunk/t/dnsbl.t
spamassassin/trunk/t/shortcircuit.t
Modified: spamassassin/trunk/MANIFEST
URL: http://svn.apache.org/viewvc/spamassassin/trunk/MANIFEST?rev=1904981&r1=1904980&r2=1904981&view=diff
==============================================================================
--- spamassassin/trunk/MANIFEST (original)
+++ spamassassin/trunk/MANIFEST Tue Nov 1 20:11:58 2022
@@ -250,7 +250,6 @@ t/basic_lint_net.t
t/basic_lint_without_sandbox.t
t/basic_meta.t
t/basic_meta2.t
-t/basic_meta_net.t
t/basic_obj_api.t
t/bayesbdb.t
t/bayesdbm.t
Modified: spamassassin/trunk/UPGRADE
URL: http://svn.apache.org/viewvc/spamassassin/trunk/UPGRADE?rev=1904981&r1=1904980&r2=1904981&view=diff
==============================================================================
--- spamassassin/trunk/UPGRADE (original)
+++ spamassassin/trunk/UPGRADE Tue Nov 1 20:11:58 2022
@@ -19,10 +19,7 @@ improved throughout.
https://wiki.apache.org/spamassassin/WelcomelistBlocklist (Bug 7826)
- Meta rules no longer use priority values, they are evaluated
- dynamically when the rules they depend on are finished. Note that if
- a meta depends on non-existing rules or rules that do not finish
- (timeout or other errors), the whole meta might not be evaluated.
- (Bug 7735)
+ dynamically when the rules they depend on are finished. (Bug 7735)
- API: New $pms->rule_ready() function. Any asynchronous eval-function
must now return undef (instead of 0 or 1), if rule result is not
Modified: spamassassin/trunk/lib/Mail/SpamAssassin/Conf/Parser.pm
URL: http://svn.apache.org/viewvc/spamassassin/trunk/lib/Mail/SpamAssassin/Conf/Parser.pm?rev=1904981&r1=1904980&r2=1904981&view=diff
==============================================================================
--- spamassassin/trunk/lib/Mail/SpamAssassin/Conf/Parser.pm (original)
+++ spamassassin/trunk/lib/Mail/SpamAssassin/Conf/Parser.pm Tue Nov 1 20:11:58 2022
@@ -1078,8 +1078,7 @@ sub compile_meta_rules {
# Will end up later in a compiled sub called from do_meta_tests:
# $_[0] = $pms
# $_[1] = $h ($pms->{tests_already_hit}),
- # $_[2] = hashref list of unrun rules
- $meta{$name} .= "(\$_[1]->{'$token'}||(\$_[2]->{'$token'}?1:0)) ";
+ $meta{$name} .= "(\$_[1]->{'$token'}||0) ";
if (!exists $conf->{test_types}->{$token}) {
dbg("rules: meta test $name has undefined dependency '$token'");
Modified: spamassassin/trunk/lib/Mail/SpamAssassin/Plugin/Check.pm
URL: http://svn.apache.org/viewvc/spamassassin/trunk/lib/Mail/SpamAssassin/Plugin/Check.pm?rev=1904981&r1=1904980&r2=1904981&view=diff
==============================================================================
--- spamassassin/trunk/lib/Mail/SpamAssassin/Plugin/Check.pm (original)
+++ spamassassin/trunk/lib/Mail/SpamAssassin/Plugin/Check.pm Tue Nov 1 20:11:58 2022
@@ -309,8 +309,8 @@ RULE:
next RULE;
}
}
- # Metasubs look like ($_[1]->{$rulename}||($_[2]->{$rulename}?1:0)) ...
- my $result = $mt->{$rulename}->($pms, $h, {});
+ # Metasubs look like ($_[1]->{$rulename}||0) ...
+ my $result = $mt->{$rulename}->($pms, $h);
if ($result) {
dbg("rules: ran meta rule $rulename ======> got hit ($result)");
$pms->got_hit($rulename, '', ruletype => 'meta', value => $result);
@@ -335,34 +335,15 @@ sub finish_meta_tests {
return if $self->{am_compiling}; # nothing to compile here
my $mp = $pms->{meta_pending};
- my $md = $pms->{conf}->{meta_dependencies};
my $mt = $pms->{conf}->{meta_tests};
my $h = $pms->{tests_already_hit};
my $retry;
- my %unrun;
RULE:
foreach my $rulename (keys %$mp) {
- %unrun = ();
- # Meta is not ready if some dependency has not run yet
- foreach my $deprule (@{$md->{$rulename}||[]}) {
- if (!exists $h->{$deprule}) {
- # Record all unrun deps for second meta evaluation
- $unrun{$deprule} = 1;
- }
- }
- # Metasubs look like ($_[1]->{$rulename}||($_[2]->{$rulename}?1:0)) ...
- my $result = $mt->{$rulename}->($pms, $h, {});
- my $result2 = $result;
- if (%unrun) {
- # Evaluate all unrun rules as true using %unrun list
- $result2 = $mt->{$rulename}->($pms, $h, \%unrun);
- }
- # Evaluated second time with all unrun rules as true. If result is not
- # the same, we can't safely finish the meta. (Bug 7735)
- if ($result != $result2) {
- next RULE;
- } elsif ($result) {
+ # Metasubs look like ($_[1]->{$rulename}||0) ...
+ my $result = $mt->{$rulename}->($pms, $h);
+ if ($result) {
dbg("rules: ran meta rule $rulename ======> got hit ($result)");
$pms->got_hit($rulename, '', ruletype => 'meta', value => $result);
} else {
@@ -375,21 +356,6 @@ RULE:
}
goto RULE if $retry--;
-
- if ($would_log_rules_all) {
- foreach my $rulename (sort keys %{$pms->{conf}->{meta_tests}}) {
- next if !$pms->{conf}->{scores}->{$rulename};
- next if exists $h->{$rulename};
- my %unrun;
- foreach my $deprule (@{$md->{$rulename}||[]}) {
- $unrun{$deprule} = 1 if !exists $h->{$deprule};
- }
- if (%unrun) {
- dbg("rules-all: unrun dependencies prevented meta $rulename from running: ".
- join(', ', sort keys %unrun));
- }
- }
- }
}
###########################################################################
Modified: spamassassin/trunk/t/basic_meta2.t
URL: http://svn.apache.org/viewvc/spamassassin/trunk/t/basic_meta2.t?rev=1904981&r1=1904980&r2=1904981&view=diff
==============================================================================
--- spamassassin/trunk/t/basic_meta2.t (original)
+++ spamassassin/trunk/t/basic_meta2.t Tue Nov 1 20:11:58 2022
@@ -17,9 +17,13 @@ plan tests => 23 * $iterations;
q{ 1.0 TEST_FOO_2 } => '',
q{ 1.0 TEST_FOO_3 } => '',
q{ 1.0 TEST_META_1 } => '',
+ q{ 1.0 TEST_META_2 } => '',
q{ 1.0 TEST_META_3 } => '',
+ q{ 1.0 TEST_META_4 } => '',
q{ 1.0 TEST_META_5 } => '',
+ q{ 1.0 TEST_META_6 } => '',
q{ 1.0 TEST_META_7 } => '',
+ q{ 1.0 TEST_META_9 } => '',
q{ 1.0 TEST_META_A } => '',
q{ 1.0 TEST_META_B } => '',
q{ 1.0 TEST_META_C } => '',
@@ -34,11 +38,7 @@ plan tests => 23 * $iterations;
%anti_patterns = (
q{ TEST_NEG_1 } => '',
- q{ TEST_META_2 } => '',
- q{ TEST_META_4 } => '',
- q{ TEST_META_6 } => '',
q{ TEST_META_8 } => '',
- q{ TEST_META_9 } => '',
);
tstlocalrules (qq{
@@ -60,30 +60,27 @@ tstlocalrules (qq{
## Unrun rule dependencies (Bug 7735)
##
- # Non-existing rule, considered "unrun" and will prevent dependent metas
- # from running (unless dual evaluation allows it)
- # Should not hit, meta is evaled twice: (!0) && (!1)
+ # Non-existing rule, should hit as !0
meta TEST_META_2 !NONEXISTINGRULE
- # Should hit, meta is evaled twice: (!0 || 0) && (!1 || 1)
+ # Should hit as !0 || 0
meta TEST_META_3 !NONEXISTINGRULE || NONEXISTINGRULE
# Disabled rule, same as above
body TEST_DISABLED /a/
score TEST_DISABLED 0
- # Should not hit
+ # Should hit as !0
meta TEST_META_4 !TEST_DISABLED
- # Should hit
+ # Should hit as !0 || 0
meta TEST_META_5 !TEST_DISABLED || TEST_DISABLED
# Unrun rule (due to local tests only), same as above
askdns TEST_DISABLED2 spamassassin.org TXT /./
- # Should not hit
+ # Should hit as !0
meta TEST_META_6 !TEST_DISABLED2
- # Should hit
+ # Should hit as !0 || 0
meta TEST_META_7 !TEST_DISABLED2 || TEST_DISABLED2
- # Other way of "disabling" a rule, with meta 0. This will let dependent
- # metas run fully, as the rule is considered run but not hitting.
+ # Other way of "disabling" a rule, with meta 0.
meta TEST_DISABLED3 0
# Should hit
meta TEST_META_I !TEST_DISABLED3
@@ -92,9 +89,9 @@ tstlocalrules (qq{
# Should not hit
meta TEST_META_8 __FOO_1 + NONEXISTINGRULE == 2
- # Should not hit
+ # Should hit as 1 + 0 + 1 == 2
meta TEST_META_9 __FOO_1 + NONEXISTINGRULE + __FOO_2 == 2
- # Should hit (both eval checks are true thanks to >1)
+ # Should hit as above
meta TEST_META_A __FOO_1 + NONEXISTINGRULE + __FOO_2 > 1
# local_tests_only
Modified: spamassassin/trunk/t/dcc.t
URL: http://svn.apache.org/viewvc/spamassassin/trunk/t/dcc.t?rev=1904981&r1=1904980&r2=1904981&view=diff
==============================================================================
--- spamassassin/trunk/t/dcc.t (original)
+++ spamassassin/trunk/t/dcc.t Tue Nov 1 20:11:58 2022
@@ -10,7 +10,7 @@ plan skip_all => "Tests don't work on wi
plan skip_all => "Net tests disabled" unless conf_bool('run_net_tests');
plan skip_all => "DCC tests disabled" unless conf_bool('run_dcc_tests');
plan skip_all => "DCC executable not found in path" unless HAS_DCC;
-plan tests => 16;
+plan tests => 12;
diag('Note: Failure may not be an SpamAssassin bug, as DCC tests can fail due to problems with the DCC servers.');
@@ -52,14 +52,3 @@ ok_all_patterns();
ok sarun ("-t < data/spam/gtubedcc_crlf.eml 2>&1", \&patterns_run_cb);
ok_all_patterns();
-# Local only, metas should not hit as no queries are made
-%patterns = (
-);
-%anti_patterns = (
- q{ DCC_CHECK }, 'dcc',
- q{ X_META_POS }, 'pos',
- q{ X_META_NEG }, 'neg',
-);
-ok sarun ("-t -L < data/spam/gtubedcc.eml 2>&1", \&patterns_run_cb);
-ok_all_patterns();
-
Modified: spamassassin/trunk/t/dnsbl.t
URL: http://svn.apache.org/viewvc/spamassassin/trunk/t/dnsbl.t?rev=1904981&r1=1904980&r2=1904981&view=diff
==============================================================================
--- spamassassin/trunk/t/dnsbl.t (original)
+++ spamassassin/trunk/t/dnsbl.t Tue Nov 1 20:11:58 2022
@@ -9,7 +9,7 @@ plan skip_all => "Can't use Net::DNS Saf
# run many times to catch some random natured failures
my $iterations = 5;
-plan tests => 22 * $iterations;
+plan tests => 21 * $iterations;
# ---------------------------------------------------------------------------
# bind configuration currently used to support this test
@@ -74,7 +74,6 @@ EOF
' 1.0 DNSBL_TXT_MISS ' => '',
' 1.0 DNSBL_TEST_WHITELIST_MISS ' => '',
'14.35.17.212.untrusted.dnsbltest.spamassassin.org' => '',
- qr/rules-all: unrun dependencies [^\n]+ (?:__|META_)?DNSBL_/ => '',
);
tstprefs("
@@ -154,8 +153,7 @@ priority DNSBL_TEST_RELAY 2000
for (1 .. $iterations) {
clear_localrules() if $_ == 3; # do some tests without any other rules to check meta bugs
- # rules-all debug needed for unrun check
- sarun ("-t -D rules-all < data/spam/dnsbl.eml 2>&1", \&patterns_run_cb);
+ sarun ("-t < data/spam/dnsbl.eml 2>&1", \&patterns_run_cb);
ok_all_patterns();
}
Modified: spamassassin/trunk/t/shortcircuit.t
URL: http://svn.apache.org/viewvc/spamassassin/trunk/t/shortcircuit.t?rev=1904981&r1=1904980&r2=1904981&view=diff
==============================================================================
--- spamassassin/trunk/t/shortcircuit.t (original)
+++ spamassassin/trunk/t/shortcircuit.t Tue Nov 1 20:11:58 2022
@@ -45,7 +45,7 @@ tstlocalrules ('
%patterns = (
' 1.0 SC_PRI_SPAM_001 ', 'hit',
'shortcircuit=spam', 'sc',
- qr/X-Spam-Status: Yes, score=103.0 required=5.0 /m, 'shortcircuit_spam_score',
+ qr/X-Spam-Status: Yes/m, 'shortcircuit_spam_header',
' 100 SHORTCIRCUIT Not all rules were run', 'shortcircuit rule desc',
);
ok (sarun ("-L -t < data/spam/001", \&patterns_run_cb));
@@ -54,7 +54,7 @@ ok_all_patterns();
%patterns = (
' 50 SC_002 ', 'hit',
'shortcircuit=spam', 'sc',
- qr/^X-Spam-Status: Yes, score=50.0 required=5.0 /m, 'SC_002 score',
+ qr/^X-Spam-Status: Yes/m, 'SC_002 header',
' 0.0 SHORTCIRCUIT Not all rules were run', 'shortcircuit rule desc',
);
ok (sarun ("-L -t < data/spam/002", \&patterns_run_cb));
@@ -63,7 +63,7 @@ ok_all_patterns();
%patterns = (
' -1.0 SC_HAM_001 ', 'SC_HAM_001',
'shortcircuit=ham', 'sc_ham',
- qr/^X-Spam-Status: No, score=-101.0 required=5.0 /m, 'SC_HAM_001 score',
+ qr/^X-Spam-Status: No/m, 'SC_HAM_001 header',
' -100 SHORTCIRCUIT Not all rules were run', 'shortcircuit rule desc',
);
ok (sarun ("-L -t < data/nice/001", \&patterns_run_cb));