Subject: Re: [sieve] [Gen-art] Gen-ART Last Call Review of draft-ietf-sieve-include-13

From: Aaron Stone <[email-masked]>
Date: Thu, 5 Jan 2012 06:32:58 -0800

On Mon, Dec 19, 2011 at 12:46 PM, Ben Campbell <[email-masked]> wrote:
> Thanks for the response! Further comments inline, with sections that appear
> to need no further comment removed:
>
> On Dec 19, 2011, at 1:13 PM, Aaron Stone wrote:
>
>
>
> On Tue, Dec 13, 2011 at 2:13 PM, Ben Campbell <[email-masked]> wrote:
>>
>>
>
> […]
>
>> Minor issues:
>>
>> -- section 3.1, paragraph 4: "Implementations MUST NOT generate errors for
>> recursive inclusions at upload time, as this would force an upload ordering
>> requirement upon script authors / generators.  However, if an active script
>> is replaced with a faulty script and would remain the active script, an
>> error MUST be generated and the upload MUST fail."
>>
>> These two statements seem contradictory on a quick reading.  In
>> particular, how can the latter assertion avoid an upload ordering
>> requirement? Or do you mean faulty in some way other than being recursive?
>
>
> If you're replacing an active script, it has to be correct all the time, and
> uploads are atomic only on a per-script basis. There's a risk that if you're
> uploading a set of scripts that include one another, at some intermediate
> stage while some scripts are uploaded but not others, they are in an invalid
> state. The managesieve spec says that scripts must be validated at upload
> time. The language above is trying to say that you can upload all of the
> scripts that may include one another in any order without generating errors
> immediately, however, if you're replacing an active script or a script
> included by the active script, then you DO have to upload a correct script
> right from the get-go.
>
>
> Is this just a question of whether the script(s) are replacing active
> scripts? That is, the license to create a transient invalid state is
> suspended if if you are replacing an active script? If so, how would one go
> about updating a set of linked scripts when one or more of them replace
> active scripts? Should one somehow deactivate the old ones, load all the
> scripts, then activate them?

Having written this out, I don't recall how an implementation would
handle this. I haven't had luck tracking down maling list discussion
on the topic. I'll have to chase up on this.

>>
>> -- section 3.4.1, paragraph 5: "If a "global" command is given the name of
>> a variable that has previously been defined in the immediate script with
>> "set", an error MUST be generated either when the script is uploaded or at
>> execution time."
>>
>> Does this conflict with the previous statement that it is okay for a
>> global and a private variable to have the same name?
>
>
> It doesn't conflict, because those variables live in separate namespaces.
> The effect of the global command is to bind the two names. An error is
> generated rather than specifying if the local overwrites the global value,
> or the global overwrites the local value.
>
>
> I take this to mean you can have a global and a local variable with the same
> name, but not if they are in the same script, right? If so, then it would
> help to add that qualification to the 2nd paragraph in 3.4. As it is, it
> says implementation MUST allow a global and non-global variable to have the
> same name with no interaction, and doesn't exclude it from happening in the
> same script.

Ok.

>
>>
>> -- section 3.4.2:
>>
>> Why do you need two ways to accomplish the same thing?
>
>
> I might be making this up, but I think the original question was whether the
> variables spec would have namespaces.
>
>
> I think I need more context to understand that response--but _my_ original
> question was more along the lines of "if we have the global name space, why
> do we need the command, and vice-versa?" It seems like either one
> accomplishes the goal (I actually like the NS as it seems like it would
> obviate the previous question about globals and locals with the same name)
> But more practically, why make an implementation implement them both? It
> seems like twice as much work, and twice as much opportunity to introduce
> bugs.

Background on my reply: there was a question early-on in the variables
document about whether namespaces would be supported, at the same time
as early drafts of include. I checked, and these were contemporary
issues in 2003.

Ultimately, Sieve Variables provides namespaces, and this document
added support for namespaces without dropping the command-form global.

>>
>> Does the global namespace have the same "requires" requirement as the
>> global command?
>
>
> Yes, but this isn't explicitly stated.
>
>
> Thanks--I think it would help to state it explicitly.

I've added some text that quotes from Sieve Variables, where this
"require" requirement is stated.

>
>>
>> -- section 4.2, paragraph 2:
>>
>> Can you elaborate on what permissions are proper? Is it different for an
>> included script than for any other script?
>>
>> -- section 4.2, paragraph 3:
>>
>> Can you elaborate on what you mean by "safe for a storage system"?
>>
>
> There are both somewhat vague warnings, basically, "Don't allow 'include
> "./../..//etc/passwd"' and don't allow 'include "foo$(`rm star`)"'.
>
>
> Including those (or some other) examples would help.
>
> […]

Done.
>
>>
>> -- section 3.4.2, paragraph 3: "Variables declared global and variables
>> accessed via the global namespace MUST be one and the same."
>>
>> Plurality mismatch. I suggest something like "a variable declared as
>> global and a variable accesses with the global namespace, otherwise having
>> the same name…"
>
>
> I might insert the word "each" after MUST to account for the plurality
> without rewriting the sentence.
>
>
>
> That works, too.
>
> […]

Done.

---

From: Barry Leiba <[email-masked]>
Date: Fri, 13 Jan 2012 10:13:49 -0500

> on the topic. I'll have to chase up on this.

I don't think this is a real problem; the text just needs a little
tweaking.  Errors should still be generated at upload time for any
detectable script errors OTHER THAN recursion.  What this is meant to
deal with is a situation where, say, we have this:

main includes sub-a
   sub-a includes sub-b
      sub-b includes sub-c

...and we want to rewrite things and  change the structure thus:

main includes sub-a
main includes sub-c
   sub-c includes sub-b
     [sub-b no longer includes sub-c]

This would require that we update sub-b *before* we update sub-c, or
else we'd get an error at upload time.  The text is meant to block the
recursion error, so we can update the scripts in any order.  However,
if the script gets run in the middle of the updates and we hit a
recursion situation, we WILL get a runtime error.

That's all that text is meant to do.

That said, it would be wise for implementations to provide a way to do
an atomic update of a set of scripts, because, clearly, multiple
scrips in some indeterminate intermediate upload state could do very
weird (and perhaps bad) things, even if no errors are generated during
their execution.

I think this isn't a protocol requirement, but a
quality-of-implementation issue.  But I do think the document could
have a paragraph discussing this, and warning of the consequences.

Barry, document shepherd