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

[Ben Campbell <[e-mail]>]

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 <[e-mail]> 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?


>  
> -- 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.


> 
> 
> -- 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.


>  
> 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.

>  
> -- 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.

[…]

>  
> -- 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.

[…]