Bug 6801 - UNPARSEABLE_RELAY returns numeric result
Summary: UNPARSEABLE_RELAY returns numeric result
Status: RESOLVED FIXED
Alias: None
Product: Spamassassin
Classification: Unclassified
Component: Rules (Eval Tests) (show other bugs)
Version: 3.3.1
Hardware: PC Linux
: P2 minor
Target Milestone: Undefined
Assignee: SpamAssassin Developer Mailing List
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-05-31 11:12 UTC by Cedric Knight
Modified: 2012-06-16 02:32 UTC (History)
3 users (show)



Attachment Type Modified Status Actions Submitter/CLA Status

Note You need to log in before you can comment on or make changes to this bug.
Description Cedric Knight 2012-05-31 11:12:42 UTC
Most eval tests return 1 or 0.  check_relays_unparseable() returns the number of unparseable lines, which is confusing when using UNPARSEABLE_RELAY in meta tests.

To reproduce: for example, use UNPARSEABLE_RELAY in a (fictional) meta rule like

meta SOMEMETA ((SUBJ_HAS_UNIQ_ID + RDNS_NONE + DCC_CHECK + RAZOR2_CHECK + EXCUSE_REMOVE + FORGED_OUTLOOK_HTML + UNPARSEABLE_RELAY) >= 6)

send an email to certain addresses @mobileemail.vodafone.ie.  The bounce includes lines like

Received: from localhost;Thu, 31 May 2012 09:56:24 +0000
Received: from localhost;Thu, 31 May 2012 09:56:24 +0000
Received: from localhost;Thu, 31 May 2012 09:56:24 +0000
Received: from localhost;Thu, 31 May 2012 09:56:24 +0000
Received: from localhost;Thu, 31 May 2012 09:56:24 +0000
Received: from localhost;Thu, 31 May 2012 09:56:24 +0000
Received: from localhostThu, 31 May 2012 09:56:24 +0000

The meta rules hit unexpectedly.  Although unparseable relays is only supposed to be a factor in the meta rule scoring, the other subrules are not required.

Suggested fix:
header __UNPARSEABLE_RELAY        eval:check_relays_unparseable()
meta UNPARSEABLE_RELAY            (__UNPARSEABLE_RELAY >= 1)
Comment 1 Mark Martinec 2012-06-05 18:28:27 UTC
> Suggested fix:
> header __UNPARSEABLE_RELAY        eval:check_relays_unparseable()
> meta UNPARSEABLE_RELAY            (__UNPARSEABLE_RELAY >= 1)

+1
Fine with me, the change looks innocent enough and may prevent surprises.
Comment 2 Kevin A. McGrail 2012-06-05 18:51:40 UTC
(In reply to comment #1)
> > Suggested fix:
> > header __UNPARSEABLE_RELAY        eval:check_relays_unparseable()
> > meta UNPARSEABLE_RELAY            (__UNPARSEABLE_RELAY >= 1)
> 
> +1
> Fine with me, the change looks innocent enough and may prevent surprises.

I feel uncomfortable voting on this because I don't know what we are doing with it.

Is this the proposed change?

Index: 20_head_tests.cf
===================================================================
--- 20_head_tests.cf    (revision 1346433)
+++ 20_head_tests.cf    (working copy)
@@ -559,12 +559,14 @@
 
 ifplugin Mail::SpamAssassin::Plugin::RelayEval
 
-header UNPARSEABLE_RELAY        eval:check_relays_unparseable()
-tflags UNPARSEABLE_RELAY        userconf
+header __UNPARSEABLE_RELAY        eval:check_relays_unparseable()
+tflags __UNPARSEABLE_RELAY        userconf
+
+meta    UNPARSEABLE_RELAY      (__UNPARSEABLE_RELAY >= 1)
+tflags  UNPARSEABLE_RELAY      userconf
 describe UNPARSEABLE_RELAY      Informational: message has unparseable relay lines
 
 
-
 header RCVD_HELO_IP_MISMATCH   eval:helo_ip_mismatch()
 describe RCVD_HELO_IP_MISMATCH Received: HELO and IP do not match, but should
Comment 3 RW 2012-06-15 14:34:10 UTC
Since someone may actually find a use for the count, I'd suggest giving the hidden rule a more meaningful name like __UNPARSEABLE_RELAY_COUNT.

Currently UNPARSEABLE_RELAY doesn't seem to be used anywhere, it's purely informational.
Comment 4 Cedric Knight 2012-06-15 15:16:32 UTC
+1 for the name __UNPARSEABLE_RELAY_COUNT

Otherwise the change is as in Comment 2.
Comment 5 Kevin A. McGrail 2012-06-16 02:32:35 UTC
thanks.  Good comments.

svn commit rules/20_head_tests.cf -m 'Fix for unparseable relay suggested by Cedric Knight bug 6801'
Sending        rules/20_head_tests.cf
Transmitting file data .
Committed revision 1350841.