Bug 1595 - Someone broke BSMTP mode
Summary: Someone broke BSMTP mode
Status: RESOLVED FIXED
Alias: None
Product: Spamassassin
Classification: Unclassified
Component: spamc/spamd (show other bugs)
Version: 2.50
Hardware: All All
: P1 critical
Target Milestone: 2.53
Assignee: Justin Mason
URL:
Whiteboard:
Keywords: backport
Depends on:
Blocks:
 
Reported: 2003-03-03 21:19 UTC by Brad "anomie" Jorsch
Modified: 2003-03-28 04:28 UTC (History)
0 users


Attachment Type Modified Status Actions Submitter/CLA Status
fixes bsmtp patch None Justin Mason [HasCLA]
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 Brad "anomie" Jorsch 2003-03-03 21:19:52 UTC 
At some point, the meaning of the struct message is_spam field has changed. In
particular, it used to be that the field was EX_ISSPAM if the message is spam,
EX_NOTSPAM if the message is not spam, or EX_TOOBIG if we return a filtered
message instead of a boolean is or is not spam (yes, i should have named it
something more obvious than either is_spam or EX_TOOBIG. Sorry.) This appears to
be in response to bug #1452.

Unforunately, whoever "fixed" that bug didn't bother to notice that
message_write uses this fact to determine whether to output only the CHECK_ONLY
score (without any BSMTP headers), or to shove the filtered message back into
the BSMTP envelope for output to the next process in line.

The end result is that--instead of outputting a valid BSMTP file--it outputs the
raw message even in BSMTP mode, which makes 'exim -bS' choke and causes ALL my
mail to bounce. Congradulations, you've created a critical bug (i.e. loss of
data with no easy workaround other than "don't use spamc").

I don't have time these days to fix it myself, except for a quick hack locally
(i've broken -c so -B works). But i suggest someone look into adding some flag
or another so message_write can determine whether it needs to produce CHECK_ONLY
output or do full output processing. And add a test to see that -B actually
outputs BSMTP output when -c is not given.
Comment 1 Justin Mason 2003-03-18 15:45:41 UTC 
eh, crap -- I missed this during the move.  it's a biggie, and we should have
fixed it for 2.51. :(    2.52 it is then.

BTW this will probably need a tester who uses -B.  anomie, you OK to test any
patches I may produce?
Comment 2 Theo Van Dinter 2003-03-24 13:40:50 UTC 
nope.  2.53.  doh.
Comment 3 Justin Mason 2003-03-26 13:08:50 UTC 
anomie, (or any other BSMTP user), could you test this patch?

if it fixes it, I'll apply throughout and it'll be in 2.53.  It adds
a new EX_ type, EX_OUTPUTMESSAGE, and uses that when message_write()
should output a message, and sets that for the non-SPAMC_CHECK_ONLY case.

I've also figured out what a BSMTP transaction looks like ;) and added
a "t" script, "spamc_B.t".
Comment 4 Justin Mason 2003-03-26 13:09:11 UTC 
Created attachment 813 [details]
fixes bsmtp
Comment 5 Justin Mason 2003-03-26 17:59:18 UTC 
taking the bug
Comment 6 Theo Van Dinter 2003-03-27 09:58:14 UTC 
OKAY: I'm not really familiar with the code, but it looks okay to me.
Comment 7 Theo Van Dinter 2003-03-27 10:00:03 UTC 
BTW: is this ready for inclusion?  there's no keyword set.
Comment 8 Antony Mawer 2003-03-27 11:07:17 UTC 
Subject: Re:  Someone broke BSMTP mode 


> BTW: is this ready for inclusion?  there's no keyword set.

I want to get approval from anomie (or someone else using BSMTP) first.

--j.
Comment 9 Theo Van Dinter 2003-03-27 14:07:35 UTC 
2.53 is due out RSN, so this is being pushed to 2.54
Comment 10 Brad "anomie" Jorsch 2003-03-28 09:00:26 UTC 
Subject: Re:  Someone broke BSMTP mode

On Thu, Mar 27, 2003 at 11:07:18AM -0800, bugzilla-daemon@hughes-family.org wrote:
> 
> I want to get approval from anomie (or someone else using BSMTP) first.

Works for me, but you'll probably get complaints from others about the
is_spam field being useless again... Perhaps something like this?

#define EX_ISSPAM_MASK            1
#define EX_OUTPUTMESSAGE_MASK     0x400

#define EX_NOTSPAM                (0 | 0)
#define EX_ISSPAM                 (EX_ISSPAM_MASK | 0)
#define EX_OUTPUTMESSAGE_NOTSPAM  (0 | EX_OUTPUTMESSAGE_MASK)
#define EX_OUTPUTMESSAGE_ISSPAM   (EX_OUTPUTMESSAGE_MASK | EX_ISSPAM_MASK)

It's either that or add a new internal field to struct message.

Hmmm... BTW, you might want to check for flags&SPAMC_REPORT,
SPAMC_REPORT_IFSPAM, or SPAMC_SYMBOLS, and not return those as output
messages (IOW, treat them like SPAMC_CHECK_ONLY as far as
message_write() is concerned).
Comment 11 Antony Mawer 2003-03-28 13:22:15 UTC 
Subject: Re:  Someone broke BSMTP mode 


> Hmmm... BTW, you might want to check for flags&SPAMC_REPORT,
> SPAMC_REPORT_IFSPAM, or SPAMC_SYMBOLS, and not return those as output
> messages (IOW, treat them like SPAMC_CHECK_ONLY as far as
> message_write() is concerned).

gross ;)

What I think I'll do is apply patch as-is for 2.53, ignore the is_spam
thing not being set for certain modes (because that's how it worked
in 2.4x anyway), and in 2.60 expand the struct message to allow this
to be done less kludgily.  (I'll also include room for an expansion
field in the struct message to keep binary compat from then on.)

--j.
Comment 12 Justin Mason 2003-03-28 13:25:12 UTC 
oh BTW -- just to make it clear -- the m.is_spam field is not broken by this patch. 

it just goes back to what it did in 2.4x, which is have a special type to
indicate "the result from the operation is in the 'out' string as a full
message, instead of using this boolean to indicate spam/nonspam status".   This
is what's documented in the API, and it's what -B requires.  So there's no
breakage from this patch.
Comment 13 Justin Mason 2003-03-28 13:28:39 UTC 
alright! Just in time for 2.53... now checked in on that branch.