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...@bugzilla.spamassassin.org on 2007/08/17 04:02:08 UTC

[Bug 5606] New: clear_headers -> many spamc/spamd tests fail

http://issues.apache.org/SpamAssassin/show_bug.cgi?id=5606

           Summary: clear_headers -> many spamc/spamd tests fail
           Product: Spamassassin
           Version: 3.2.3
          Platform: All
        OS/Version: All
            Status: NEW
          Severity: normal
          Priority: P5
         Component: Regression Tests
        AssignedTo: dev@spamassassin.apache.org
        ReportedBy: jeff@sdsc.edu


Due to the use of the clear_headers option in my user_prefs file, 17 of the
spamc and spamd test scripts fail.  For example, t/spamd.t produces

 Not found: flag =  X-Spam-Flag: YES

messages because I have told SA not to add that header to my mail.  This does
not occur in version 3.1.9 and earlier, and I first saw it in 3.2.0.  It appears
that the 3.2.x series uses the user_prefs file of the user running "make test"
rather than a test-specific set of preferences for the spamc/spamd tests. 
Please restore the older behavior.



------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.

[Bug 5606] [review] spamc/spamd tests fail due to broken -x logic

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


jm@jmason.org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
            Summary|spamc/spamd tests fail due  |[review] spamc/spamd tests
                   |to broken -x logic          |fail due to broken -x logic
  Status Whiteboard|                            |needs 2 votes for 3.2




------- Additional Comments From jm@jmason.org  2007-09-26 15:38 -------
(oh, it fixes the reported bug, btw, tested by adding "clear_headers" to
~/.spamassassin/user_prefs and "prove t/spamd.t" before and after applying)



------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.

[Bug 5606] clear_headers -> many spamc/spamd tests fail

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





------- Additional Comments From sidney@sidney.com  2007-08-16 23:08 -------
> None of the tests should be influenced by outside configuration

Yes, of course. Long ago (bug 68) SATest.pm was modified to call spamd with the
-x option just for this very reason. But apparently something has changed since
3.1 branch that causes the -x option to not to have that effect when tests are run.

Ok, I've tracked it down. In version 3.2 spamd calls
Mail::SpamAssassin::init_learner() and that calls $self->init(1) which reads in
the user_pref file. In version 3.1 spamd doesn't call init_learner.

I don't know enough about what it is doing to say what the fix should be.




------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.

[Bug 5606] [review] spamc/spamd tests fail due to broken -x logic

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





------- Additional Comments From spamassassin@dostech.ca  2007-11-06 13:02 -------
Why we always call $spamtest->init(1), regardless of whether or not user_prefs
are allowed (-x / --nouser-config), isn't clear to me.



------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.

[Bug 5606] [review] spamc/spamd tests fail due to broken -x logic

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


jm@jmason.org changed:

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




------- Additional Comments From jm@jmason.org  2007-12-28 05:12 -------
thanks, applied to 3.2.x:

: jm 224...; svn commit -m "bug 5606: too-early init_learner() call causes
root's user prefs file to be read when spamd is started; this is inappropriate.
fix" spamd/spamd.raw
Sending        spamd/spamd.raw
Transmitting file data .
Committed revision 607233.




------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.

[Bug 5606] clear_headers -> many spamc/spamd tests fail

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





------- Additional Comments From sidney@sidney.com  2007-08-16 21:00 -------
I can confirm this in branch/3.2 and in trunk.

The problem appears to be that spamd reads ~/.spamassassin/user_prefs when it
starts up even if the -x option is telling it not to read per-user config files
for each message.

SATest.pm uses -x with spamd and -p with spamassassin to prevent contaminating
the tests with user's actual preference file. Unfortunately, that isn't what
spamd -x means. We either need to decide that spamd should not read the
user_prefs file of the userid it is started up under, or else should add an
option that does the same thing as spamassassin -p.

I think that the former makes more sense, since spamd is already reading the
various *.pre files for system configuration, and each child can read the
appropriate user's user_prefs. It is confusing to have a base user_prefs file
and then read a per-user user_prefs file on top of it.





------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.

[Bug 5606] spamc/spamd tests fail due to broken -x logic

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


jm@jmason.org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
            Summary|clear_headers -> many       |spamc/spamd tests fail due
                   |spamc/spamd tests fail      |to broken -x logic






------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.

[Bug 5606] [review] spamc/spamd tests fail due to broken -x logic

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





------- Additional Comments From jm@jmason.org  2007-09-27 05:16 -------
applied to trunk:

: jm 158...; svn commit -m "bug 5606: too-early init_learner() call causes
root's user prefs file to be read when spamd is started; this is inappropriate.
 fix" spamd/spamd.raw
Sending        spamd/spamd.raw
Transmitting file data .
Committed revision 579983.




------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.

[Bug 5606] clear_headers -> many spamc/spamd tests fail

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


jm@jmason.org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Severity|normal                      |minor
   Target Milestone|Undefined                   |3.2.4




------- Additional Comments From jm@jmason.org  2007-08-20 04:09 -------
we should try to fix this soon



------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.

[Bug 5606] spamc/spamd tests fail due to broken -x logic

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





------- Additional Comments From jm@jmason.org  2007-09-26 15:26 -------
(In reply to comment #3)
> Ok, I've tracked it down. In version 3.2 spamd calls
> Mail::SpamAssassin::init_learner() and that calls $self->init(1) which reads in
> the user_pref file. In version 3.1 spamd doesn't call init_learner.

This was from the fix for bug 3466 ('RFE: split out opportunistic work from the
current check/scan functions'):

: jm 184...; svn diff -r490113:490114 spamd/spamd.raw
Index: spamd/spamd.raw
===================================================================
--- spamd/spamd.raw     (revision 490113)
+++ spamd/spamd.raw     (revision 490114)
@@ -844,6 +844,10 @@
   }
 );

+$spamtest->init_learner({
+  opportunistic_expire_check_only => 1,
+});
+
 # if $clients_per_child == 1, there's no point in copying configs around
 unless ($clients_per_child > 1) {
   # unset $copy_config_p so we don't bother trying to copy things back
@@ -1409,7 +1413,9 @@
   }

   # Go ahead and check the message
-  my $status = $spamtest->check($mail);
+  $spamtest->init(1);
+  my $status = Mail::SpamAssassin::PerMsgStatus->new($spamtest, $mail);
+  $status->check();

   my $msg_score     = sprintf( "%.1f", $status->get_score );
   my $msg_threshold = sprintf( "%.1f", $status->get_required_score );
@@ -1528,6 +1534,16 @@
   # bug 3808: log scan results to any listening plugins, too
   $spamtest->call_plugins("log_scan_result", { result => $log });

+  # bug 3466: handle the bayes expiry bits after the results were returned to
+  # the client.  keeps clients from timing out.  if bayes_expiry_due is set,
+  # then the opportunistic check has already checked.  go ahead and do another
+  # sync/expire run.
+  if ($status->{'bayes_expiry_due'}) {
+    dbg("spamd: bayes expiry was marked as due, running post-check");
+    $spamtest->rebuild_learner_caches();
+    $spamtest->finish_learner();
+  }
+
   $status->finish();    # added by jm to allow GC'ing
   $mail->finish();


Theo, any chance of a comment?  I'm guessing that moving the 

+$spamtest->init_learner({
+  opportunistic_expire_check_only => 1,
+});

call to after preload_modules() will fix this bug.



------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.

[Bug 5606] spamc/spamd tests fail due to broken -x logic

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


jm@jmason.org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
Attachment #4104 is|0                           |1
           obsolete|                            |




------- Additional Comments From jm@jmason.org  2007-09-26 15:38 -------
Created an attachment (id=4134)
 --> (http://issues.apache.org/SpamAssassin/attachment.cgi?id=4134&action=view)
alt fix

here's that fix I mentioned -- I think this is less intrusive, and more
correct.  Theo -- if you could review it, that'd be great ;)



------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.

[Bug 5606] clear_headers -> many spamc/spamd tests fail

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





------- Additional Comments From felicity@apache.org  2007-08-16 21:58 -------
(In reply to comment #1)
> We either need to decide that spamd should not read the
> user_prefs file of the userid it is started up under, or else should add an
> option that does the same thing as spamassassin -p.

None of the tests should be influenced by outside configuration.  Which means
installed default config, local state dir config, site config, or user config. 
If any of those are being used (which apparently they are :(), that's a bug.



------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.

[Bug 5606] [review] spamc/spamd tests fail due to broken -x logic

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





------- Additional Comments From jeff@sdsc.edu  2007-09-27 13:54 -------
I am +1 on Justin's alt fix, which resolves the regression test problem. 
Additionally, I see no unwanted side effects when using it in my production
spamd.  Thanks, everyone.



------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.

[Bug 5606] spamc/spamd tests fail due to broken -x logic

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





------- Additional Comments From jeff@sdsc.edu  2007-08-20 14:04 -------
Created an attachment (id=4104)
 --> (http://issues.apache.org/SpamAssassin/attachment.cgi?id=4104&action=view)
proposed fix

The attached patch resolves this problem by calling init($opt{'user-config'})
just before the new call to init_learner().  I'll leave it to someone more
familiar with the code to decide whether it is too ugly to commit as is.



------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.

[Bug 5606] [review] spamc/spamd tests fail due to broken -x logic

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


maddoc@maddoc.net changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Status Whiteboard|needs 1 votes for 3.2       |can be commited




------- Additional Comments From maddoc@maddoc.net  2007-12-22 16:31 -------
+1



------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.

[Bug 5606] [review] spamc/spamd tests fail due to broken -x logic

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


sidney@sidney.com changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Status Whiteboard|needs 2 votes for 3.2       |needs 1 votes for 3.2




------- Additional Comments From sidney@sidney.com  2007-12-21 14:05 -------
init() caches whether it has already been called, returning immediately if it
has. It is documented in the comments for init() that it can safely be called
multiple times.

The point of this fix is to move the call to init_learner until after the call
to preload_modules_with_tmp_homedir() because that calls
$spamtest->compile_now(0,1) which calls init() with the first argument to
compile_now, i.e., init(0). After that it is safe to call init(1) as it has no
effect.

+1 on this patch




------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.