Re: Working group last call on draft-ietf-sieve-include

Nits included in my -11. Question below about naming.

On Mon, Jul 18, 2011 at 6:21 AM, Alexey Melnikov
<[email protected]> wrote:
> Barry Leiba wrote:
>
>> The sieve-include doc looks ready to go.  Because Aaron and Cyrus are
>> both editors of this document, I'm playing doc shepherd and handling
>> chair duties related to the document.  And so here we start working
>> group last call on draft-ietf-sieve-include-10, which you can find
>> here: http://tools.ietf.org/html/draft-ietf-sieve-include-10
>>
>> We've had good feedback on this, and I think it's solid.  Everyone,
>> please give it a last review and let us know whether you agree, or
>> whether you find anything that needs changing.  WGLC will end at the
>> end of July, just after the upcoming IETF meeting finishes.
>>
>> Posts confirming that you've reviewed this version and agree that it's
>> ready are welcome.
>>
> I am mostly happy with this version, but I spotted several nits or minor
> errors:
>
> 2.  Conventions Used in This Document
>
>  script execution
>     an instance of a Sieve interpreter invoked for a given message
>     delivery, starting with the user's active script and continuing
>     through any included scripts until the message is delivered.
>
> Pedantic comment: does "reject" or "discard" qualify as "message delivery"?
>
> 3.1.  General Considerations
>
>  Sieve implementations MUST generate an error at execution time if an
>  included script does not exist.
>
> This is no longer true due to the addition of the :optional tag.
>
>
> 3.2.  Control Structure include
>
>        Usage:   include *[PARAMETERS] <value: string>
>                 PARAMETERS = LOCATION / :once / :optional
>                 LOCATION = :personal / :global
>
>  The "include" command takes an optional "location" parameter, an
>  optional ":once" parameter, an optional ":optional" parameter, and a
>  single string argument representing the name of the script to include
>  for processing at that point.  It is RECOMMENDED that implementations
>  restrict script names according to MANAGESIEVE [RFC5804] Section 1.7.
>
> Did you mean Section 1.6 ("Script Names") instead of 1.7 ("Capabilities")?

Fixed.

> Also, I think this requirement makes [RFC5804] Normative.
>
> And finally, why is this not a MUST? Making a SHOULD level requirement seems
> a bit weak in this case.

Is it normative even without the MUST (i.e. if I leave it as RECOMMENDED)?

In the case of a MUST, it means that a valid includes implementation
imposes a script naming restriction. If a site isn't using
managesieve, would that site really need to accept the name
restrictions?

I'm comfortable either way.

> 4.  Security Considerations
>
>  Sieve implementations MUST ensure that script names are checked for
>  validity and proper permissions prior to inclusion, in order to
>  prevent a malicious user from gaining acess to files accessible to
>
> typo: access
>
>  the mail server software that should not be accessible to the user.
>
>
> 5.1.  "include" Extension Registration
>
>     Capability name: include
>     Description:     adds the "include" command to execute other Sieve
>                      scripts, and the "global" command and "global"
>                      variables namespace to access variables shared
>                      among included scripts.
>
> The "return" action is not mentioned here.
>
> _______________________________________________
> sieve mailing list
> [email protected]
> https://www.ietf.org/mailman/listinfo/sieve
>
_______________________________________________
sieve mailing list
[email protected]
https://www.ietf.org/mailman/listinfo/sieve