Subject: WGLC on draft-freed-sieve-ihave-02.txt

From: Alexey Melnikov <[email-masked]>
Date: Mon, 08 Sep 2008 15:00:13 +0100

Hi folks,

This message officially starts the Sieve Working Group Last Call for the following document: 

SIEVE Email Filtering: Ihave Extension
<http://www.ietf.org/internet-drafts/draft-freed-sieve-ihave-02.txt>

The Working Group Last Call for this document starts on September 8th and will end on September 22nd.

Please send any comments to the Sieve mailing list or directly to me. If you chose to do the latter,
please CC Ned Freed <[email-masked]> and Cyrus Daboo <[email-masked]>.
Reviews that found no issues are also welcomed, so if you review
the document and find it acceptable, please let the mailing list/authors+me know as well.

Thank you,
Alexey, Sieve WG co-chairs

---

From: Stephan Bosch <[email-masked]>
Date: Wed, 10 Sep 2008 02:03:42 +0200

> and will end on September 22nd.

I just scanned though this draft specification and, although I like the 
potential of this new Sieve feature, I am very worried about the 
implementation of all this. The issue described here was already coined 
in November of 2006 (e.g. 
http://www.imc.org/ietf-mta-filters/mail-archive/msg03315.html), but to 
my knowledge it was never discussed further.

I've built a compiler that compiles Sieve scripts into a byte code 
representation. I'd like to see this ihave feature work for my 
implementation even when the compiler is completely oblivious of the 
extensions listed by the ihave test. Given the fact that most extensions 
introduce new language elements, the compiler must have some means of 
ignoring/discarding the regions of the script that need the obscure 
extensions. Alternatively, doing the actual ihave test at runtime 
requires that the compiler can distinguish between erroneous language 
features and features that would have been introduced by the unknown 
extension (in order to produce compile time errors or runtime errors 
respectively). This would require knowledge about that obscure extension.

As the document itself indicates, it will be very difficult to 'parse 
around' the script regions that depend on the obscure extensions listed 
by ihave for two reasons: ihave is a test that can be arbitrarily 
combined with other tests using allof/anyof and the extensions activated 
by ihave remain active even after the block of the (els)if command has 
ended. Now I am wondering why this choice was made. What is the benefit 
of being able to write complex conditional expressions with the ihave 
test and why not 'scope' the use of the extension within the block it 
controls? (Note that these issues are somewhat interdependent).

Both of these problems could for instance be mitigated by introducing 
ihave as a control structure instead of a test command, e.g.:

require "ihave";

redirect "[email-masked]";

ihave "fileinto" {
	fileinto "INBOX.folder";
}

keep;

Here, the use of the fileinto extension is scoped within the block 
controlled by the ihave command and it is invalid outside that block. If 
the compiler is completely oblivious of the fileinto extension it can 
now safely discard the ihave command and implicitly be rid of all script 
regions (in this case the block containing the fileinto command) that 
depend on the unknown extension. It thus becomes similar to C's #ifdef 
for Sieve compilers that handle ihave at compile time.

The only thing the above sample script omits is a form of 'else' case to 
execute when one of the listed extensions is missing. Other solutions 
are of course imaginable (more are listed in the posting referenced above).

Regards,

--
Stephan Bosch
[email-masked]

---

From: Arnt Gulbrandsen <[email-masked]>
Date: Wed, 10 Sep 2008 11:17:00 +0200

Stephan Bosch writes:
> Given the fact that most extensions introduce new language elements, 
> the compiler must have some means of ignoring/discarding the regions 
> of the script that need the obscure extensions.

Most extensions do not extend the grammar. They only add new commands, 
tests, etc., which follow the basic Sieve grammar.

> Alternatively, doing the actual ihave test at runtime requires that 
> the compiler can distinguish between erroneous language features and 
> features that would have been introduced by the unknown extension (in 
> order to produce compile time errors or runtime errors respectively). 
> This would require knowledge about that obscure extension.

I think there's an unstated requirement there: in order to be used with 
ihave, an extension must obey the grammar on page 36 of RFC 5228.

Arnt

---

From: Stephan Bosch <[email-masked]>
Date: Wed, 10 Sep 2008 13:08:11 +0200

> ihave, an extension must obey the grammar on page 36 of RFC 5228.
 From RFC 5228, Section 1.1, page 4:

'The intent is to allow for extension without changing the grammar.'

So I doubt there will ever be any extension that changes the grammar anyway.

Regards,

Stephan

(yes, I clicked send on the previous message too early)

---

From: Stephan Bosch <[email-masked]>
Date: Wed, 10 Sep 2008 13:04:44 +0200

> tests, etc., which follow the basic Sieve grammar.
That is what I mean with new language elements. I am not talking about 
the parsing stage of the compiler, but rather contextual analysis and 
code generation.

---

From: "Mark E. Mallett" <[email-masked]>
Date: Wed, 10 Sep 2008 15:56:01 -0400

> my knowledge it was never discussed further.

That one was my note.  My problem was mainly that, according to the
version of the draft I was commenting on, the capability would only be
enabled inside of a block where an 'ihave' test had contributed to a
conditional evaluating to true.  I thought it would be difficult to
coordinate the truth of the conditional evaluation, which might contain
a lot more than just an 'ihave', and the enabling of the capability.
(it's more than that, read the text at the link I guess, even though I
didn't elaborate at length I tried to present the gist of the problem.
I also would have prefered to have explicit enable/disable of the
capability, distinct from the test, but the way it evolved is ok).

This has been addressed in the later 'ihave' drafts by having it say
that a capability is enabled when a ihave test successfully operates on
that capability, given that an earlier definition says that an ihave
test is successful when all of the capabilities given to it are
available.

Anyway, I think that was quite a bit different from what you are
saying here:

> by ihave   [...]

And yeah, that can present some difficulties.  But I think as long as
you know what the lexical elements of a test or an action are and how
each one is terminated, or the syntax of a comparator token, it's
possible to find a way to swallow an unknown element such as action,
test, comparator name, etc., and generate code with enough information
that it can resonably report an error at runtime should it be
encountered.  I'm looking forward to doing this myself :)

> for Sieve compilers that handle ihave at compile time.

FWIW, my implementation actually supports #ifdef-like constructs that
can be used to enable or disable code based on various capabilities;
I rather like putting that level of control at, well, that level, but
obviously one can't mandate that everyone implement things that way.

mm

---

From: Ned Freed <[email-masked]>
Date: Sat, 13 Sep 2008 19:42:48 -0700 (PDT)

> my knowledge it was never discussed further.

Ihave no longer tracks with block scope so this is no longer an issue. The way
it works now is one ihave has turned on a feature it stays on.

> representation.

That's how our implementation works as well. The only issue that came up while
implementing ihave was that we needed to turn off a bunch of checks the
compiler was doing - you cannot assume an extension won't change the  minimum
or maximum number of arguments, or allow some additional tagged argument, or
other stuff along those lines.

> I'd like to see this ihave feature work for my
> extensions listed by the ihave test.

That's perfectly permissible, but an alternative is to do the ihave check at
compile time. THe prohibition on variables in ihave arguments makes it possible
to perform the check at compile time and reduce the ihave test to a constant
false if the extension doesn't exist and an activation of the extension and
a constant true if the extension is supported.

I would have done it your way if there was a case where we couldn't determine
the availability of an extension at compile time, but we don't have any of
those.

> Given the fact that most extensions
> extensions.

Not necessarily. Additional tests, actions, arguments, and so on can be added
bu the grammar never changes. So you can always parse the script and, in the
case of an unknown element, simply compile it into something that signals an
error if the execution path ever gets to that point.

> Alternatively, doing the actual ihave test at runtime

No, such knowledge is not required. See above.

> ended.

I agree that this is difficult. The good news is that it isn't necessary.

> Now I am wondering why this choice was made. What is the benefit

Cleaner, more compact scripts, for one thing. I also didn't like the idea of
introducing a new control element when it wasn't necessary.

> keep;

Sure, this is one way to do it. But it is less flexible, more verbose, and IMO
less clear. And I don't think it assists implementations nearly as much as you
seem to think. In fact in our implementation it wouldn't have been one iota
easier to implement.

> are of course imaginable (more are listed in the posting referenced above).

And else clause are going to be very common because most of the time you can't
just not do something, you have to do something else instead. So now you either
have to reuse else or invent some other control for this purpose - another
reason why tests are better.

				Ned

---

From: Ned Freed <[email-masked]>
Date: Sat, 13 Sep 2008 20:20:27 -0700 (PDT)

> ihave, an extension must obey the grammar on page 36 of RFC 5228.

I can and will put this in as as requirement, but my understanding has always
been that extensions would not change the base grammar because doing so
requires that implementation change their core parsers - not good.

				Ned

---

From: Stephan Bosch <[email-masked]>
Date: Sun, 14 Sep 2008 22:54:01 +0200

> No, such knowledge is not required. See above.
Ok, first of all, I must agree with the statements you make that ihave 
as a test command is cleaner in many ways. I also agree that doing the 
ihave checks on unknown language elements (mostly) at runtime seems to 
be a viable solution. I have one final remark/concern on this solution 
however. Consider the following script:

require "ihave";
require "fileinto";

if ihave "frop" {
   friep "This is a nifty feature.";
}

if header "X-Important" "yes" {
   filinto "Important";
}

Now assume that this script is uploaded through ManageSieve. As you 
know, ManageSieve compiles the script during upload to prevent the user 
from installing a broken script. From what I understand from your 
explanation, the obvious typo 'filinto' will not be noticed during 
compile (because it may be part of the obscure "frop" extension that 
could be enabled by ihave throughout the rest of the script) and the 
ManageSieve upload will succeed. During runtime however, the script will 
intermittently fail when the header test is true. Is there a way to get 
around this issue?

Anyways, I think I should first give implementing this extension a try 
before posting more concerns. I may be a bit biased towards finding more 
difficulties if I have never actually tried to implement it. I'll keep 
you posted when I do.

Regards,

Stephan

---

From: Ned Freed <[email-masked]>
Date: Sun, 14 Sep 2008 14:04:13 -0700 (PDT)

> I have one final remark/concern on this solution

> from installing a broken script.

Yes, it is supposed to do a validity check. The exact requirement currently
is:

  The server MUST check the submitted script for syntactic validity, which
  includes checking that all Sieve extensions mentioned in Sieve script "require"
  statement(s) are supported by the Sieve interpreter.

> From what I understand from your
> ManageSieve upload will succeed.

Yes, that's indeed the case.

> During runtime however, the script will

Not that I know of. I wish there were, but the problem is the only way I see to
"fix" this is to go back to making ihave only activate extensions for the
duration of the enclosed block. I actually have a way to implement that, but
it's due in large part to our having a separate compilation step. I fear that
reimposing that requirement will make ihave difficult or even impossible to
support ihave in a more straightforward implementation. I also think that while
having ihave track block structure may be more obvious to veteran programmers,
it isn't nearly so clear to people with less programming experience, and while
absolute newbies aren't the target audience for handcoded sieves, people
with only limited programming experience are.

I do note that this doesn't violate the managesieve requirement, since only a
syntax and require check are called for there. Of course nothing prevents a
managesieve implementation from doing more checks (our plan for our managesieve
implementation is to simply call our compiler and see what it thinks).

> difficulties if I have never actually tried to implement it. I'll keep

Thanks will be interesting, thanks.

				Ned

P.S. One unfortunate tendency I've seen is for automatically generated Sieves
to unconditionally include a require clause listing every extension the
generator knows about, regardless of whether the current Sieve actually uses
it. Of course this is bad for all sorts of reasons, including unecessarily
limiting script portability and increased overhead (e.g., saving match results
and scanning for substitutions if variables are enabled, per-recipient
evaluation if envelope is used, etc.). Unnecesary use of ihave will add "limits
the ability to perform validity checks" to that list.

---

From: Arnt Gulbrandsen <[email-masked]>
Date: Mon, 15 Sep 2008 11:18:58 +0200

Ned Freed writes, answering Stephan Bosch:
>> From what I understand from your explanation, the obvious typo 
>> 'filinto' will not be noticed during compile (because it may be part 
>> of the obscure "frop" extension that could be enabled by ihave 
>> throughout the rest of the script) and the ManageSieve upload will 
>> succeed.
>

Not quite, unless the variables exension is also used. Here's the 
example again, a little uglified:

require "ihave";
require "fileinto";

if ihave "frop" {
     # point A
}

# point B

     # point C
}

At point A, the script author/generator knows that frop is available, 
and can use friep. At point B, the script author doesn't know that any 
more: At point B, use of frop might cause a runtime error.

At point C, the author/generator still doesn't know that frop is 
available. If the managesieve server assumes that the script is not 
meant to cause a runtime error, the managesieve server can give an 
error for filinto at point C.

Use of variables changes that. It would be possible to set a variable at 
point A and test that variable just before point C, so if variables is 
used, then filinto can't be flagged at managesieve put time.

My conclusion is that frop isn't really available except at A. The rule 
as it stands is sufficient to hamper error detection, but it only 
allows using friep/frop in rather odd cases, and trying to get it right 
causes too much wear and tear on programmer brains.

I appreciate why the rule is there, though...

Arnt

---

From: Ned Freed <[email-masked]>
Date: Mon, 15 Sep 2008 17:27:36 -0700 (PDT)

> I appreciate why the rule is there, though...

Agreed on all points.

				Ned