Subject: Re: [sieve] WGLC: draft-ietf-sieve-imap-sieve-04.txt

From: Alexey Melnikov <[email-masked]>
Date: Sun, 3 Jun 2012 20:31:58 +0100

On 30 May 2012, at 21:30, Cyrus Daboo <[email-masked]> wrote:

> Hi folks,
> I am starting a working group last call on draft-ietf-sieve-imap-sieve-04.txt. This is the last document this working group has on its plate and once finished, we intend to shut down the working group.
> 
> The last call on draft-ietf-sieve-imap-sieve-04.txt will run for two weeks. Please review this draft and send comments to the list. This document has been around for a while, so if you have done a detailed review before, just checking the changes now should be sufficient. In any case, the chairs want to heard from everyone who has done a review and those who have decided for or against doing an implementation of this in the near future.

I have reviewed the latest version and happy for this to be published as a Proposed Standard.

---

From: Aaron Stone <[email-masked]>
Date: Fri, 15 Jun 2012 18:08:58 -0700

Review with my contributor hat on:

Major:
RFC 5464 says:
   All entries MUST have either "/shared" or "/private" as a prefix.
   Entry names are case-insensitive.

I think that means our metadata key should be
"/shared/imapsieve/script". Text below implements this change, but
please double-check.

Minor and Nits:

Global nit: perform regex s/section (\d+)/Section $1/g

Section 1.2: remove extra comma
OLD
  At times other than message delivery, delivery "envelope"
  information, might not be available.
NEW
  At times other than message delivery, delivery "envelope"
  information might not be available.

Section 2.1: feels like there should be a MUST in here somewhere?

OLD
   The corresponding Sieve implementation uses the Sieve capability
   string "imapsieve", and Sieve scripts that depend upon the IMAP
   events MUST include that string in their "required" lists.
NEW
   The corresponding Sieve implementation MUST provide the "imapsieve"
   capability string (see [RFC5228] Section 6.1), and Sieve scripts that depend
   upon IMAP events MUST include that string in their "required" lists.

Section 2.2: Add a MUST regarding individual script execution, because
I missed this requirement the first two times I read through it:
OLD
   If more than one message is affected at the same time, each message
   triggers the execution of a Sieve script separately.  The scripts MAY
   be run in parallel.
NEW
   If more than one message is affected at the same time, each message
   MUST trigger execution of the Sieve script separately.  The scripts MAY
   be run in parallel.

Section 2.3.1: Convert from prose to list, and rephrase to include a
lot of MUSTs
OLD
   When an applicable event occurs on an IMAP mailbox, if there is an
   IMAP metadata entry named "/IMAPSieve/Script" for the mailbox, that
   entry is used.  If there is not, but there is an IMAP metadata entry
   named "/IMAPSieve/Script" for the server, that entry is used
   (providing a way to define a global script for all mailboxes on a
   server).  If neither entry exists, then no script will be invoked.
NEW
   When an applicable event occurs, the IMAP server MUST inspect
   the following metadata entries, in order, to determine which Sieve
   script to invoke:
   o "/IMAPSieve/Script" for the target mailbox,
   o "/IMAPSieve/Script" for the server.
   If neither entry exists, then no script will be invoked.

MINOR: I just realized that there might be confusion over an "empty"
script name. I propose that a global server script, and a mailbox
script with no value, means "run this script for every mailbox, except
this one". If that's OK with you, I propose this text immediately
following the text above:
NEW
    If the script name is an empty string, the server MUST NOT
    inspect any further metadata entries (this allows a user to specify
    a global server script, but disable it for specific mailboxes).

Given the above, and the Major issue at the top, I think this
paragraph can be removed:

OLD
   If an "/IMAPSieve/Script" metadata entry was selected above, the
   shared value of that metadata name (its "value.shared" attribute)
   MUST be the name of the Sieve script that will be invoked in response
   to the IMAP event.  Note that only the value.shared attribute is
   used; any value.priv attributes are ignored.
NEW
   [nothing]

The registration template in RFC 5464 is changed since early drafts, therefore:

Section 7.4:
OLD
   To: [email-masked]
   Subject: IMAP METADATA Registration
   Please register the following IMAP METADATA item:
   [x] Entry           [ ] Attribute
   [x] Mailbox         [ ] Server
   Name: /IMAPSieve/Script
   Description: This entry name is used to define mailbox metadata
   associated with IMAP events in Sieve for the associated mailbox.
   Specifically, this specifies the Sieve script that will be invoked
   when IMAP events occur on the specified mailbox.
   Content-type: text/plain; charset=utf-8
   RFC number: [[this RFC]]
   Contact address: Sieve mailing list <[email-masked]>
NEW
       To: [email-masked]
       Subject: IMAP METADATA Entry Registration

       Type:         Mailbox
       Name:         /shared/imapsieve/script
       Description:  This entry name is used to define mailbox metadata
   associated with IMAP events in Sieve for the associated mailbox.
   Specifically, this specifies the Sieve script that will be invoked
   when IMAP events occur on the specified mailbox.
       Content-type: text/plain; charset=utf-8
       RFC Number:   [this RFC]
       Contact:      Sieve mailing list <[email-masked]>

Section 7.5:
OLD
  To: [email-masked]
   Subject: IMAP METADATA Registration
   Please register the following IMAP METADATA item:
   [x] Entry           [ ] Attribute
   [ ] Mailbox         [x] Server
   Name: /IMAPSieve/Script
   Description: This entry name is used to define metadata associated
   globally with IMAP events in Sieve for the associated server.
   Specifically, this specifies the Sieve script that will be invoked
   when IMAP events occur on any mailbox in the server that does not
   have its own mailbox-level /IMAPSieve/Script entry.
   Content-type: text/plain; charset=utf-8
   RFC number: [[this RFC]]
   Contact address: Sieve mailing list <[email-masked]>
NEW
       To: [email-masked]
       Subject: IMAP METADATA Entry Registration

       Type:         Server
       Name:         /shared/imapsieve/script
       Description:  This entry name is used to define metadata associated
   globally with IMAP events in Sieve for the associated server.
   Specifically, this specifies the Sieve script that will be invoked
   when IMAP events occur on any mailbox in the server that does not
   have its own mailbox-level /IMAPSieve/Script entry.
       Content-type: text/plain; charset=utf-8
       RFC Number:   [this RFC]
       Contact:      Sieve mailing list <[email-masked]>

Draft looks awesome besides the above, and I'm looking forward to publication!

Best,
Aaron

On Wed, May 30, 2012 at 1:30 PM, Cyrus Daboo <[email-masked]> wrote:
> Hi folks,
> I am starting a working group last call on
> draft-ietf-sieve-imap-sieve-04.txt. This is the last document this working
> group has on its plate and once finished, we intend to shut down the working
> group.
>
> The last call on draft-ietf-sieve-imap-sieve-04.txt will run for two weeks.
> Please review this draft and send comments to the list. This document has
> been around for a while, so if you have done a detailed review before, just
> checking the changes now should be sufficient. In any case, the chairs want
> to heard from everyone who has done a review and those who have decided for
> or against doing an implementation of this in the near future.
>
> --
> Cyrus Daboo
>
> _______________________________________________
> sieve mailing list
> [email-masked]
> https://www.ietf.org/mailman/listinfo/sieve

---

From: Barry Leiba <[email-masked]>
Date: Fri, 15 Jun 2012 21:50:13 -0400

Thanks, Aaron, for the review.

> "/shared/imapsieve/script".

Well, it doesn't mean that it couldn't be, say,
"/shared/IMAPSieve/Script".  It says it's insensitive, so I can put it
in any case I like.  But as long as I have to change it to add the
prefix (the text was written when there was no prefix and "value.xxx"
was used), I also changed it to be all lower case.  Why not?

>   upon IMAP events MUST include that string in their "required" lists.

Absolutely not: that would make this the only Sieve extension to say
that.  All the extensions I've looked at (Vacation, Notify,
Editheader, Ihave, MIME-loop, Reject) do NOT use 2119 language when
they talk about the capability string.

>   MUST trigger execution of the Sieve script separately.  The scripts MAY

No.  If you're only taking text to be normative if it has a 2119
keyword in it, you need to read documents differently.  "This is how
the protocol works" statements don't need to have "MUST" all over
them, and I see no need for a MUST here.

>   If neither entry exists, then no script will be invoked.

Same as above.  The original text is fine.  I dislike the list format
here, and see no reason for a MUST.

>    a global server script, but disable it for specific mailboxes).

No reason to tell it not to inspect further, because we've already
said that if it finds a mailbox entry, it has what it needs.  But I
like the idea of saying that an empty entry value means that no script
is run, so I'll add that.  New paragraph 3:

                If a "/shared/imapsieve/script" metadata entry was selected
                above, its value is used as the name of the Sieve script
                that will be invoked in response to the IMAP event.
                If the value is empty, then no script is run.

I'll post a new version after I send this.

Barry

---

From: Aaron Stone <[email-masked]>
Date: Fri, 15 Jun 2012 19:22:10 -0700

Thanks! No sweat on the comments not taken :)

> they talk about the capability string.

OK.

> them, and I see no need for a MUST here.

OK.

> here, and see no reason for a MUST.

OK.

>                If the value is empty, then no script is run.

I dig your subtle style.

---

From: Aaron Stone <[email-masked]>
Date: Mon, 18 Jun 2012 10:25:48 -0700

I have reviewed the changes in -05, and recommend this document for publication.

Cheers,
Aaron