Re: [hybi] Fwd: publication request: draft-ietf-hybi-permessage-compression-05

[Barry Leiba <[e-mail]>]

> I have sent a publication request to the IESG for the permessage-compression draft.

And I'm afraid that I'm sending it back: I think it's not in
appropriate shape to ask the community and the IESG to review and
accept it.  What I've read has been unclear, and I'm very much
concerned that implementors won't be able to follow it well enough to
implement it correctly.  See my (partial) review below; I stopped
after Section 3.

I'm asking the working group to do some careful review to make sure
that it's clear, understandable, and ready for people outside the
working group to implement.  Make sure everything holds together and
that someone who is NOT already familiar with how this works can still
understand and correctly implement it.  If the working group needs
more help with this, it can ask for it on apps-discuss.

Barry

========================================
-- General --

   [in various places...}
   _This section is non-normative._

   2. Conformance Requirements
   Everything in this specification except for sections explicitly
   marked non-normative is normative.

I understand that all of this corresponds to the same text in the
WebSockets document.  I can't tell you how much I dislike this style,
and I ask (but not demand) that you please consider removing it.  I'm
happy to discuss this.

I believe you need to be clear from the language you use (and I don't
just mean MUST and SHOULD) when something is not normative.  Calling
things "examples" already conveys this.  Section titles such as
"Introduction" conveys this.  Most documents manage without this sort
of thing.  Why does WebSockets (and its successor documents, such as
this one) need it?

-- Introduction --

   The simplest "Sec-WebSocket-Extensions" header in the client's
   opening handshake to request permessage-deflate is the following:

       Sec-WebSocket-Extensions: permessage-deflate

   The simplest header from the server to accept this extension is the
   same.

Why is this in the Introduction?  It seems seriously out of place there.

-- Section 3 --

I find the organization of this to be awkward.  The first paragraph
doesn't make much sense to me, for one thing.  I suggest two things to
start with:

1. Change the overall document title:

OLD
WebSocket Per-message Compression

NEW
WebSocket Per-message Compression Extension Framework

That better explains what the document is specifying (the abbreviated
title can remain "WebSocket Per-message Compression"), and means that
the last sentence of the first paragraph of Section 3 can be removed.
For the rest of the paragraph, how about this (I'm also pulling up
something from the paragraphs below)?:

NEW
   Per-Message Compression Extensions (PMCEs) are individually
   defined, and are registered in the WebSocket Extension Name
   Registry.  One such extension is defined in Section 5 of this
   document and is registered in Section 7.1.  Other PMCEs may
   be defined in other documents.  Each PMCE defines the content
   of its extension token, including any applicable extension
   parameters.
END

Note that you now have an abbreviation (PMCE) that you can use freely.

Second paragraph:

   To request use of a Per-message Compression Extension, a client MUST
   include an element with its extension token in the
   "Sec-WebSocket-Extensions" header in its opening handshake.

You should avoid a lot of "it"s, unless the antecedent is quite clear.
 Here, the antecedents are not clear.  It looks like "its extension
token" means "the client's extension token," and it looks like "its
opening handshake" means "the S-WS-E header's opening handshake," or
perhaps "the token's opening handshake."

Also, is the client "requesting" use of one particular PMCE, or
"offering" a list of PMCEs?  It seems to me that it's the latter: the
client offers a list, and the server selects zero or one from the list
and sends it in the response.  Yes?

   The
   element contains extension parameters as specified by the
   specification of the extension.

This loops around in a circle, and is hard to make sense of.

   A client MAY list multiple Per-
   message Compression Extensions with the same name to offer use of the
   same algorithm with different configurations.

May a client also list multiple extensions with *different* names?

How about this for the whole paragraph?:

NEW
   To offer use of a PMCE, a client MUST include an element with the
   offered extension's token in the "Sec-WebSocket-Extensions" header
   in the opening handshake of the WebSocket connection.  A client
   offers multiple PMCE choices to the server by including multiple
   tokens, one for each PMCE offered.  The set of tokens MAY include
   multiple PMCEs with the same name to offer use of the same
   algorithm with different configurations.
END

Please look for other "it" occurrences, and make sure they're similarly clear.

Similarly, for the third paragraph, maybe this?:

NEW
   To accept use of a PMCE, a server MUST include an element with the
   accepted extension token in the "Sec-WebSocket-Extensions" header in
   the opening handshake of the WebSocket connection.  The accepted
   token MUST be one of those offered by the client during the opening
   handshake, and MUST represent a PMCE that is fully supported by the
   server.  The server rejects all offered PMCEs by not including any
   PMCE tokens, in which case the connection proceeds without
   per-message compression.
END

Fourth paragraph:

   If a client doesn't support the extension and its parameters replied
   from the server, the client MUST _Fail the WebSocket Connection_.
   Otherwise, once _the WebSocket Connection is established_, both
   endpoints MUST use the algorithm described in Section 4 to exchange
   messages.

Why would a server respond with an extension/parameters that the
client doesn't support?  The client already sent a list of what it's
willing to use.  In the re-writes I've done above, that's made clear,
and this paragraph is, if anything, just specifying how to handle a
server response that's in violation of the protocol.  You also refer
to the Section 4 algorithm, but say nothing about what compression
mechanism will be used.  So maybe this?:

NEW
   * If the server responds with no PMCE tokens, once _the WebSocket
   Connection is established_ it proceeds without per-message
   compression.

   * If the server gives an invalid response, such as accepting a
   PMCE that the client did not offer, the client MUST _Fail the
   WebSocket Connection_.

   * Otherwise, once _the WebSocket Connection is established_, both
   endpoints MUST use the algorithm described in Section 4 to exchange
   messages, using the PMCE returned by the server.
OLD

By my reckoning, this has just re-written the entire Section 3.  This
bothers me, because it makes me question the level of review and
editing that the document has gone through.  As I look at Section 3.1,
I see that it appears to be inadequate: it is *not* a "negotiation
example"; it is a series of out-of-context examples of isolated header
field values.  This actually *would* benefit from a full example of an
opening handshake that uses this, where a client offers a few PMCEs
(perhaps two with the same name and a third with another name), and
showing how the server responds.
========================================