Re: [Mailman-Developers] [ mailman-Bugs-1416853 ] Jan 14 change to Handlers/SpamDetect.py isincomplete

Dan Astoorian wrote:
OK
Actually, as you implicitly point out, this isn't necessary since only list posts should be in the moderation queue in the first place. The only other messages that currently get held are messages to the owner that get caught by a Spam filter 'hold' rule, and these shouldn't be held. At least that's how it seems pending other input.
-- Mark Sapiro <msapiro@value.net> The highway is for gamblers, San Francisco Bay Area, California better use your sense - B. Dylan

On Fri, 2006-01-27 at 14:30 -0800, Mark Sapiro wrote:
That seems right to me. It doesn't make any sense to hold a message ultimately destined for the list owner or moderator because those are the people who have to approve the message (what? so they can then read it again in their inbox?).
If in response to a certain spam designation the owner destined message is discarded, that's fine. But it shouldn't be held, and rejecting spam is almost always a bad idea.
-Barry

Barry Warsaw wrote:
OK, Barry. I've come up with this patch (for the current CVS). If its OK, I want to start up for the release of 2.1.8a1. -- Tokio Kikuchi, tkikuchi@ is.kochi-u.ac.jp http://weather.is.kochi-u.ac.jp/ Index: SpamDetect.py =================================================================== RCS file: /cvsroot/mailman/mailman/Mailman/Handlers/SpamDetect.py,v retrieving revision 2.3.2.11 diff -u -r2.3.2.11 SpamDetect.py --- SpamDetect.py 14 Jan 2006 10:11:19 -0000 2.3.2.11 +++ SpamDetect.py 28 Jan 2006 02:01:34 -0000 @@ -102,15 +102,6 @@ if mo: # we've detected spam, so throw the message away raise SpamDetected - # Before we go to header_filter_rules, we exclude internally generated - # owner notification from checking, because 1) we collect headers from - # all the attachments but this will cause matching the filter rule again, - # and 2) list owners may want to check header name / value pair like - # 'Precedence: bulk' which is also generated by mailman. Both will - # cause loop of holding owner notification messages if the action is - # set to 'hold'. - if msgdata.get('toowner') and msg.get('x-list-administrivia') == 'yes': - return # Now do header_filter_rules # TK: Collect headers in sub-parts because attachment filename # extension may be a clue to possible virus/spam. @@ -132,9 +123,18 @@ if action == mm_cfg.DISCARD: raise Errors.DiscardMessage if action == mm_cfg.REJECT: + if msgdata.get('toowner'): + # Don't send rejecting message if addressed + # to '-owner'. We just discard here. + raise Errors.DiscardMessage raise Errors.RejectMessage( _('Message rejected by filter rule match')) if action == mm_cfg.HOLD: + if msgdata.get('toowner'): + # Don't hold '-owner' addressed message. We just + # pass it here but list-owner can set this to be + # discarded on the GUI if he wants. + return hold_for_approval(mlist, msg, msgdata, HeaderMatchHold) if action == mm_cfg.ACCEPT: return

Tokio Kikuchi wrote:
OK, Barry. I've come up with this patch (for the current CVS). If its OK, I want to start up for the release of 2.1.8a1.
The pass through of the 'hold' action if the message is to -owner seems right to me, but discarding instead of rejecting a 'reject' action if the message is to -owner seems wrong.
I think we should not change the disposition for a 'reject' action. The rule can be for lots of purposes, not just spam and if the owner has configured the rule to reject the message, I don't think we should discard it just because it is to -owner and not to the list.
I have one idea for 2.1.8a1 before we wrap it up. I'll address that in a separate post.
-- Mark Sapiro <msapiro@value.net> The highway is for gamblers, San Francisco Bay Area, California better use your sense - B. Dylan

Mark Sapiro wrote:
Well, then it should be passed. Rejection makes it another loop of rejection notices if an initial poster forges the From: address as list-owner. Yes, I tested this. Nothing could stop this if the mailman qrunner wasn't stopped. (Maybe growing size of rejection notice would hit the MTA limit.)
OK.
-- Tokio Kikuchi, tkikuchi@ is.kochi-u.ac.jp http://weather.is.kochi-u.ac.jp/

Tokio Kikuchi wrote:
Ahh. I hadn't thought of that. Maybe discard it, but put something in the comment about the potential loop if we reject.
-- Mark Sapiro <msapiro@value.net> The highway is for gamblers, San Francisco Bay Area, California better use your sense - B. Dylan

On Fri, 2006-01-27 at 14:30 -0800, Mark Sapiro wrote:
That seems right to me. It doesn't make any sense to hold a message ultimately destined for the list owner or moderator because those are the people who have to approve the message (what? so they can then read it again in their inbox?).
If in response to a certain spam designation the owner destined message is discarded, that's fine. But it shouldn't be held, and rejecting spam is almost always a bad idea.
-Barry

Barry Warsaw wrote:
OK, Barry. I've come up with this patch (for the current CVS). If its OK, I want to start up for the release of 2.1.8a1. -- Tokio Kikuchi, tkikuchi@ is.kochi-u.ac.jp http://weather.is.kochi-u.ac.jp/ Index: SpamDetect.py =================================================================== RCS file: /cvsroot/mailman/mailman/Mailman/Handlers/SpamDetect.py,v retrieving revision 2.3.2.11 diff -u -r2.3.2.11 SpamDetect.py --- SpamDetect.py 14 Jan 2006 10:11:19 -0000 2.3.2.11 +++ SpamDetect.py 28 Jan 2006 02:01:34 -0000 @@ -102,15 +102,6 @@ if mo: # we've detected spam, so throw the message away raise SpamDetected - # Before we go to header_filter_rules, we exclude internally generated - # owner notification from checking, because 1) we collect headers from - # all the attachments but this will cause matching the filter rule again, - # and 2) list owners may want to check header name / value pair like - # 'Precedence: bulk' which is also generated by mailman. Both will - # cause loop of holding owner notification messages if the action is - # set to 'hold'. - if msgdata.get('toowner') and msg.get('x-list-administrivia') == 'yes': - return # Now do header_filter_rules # TK: Collect headers in sub-parts because attachment filename # extension may be a clue to possible virus/spam. @@ -132,9 +123,18 @@ if action == mm_cfg.DISCARD: raise Errors.DiscardMessage if action == mm_cfg.REJECT: + if msgdata.get('toowner'): + # Don't send rejecting message if addressed + # to '-owner'. We just discard here. + raise Errors.DiscardMessage raise Errors.RejectMessage( _('Message rejected by filter rule match')) if action == mm_cfg.HOLD: + if msgdata.get('toowner'): + # Don't hold '-owner' addressed message. We just + # pass it here but list-owner can set this to be + # discarded on the GUI if he wants. + return hold_for_approval(mlist, msg, msgdata, HeaderMatchHold) if action == mm_cfg.ACCEPT: return

Tokio Kikuchi wrote:
OK, Barry. I've come up with this patch (for the current CVS). If its OK, I want to start up for the release of 2.1.8a1.
The pass through of the 'hold' action if the message is to -owner seems right to me, but discarding instead of rejecting a 'reject' action if the message is to -owner seems wrong.
I think we should not change the disposition for a 'reject' action. The rule can be for lots of purposes, not just spam and if the owner has configured the rule to reject the message, I don't think we should discard it just because it is to -owner and not to the list.
I have one idea for 2.1.8a1 before we wrap it up. I'll address that in a separate post.
-- Mark Sapiro <msapiro@value.net> The highway is for gamblers, San Francisco Bay Area, California better use your sense - B. Dylan

Mark Sapiro wrote:
Well, then it should be passed. Rejection makes it another loop of rejection notices if an initial poster forges the From: address as list-owner. Yes, I tested this. Nothing could stop this if the mailman qrunner wasn't stopped. (Maybe growing size of rejection notice would hit the MTA limit.)
OK.
-- Tokio Kikuchi, tkikuchi@ is.kochi-u.ac.jp http://weather.is.kochi-u.ac.jp/

Tokio Kikuchi wrote:
Ahh. I hadn't thought of that. Maybe discard it, but put something in the comment about the potential loop if we reject.
-- Mark Sapiro <msapiro@value.net> The highway is for gamblers, San Francisco Bay Area, California better use your sense - B. Dylan
participants (3)
-
Barry Warsaw
-
Mark Sapiro
-
Tokio Kikuchi