Subject: Final sanity check on draft-ietf-sieve-mime-loop-04.txt

From: Alexey Melnikov <[email-masked]>
Date: Fri, 07 Mar 2008 19:09:16 +0000

Cyrus and Tony have updated Sieve MIME loops document. Cyrus should send 
out a message (I apologize if he did already and I missed it) about 
which changes were addressed and which were not (and why).

I would like to ask all people who commented on the document earlier to 
review if their comments were addressed. If there are no comments on the 
document before March 21st, I would recommend its publication.

Regards,
Alexey,

---

From: Arnt Gulbrandsen <[email-masked]>
Date: Fri, 7 Mar 2008 23:17:54 +0100

I lost track of that draft. Sorry. Three comments and a bonus.

1. for_every_part jars. We don't have file_into, we don't have any_child.

I don't mind underscores per se, but I do mind heterodoxy. Using 
gluedtogethernames sometimes, using underscore_names sometimes and 
camelCaseNames sometimes makes the language unnecessarily messy. (Yes, 
I know we don't have camelCase anywhere. At least not that I've 
noticed.)

2. break has been difficult in C for decades when nested loops are used, 
do we need to repeat it? Wouldn't it be better to tie break and its 
loop explicitly, e.g. with an identifier? The identifier would be 
optional on the loop and mandatory on the break.

3. Perhaps I'm tired, but it wasn't clear to me whether if two loops are 
nested, both operate on _every_ bodypart, O(n^2) and all, or whether 
the inner only operates on children of the bodypart currently accessed 
by the outer loop. The former seems correct, but I'm not thrilled to 
have O(n^2) mandatory behaviour, and O(n^m) possible if more than two 
levels is supported.

Bonus. Philosophy. I like the way :anychild without for_every_part 
works. That makes it possible to do parallel processing instead of 
sequential. If I want to process, say, ten or a hundred thousand 
messages with a single sieve script, that becomes important. I can 
transform the test to a database lookup and the query planner will 
generally do well. This makes some clever reprocessing of an entire 
mailbox possible. "Your new sieve script would have filed <these> 
messages into <that mailbox>. Do you want to move them now?". break and 
perhaps for_every_part torpedoes such transformations. I wonder whether 
that's necessary. I wonder whether it's really the same problem as the 
O(n^m) behaviour of nested for_every_part.

Arnt

---

From: "Nigel Swinson" <[email-masked]>
Date: Tue, 18 Mar 2008 18:04:52 -0000

>    document before March 21st, I would recommend its publication.

4.1. Test "header"
-   The "header" test is extended with the addition of a new ":mime"
-   tagged argument and its associated options.
+   The "header" test is extended with the addition of new ":mime"
+   and ":anychild" tagged arguments and their associated options.

I guess this doesn't have to be the case, but I think it would make sense to
use header :anychild without a :mime argument in order to test header of
attached rfc2822 messages, or indeed to test the Content-Type headers of
child body parts as unparsed strings.  It seems clear if you use :anychild,
then you are asking for the mime structure to be evaluated.  I think this is
actually implied anyway when it goes on to say:

   When used inside the context of a "for_every_part" iterator, and with
   an ":anychild" tagged argument, the "header" test will examine the
   current MIME part context and all it's nested MIME body parts,
   returning true if any of them satisfies the test.

   :param  parses the header looking for MIME parameters in the header.
      The supplied string-list lists the names of any parameters to be
-      tested.  If any one named parameter value matches the test string
-      value, the test will return true.
+      tested.  If any one named parameter value matches any of the test
+      string values, the test will return true.

4.2. Test "address"

-   The "address" test is extended with the addition of a new ":mime"
-   tagged argument, which takes a number of other arguments.
+   The "address" test is extended with the addition of new ":mime"
+   and ":anychild" tagged arguments and their associated options.

4.3. Test "exists"

-   The "exists" test is extended with the addition of a new ":mime"
-   tagged argument, which takes one other argument.
+   The "exists" test is extended with the addition of new ":mime"
+   and ":anychild" tagged arguments.

5. Action Replace

Should we also preserve some trace fields?  I'd have thought typically
replace would be used to remove malicious content, in which case would it
not be helpful to at least have some way of knowing where the message came
from that produced the butchered message?  I'd suggest:

-  If the entire message is being replaced, a ":subject" parameter
+  If the entire message is being replaced, the optional ":subject"
parameter
   specifies a subject line to attach to the message that is generated.
   UTF-8 characters can be used in the string argument; implementations
   MUST convert the string to [RFC2047] encoded words if and only if
   non-ASCII characters are present.  Implementations MUST preserve the
   previous Subject header as an Original-Subject header.

   If the entire message is being replaced, a ":from" parameter may be
   used to specify an alternate address to use in the From field of the
   message that is generated.  The string must specify a valid [RFC2822]
   mailbox-list.  Implementations SHOULD check the syntax and generate
   an error when a syntactically invalid ":from" parameter is specified.
   Implementations MAY also impose restrictions on what addresses can be
   specified in a ":from" parameter; it is suggested that values that
   fail such a validity check simply be ignored rather than causing the
   replace action to fail.  Implementations MUST preserve the previous
   From header as an Original-From header.

+  Implementations MUST preserve all other header fields from the original
+  message with the exception of those relating to the MIME structure which
+  will need to be replaced.

Although I do note that the security section does at least indicate that
replace creates an entirely new message, so I'm sorry if we covered this
before and I just forgot.

6. Action Enclose
   If multiple "enclose" actions are executed by a script, only the text
-  specified on the last one is used when creating the enclosed message.
+  specified on the last one is used when creating the enclosed message,
+  the message in this case would not be enclosed multiple times.
   This action does not affect messages that are forwarded via a
-  "redirect" action.
+  "redirect" action, rather the unenclosed message would be what was
+  redirected.

   Specifically, the original message becomes a multipart/mixed message
   with two parts: a text/plain portion with the string argument as its
   body, and a message/rfc822 portion with the original message
   enclosed.  The Content-Type: header field becomes multipart/mixed.
-  The Subject: header is specified by the :subject argument.  Any
+  The optional Subject: header is specified by the :subject argument,
+  if not present the subject will be taken from the enclosed message.  Any
   headers specified by :headers are copied from the old message into
   the new message.  If not specified by :headers, Date: and From:
   headers should be synthesized to reflect the current date and the
   user running the Sieve action.

7. Action extract_text
   If extract_text is used outside the context of a "for_every_part"
   loop, the action will set the variable identified by varname to the
   empty string.

I suggest the above SHOULD be a compilation error, otherwise it should
return the empty string.

Cheers

Nigel

---

From: "Mark E. Mallett" <[email-masked]>
Date: Wed, 19 Mar 2008 16:55:55 -0400

Hi,

nitpicking:

3.  Sieve Loops

5th(ish) paragraph contains:

   > If that
   > MIME part is a terminal MIME part (i.e. does not contain other MIME
   > parts) then the nested "for_every_loop" is simply ignored.

I believe this intends to say

   ... the nested "for_every_part" loop is imply ignored.

==========

4.1.  Test "header"

   > returning true if any of them satisfies the test.

"it's" should be "its" -- picky, picky.

==========

9.2.  Example 2

In the example, the "enclose" action:

   >     enclose :subject "Warning" :text
   > WARNING! The enclosed message contains executable attachments.
   > These attachments types may contain a computer virus program
   > that can infect your computer and potentially damage your data.
   > 
   > Before clicking on these message attachments, you should verify
   > with the sender that this message was sent by them and not a
   > computer virus.
   > .

needs to end with a semicolon.

==========

less nitty:

I was about to go into some stuff about how I thought it would be better
to lay a fundamental foundation of MIME awareness on which this
extension could be layered; about how :mime not only seems superflous
but conflicts with :mime syntax and meaning in other extensions; about
how I'd rather see a generalized parameter extraction for headers that
had a parameter-list syntax (as opposed to :type and so forth).  But
then I realized I said a bunch of that last summer, but apparently
without any sympathy :)  That's the way it goes, but just for reference,
I still am of many of these opinions (and perhaps others ...):

   http://www.imc.org/ietf-mta-filters/mail-archive/msg03658.html

even though I see I fumble-fingered some bit(s) of it.

Yours,
mm