PATCH: Access priority

PATCH: Access priority

[Ben Schmidt]

[-- Attachment #1: Type: text/plain, Size: 639 bytes --]

This patch fixes something that I got bitten by last week, and since I spotted the 
place in the code while I was working on the other stuff, I did this too! It 
applies over 1.2.17.

The problem is that if you have a list that is 'moderated' or 'modnonsubposts', 
the 'allow' access keyword is ineffective, but behaves like 'moderate'. This patch 
fixes that so that access rules have priority, and do what they say.

I think this is more intuitive.

Do others agree? Or could this change cause problems? If not, or the perceived 
problems are minor enough, could this be included in mlmmj?

I have tested it, and it works for me.

Ben.


[-- Attachment #2: mlmmj-access.patch --]
[-- Type: text/x-patch, Size: 781 bytes --]

diff -ru mlmmj-1.2.17/src/mlmmj-process.c mlmmj-1.2.17-mod/src/mlmmj-process.c
--- mlmmj-1.2.17/src/mlmmj-process.c	2010-01-11 00:40:57.000000000 +1100
+++ mlmmj-1.2.17-mod/src/mlmmj-process.c	2010-01-25 02:23:23.000000000 +1100
@@ -851,6 +852,9 @@
 	}
 
 startaccess:
+	if(!moderated)
+		moderated = statctrl(listdir, "moderated");
+
 	noaccessdenymails = statctrl(listdir, "noaccessdenymails");
 
 	access_rules = ctrlvalues(listdir, "access");
@@ -917,11 +921,11 @@
 			myfree(donemailname);
 			myfree(discardname);
                 	exit(EXIT_SUCCESS);
+		} else if (accret == ALLOW) {
+			moderated = 0;
 		}
 	}
 
-	if(!moderated)
-		moderated = statctrl(listdir, "moderated");
 	if(moderated) {
 		mqueuename = concatstr(3, listdir, "/moderation/",
 				       randomstr);

Re: PATCH: Access priority

[Ben Schmidt]

[-- Attachment #1: Type: text/plain, Size: 822 bytes --]

Here is an updated version of this patch that applies on top of the revised 
notmetoo patch I just emailed.

Ben.



On 25/01/10 10:02 PM, Ben Schmidt wrote:
> This patch fixes something that I got bitten by last week, and since I
> spotted the place in the code while I was working on the other stuff, I
> did this too! It applies over 1.2.17.
>
> The problem is that if you have a list that is 'moderated' or
> 'modnonsubposts', the 'allow' access keyword is ineffective, but behaves
> like 'moderate'. This patch fixes that so that access rules have
> priority, and do what they say.
>
> I think this is more intuitive.
>
> Do others agree? Or could this change cause problems? If not, or the
> perceived problems are minor enough, could this be included in mlmmj?
>
> I have tested it, and it works for me.
>
> Ben.
>

[-- Attachment #2: mlmmj-access-2.patch --]
[-- Type: text/x-patch, Size: 827 bytes --]

diff -ru mlmmj-1.2.17/src/mlmmj-process.c mlmmj-1.2.17-mod/src/mlmmj-process.c
--- mlmmj-1.2.17/src/mlmmj-process.c	2010-01-26 00:13:49.000000000 +1100
+++ mlmmj-1.2.17-mod/src/mlmmj-process.c	2010-01-26 00:15:26.000000000 +1100
@@ -852,6 +854,9 @@
 	}
 
 startaccess:
+	if(!moderated)
+		moderated = statctrl(listdir, "moderated");
+
 	noaccessdenymails = statctrl(listdir, "noaccessdenymails");
 
 	access_rules = ctrlvalues(listdir, "access");
@@ -918,13 +923,13 @@
 			myfree(donemailname);
 			myfree(discardname);
                 	exit(EXIT_SUCCESS);
+		} else if (accret == ALLOW) {
+			moderated = 0;
 		}
 	}
 
 	notmetoo = statctrl(listdir, "notmetoo");
 
-	if(!moderated)
-		moderated = statctrl(listdir, "moderated");
 	if(moderated) {
 		mqueuename = concatstr(3, listdir, "/moderation/",
 				       randomstr);

Re: PATCH: Access priority

[Morten Shearman Kirkegaard]

On Mon, 2010-01-25 at 22:02 +1100, Ben Schmidt wrote: 
> The problem is that if you have a list that is 'moderated' or
> 'modnonsubposts', the 'allow' access keyword is ineffective, but
> behaves like 'moderate'. This patch fixes that so that access rules
> have priority, and do what they say.
> 
> I think this is more intuitive.

Since we now have the "moderate" keyword, then maybe you are right. It
made more sense when it was just "allow" or "deny." Maybe "moderate"
should have been "hold" or something like that.

> Do others agree? Or could this change cause problems?

It would break backwards compatibility on a major feature, so I don't
want to include it in the 1.2-series.

I also think change this needs a bit more thought. I've tried to graph
the flow as it is in mlmmj now, and what I think you are proposing.
Please correct me, if I misinterpreted your text - I did not look into
the code patch.

The "moderation" boxes represent the "moderated" and "modnonsubposts"
tunables.


In mlmmj now:

   +--------+ deny   +--------+
   | access |------->| reject |
   +--------+        +--------+
allow |  | moderate
      |  +-----------+
      v              v
+------------+    +------+
| moderation |--->| hold |
+------------+    +------+
      |              |
      v              |
   +------+          |
   | send |<---------+
   +------+


Proposed flow:

allow +--------+ deny   +--------+
+-----| access |------->| reject |
|     +--------+        +--------+
|         | moderate
|         v
|   +------------+    +------+
|   | moderation |--->| hold |
|   +------------+    +------+
|         |              |
|         v              |
|      +------+          |
+----->| send |<---------+
       +------+


Pros: With the proposed flow, it is possible to force a mail through via
an access rule. The keywords might also make more sense to the
administrators.


Cons: With the proposed flow, it is impossible to force a mail to be
held for moderation. If a list is modnonsubposts, but the administrators
would like a chance to examine multipart mails, before they are sent to
the list, it can be done like this today:

moderate ^Content-Type: multipart/
allow


One possible solution - at least to the bypass moderation problem -
might be to add another keyword. We could add "send" to force the mail
past normal moderation procedures.

A possible new flow:

 send +--------+ deny   +--------+
+-----| access |------->| reject |
|     +--------+        +--------+
|  allow |  | moderate
|        |  +-----------+
|        v              v
|  +------------+    +------+
|  | moderation |--->| hold |
|  +------------+    +------+
|        |              |
|        v              |
|     +------+          |
+---->| send |<---------+
      +------+

send ^X-Password: 90e85684b1285a9f$
moderate ^Content-Type: multipart/
allow

I know it does not fix the meaning of "allow" vs. "moderate" on lists
which are also subject to regular moderation, but I don't see a way to
do that without breaking backwards compatibility.

Any thoughts on this?

Morten

-- 
Morten Shearman Kirkegaard <moki@fabletech.com>
CTO, FableTech
http://fabletech.com/

Re: PATCH: Access priority

[Ben Schmidt]

> One possible solution - at least to the bypass moderation problem -
> might be to add another keyword. We could add "send" to force the mail
> past normal moderation procedures.

Exactly what I was thinking after reading the issues you raised above.

> I know it does not fix the meaning of "allow" vs. "moderate" on lists
> which are also subject to regular moderation, but I don't see a way to
> do that without breaking backwards compatibility.

It wouldn't be too big a problem--if it were documented. But it isn't,
so the current behaviour is just unexpected, and I would say wrong. At
least if it's documented, it's unexpected if you don't read the docs,
but right.

Shall I put together a new patch?

Ben.

Re: PATCH: Access priority

[Franky Van Liedekerke]

On Tue, 16 Feb 2010 17:52:51 +0100
Morten Shearman Kirkegaard <moki@fabletech.com> wrote:

> On Mon, 2010-01-25 at 22:02 +1100, Ben Schmidt wrote: 
> > The problem is that if you have a list that is 'moderated' or
> > 'modnonsubposts', the 'allow' access keyword is ineffective, but
> > behaves like 'moderate'. This patch fixes that so that access rules
> > have priority, and do what they say.
> > 
> > I think this is more intuitive.
> 
> Since we now have the "moderate" keyword, then maybe you are right. It
> made more sense when it was just "allow" or "deny." Maybe "moderate"
> should have been "hold" or something like that.
> 
> > Do others agree? Or could this change cause problems?
> 
> It would break backwards compatibility on a major feature, so I don't
> want to include it in the 1.2-series.
> 
> I also think change this needs a bit more thought. I've tried to graph
> the flow as it is in mlmmj now, and what I think you are proposing.
> Please correct me, if I misinterpreted your text - I did not look into
> the code patch.
> 
> The "moderation" boxes represent the "moderated" and "modnonsubposts"
> tunables.
> 
> 
> In mlmmj now:
> 
>    +--------+ deny   +--------+
>    | access |------->| reject |
>    +--------+        +--------+
> allow |  | moderate
>       |  +-----------+
>       v              v
> +------------+    +------+
> | moderation |--->| hold |
> +------------+    +------+
>       |              |
>       v              |
>    +------+          |
>    | send |<---------+
>    +------+
> 
> 
> Proposed flow:
> 
> allow +--------+ deny   +--------+
> +-----| access |------->| reject |
> |     +--------+        +--------+
> |         | moderate
> |         v
> |   +------------+    +------+
> |   | moderation |--->| hold |
> |   +------------+    +------+
> |         |              |
> |         v              |
> |      +------+          |
> +----->| send |<---------+
>        +------+
> 
> 
> Pros: With the proposed flow, it is possible to force a mail through
> via an access rule. The keywords might also make more sense to the
> administrators.
> 
> 
> Cons: With the proposed flow, it is impossible to force a mail to be
> held for moderation. If a list is modnonsubposts, but the
> administrators would like a chance to examine multipart mails, before
> they are sent to the list, it can be done like this today:
> 
> moderate ^Content-Type: multipart/
> allow
> 
> 
> One possible solution - at least to the bypass moderation problem -
> might be to add another keyword. We could add "send" to force the mail
> past normal moderation procedures.
> 
> A possible new flow:
> 
>  send +--------+ deny   +--------+
> +-----| access |------->| reject |
> |     +--------+        +--------+
> |  allow |  | moderate
> |        |  +-----------+
> |        v              v
> |  +------------+    +------+
> |  | moderation |--->| hold |
> |  +------------+    +------+
> |        |              |
> |        v              |
> |     +------+          |
> +---->| send |<---------+
>       +------+
> 
> send ^X-Password: 90e85684b1285a9f$
> moderate ^Content-Type: multipart/
> allow
> 
> I know it does not fix the meaning of "allow" vs. "moderate" on lists
> which are also subject to regular moderation, but I don't see a way to
> do that without breaking backwards compatibility.
> 
> Any thoughts on this?
> 
> Morten
> 

Hmmm ... my 2 cents:
- I always though of allow/reject as being on the IN-side: allow a mail
  to be send to the list and go on with the next steps (or reject), so
  personally I prefer the third flow. But functionally the second and
  third flow are almost alike, with the advantage of the third flow
  that you can force certain mails to be moderated
- if the third flow is choosen, don't call the new keyword plain
  "send", call it eg. "force-send" or "bypass-mod" or so. People tend
  to like descriptive keywords :-)
- whatever flow is choosen, I find it very nicely explained and want
  this ASCII art in a README :-)

Franky

Re: PATCH: Access priority

[Mads Martin Jørgensen]

On 16/02/2010, at 23.03, Franky Van Liedekerke wrote:

> - whatever flow is choosen, I find it very nicely explained and want
>  this ASCII art in a README :-)


Yeah, me too! Maybe brush it up and use it as the mlmmj logo?

-- 
Mads Martin Joergensen, http://mmj.dk
"Why make things difficult, when it is possible to make them cryptic
 and totally illogical, with just a little bit more effort?"
                                 -- A. P. J.

Re: PATCH: Access priority

[Morten Shearman Kirkegaard]

[-- Attachment #1: Type: text/plain, Size: 1495 bytes --]

On Wed, 2010-02-17 at 08:40 +1100, Ben Schmidt wrote:
> > One possible solution - at least to the bypass moderation problem -
> > might be to add another keyword. We could add "send" to force the mail
> > past normal moderation procedures.
> 
> Exactly what I was thinking after reading the issues you raised above.
...
> Shall I put together a new patch?

Yes, that would be nice.

I can't think of many real-world scenarios where it would be useful, but
if you have the time to write a patch, let's have that feature. It
should not complicate things by much.

On Tue, 2010-02-16 at 23:03 +0100, Franky Van Liedekerke wrote:
> my 2 cents:
> - I always though of allow/reject as being on the IN-side: allow a mail
>   to be send to the list and go on with the next steps (or reject), so
>   personally I prefer the third flow. But functionally the second and
>   third flow are almost alike, with the advantage of the third flow
>   that you can force certain mails to be moderated

It is, and I think it makes good sense.

> - if the third flow is choosen, don't call the new keyword plain
>   "send", call it eg. "force-send" or "bypass-mod" or so. People tend
>   to like descriptive keywords :-)

Why break a tradition og choosing ambiguous keywords? ;-)

> - whatever flow is choosen, I find it very nicely explained and want
>  this ASCII art in a README :-)

Patch attached. Will that do?


Morten

-- 
Morten Shearman Kirkegaard <moki@fabletech.com>
CTO, FableTech
http://fabletech.com/

[-- Attachment #2: patch-mlmmj-1.2.17-README.access.diff --]
[-- Type: text/x-patch, Size: 1821 bytes --]

Index: README.access
===================================================================
RCS file: /home/mmj/MMJCVS/mlmmj/README.access,v
retrieving revision 1.5
diff -u -r1.5 README.access
--- README.access	9 May 2007 16:59:23 -0000	1.5
+++ README.access	17 Feb 2010 21:30:20 -0000
@@ -23,9 +23,42 @@
 - "Regexp" is a POSIX.2 extended regular expression. Matching is done case
   insensitive.
 
+The action "allow" will pass the mail on to the next step in processing. The
+mail may still be held for moderation, if it would have been so without access
+rules.
+
+The action "deny" will not send the mail to the mailing list, but will send a
+rejection mail to the sender.
+
+The action "discard" will not send the mail to the list, and will not send a
+rejection mail.
+
+The action "moderate" will hold the mail for moderation.
+
 IMPORTANT: if "moderate" is used then don't forget to add people who should
            function as moderators in listdir/control/moderators
 
+
+The flow out of the access system is like this:
+
+                      |
+                      v
++--------+   deny +--------+ discard   +---------+
+| reject |<-------| access |---------->| discard |
++--------+        +--------+           +---------+
+               allow |  | moderate
+                     |  +-----------+
+                     v              v
+               +------------+    +------+
+               | moderation |--->| hold |
+               +------------+    +------+
+                     |              |
+                     v              |
+                  +------+          |
+                  | send |<---------+
+                  +------+
+
+
 First a simple example. This rule set will reject any mail that is NOT plain
 text, or has a subject that contains "BayStar", and allow anything else:
 

Re: PATCH: Access priority

[Franky Van Liedekerke]

On Wed, 17 Feb 2010 22:40:57 +0100
Morten Shearman Kirkegaard <moki@fabletech.com> wrote:

> > - if the third flow is choosen, don't call the new keyword plain
> >   "send", call it eg. "force-send" or "bypass-mod" or so. People
> > tend to like descriptive keywords :-)
> 
> Why break a tradition og choosing ambiguous keywords? ;-)

hehheh ... try walking on the dark side of life once :-)
 
> > - whatever flow is choosen, I find it very nicely explained and want
> >  this ASCII art in a README :-)
> 
> Patch attached. Will that do?

yup, it describes the current workflow just fine.

Franky

Re: PATCH: Access priority

[Morten Shearman Kirkegaard]

On Wed, 2010-02-17 at 23:56 +0100, Franky Van Liedekerke wrote:
> On Wed, 17 Feb 2010 22:40:57 +0100
> Morten Shearman Kirkegaard <moki@fabletech.com> wrote:
> > > - whatever flow is choosen, I find it very nicely explained and want
> > >  this ASCII art in a README :-)
> > 
> > Patch attached. Will that do?
> 
> yup, it describes the current workflow just fine.

Committed to CVS.


Morten

-- 
Morten Shearman Kirkegaard <moki@fabletech.com>
CTO, FableTech
http://fabletech.com/

Re: PATCH: Access priority

[Ben Schmidt]

[-- Attachment #1: Type: text/plain, Size: 1210 bytes --]

>>> One possible solution - at least to the bypass moderation problem -
>>> might be to add another keyword. We could add "send" to force the mail
>>> past normal moderation procedures.
>>
>> Exactly what I was thinking after reading the issues you raised above.
> ...
>> Shall I put together a new patch?
>
> Yes, that would be nice.

Attached. It applies on top of the ASCII art patch.

Mind you, it changes the ASCII art to something different...I found it
pretty impossible to try to incorporate the 'send' into the existing
picture, and ended up turning the whole thing around. I don't think it's
as good that way around, but at least it fits. Maybe you can do a better
job incorporating it into the old one, Morten.

The patch has not been tested yet, though it builds OK. But definitely
don't commit it until at least someone has tested it. I will try to give
it a whirl early next week.

>> - if the third flow is choosen, don't call the new keyword plain
>>    "send", call it eg. "force-send" or "bypass-mod" or so. People tend
>>    to like descriptive keywords :-)
>
> Why break a tradition of choosing ambiguous keywords? ;-)

I don't think it's too ambiguous, so have stuck with 'send'.

Ben.




[-- Attachment #2: mlmmj-access-3.patch --]
[-- Type: text/x-patch, Size: 3771 bytes --]

diff -r 490e905b4aca README.access
--- a/README.access	Thu Feb 18 18:28:52 2010 +1100
+++ b/README.access	Thu Feb 18 19:00:24 2010 +1100
@@ -17,7 +17,7 @@
 discard or moderate the post.
 
 The syntax is quite simple: action[ [!]regexp]
-- "Action" can be "allow", "deny", "discard" or "moderate".
+- "Action" can be "allow", "send", "deny", "discard" or "moderate".
 - The optional "!" makes the rule a match, if NO header matches the regular
   expression.
 - "Regexp" is a POSIX.2 extended regular expression. Matching is done case
@@ -27,6 +27,9 @@
 mail may still be held for moderation, if it would have been so without access
 rules.
 
+The action "send" will send the mail unconditionally. It will not be
+moderated.
+
 The action "deny" will not send the mail to the mailing list, but will send a
 rejection mail to the sender.
 
@@ -39,24 +42,27 @@
            function as moderators in listdir/control/moderators
 
 
-The flow out of the access system is like this:
+The flow through the access system is like this:
 
-                      |
-                      v
-+--------+   deny +--------+ discard   +---------+
-| reject |<-------| access |---------->| discard |
-+--------+        +--------+           +---------+
-               allow |  | moderate
-                     |  +-----------+
-                     v              v
-               +------------+    +------+
-               | moderation |--->| hold |
-               +------------+    +------+
-                     |              |
-                     v              |
-                  +------+          |
-                  | send |<---------+
-                  +------+
+               deny       +------+
+       +----------------->| deny |
+       |                  +------+
+       |
+       |       discard    +---------+
+       |  +-------------->| discard |
+       |  |               +---------+
+       |  |                   ^
+       |  |                   | expire
+    +--------+ moderate   +------+           +------+
+--->| access |----------->| hold |---------->| send |--->
+    +--------+            +------+ confirm   +------+
+       |  |                   ^                ^  ^
+       |  |                   | yes            |  |
+       |  |    allow      +------------+ no    |  |
+       |  +-------------->| moderation |-------+  |
+       |                  +------------+          |
+       |       send                               |
+       +------------------------------------------+
 
 
 First a simple example. This rule set will reject any mail that is NOT plain
diff -r 490e905b4aca src/mlmmj-process.c
--- a/src/mlmmj-process.c	Thu Feb 18 18:28:52 2010 +1100
+++ b/src/mlmmj-process.c	Thu Feb 18 19:00:24 2010 +1100
@@ -55,6 +55,7 @@
 
 enum action {
 	ALLOW,
+	SEND,
 	DENY,
 	MODERATE,
 	DISCARD
@@ -173,6 +174,9 @@
 		if (strncmp(rule_ptr, "allow", 5) == 0) {
 			rule_ptr += 5;
 			act = ALLOW;
+		} else if (strncmp(rule_ptr, "send", 4) == 0) {
+			rule_ptr += 4;
+			act = SEND;
 		} else if (strncmp(rule_ptr, "deny", 4) == 0) {
 			rule_ptr += 4;
 			act = DENY;
@@ -852,6 +856,9 @@
 	}
 
 startaccess:
+	if(!moderated)
+		moderated = statctrl(listdir, "moderated");
+
 	noaccessdenymails = statctrl(listdir, "noaccessdenymails");
 
 	access_rules = ctrlvalues(listdir, "access");
@@ -918,13 +925,15 @@
 			myfree(donemailname);
 			myfree(discardname);
                 	exit(EXIT_SUCCESS);
+		} else if (accret == SEND) {
+			moderated = 0;
+		} else if (accret == ALLOW) {
+			/* continue processing as normal */
 		}
 	}
 
 	notmetoo = statctrl(listdir, "notmetoo");
 
-	if(!moderated)
-		moderated = statctrl(listdir, "moderated");
 	if(moderated) {
 		mqueuename = concatstr(3, listdir, "/moderation/",
 				       randomstr);

Re: PATCH: Access priority

[Morten Shearman Kirkegaard]

On Fri, 2010-02-19 at 01:27 +1100, Ben Schmidt wrote:
> > > Shall I put together a new patch?
> >
> > Yes, that would be nice.
> 
> Attached.

Cool. You are incredibly productive.

> Mind you, it changes the ASCII art to something different...

I like your version.

> The patch has not been tested yet, though it builds OK. But definitely
> don't commit it until at least someone has tested it. I will try to give
> it a whirl early next week.

It looks very clean. Please let me know when you've tested it, and I
will commit it.


Morten

-- 
Morten Shearman Kirkegaard <moki@fabletech.com>
CTO, FableTech
http://fabletech.com/

[mlmmj] Re: PATCH: Access priority

[Ben Schmidt]

>> The patch has not been tested yet, though it builds OK. But definitely
>> don't commit it until at least someone has tested it. I will try to give
>> it a whirl early next week.
>
> It looks very clean. Please let me know when you've tested it, and I
> will commit it.

I have done some testing now; it seems to work.

Can it be committed now, Morten?

Ben.

Re: [mlmmj] Re: PATCH: Access priority

[Morten Shearman Kirkegaard]

Hi Ben,

On Wed, 2010-03-10 at 09:32 +1100, Ben Schmidt wrote:
> >> The patch has not been tested yet, though it builds OK. But definitely
> >> don't commit it until at least someone has tested it. I will try to give
> >> it a whirl early next week.
> >
> > It looks very clean. Please let me know when you've tested it, and I
> > will commit it.
> 
> I have done some testing now; it seems to work.
> 
> Can it be committed now, Morten?

I have committed it to CVS now. Thanks for the patch.


Best regards,
Morten

-- 
Morten Shearman Kirkegaard <moki@fabletech.com>
CTO, FableTech
http://fabletech.com/