6.0.0-beta1
7/11/25

[#5816] Added more tests to maildrop Script and fixed REJECT action
Summary Added more tests to maildrop Script and fixed REJECT action
Queue Ingo
Queue Version HEAD
Type Enhancement
State Resolved
Priority 2. Medium
Owners chuck (at) horde (dot) org
Requester horde (at) nospam (dot) obeliks (dot) de
Created 10/18/2007 (6476 days ago)
Due
Updated 12/27/2007 (6406 days ago)
Assigned
Resolved 12/27/2007 (6406 days ago)
Milestone
Patch No

History
12/27/2007 05:06:59 AM Chuck Hagenbuch Comment #18
State ⇒ Resolved
Reply to this comment
Okay, I've removed the blacklist/whitelist changes for now, which 
makes the main bit of this ticket resolved. If you have a chance later 
to submit the black/whitelist changes in the future, that'd be great!
12/26/2007 08:18:14 PM horde (at) nospam (dot) obeliks (dot) de Comment #17 Reply to this comment
We're getting much closer to release. Any updates on a patch?
I'm sorry, my time is quite limited right now, since I'm very near to 
the deadline of a project myself. I certainly can't provide a solution 
for a more sophisticated expression model, like for example in Sieve 
support.



Improving support for black/whitelist isn't that hard in concept 
though. It would be best not to match against a header value, but 
against the predefined $FROM-Variable (cp. 
http://www.courier-mta.org/maildrop/maildropfilter.html) that contains 
the sender _address_ only.

This would however require a condition that isn't based on regular 
expressions being matched against the header lines, but a condition 
like ($FROM eq "address1"), which is why I though creating a more 
flexible recipe/condition model would be the right way to go.



To work around that, the easiest and most elegant way would probably 
be to create a new Maildrop_ListRecipe class, that inherits from 
Maildrop_Recipe and overrides addCondition() as well as generate() 
(the latter can be copied nearly 100%, just the condition->text 
transformation has to be altered -- maybe it would be better to change 
it in the original Maildrop_Recipe and only have to change both 
addCondition() implementations).



I'm sorry I can't provide a patch right now, maybe I can help out for 
the next release. If nobody can be found doing this work until the 
next release, it would probably still be best to revert to the old 
"contains"-Condition for While/Blacklists.
12/24/2007 09:11:28 PM Chuck Hagenbuch Comment #16 Reply to this comment
We're getting much closer to release. Any updates on a patch?
11/20/2007 08:47:50 PM Chuck Hagenbuch Comment #15
State ⇒ Feedback
Priority ⇒ 2. Medium
Reply to this comment

[Show Quoted Text - 10 lines]
We're near an RC, but not a final release. There should really not be 
multiple addresses in From:, but I see the point about getaddr. I'll 
wait for (or look into myself, though that might be wishful) a patch 
for now.



Thanks for all the help on maildrop support!
11/19/2007 09:35:07 PM horde (at) nospam (dot) obeliks (dot) de Comment #14 Reply to this comment
I missed one thing: imho, white- and blacklist should use 'is' as
match type, not 'contains'... if I blacklist "one@domain.com", I
don't want to blacklist "someone@domain.com" in one go...
I recently noticed why 'contains' was used so far... because the From: 
header mostly consists of more than just the sender address. Using 
'is' surely isn't the right solution, the proper way to do this is 
using maildrops getaddr() on the From header and checking each listed 
email address...

I'll take a shot at this when dealing with the changes I mentioned in 
bug 5843.



If a release is due, maybe you should switch back to 'contains' for 
time being. Otherwise just wait for my patch ;)


11/19/2007 07:49:38 PM Chuck Hagenbuch Comment #13 Reply to this comment
Example added to backends.php.dist.
11/19/2007 11:54:06 AM raptor-horde (at) xpls (dot) de Comment #12 Reply to this comment
In a standard courier-mta setup with maildrop filtering, the
resulting filter script becomes invalid due to the leading "INBOX."
within the path. strip_inbox is actually needed in this case.
And so strip_inbox is configurable... or are you suggesting a change
in one of the .dist files? I'm confused, sorry.
It would be nice to have this option shown at least as an example 
within one of the dist files . Otherwise I would suggest as setting it 
to true by default, to not break existing installations. Great work, 
btw :)


11/19/2007 05:07:52 AM Chuck Hagenbuch Comment #11 Reply to this comment
In a standard courier-mta setup with maildrop filtering, the
resulting filter script becomes invalid due to the leading "INBOX."
within the path. strip_inbox is actually needed in this case.
And so strip_inbox is configurable... or are you suggesting a change 
in one of the .dist files? I'm confused, sorry.
11/18/2007 07:00:01 PM raptor-horde (at) xpls (dot) de Comment #10 Reply to this comment
In a standard courier-mta setup with maildrop filtering, the resulting 
filter script becomes invalid due to the leading "INBOX." within the 
path. strip_inbox is actually needed in this case.


10/22/2007 09:33:08 PM Chuck Hagenbuch Comment #9
Version ⇒ HEAD
Reply to this comment
These changes will be in Ingo 1.2.
10/22/2007 09:32:40 PM Chuck Hagenbuch Comment #8
State ⇒ Resolved
Assigned to Chuck Hagenbuch
Reply to this comment
Committed to HEAD (only very minor rejects, easily fixed), thanks!
10/22/2007 05:06:26 PM horde (at) nospam (dot) obeliks (dot) de Comment #7 Reply to this comment
Fixes should always be for HEAD and then backported, unless the
problem doesn't exist in HEAD, but I'll take a look. Thanks for the
updated patch.
I just applied my changes to the file I had on my system, but I'll 
diff against HEAD next time... but since not much changed in the 
maildrop.php except for vacation support, the patch might even succeed 
with some fuzziness ;)
10/22/2007 05:01:54 PM Chuck Hagenbuch Comment #6 Reply to this comment
Fixes should always be for HEAD and then backported, unless the 
problem doesn't exist in HEAD, but I'll take a look. Thanks for the 
updated patch.
10/22/2007 04:59:54 PM Chuck Hagenbuch Deleted Original Message
 
10/22/2007 04:58:23 PM horde (at) nospam (dot) obeliks (dot) de Comment #5
New Attachment: horde-ingo_maildrop-support.diff Download
Reply to this comment
Hmm - I get this trying to apply the patch:

Maya:/var/www/horde/ingo/lib/Script chuck$ patch <
/Users/chuck/Desktop/horde-ingo_maildrop-support\[2\].diff
patching file maildrop.php
patch: **** malformed patch at line 21: @@ -181,7 +190,7 @@
I'm sorry, I shouldn't have tried to manually edit the diff file ;) 
Here comes a new version, straight from `diff`. Fixes one typo too 
(missing comma).

Keep in mind that it's patched against 1.1.2 though
10/22/2007 04:46:59 PM Chuck Hagenbuch Comment #4
State ⇒ Feedback
Reply to this comment
Hmm - I get this trying to apply the patch:



Maya:/var/www/horde/ingo/lib/Script chuck$ patch < 
/Users/chuck/Desktop/horde-ingo_maildrop-support\[2\].diff

patching file maildrop.php

patch: **** malformed patch at line 21: @@ -181,7 +190,7 @@
10/22/2007 04:45:33 PM Chuck Hagenbuch Deleted Original Message
 
10/22/2007 04:45:24 PM Chuck Hagenbuch Deleted Original Message
 
10/18/2007 07:39:23 PM horde (at) nospam (dot) obeliks (dot) de Comment #3
New Attachment: horde-ingo_maildrop-support[2].diff
Reply to this comment
My apologies, I did it again ;)



Here comes the second update to the original patch, I've added 
"matches", "not matches", "exists" and "not exist" tests.



I also have a nice idea on how to implement relational tests, but that 
would require more restructuring, which is why I'll probably wait 
until I have a more recent version of the original file on my system.



Body and Size tests should be easy too.



Updated patch attached.
10/18/2007 01:04:08 PM horde (at) nospam (dot) obeliks (dot) de Comment #2
New Attachment: horde-ingo_maildrop-support[1].diff
Reply to this comment
I missed one thing: imho, white- and blacklist should use 'is' as 
match type, not 'contains'... if I blacklist "one@domain.com", I don't 
want to blacklist "someone@domain.com" in one go...



Another thing I didn't mention before: I changed the DISCARD action 
from "to /dev/null" to "exit", which looks cleaner to me.



Updated patch attached.
10/18/2007 12:38:16 PM horde (at) nospam (dot) obeliks (dot) de Comment #1
Priority ⇒ 1. Low
State ⇒ New
New Attachment: horde-ingo_maildrop-support.diff Download
Queue ⇒ Ingo
Type ⇒ Enhancement
Summary ⇒ Added more tests to maildrop Script and fixed REJECT action
Reply to this comment
This patch addresses three issues (all are still relevant in cvs, but 
the patch was generated against 1.1.2):



1. It adds the "is" test, as well as all negated versions of the 
currently available tests (i.e. 'not contain', 'is', 'not is', 'not 
begins with', 'not ends with') by prefixing the patterns with "! ".



2. It fixes the REJECT action for maildrop scripts. The current 
version uses "EXITCODE=reject reason", which is bogus, because 
maildrop needs a numerical exit code value. In order to pass a reject 
reason to the MTA, the latter must support reading it from the program 
output (e.g. postfix >= 2.3).

My code sets EXITCODE=77 (EX_NOPERM) and echoes "5.7.1 Reject Reason" 
to stdout before exiting. This way, if the MTA supports reading 
RFC3463 Status Codes from program output, it will provide the intended 
error message, otherwise it should still reject the message permanently.

fyi: 5.7.1 is the status code Postfix uses to reject messages (if no 
other code is given)



3. The current Maildrop script strips a leading "INBOX." from path 
names, which might be correct if INBOX is used as the root of the 
Maildir. However, IMAP servers like Dovecot support having root 
folders next to the INBOX. In such setups, the current code fails 
completely.

Therefore, I introduced another backend-parameter, strip_inbox, which 
let's the admin configure this behaviour. Maybe a more elegant 
approach would be to introduce a third mailbox format ("mbox", 
"maildir", "maildir-inbox" or similar).





The code has been tested briefly, and has so far created no problems 
on my system (postfix+dovecot+custom maildrop script).



hth, Bernhard Frauendienst

Saved Queries