Bug 3113 - Pre-Existing X-Spam headers not removed
Summary: Pre-Existing X-Spam headers not removed
Status: RESOLVED FIXED
Alias: None
Product: Spamassassin
Classification: Unclassified
Component: Libraries (show other bugs)
Version: SVN Trunk (Latest Devel Version)
Hardware: PC Linux
: P5 major
Target Milestone: 3.0.0
Assignee: SpamAssassin Developer Mailing List
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2004-03-01 02:05 UTC by Robert Kiessling
Modified: 2004-03-01 06:02 UTC (History)
0 users


Attachment Type Modified Status Actions Submitter/CLA Status
Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Robert Kiessling 2004-03-01 02:05:56 UTC 
2.70-r6565 does not do remove X-Spam headers before handling the message.

Reason is that delete_header does not affect the pristine headers, but those
are used by rewrite_header().

The appended patch is a simple fix.

*** /root/src/Mail-SpamAssassin-2.70/lib/Mail/SpamAssassin/PerMsgStatus.pm 
2004-02-11 09:55:02.000000000 +0100
--- PerMsgStatus.pm     2004-03-01 11:02:39.000000000 +0100
***************
*** 755,761 ****
    my ($self) = @_;
  
    # put the pristine headers into an array
!   my(@pristine_headers) = $self->{msg}->get_pristine_header() =~ /^([^:]+:[ ]
+(?:.*\n(?:\s+\S.*\n)*))/mig;
    my $addition = 'headers_ham';
  
    if($self->{is_spam}) {
--- 755,761 ----
    my ($self) = @_;
  
    # put the pristine headers into an array
!   my(@pristine_headers) = grep { !/^x-spam-/i } $self->{msg}
->get_pristine_header() =~ /^([^:]+:[ ]+(?:.*\n(?:\s+\S.*\n)*))/mig;
    my $addition = 'headers_ham';
  
    if($self->{is_spam}) {
Comment 1 Theo Van Dinter 2004-03-01 10:42:12 UTC 
Subject: Re:  New: Pre-Existing X-Spam headers not removed

On Mon, Mar 01, 2004 at 02:05:57AM -0800, bugzilla-daemon@bugzilla.spamassassin.org wrote:
> 2.70-r6565 does not do remove X-Spam headers before handling the message.

Correct.  Nor should it.

> Reason is that delete_header does not affect the pristine headers, but those
> are used by rewrite_header().

Yep -- they're pristine.
Comment 2 Theo Van Dinter 2004-03-01 10:43:29 UTC 
if you pass in marked up mail to SA to check, it won't ignore those headers.  it's a design decision.
Comment 3 Justin Mason 2004-03-01 11:04:57 UTC 
to clarify: it should never allow a message through as nonspam based on those
headers.  That's a design decision too, to avoid spammers forging
"X-Spam-Status: No" and getting past SA scanning that way.
Comment 4 Robert Kiessling 2004-03-01 11:16:41 UTC 
Some more remarks on this (thought this would not be contentions,
but here we go):

- 2.63 did remove pre-existing X-Spam headers

- I don't find in Changes where this (serious) change of 
  behaviour is mentioned

- There is a call to $self->{msg}->delete_header('X-Spam-.*') in 
  PerMsgstatus. What does this achieve if the intended purpose is 
  not what a superficial reading implies (and earlier SA versions 
  actually did at the same place)?

- If old X-Spam headers are not deleted, you lose the ability to do 
  simple and clean RE matches on headers for the very condition SA is there 
  to decide: "this is spam, as seems by the local SA". Up to 2.63, 
  a "^X-Spam-Status: Yes" was sufficient for this. This no longer works 
  since it also matches SA headers of the incoming message, i.e. 
  some I cannot rely on.

- Surely it won't take long until Spammers find this out and add 
    X-Spam-Status: No
  themselves.

- If old X-Spam headers are not deleted, you have no way to distinguish
  between X-Spam headers coming from the original message while spamc 
  was not able to contact spamd, and X-Spam headers added locally.

- If old headers shouldn't be deleted, renaming them to something like 
  X-Old-Spam-* sounds like an easy solution.

I would kindly urge you to rethink this design decision since it created 
huge problems.

Robert
Comment 5 Theo Van Dinter 2004-03-01 15:02:23 UTC 
Oops.  I should appologize.  For this whole time I thought you meant in terms of check(), but you're 
talking about "report_safe 0" output.  Doh!  Sorry about that.   Yeah, in looking at the code, the rewrite 
function should definitely strip out the original X-Spam-* headers -- I don't know why that wasn't in 
there actually.

- I don't find in Changes where this (serious) change of 
  behaviour is mentioned

3.0 isn't released, so the Changes file hasn't been updated.

- There is a call to $self->{msg}->delete_header('X-Spam-.*') in 
  PerMsgstatus. What does this achieve if the intended purpose is 
  not what a superficial reading implies (and earlier SA versions 
  actually did at the same place)?

that check() doesn't see the x-spam headers.

- If old headers shouldn't be deleted, renaming them to something like 
  X-Old-Spam-* sounds like an easy solution.

for report_safe 0, we should just grep out the X-Spam headers.  I'm not sure why that wasn't being 
done before actually (it should have been in there...), but it's committed now. :)

"I would kindly urge you to rethink this design decision since it created 
huge problems."

since 3.0 isn't released, it hasn't created any problems afaik.  don't run devel releases in production. ;)