Re: WGLC on draft-ietf-sieve-managesieve-01.txt (review)
2008-11-06 06:21:26
Hello,
Alexey Melnikov wrote:
The Working Group Last Call for this document starts on November 3rd and
will end on November 21st (so it is almost 3 weeks, so that the IETF
meeting in Dublin is covered).
Please send any comments to the Sieve mailing list or directly to me. If
you chose to do the latter, please CC Cyrus Daboo <cyrus(_at_)daboo(_dot_)name>.
Reviews that found no issues are also welcomed, so if you review the
document and find it acceptable, please let the mailing
list/authors+chairs know as well.
Since this is the last call, I decided to do a full review of the
document. Thus far I have not implemented any of the recent
changes/additions to the document, so I may find other issues when I do
(hopefully before the 21st of November).
My review is structured in a per-section manner. Comments include typos,
but also requests for clarification:
1.3:
- Use of example with 'the QUOTA/MAXSCRIPTS response' code is confusing.
MAXSCRIPTS is not part of the standard, but the example does not mention
that explicitly. Also, I can imagine that such detailed response can
have some merit. So, why is it not included in the standard along with
something like MAXSIZE?
- Typos in explanation of NONEXISTENT and ALREADYEXISTS :
s/references/referenced.
1.7:
- Example at the end of this section lists STARTTLS after
authentication, which is strange considering that STARTTLS is only valid
in a non-authenticated state.
- Explain the purpose of the OWNER capability. Common question would be:
clients already know their identity, so why advertise it as a
capability? Also, in my mind, listing status information in a capability
response seems hardly appropriate.
2.1:
- From text: "When both [TLS] and SASL security layers are in effect,
the TLS encoding MUST be applied (when sending data) after the SASL
encoding, regardless of the order in which the layers were negotiated."
Isn't this order fixed correctly already by the fact that the STARTTLS
command is only valid before authentication(== SASL and any security
layer it may install). I mean that I do not see how the layers can be
negotiated in a different order.
- From text: "To ensure interoperability, client and server
implementations of this extension MUST implement the [SCRAM] SASL
mechanism." This sentence seems out of place. What extension? Why SCRAM?
2.2.1:
- Item 1, first sentence: s/than/then
2.6:
- Clarify what is meant by syntax checking. In the strictest sense it
could mean that any script that complies with the basic grammar of the
Sieve language would pass. Command syntax, i.e. which arguments are
required/allowed, would then be ignored. Also, wouldn't it be
helpful/useful/desirable to include contextual checks here as well? E.g.
unknown/invalid comparators etc. I first encountered this distinction
when discussing the ihave extension to the Sieve language. I implicitly
interpreted this requirement as a full script check (i.e. as would be
done for full compilation), which seems overly restrictive when reading
this standard more carefully.
2.11:
- Naming of CHECKSCRIPT extension is not consistent with RENAME
extension. To be consistent CHECK would be a better choice. (Yes, I am
nitpicking, sorry)
2.11.2:
- For clarity, mention that the CHECKSCRIPT command is in effect
identical to the PUTSCRIPT command when in ANONYMOUS Sieve script
verification mode.
2.11.3:
- NOOP: why expect a NO response from older servers? It is advertised as
a capability; if it is not advertised, don't issue the NOOP command.
2.11.4:
- Explain the use and merit of the UNAUTHENTICATE command. The only
merit I can think of is that reconnecting can cause some extra overhead
for setting up the connection and re-negotiating security/TLS layers.
But is this time optimization worth complicating the protocol?
Discussion item: who would be willing to implement this? I personally
have no real problems with the addition of this command, but I cannot
imagine that I will ever want to implement it. Perhaps I am overlooking
something?
General:
- The described protocol is both referred to as "ManageSieve" and
"Manage Sieve" (e.g. IANA Considerations)
I hope this review is useful. I'll try to update my implementation to
match the most recent changes in the coming week or so.
Kind regards,
--
Stephan Bosch
stephan(_at_)rename-it(_dot_)nl
|
|