Re: [apps-discuss] Review of draft-melnikov-smtp-priority-14

[Alexey Melnikov <[e-mail]>]

Hi Ned,
Thank you for very constructive comments.

On 11/06/2012 15:29, Ned Freed wrote:
> This entire discussion has been growing increasingly abstract, and that's
> rarely a good thing in standards work.
>
> Rather than continue in that direction, I decided to do something I should
> have done a lot sooner: I implemented the draft.
>
> <disclaimer>
>
> The fact that I have implemented this draft in Oracle Messaging Server does not
> imply any commitment on the part of Oracle to actually support this extension.
> And should Oracle decide to support it, this doesn't mean it will be done
> in any particular release or in any particular time frame.
>
> </disclaimer>
>
> (Sorry about having to include that, but rules are rules.)
>
> Implementation work is usually very instructive, and this time was no
> exception. I'll list the simple things I learned first:
>
> (1) The draft says that the extension is valid for LMTP, but doesn't give any
>      guidance as to what that means. This could be interpreted as saying that
>      an implementation that uses LMTP has to support the MT-PRIORITY extension
>      there to be compliant. That would in turn mean that prioritization has
>      to be done on the LMTP server side, which may be a bit difficult as there's
>      no queue there. Our implementation, and I suspect many others, handles
>      prioritization on the LMTP client side.
A very good point. Yes.
>      That said, it's entirely possible for the MT-PRIORITY to affect the LMTP
>      server by changing processing or I/O handling in some way, or affect
>      subsequent store access behavior, or may be needed just so it can be
>      logged. So the extension should be allowed over LMTP, but some clarifying
>      text is needed to say an implementation MAY choose to handle
>      prioritization on the client side of the LMTP connection only.
I've added a new section on this in -19.
> (2) The draft also allows MT-PRIORITY on submission, which of course makes
>      complete sense. However, given the overall state of MUAs in the world
>      today,  it is next to certain that clients are going to be used that don't
>      support it. (And please don't try the line that this can always be dealt
>      with by requiring the use of certain clients. I'm talking about the real
>      world, not fantasyland.) As such, there are going to be cases where the
>      MSA needs to attach an MT-PRIORITY to messages that don't have one.

This was always allowed. It is implied from the procedure of calculating the
priority as specified by the sender (and using 0 if not specified), then
changing it due to the local policy.

Actually, I thought the current text was quite clear:

<t>The SMTP server MAY also alter the message priority (to lower or
     to raise it) in order to enforce some other site policy.
     For example an MSA might have a mapping table that assigns priorities
     to messages based on authentication credentials.
</t>

I added "(Note that this also includes the case when the priority is not 
explicitly specified.)" after the first sentence quoted above.

I will also added an extra example that demonstrates that.

>      It
>      would be good to mention this, but even if it's not discussed there needs
>      to be an enhanced status code defined to indicate when it has been done,
>      and more generally to indicate when the MT-PRIORITY has been upgraded.
I changed the existing Enhanced Status Code for "the priority was 
lowered" to "the priority was changed". Pete asked for it earlier 
anyway, but I just didn't see much use case, so didn't do that. I don't 
think there is a need in  2 Enhanced Status Codes, because the selected 
priority is communicated back in the response anyway.

> (3) Implementations that support Sieve need to provide a way for Sieves to
>      test the current MT-PRIORITY value. I implemented this as an environment
>      item because that's a place that allows for implementation-specific values.
>      The alternative would be to do it as a full-blown Sieve extension and add
>      it to the envelope test.

Personally I have no preferences and I think either will work. 
Considering that you implemented the extended environment test, we might 
as well use that.

> Given that MT-PRIORITY is an SMTP extension the
>      envelope test is the natural place to do it, but a problem with that may
>      be it restricts the test to implementations and situations where the
>      envelope test itself is supported. This is not a problem for our
>      implementation, but it may be a problem for others.
>
>      Another issue is the lack of a defined comparator that will work with
>      MT-PRIORITY values. i;ascii-numeric doesn't handle signed comparisons.
>      An i;ascii-integer comparator is needed, so I added one and I think I'll
>      go ahead and add the definition to the comparator registry.
>
>      It may also be a bad idea to add all this in the current draft, especially
>      given where this draft is in the process. But it needs to be done somewhere.

I think doing this in a separate document would be best at this stage in 
the process.

> (4) The draft doesn't say anything about logging. The current MT-PRIORITY
>      value does appear in Received: fields, but that's not the same thing.
>      Logging of how MT-PRIORITY is used is going to be a requirement in some
>      environments, so a general suggestion along the lines of "MT-PRIORITY
>      values SHOULD be logged as part of any logging of message transactions" is
>      in order.

Agreed. Added in -19.

> (5) The suggested text for the X.3.TBD3 error code doesn't conform to the
>      requirement that the text begin with the new priority value. (Note that
>      the new error code suggested under (2) should have the same requirement.)

You mean where it is first defined? I copied the text from the IANA 
registration section to make this clear.

> (6) The draft talks about greylisting, but fails to mention that
>      connection-level and SMTP HELO/HELO greylisting options exist, and when
>      these options are used there's an issue if the trust relationship is
>      established through the use of SASL. This needs to be pointed out.

Added.

> I also spotted a minor typo in the Introduction: sattelite ->  satellite.

Fixed.

> The draft also spells out numbers in some places and writes them as
> actual numbers in others, making it hard to search for things. I don't care
> which approach is used as long as it is consistent.

I think I fixed these.

> Now on to the more difficult stuff.

I mostly agree with these comments. They will be addressed in -20.