Re: [imapext] AD review of draft-ietf-imapapnd-rfc2088bis-03

[Alexey Melnikov <[e-mail]>]

Hi Barry,
Thank you for your review.

> On 4 Mar 2016, at 23:03, Barry Leiba <[e-mail]> wrote:
> 
> Here's my review of draft-ietf-imapapnd-rfc2088bis-03.  Much of this
> is editorial, but there are a couple of substantive things here.
> 
> -- Introduction --
> 
> "(RFC 3501)" should be a citation, "[RFC3501]".  (And then you can
> remove the citation at the beginning of Section 3, if you like (or
> leave it, if you prefer).)
> 
> -- Section 3 --
> 
>   If the server does
>   not advertise either of the above capabilities, the client must use
>   synchronizing literals instead.
> 
> Minor point, but I'd word this like this (because I find "instead" to
> be a bit inapt):
> 
> NEW
>   If the server does
>   not advertise either of the above capabilities, the client can only
>   use synchronizing literals.
> END
> 
>   The protocol receiver of an IMAP server must check the end of every
>   received line
> 
> That "must" should probably be "MUST".
> 
> We probably should also take this opportunity to fix a bit of
> confusion that's come up with respect to this paragraph in the past.
> How about this?:
> 
> OLD
>   The protocol receiver of an IMAP server must check the end of every
>   received line (a sequence of octets that end with a CRLF) for an open
>   brace ('{') followed by an octet count, a plus ('+'), and a close
>   brace ('}') immediately preceeding the CRLF.  If it finds this
>   sequence, it is the octet count of a non-synchronizing literal and
>   the server MUST treat the specified number of following octets and
>   the following line as part of the same command.  A server MAY still
>   process commands and reject errors on a line-by-line basis, as long
>   as it checks for non-synchronizing literals at the end of each line.
> 
> NEW
>   The protocol receiver of an IMAP server MUST check the end of every
>   received line (a sequence of octets that ends with a CRLF) for an
>   open brace ('{') followed by an octet count, a plus ('+'), and a
>   close brace ('}') immediately preceeding the CRLF.  If it finds this
>   sequence, it is the octet count of a non-synchronizing literal and
>   the server MUST treat the specified number of following octets and
>   the following octets through the next CRLF as part of the same
>   command.
> 
>   It's important to note that the literal is not delimited by CRLF.
>   It ends after the number of bytes specified by the octet count, and
>   the current command continues from there.  There might be a CRLF
>   immediately after, which ends the command.  Or there might be more
>   octets, specifying other command parameters, before the CRLF.  If
>   a SPACE character is needed between parameters, it's important that
>   the SPACE appear after the literal, in its appropriate place.
> 
>   A server MAY still process commands and reject errors on a
>   line-by-line basis, as long as it checks for non-synchronizing
>   literals at the end of each line.
> 
> END
> 
> ...and...
> 
> OLD
>  Example:
> 
>   C: A001 LOGIN {11+}
>   C: FRED FOOBAR {7+}
>   C: fat man
>   S: A001 OK LOGIN completed
> 
> NEW
>   Example:
> 
>   C: A001 LOGIN {11+}
>   C: FRED FOOBAR {7+}
>   C: fat man
>   S: A001 OK LOGIN completed
> 
>   This is semantically equivalent to this version that uses quoted
>   strings instead of literals:
> 
>   C: A001 LOGIN "FRED FOOBAR" "fat man"
>   S: A001 OK LOGIN completed
> 
>   Note that the SPACE after FOOBAR in the first version corresponds
>   to the SPACE between the two quoted strings in the second.
> END

These look fine to me.

> I used to get questions from implementors about the CRLF and SPACE
> things.  If you really think this is unnecessary, feel free to opt out
> of this suggestion.

I don't mind adding your text.
> 
> -- Section 4 --
> 
>   a compliant LITERAL+ server
>   implementation has to make a choice between several non-optimal
>   choices:
> 
> There are only two choices, and in no one's reckoning does two count
> as "several".  

I thought several was any number other than 1.

> Maybe change "several" to "two"?
> 
> In bullet 1:
> 
>       (The server is allowed to send the tagged BAD/NO response before
>       reading the whole non-synchronizing literal.)
> 
> Substantive: Shouldn't that be "the server is not allowed" (missing "not")?

No, the sentence is correct as written. Some servers send BAD right after observing the non-synchronising literal prefix and that is Ok.
> 
> Please change "most of commands" to "most commands".
> 
> "Denial Of Service attacks" shouldn't be capitalized, but should be
> hyphenated; make it "denial-of-service attacks" (and similarly, remove
> the capitals in Section 9).

Ok.
> 
> -- Section 5 --
> Substantive: Shouldn't references to "APPEND" be removed from here,
> since we re-spun LITERAL- as applying to all commands?

I double check, but I prefer to keep it.

> Also, the last
> sentence doesn't really make sense.  In order to reject the command
> with BAD and TOOBIG, the server has to read (and discard) the literal
> -- that is, it's already processing according to bullet 1 in Section
> 4.
> 
> So:
> 
> OLD
>   The "LITERAL-" extension is almost identical to "LITERAL+", with one
>   exception: when "LITERAL-" is advertised, non-synchronizing literals
>   used in any command MUST NOT be larger than 4096 bytes.  Any literal
>   larger than 4096 bytes MUST be sent as an RFC 3501 synchronizing
>   literal.  A "LITERAL-" compliant server that encounters a non-
>   synchronizing literal in APPEND larger than 4096 bytes MUST reject
>   such APPEND command with a tagged BAD response that contains the
>   TOOBIG response code [RFC4469].  It then MAY proceed as described in
>   Section 4.
> 
> NEW
>   The "LITERAL-" extension is almost identical to "LITERAL+", with one
>   exception: when "LITERAL-" is advertised, non-synchronizing literals
>   used in any command MUST NOT be larger than 4096 bytes.  Any literal
>   larger than 4096 bytes MUST be sent as an RFC 3501 synchronizing
>   literal.  A "LITERAL-" compliant server that encounters a non-
>   synchronizing literal larger than 4096 bytes MUST read (and discard)
>   the literal, and then reject the command with a tagged BAD response
>   that contains the TOOBIG response code [RFC4469].

Hmm. Or it can close the connection. I will try to reword, as your text seems to imply that that is the only option.
> 
> END
> 
> Substantive: I also suggest adding this paragraph, to make things
> perfectly clear:
> 
> INSERT
>   Note that the form of the non-synchronizing literal does not change:
>   it still uses the "+" in the literal itself, even if the applicable
>   extension is "LITERAL-".
> END

Ok.
> 
> -- Section 9 --
> 
>   Section 4 motivates creation of the "LITERAL-" extension
>   that partially improves the situation.
> 
> I would just say 'The "LITERAL-" extension partially improved this situation.'
> 
> -- Section 10 --
> 
> OLD
>   This document requests that IANA updates the above registry to
>   include the entry for LITERAL+ capability pointing to this document.
> 
> NEW
>   This document requests that IANA update the above registry to
>   replace the reference for LITERAL+ to point to this document.
> 
> END

Yes, good catch.
> 
> (And as a nit, change "adds" to "add" in the next paragraph; it should
> be subjunctive mood.)
> 
> -- 
> Barry, ART Director