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

From: Barry Leiba <[email-masked]>
Date: Fri, 4 Mar 2016 18:03:20 -0500

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

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.

-- 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".  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")?

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).

-- Section 5 --
Substantive: Shouldn't references to "APPEND" be removed from here,
since we re-spun LITERAL- as applying to all commands?  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].

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

-- 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

(And as a nit, change "adds" to "add" in the next paragraph; it should
be subjunctive mood.)

-- 
Barry, ART Director

---

From: Barry Leiba <[email-masked]>
Date: Sat, 5 Mar 2016 11:19:01 -0500

>> as "several".
> I thought several was any number other than 1.

This is just for fun now, because it really doesn't matter either way:
Well, dictionary.reference.com says...

   1. being more than two but fewer than many in number or kind:
         several ways of doing it.

In general, if you said "Barry has several Furbies," and it turned out
I had two, no one would say you were wrong, but many might accuse you
of exaggerating.  (In fact, I have no Furbies
<https://en.wikipedia.org/wiki/Furby>; sorry.)

> No, the sentence is correct as written. Some servers send BAD right after
> observing the non-synchronising literal prefix and that is Ok.

Hm, but then I think the sentence is meaningless.  The server still
has to read and discard the literal, so whether it sends the BAD
before or after it does that hardly matters.  What are you really
trying to say with that parenthetical that's useful?

>> since we re-spun LITERAL- as applying to all commands?
>
> I double check, but I prefer to keep it.

Really?  Why.  My understanding was that the working group agreed that
the 4K limitation on LITERAL- applies to all literals, not just those
in APPEND commands.  So why should this text explicitly talk about
APPEND?

>> Also, the last
...
> Hmm. Or it can close the connection. I will try to reword, as your text
> seems to imply that that is the only option.

I'll look forward to your rewording.

Barry

---

From: Alexey Melnikov <[email-masked]>
Date: Sat, 5 Mar 2016 16:58:44 +0000

Hi Barry,

> trying to say with that parenthetical that's useful?

This is a valuable implementation advice.

---

From: Alexey Melnikov <[email-masked]>
Date: Sat, 5 Mar 2016 17:09:54 +0000

> On 5 Mar 2016, at 16:58, Alexey Melnikov <[email-masked]> wrote:
> 
> Hi Barry,
> 

This also potentially affects how client implementations are coded.

---

From: Barry Leiba <[email-masked]>
Date: Sat, 5 Mar 2016 13:36:28 -0500

> This also potentially affects how client implementations are coded.

I don't agree that it's valuable as written, as I don't think it says
enough to be useful.  Consider a non-expert implementor, who codes
like this:

- Read up to CRLF
- See "{666666+}" at the end
- Send BAD
- Now what?  Read the next command?  That won't work, because the
client already sent the literal.  Close the socket?  With or without
sending untagged BYE first?  And why would you want to close the
socket when you've already said BAD to the command itself?  Are we
advising servers to kick off clients that have protocol errors?  And
then what if the client just reconnects?  Kick it off again?

Rather than having a cryptic parenthetical, if you want to give useful
implementation advice it'd be better to give real advice about the
options that are reasonable, and what the pros and cons are.  And I
personally (just opinion here) don't think that sending BAD before
discarding the literal is something implementors should be advised to
do, even if some implementations happen to do it... unless one can
articulate what advantages it gives to the server or to the client.

If your argument is that sending BAD and then immediately closing the
socket (or sending BAD, then BYE, then closing the socket), without
bothering the read the literal... saves the server work and resources,
then maybe it's reasonable advice.  But then the advice isn't just to
send BAD -- it's the whole sequence, together.

Get what I'm trying to say?

b

---

From: Alexey Melnikov <[email-masked]>
Date: Sat, 5 Mar 2016 08:08:42 +0000

Hi Barry,
Thank you for your review.

> On 4 Mar 2016, at 23:03, Barry Leiba <[email-masked]> wrote:
> 

These look fine to me.

> things.  If you really think this is unnecessary, feel free to opt out

I don't mind adding your text.

> Maybe change "several" to "two"?
> 
> In bullet 1:
> 

No, the sentence is correct as written. Some servers send BAD right after observing the non-synchronising literal prefix and that is Ok.
> the capitals in Section 9).

Ok.

>   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.
>   it still uses the "+" in the literal itself, even if the applicable

Ok.
>   replace the reference for LITERAL+ to point to this document.

Yes, good catch.

---

From: Alexey Melnikov <[email-masked]>
Date: Sun, 6 Mar 2016 21:21:43 +0000

Hi Barry,
I've applied most of the comments (thank you!). Regarding the two
remaining issues:

> Substantive: Shouldn't that be "the server is not allowed" (missing "not")?

I reply to this in a separate email.

> -- that is, it's already processing according to bullet 1 in Section

I fixed the problem you stated in the last sentence, see below.

>    that contains the TOOBIG response code [RFC4469].

Unfortunately TOOBIG is defined as:

4.2.  TOOBIG Response

   The TOOBIG response code is returned if the resulting message will
   exceed the 4-GB IMAP message limit.  This might happen, for example,
   if the client specifies 3 URLs for 2-GB messages.  Note that even if
   the server doesn't return TOOBIG, it still has to be defensive
   against misbehaving or malicious clients that try to construct a
   message over the 4-GB limit.  The server may also wish to return the
   TOOBIG response code if the resulting message exceeds a server-
   specific message size limit.

So this is really APPEND specific. And I think having a response code is
important to debugging and problem diagnostic. So I initially rewrote
the last sentence as:

   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. If
   responding to an APPEND command, the tagged BAD response that
   contains the TOOBIG response code <xref target="RFC4469"/>.

However, I realized that the server should still be allowed to return
BYE and disconnect. So my final version is:

   A "LITERAL-" compliant server that encounters a non-synchronizing
   literal larger than 4096 bytes proceeds as described in
   Section 4. If responding to an APPEND command, the tagged BAD
   response MUST contains the TOOBIG response code [RFC4469].
   If responding with untagged BYE response, it SHOULD include
   the TOOBIG response code.

---

From: Alexey Melnikov <[email-masked]>
Date: Sun, 6 Mar 2016 21:29:21 +0000

Hi Barry,

> then what if the client just reconnects?  Kick it off again?

But you know that this particular problem is not really solvable.

> articulate what advantages it gives to the server or to the client.

Oh, I think you misinterpreted what the text is trying to say: it
doesn't recommend discarding the literal. Implementations that return
BAD must still eat the data. But this sentence is saying that it is OK
to say BAD immediately after observing a large literal, without waiting
till the full command is received.

> then maybe it's reasonable advice.

This is not better than just sending BYE.

> But then the advice isn't just to
> _______________________________________________
> imapext mailing list
> [email-masked]
> https://www.ietf.org/mailman/listinfo/imapext
>

---

From: Barry Leiba <[email-masked]>
Date: Mon, 7 Mar 2016 06:40:57 +0000

>    the TOOBIG response code.

I think that's better than how we started, by leaving the actual
advice in one place (Section 4), and referring to it.

I still think Section 4 could be better about explaining why it would
(or wouldn't) be useful to send BAD before flushing the literal and
the rest of the command, rather than after, but no one else has piped
up so we'll go from here.

I'll request last call now.

Barry