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

[Aaron Stone <[e-mail]>]

On Mon, Dec 19, 2011 at 12:46 PM, Ben Campbell <[e-mail]> 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 <[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?

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.