ietf-mta-filters
[Top] [All Lists]

Re: AD review of draft-ietf-sieve-3028bis-12

2007-03-29 10:28:11

Chris Newman wrote:

I have reviewed draft-ietf-sieve-3028bis-12.txt and that draft has sufficient quality that I will vote "yes" for it to become proposed standard.

Thanks :-)

However, I have found enough issues during review that the improvement in a draft 13 based on the non-controversial comments I've made would be worth the delay, IMHO.

One particular issue, marked with "***" below is a cross-WG compatibility issue -- the sort of issue it is my job as area director to catch.

As AD I do not wish to impose engineering decisions on WGs except in egregious cases, so I do not require the WG to fix any of the issues listed below. The WG is free to say "-12" is good enough. In that case consider these comments fodder for 3028ter.

After discussing this with Cyrus: chairs would like to publish -12 + RFC-editor's notes.

Below I am replying to your comments. I've split them into 2 categories: the ones that should be just applied and the ones that should be either deferred till 3028ter or at least discussed.

With my contributor's hat on, I think the following changes should be done as RFC editor's notes:

Section 1, 4th paragraph

  the Mail Transfer Agent (MTA) does final delivery, such as
  traditional Unix mail, it is reasonable to sort when the MTA deposits
  mail into the user's mailbox.              ^^^^

Q: Should that be "filter"?

(I don't care either way, but I think filter is probably better)
[...]

Section 2.5

  actions.  In this document, tests are given to if/elsif/else to

Q: Are tests "given" to else?

Your are right. Remove "else" here.

Section 2.6.2

  To simplify this specification, tagged arguments SHOULD NOT take
  tagged arguments as arguments.

Q: How does what future standards choose to do with tagged arguments make this specification simpler? Perhaps just remove "To simplify this specification, "?

Agreed.
[...]

Section 2.7.1

  in use.  In contrast, the comparator "i;basic;uca=3.1.1;uv=3.2"
  defines a character to be any UTF-8 octet sequence encoding one

Perhaps just say "a Unicode-based comparator would define ..." since the basic comparator got split out of the base spec and could change dramatically.

Agreed.

Section 2.10.4

  Site policy MAY limit numbers of actions taken and MAY impose

numbers -> the number

Ok.
[...]

Section 4.2

  The message is send back out with the address from the redirect

 send->sent

Yes.
[...]

Section 5.9

  Note that for a message that is exactly 4,000 octets, the message is
  neither ":over" 4000 octets or ":under" 4000 octets.

                               ^^
                               nor

Yes.

  As the range of mail systems that this document is intended to apply
  to is quite varied, a method of advertising which capabilities an
  implementation supports is difficult due to the wide range of
  possible implementations.

I suggest removing all text up to and including the "," to simplify this sentence.

That would be fine with me.

Section 8.1

  multiline-dotstuff = "." 1*octet-not-crlf CRLF

This name is misleading because it covers all lines starting with a dot, whether or not they are dot-stuffed. Perhaps it should be "multiline-dotstart".

Sure.
[...]

Section 13

  [COLLATION] Newman, C., Duerst, M., and A. Gulbrandsen "Internet
                   Application Protocol Collation Registry" draft-
                   newman-i18n-comparator-07.txt (work in progress),
                   March 2006.

Now RFC 4790...Thanks Arnt!

Yes. But note that RFC editor will fix this automatically.

Section 15

This is missing some important changes.

10. Added encoded-character capability and deprecated (but did not remove)
   use of arbitrary binary in Sieve scripts.
11. Updated IANA registration template, and permit capability prefix
registrations. Prefix registrations outside "vnd." require IESG approval.
12. Added .sieve as a valid extension for sieve scripts.

Yes!

*======================
*Below are the changes that needs to be discussed/deferred till 3028ter (once again, with my contributor's hat on):

   Implementations MUST support integer values in the inclusive range
   zero to 2,147,483,647 (2^31 - 1), but MAY support larger values.

Q: Is there a Sieve capability to indicate support for larger numbers?

No, there isn't any.
[...]

section 2.4.2.1. String Lists

 Conversely, in any case where a list of strings is appropriate, a

Awkward wording.

Q: Perhaps this text should mention empty lists are forbidden to align with the
  ABNF?

That would be fine with me, even though I don't think this is necessary.
[...]

Section 2.6.3

  different orderings on UTF-8 [UTF-8] characters.

The ordering and equality rules of a comparator apply to strings, not characters. So this could be made more precise.

The change seems fine to me.

  There are three match types describing the matching used in this
  specification: ":is", ":contains", and ":matches".  Match type
  arguments are supplied to those commands which allow them to specify
  what kind of match is to be performed.

Awkward wording.

How about the following:

Match type arguments control what kind of match is to be performed by commands.

(The "which allow the to specify" is not important, as commands that don't allow for match types obviously don't care about them.)
However, is the updated sentence adds any value to the spec?

[...]

Section 2.10.5

  Implementations MUST NOT execute at all any script which requires an

Perhaps drop the "at all"

The text is trying to prevent people to do partial script execution. Any suggestions how to improve the text?

        Experience with PostScript suggests that mechanisms that allow
        a script to work around missing extensions are not used in
        practice.

This statement would argue that the proposed "ihave" action needs to go experimental

I think this is what Ned was planning to do anyway.

until utility is demonstrated at which point this text would be misleading.

Should this sentence just be dropped?

* *

  Extensions which define actions MUST state how they interact with
  actions discussed in the base specification.

Q: Isn't this redundant with the last paragraph of section 2.10.1?

It is, but I don't think it hurts to repeat this.

2.10.7. Limits on Execution

Q: There is no mention of limits on script size, string size, or list size. Should there be?

Hmm. We do have limits on size of variables. Opinions?

Section 5.1 ***

There's some tricky interaction with IDN and EAI here. It could be clarified with text like: A Sieve implementation for use with Internet email MUST support the use of IDN-encoded domain names [IDN] in the key-list.

Isn't this implied? I mean any ACE-encoded IDN domain name is a valid ASCII domain name.

A Sieve implementation MAY support use of UTF-8 for domain names in the key-list (a subsequent Sieve extension could add a capability name to require that behavior).

Addition of this text is fine with me.

Section 5.4

  command.  The null reverse-path is matched against as the empty
  string, regardless of the ADDRESS-PART argument specified.

Awkward wording.

Personally, I would have preferred unknown envelope parts were ignored.

If I remember correctly, the WG discussed this and the existing text is based on WG consensus.

In the case of EAI, I'd include "alt-address" in most tests but wouldn't care if the implementation supports it. I could always add a require "alt-address" to the script if I really needed it to work. With the "unknown envelope parse

-12 says "parts", not "parse".
(So I don't know if the rest of your argument is valid)

SHOULD result in an error", one has to use the ihave extension to support the "don't care if it works" case.

[...]

Section 10

  Implementations SHOULD take measures to prevent scripts from looping.

Q: Isn't this trivially true because Sieve scripts have no loop command? Perhaps you meant to say "creating mail loops" instead of "looping".

I think you are right.