Re: Questions and remarks on draft-ietf-sieve-include-01.txt

[Aaron Stone <aaron@xxxxxxxxxxxxxx>]

Thanks for your comments. I'd been preparing to send an email to the list
announcing the changes and requesting feedback. You've touched on all the
good points. So let's run with this thread!

On Sat, 11 Apr 2009 01:39:58 +0200, Stephan Bosch <stephan@xxxxxxxxxxxx>
wrote:
> Hi,
> 
> First of all, I am very glad that the work on the specification is being 
> continued now. This week I quickly updated my implementation to match 
> the new specification. Merging import and export into a single global 
> command is a good choice. The use of a global variable namespace is also 
> a good idea (but I did not implement that yet).

I actually wonder if we should drop the global keyword and rely on
namespaces entirely.

> During implementation I collected some remarks and questions:
> 
> - Where the ManageSieve protocol specifies what characters are allowed 
> for a script name, the include extension for the Sieve language does 
> not. Would it be useful to adopt the same limitations? Especially things 
> like '/' can cause problems.

Good suggestion. I think this makes sense to give a consistent opinion on
what script names should look like, but on the other hand, perhaps it's
possible that someone isn't using ManageSieve but IS using include and
might need to get at weird names? Do we care in that case?

> - For the global command I would expect text stating that the variables 
> extension is required when it is used.

Will add.

> - The global command is required to follow 'require' or another global 
> command. I am worried what happens when other extensions have commands 
> with similar requirements. Shouldn't we account for this eventuality?

I don't like this restriction anyways. Any objection to lifting it?

> - Are there any special security implications for using variables in the 
> value argument in the include command, i.e. to include a script 
> specified by a variable? Is that even (intended to be) allowed?

Oh, that's a good question. That'd be an easy vector for contamination of
email contents into a filesystem request. Can we reasonably place such a
restriction, though? I'd hate to force implementations to special-case
their string expansions.

> - The scope of the :once modifier could be a bit confusing. I am 
> assuming it holds for the whole Sieve execution and not only for the 
> identical include commands within the current script.

Correct. Could you suggest how I might clarify that it's the whole
execution? I feel like I'm missing the right word for being inside one file
vs. being inside one delivery/execution instance.

> - Take a good look at the examples: they have small typos and syntax 
> problems:
>    * page 6; spam_tests: reject command has mandatory message argument
>    * page 7; spam_tests: reject command has mandatory message argument
>    * page 8; active script: test-mailbox is not a valid variable name
>    * page 8; active script: missing require for relational extension
>    * page 9; active scropt: :count match type needs an argument, e.g.
>      "eq"
>    * page 9; active script: "spam-${test}; <- missing "

Will correct these.

> Please let me know what you think.

Thanks!
Aaron