You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@spamassassin.apache.org by bu...@issues.apache.org on 2011/03/23 12:17:26 UTC

[Bug 6560] New: Network #testrules shouldn't be auto-promoted!

https://issues.apache.org/SpamAssassin/show_bug.cgi?id=6560

           Summary: Network #testrules shouldn't be auto-promoted!
           Product: Spamassassin
           Version: SVN Trunk (Latest Devel Version)
          Platform: PC
        OS/Version: Linux
            Status: NEW
          Severity: critical
          Priority: P2
         Component: RuleQA
        AssignedTo: dev@spamassassin.apache.org
        ReportedBy: wtogami@gmail.com


Please see Bug #6220 where somehow a sandbox file beginning with "#testrules"
had all six of its network rules auto-promoted into sa-update.

http://svn.apache.org/repos/asf/spamassassin/trunk/rulesrc/sandbox/smf/30_bug_6220_sem.cf

This bug is to investigate and fix whatever caused it to ignore "#testrules".

-- 
Configure bugmail: https://issues.apache.org/SpamAssassin/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.

[Bug 6560] Network #testrules shouldn't be auto-promoted!

Posted by bu...@bugzilla.spamassassin.org.
https://issues.apache.org/SpamAssassin/show_bug.cgi?id=6560

Darxus <Da...@ChaosReigns.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |Darxus@ChaosReigns.com

--- Comment #9 from Darxus <Da...@ChaosReigns.com> 2011-06-27 21:59:03 UTC ---
Daryl provided a generated but un-released sa-update for testing this:

http://mail-archives.apache.org/mod_mbox/spamassassin-dev/201105.mbox/%3C4DD482AD.10304@dostech.ca%3E

Warren thought he tested it, but was apparently looking at the wrong tarball
(said in IRC today).  Kevin also tested it, but presumably didn't specifically
look for the SEM rules that were being a problem.  

(In case anybody else has been thinking "Wait, didn't we test this?")

-- 
Configure bugmail: https://issues.apache.org/SpamAssassin/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.

[Bug 6560] Network #testrules shouldn't be auto-promoted!

Posted by bu...@issues.apache.org.
https://issues.apache.org/SpamAssassin/show_bug.cgi?id=6560

--- Comment #2 from Warren Togami <wt...@gmail.com> 2011-03-23 16:11:29 EDT ---
NOTE: The only reason #testrules was used in this particular sandbox file was
Bug #6527 where "tflags nopublish" not only excluded it from auto-promotion but
also from nightly masscheck.

-- 
Configure bugmail: https://issues.apache.org/SpamAssassin/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.

[Bug 6560] Network #testrules shouldn't be auto-promoted!

Posted by bu...@issues.apache.org.
https://issues.apache.org/SpamAssassin/show_bug.cgi?id=6560

--- Comment #8 from Karsten Bräckelmann <gu...@rudersport.de> 2011-03-24 18:56:00 EDT ---
(In reply to comment #6)
> autopromotion runs off of trunk.  The code will update itself to the latest
> rev.  So just commit to trunk.  If it breaks its no big deal.  Rule updates are
> disabled.  Try to do it ASAP though just in case it breaks so that it can be
> fixed before the weekly mass-check is tagged.

Great, let me break it then. :)

Sending listpromotable
Committed revision 1085175.

  Bug 6560, specifically declared #testrules may only be overridden on a
  rule-by-rule basis by tflags publish, not tflags net.


Keeping this bug open, until the untested code at least has proven not to break
the process.


(In reply to comment #7)
> May I propose that we insert a temporary "short-circuit" into the auto-update
> mechanism to allow for manual inspection prior to it being pushed live?

You may, though I suggest to do it somewhere else and not hijack this
#testrules bug. ;)  The dev list seems appropriate for kicking around ideas.

-- 
Configure bugmail: https://issues.apache.org/SpamAssassin/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.

[Bug 6560] Network #testrules shouldn't be auto-promoted!

Posted by bu...@issues.apache.org.
https://issues.apache.org/SpamAssassin/show_bug.cgi?id=6560

--- Comment #5 from Warren Togami <wt...@gmail.com> 2011-03-23 17:12:00 EDT ---
I looked into if this change will cause anything else unexpected.

http://svn.apache.org/repos/asf/spamassassin/trunk/rulesrc/sandbox/jm/20_bug_5984.cf

RCVD_IN_BRBL_LASTEXT was never meant to escape testing? =)

-- 
Configure bugmail: https://issues.apache.org/SpamAssassin/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.

[Bug 6560] Network #testrules shouldn't be auto-promoted!

Posted by bu...@issues.apache.org.
https://issues.apache.org/SpamAssassin/show_bug.cgi?id=6560

--- Comment #7 from Warren Togami <wt...@gmail.com> 2011-03-24 01:28:29 EDT ---
May I propose that we insert a temporary "short-circuit" into the auto-update
mechanism to allow for manual inspection prior to it being pushed live?

Let all but the DNS update happen then post the URL somewhere public so many
eyes can download and examine diffs from the previous rules.

-- 
Configure bugmail: https://issues.apache.org/SpamAssassin/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.

[Bug 6560] Network #testrules shouldn't be auto-promoted!

Posted by bu...@issues.apache.org.
https://issues.apache.org/SpamAssassin/show_bug.cgi?id=6560

--- Comment #6 from Daryl C. W. O'Shea <sp...@dostech.ca> 2011-03-24 01:07:38 EDT ---
(In reply to comment #4)
> Didn't commit to trunk either, since I don't know which branch actually is used
> for autopromotion. Doe this need to be committed to both, 3.3 and trunk?

autopromotion runs off of trunk.  The code will update itself to the latest
rev.  So just commit to trunk.  If it breaks its no big deal.  Rule updates are
disabled.  Try to do it ASAP though just in case it breaks so that it can be
fixed before the weekly mass-check is tagged.

-- 
Configure bugmail: https://issues.apache.org/SpamAssassin/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.

[Bug 6560] Network #testrules shouldn't be auto-promoted!

Posted by bu...@bugzilla.spamassassin.org.
https://issues.apache.org/SpamAssassin/show_bug.cgi?id=6560

Kevin A. McGrail <km...@pccc.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |RESOLVED
         Resolution|                            |FIXED

--- Comment #12 from Kevin A. McGrail <km...@pccc.com> 2011-06-27 22:30:53 UTC ---
(In reply to comment #11)
> (In reply to comment #10)
> > 
> > As they were flagged as T_ which is what the cf file said to do, I wouldn't
> > have flagged it as wrong.  A few DNS queries don't bug me and it won't swing
> > ham/spam scores excep by almost negligible amounts.
> 
> They are wrong as in we never intended for any T_ rule to become active?

#Testrules and nopublish are not the same thing.  I worry people are confusing
the two OR the code doesn't implement what people thought it implemented. 

Considering this bug resolved and outstanding issues duplicate of bug 6527.

-- 
Configure bugmail: https://issues.apache.org/SpamAssassin/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.

[Bug 6560] Network #testrules shouldn't be auto-promoted!

Posted by bu...@issues.apache.org.
https://issues.apache.org/SpamAssassin/show_bug.cgi?id=6560

--- Comment #4 from Karsten Bräckelmann <gu...@rudersport.de> 2011-03-23 16:38:05 EDT ---
Didn't commit to trunk either, since I don't know which branch actually is used
for autopromotion. Doe this need to be committed to both, 3.3 and trunk?

-- 
Configure bugmail: https://issues.apache.org/SpamAssassin/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.

[Bug 6560] Network #testrules shouldn't be auto-promoted!

Posted by bu...@issues.apache.org.
https://issues.apache.org/SpamAssassin/show_bug.cgi?id=6560

--- Comment #3 from Karsten Bräckelmann <gu...@rudersport.de> 2011-03-23 16:32:24 EDT ---
Created an attachment (id=4858)
 --> (https://issues.apache.org/SpamAssassin/attachment.cgi?id=4858)
proposed fix

This rather trivial patch moves tflags (userconf|learn|net) handling *after*
checking for #testrules. Only an explicit tflag publish can override testrules.

Please note that the patch is entirely UNTESTED. Don't have an environment to
run the code. Please review carefully. ;)

-- 
Configure bugmail: https://issues.apache.org/SpamAssassin/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.

[Bug 6560] Network #testrules shouldn't be auto-promoted!

Posted by bu...@bugzilla.spamassassin.org.
https://issues.apache.org/SpamAssassin/show_bug.cgi?id=6560

--- Comment #11 from Warren Togami <wt...@gmail.com> 2011-06-27 22:09:03 UTC ---
(In reply to comment #10)
> 
> As they were flagged as T_ which is what the cf file said to do, I wouldn't
> have flagged it as wrong.  A few DNS queries don't bug me and it won't swing
> ham/spam scores excep by almost negligible amounts.

They are wrong as in we never intended for any T_ rule to become active?

-- 
Configure bugmail: https://issues.apache.org/SpamAssassin/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.

[Bug 6560] Network #testrules shouldn't be auto-promoted!

Posted by bu...@bugzilla.spamassassin.org.
https://issues.apache.org/SpamAssassin/show_bug.cgi?id=6560

Kevin A. McGrail <km...@pccc.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |kmcgrail@pccc.com

--- Comment #10 from Kevin A. McGrail <km...@pccc.com> 2011-06-27 22:06:03 UTC ---
(In reply to comment #9)
> Daryl provided a generated but un-released sa-update for testing this:
> 
> http://mail-archives.apache.org/mod_mbox/spamassassin-dev/201105.mbox/%3C4DD482AD.10304@dostech.ca%3E
> 
> Warren thought he tested it, but was apparently looking at the wrong tarball
> (said in IRC today).  Kevin also tested it, but presumably didn't specifically
> look for the SEM rules that were being a problem.  
> 
> (In case anybody else has been thinking "Wait, didn't we test this?")

As they were flagged as T_ which is what the cf file said to do, I wouldn't
have flagged it as wrong.  A few DNS queries don't bug me and it won't swing
ham/spam scores excep by almost negligible amounts.

-- 
Configure bugmail: https://issues.apache.org/SpamAssassin/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.

[Bug 6560] Network #testrules shouldn't be auto-promoted!

Posted by bu...@issues.apache.org.
https://issues.apache.org/SpamAssassin/show_bug.cgi?id=6560

--- Comment #1 from Karsten Bräckelmann <gu...@rudersport.de> 2011-03-23 14:45:02 EDT ---
The problem is in build/mkupdates/listpromotable, starts around line 207, see
the following 20 lines. Also related, bug 5545.

The following few lines of comments fully explain the logic.

    # all of these tflags force publication;
    # include "net", since otherwise this script has to be aware [...]

  # rule was from a file marked with "#testrules" (bug 5545)
  # note: this is after "tflags publish" support, so you can override
  # it on a rule-by-rule basis anyway

Of course, the rules autopromoted unintentionally are net rules.


Suggest the following change in the autopromotion / exclusion logic.

* ONLY tflags publish comes first and can actually override #testrules.
* Then chicken out, if #testrules has been set.
* Last, check for the other tflags as currently done early.

* Go on with the rest of the autopromotion magic...

-- 
Configure bugmail: https://issues.apache.org/SpamAssassin/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.