Subject: [apps-discuss] Comments on draft-ietf-appsawg-sieve-duplicate-02

From: S Moonesamy <[email-masked]>
Date: Wed, 22 Jan 2014 07:32:05 -0800

Hi Stephan,

I have a few comments on draft-ietf-appsawg-sieve-duplicate-02.

In Section 3:

   "In its basic form, the "duplicate" test keeps track of which messages
    were seen before by this test during an earlier Sieve execution.
    Messages are identified by their message ID as contained in the
    Message-ID header."

The minimum nunber of message-id fields in a RFC 5322 message is 
zero.  A few MTAs do not add the message-id header when there isn't 
one in a message.  I'll mention it without suggesting any change as 
the "duplicate" test should work in most cases.

   "Implementations MUST prevent making any definitive modifications to
    the internal duplicate tracking list until the Sieve script execution
    finishes successfully"

It would be easier to say that implementations MUST only update the 
internal duplicate tracking list when the Sieve script execution 
finishes successfully instead of having  RFC 2119 requirement about 
preventing modifications.

   'But, irrespective of what implementation is chosen, situations in
    which the "duplicate" test erroneously yields "true" MUST be
    prevented.'

That's the same requirement as the one I quoted (implementations must 
...).  The draft mentions that there is a race condition.  I would 
leave it at that as the draft explains under which circumstances the 
condition can occur.

   'The "duplicate" test MUST only check for duplicates amongst message
    ID values encountered in previous executions of the Sieve script; it
    MUST NOT consider ID values encountered earlier in the current Sieve
    script execution as potential duplicates.  This means that all
    "duplicate" tests in a Sieve script execution, including those
    located in scripts included using the "include" [INCLUDE] extension,
    MUST always yield the same result if the arguments are identical.'

There is already a requirement where the internal tracking list is 
only updated on successful execution of the Sieve script.  I gather 
that the above is trying to cover the case where there is an 
"include" extension.  I suggest keeping the above simple by 
discussing that case and using one RFC 2119 key word for clarity.

In Section 6:

   "A flood of unique messages could cause the list of tracked message ID
    values to grow indefinitely.  Implementations SHOULD apply limits on
    the number and lifespan of entries in that list."

There is already "implementations SHOULD limit the number of entries 
..." in Section 3.  If the implementation follows that recommendation 
the list of tracked values should not grow indefinitely.

Regards,
S. Moonesamy

---

From: Ned Freed <[email-masked]>
Date: Thu, 23 Jan 2014 11:43:48 -0800 (PST)

> the "duplicate" test should work in most cases.

Also note that this text is talking about the "basic form".

> preventing modifications.

This seems like a reasonable change to me.

> leave it at that as the draft explains under which circumstances the

Not necessarily. We have no way of knowing what way people will implement 
things, so stating the more general requirement is helpful.

> discussing that case and using one RFC 2119 key word for clarity.

Updating the internal tracking list isn't the only thing that may be going on.
There could be caching involved as well, and it's important that cacching not
generate a match.

And it is not just the include case. Just as one counterexample, sieves can be
and often are built up from templates that result in tests of all kinds being
repeated without any use of the include extension.

> the list of tracked values should not grow indefinitely.

I agree that this one is redundant.

				Ned

---

From: S Moonesamy <[email-masked]>
Date: Thu, 23 Jan 2014 12:41:46 -0800

Hi Ned,
>Also note that this text is talking about the "basic form".

Ok.

>Not necessarily. We have no way of knowing what way people will 
>implement things, so stating the more general requirement is helpful.

Ok.

>repeated without any use of the include extension.

Thanks for the explanation.  I am okay with the current text.

Regards,
S. Moonesamy