As things stand the spam threshold is configurable per user. Shouldn't the threshold used be either the higher of the two, or perhaps an offset from the normal configured spam threshold.

(In reply to comment #1)
> As things stand the spam threshold is configurable per user. Shouldn't the
> threshold used be either the higher of the two, or perhaps an offset from
> the normal configured spam threshold.

I like the idea of making it an offset rather than an absolute value.  So, to clarify, what you're saying is that if your normal spam threshold is 3 and you want the override to be 8, instead of passing -T 8.0, you'd pass -T 5.0?  The one thing is that you wouldn't be able to pass a negative number on the command line, but actually I don't think that makes semantic sense anyway (you want to discard spam that SpamAssassin has *not* marked as spam??) so that may not be a problem.

Created attachment 5131 [details]
Second patch; uses offset value instead of absolute

This version of the patch makes -T into an offset value instead of an absolute threshold override, as discussed previously.

(In reply to comment #2)

imo, absolute value would make it easier for the average user and safer.

Offset can lead to confusion.

if default is 5.0
to stick how several known glue systems' naming change could be:

low/tag spam to 3.0
high/kill spam 15.0

I don't really like the idea of adding complexity to spamc but my voice is one among many.

(In reply to comment #4)
> (In reply to comment #2)
> 
> imo, absolute value would make it easier for the average user and safer.

Well, both patches seem to be bug-free, it's just that the first one uses an absolute value for the threshold and the second uses an offset.  You can choose which one to apply based on what you want -T to do.

As I was advised on the mailing list, I'm just noting here that I have now been running this patch on my server for several months with no problems.  :-)

Frankly, I don't like the idea of adding such decision / override option to spamc.

First of all, spamc is meant to be a light-weight client, merely feeding messages to the daemon, and dealing with the result in an absolute minimal way. spamc deliberately does not decide about the score, nor rewrite the message or even add headers. That is entirely left to spamd.

Thus, if a feature like this would be implemented, I am strongly in favor of dealing with it in spamd.

Moreover, this basically duplicates existing functionality, and introduces the notion of "discard" -- something SA does not do. SA scores, but it does not deliver, nor discard.

The environment described in comment 0 is almost identical to adding the X-Spam-Level header, which is not by coincidence meant to easily allow for this shades of gray differentiation. The only difference between this existing behavior and the request is, what the spamc calling tool acts upon -- the $is_spam return value, instead of grepping for a single line.

Changing, overriding the decision (exitcode) whether a message is deemed spam or not in this way seems pretty confusing, too. A message's score exceeding the required_score threshold is classified spam. The return value of spamc should not claim anything else.


Setting Severity back to enhancement, as Jez, the reporter, even set it himself. I don't see how this would be a blocker.

(In reply to comment #7)
> First of all, spamc is meant to be a light-weight client, merely feeding
> messages to the daemon, and dealing with the result in an absolute minimal
> way. spamc deliberately does not decide about the score, nor rewrite the
> message or even add headers. That is entirely left to spamd.

I fail to see the problem.  spamc can optionally care about the score - it's a very convenient place to do it because it already has the score stored in a struct in memory.  Serializing it to text and then deserializing again is horribly inefficient.

> Thus, if a feature like this would be implemented, I am strongly in favor of
> dealing with it in spamd.

By definition this can't be dealt with in spamd, because it is supposed to offer a lightweight way of defining a "grey area" for the spam score.

> Moreover, this basically duplicates existing functionality

No it doesn't, or I wouldn't have had to add it.  Existing functionality is slow, slow, slow.  This is so much quicker that I'd say it counts as new functionality.

> , and introduces
> the notion of "discard" -- something SA does not do. SA scores, but it does
> not deliver, nor discard.

It doesn't really introduce the notion of discard, I just suggested that one interpretation a shell script might have for a return value of 1 is to discard the message - which is what often happens already - but of course the script can do what it wants.

> The environment described in comment 0 is almost identical to adding the
> X-Spam-Level header, which is not by coincidence meant to easily allow for
> this shades of gray differentiation. The only difference between this
> existing behavior and the request is, what the spamc calling tool acts upon
> -- the $is_spam return value, instead of grepping for a single line.

Quite, and when that tool is a shell script, it has very little to work with.  grepping for a score that has been serialized, deserializing it again, and re-converting it to an integer is horrible to do in bash and, apart from anything else, inefficient as hell.  There's a good reason I didn't want to go down that route.

> Changing, overriding the decision (exitcode) whether a message is deemed
> spam or not in this way seems pretty confusing, too. A message's score
> exceeding the required_score threshold is classified spam. The return value
> of spamc should not claim anything else.

If if changed the existing behaviour, I'd agree it could be confusing, but it doesn't.  By default, nothing changes with this patch.  You have to add the extra commandline argument.  Who is going to do that without reading the documentation?  If they randomly add this commandline arg and get confused by it, then I'd say it's their fault.

(In reply to comment #8)
> Quite, and when that tool is a shell script, it has very little to work
> with.  grepping for a score that has been serialized, deserializing it
> again, and re-converting it to an integer is horrible to do in bash and,
> apart from anything else, inefficient as hell.  There's a good reason I
> didn't want to go down that route.

The RE is straight forward, and documented in the procmail recipe example. There is nothing to serialize and deserialize, since the RE works on the already existing X-Spam-Level header.

  grep -E '^X-Spam-Level: \*{8,}


BTW, which variant of "rewriting" are you using? Rewriting headers, or attaching the spam to a wrapper message?

(In reply to comment #9)
> (In reply to comment #8)
> > Quite, and when that tool is a shell script, it has very little to work
> > with.  grepping for a score that has been serialized, deserializing it
> > again, and re-converting it to an integer is horrible to do in bash and,
> > apart from anything else, inefficient as hell.  There's a good reason I
> > didn't want to go down that route.
> 
> The RE is straight forward, and documented in the procmail recipe example.
> There is nothing to serialize and deserialize, since the RE works on the
> already existing X-Spam-Level header.
> 
>   grep -E '^X-Spam-Level: \*{8,}

So grepping is as efficient as checking a process's return value?

> BTW, which variant of "rewriting" are you using? Rewriting headers, or
> attaching the spam to a wrapper message?

Rewriting headers.

(In reply to comment #10)
> >   grep -E '^X-Spam-Level: \*{8,}
> 
> So grepping is as efficient as checking a process's return value?

Of course not. But if *that* difference makes such a huge deal to you, you must be shocked when you figure out SA itself is written in Perl, not C...

> > BTW, which variant of "rewriting" are you using? Rewriting headers, or
> > attaching the spam to a wrapper message?
> 
> Rewriting headers.

Good, so you have the headers I referred to. :)  However, this also makes me wonder why exactly you are using this approach -- other than optimization.

With rewriting headers (report_safe 0), there is about one difference in rewriting: The X-Spam-Report header with rules' scores and descriptions. Your comment 0 suggests rewriting would not happen for ham -- which is false for header rewriting. X-Spam headers are added always, with some (by default) being restricted to spam.

You can change that and add them unconditionally. So your 3..8 gray area will be "rewritten" just the same as spam, which is what you want. (Granted, ham gets this header, too.) By setting required_score 8, spamc exits with the desired code...


Jez, please don't get me wrong -- it is great to see you contributing the patch that fixes a particular issue for you. This is open source at its best.

I am always open to being convinced. However, at this point, this is just an extreme over-optimization edge case, and borderline consistent with SA terms. My personal opinion is against inclusion in upstream.

Jez, I look forward to other patches but this one can't be accepted upstream.