Subject: Re: [hybi] websockets protocol getting solid - cross-reviews; deadline April 15

From: timeless <[email-masked]>
Date: Mon, 11 Apr 2011 01:38:22 +0300

On Tue, Mar 29, 2011 at 11:35 PM, Arthur Barstow <[email-masked]> wrote:
> All comments should be submitted to [email-masked] by April 15 at the latest.

http://tools.ietf.org/html/draft-ietf-hybi-thewebsocketprotocol-06

> This protocol is intended to fail to establish a connection with
> servers of pre-existing protocols like SMTP or HTTP, while allowing

s/or/and/

>   5.   If the <scheme> component of /uri/ is "ws", set /secure/ to
>        false; otherwise, if the <scheme> component is "wss", set
>        /secure/ to true; otherwise, fail this algorithm.

it seems odd to use otherwise twice, i think the first instance can be omitted

>   o  The /resource name/ string must be a non-empty string of
>      characters in the range U+0021 to U+007E that starts with a U+002F
>      SOLIDUS character (/).

s/that starts/and start/

>   Any WebSocket URIs not meeting the above criteria are considered
>   invalid, and a client MUST NOT attempt to make a connection to an
>   invalid WebSocket URI.

s/, and a/.  A/

>   Payload length:  7 bits

>      in network byte order.  The payload length is the length of the
>      Extension data + the length of the Application Data.  The length
>      of the Extension data may be zero, in which case the Payload
>      length is the length of the Application data.

This block uses both "Application data" and "Application Data", I
think the former is intended based on the definitions that follow:

>    Extension data:  n bytes
>   Application data:  n bytes

>   be counted before first byte is sent.  With fragmentation, a server
>   or intermediary may choose a reasonable size buffer, and when the

a client, server, or intermediary ?

>   4.   The request MUST contain a "Host" header whose value is equal to
>        the authority component of the WebSocket URI.

Is there a reason to use 'authority component' instead of /host/ or
defining '/authority component/' in <Section 3.1>?
>   The steps to *parse a WebSocket URI's components* from a string /uri/
>   are as follows.  These steps return either a /host/, a /port/, a
>   /resource name/, and a /secure/ flag, or they fail.

>   6.  Optionally, a "Sec-WebSocket-Protocol header, with a list of
>       values indicating which protocols the client would like to speak,
>       ordered by preference.

The ordered by preference part only appears to be listed in the Server
portion of the spec, as it's relatively important for the client to
understand that order matters, it seems useful to indicate this in the
client section too.

>7.2.2.  Server-initiated closure
>   Certain algorithms require or recommend that the server _abort the
>   WebSocket connection_ during the opening handshake.  To do so, the
>   server must simply _close the WebSocket connection_ (Section 7.1.1).

Is there a reason 'must' here isn't in caps?

>   1003
>      1003 indicates that an endpoint is terminating the connection
>      because it has received a type of data it cannot accept (e.g. an
>      endpoint that understands only text data may send this if it
>      receives a binary message.)

s/.)/)./

> 8.1.  Negotiating extensions
>         extension-list = 1#extension
>         extension = extension-token *( ";" extension-param )
>         extension-token = registered-token | private-use-token
>         registered-token = token
>         private-use-token = "x-" token
>         extension-param = token [ "=" ( token | quoted-string ) ]
..
>   is exactly equivalent to
>         Sec-WebSocket-Extensions: foo, bar; baz=2

I don't see <,> covered by the ABNF

>   Note that the client is only offering to
>   use any advertised extensions, and MUST NOT use them unless the
>   server accepts the extension.

perhaps s/accepts/responds indicating that it wants to use/

>  The extensions
>   listed by the server in response represent the extensions actually in
>   use.

perhaps s/use/use for the connection/

>      corresponding bytes using their percent-encoded form as defined in
>      the URI and IRI specification.  [RFC3986] [RFC3987]

"specifications" ?

----
The IETF, IANA (10.1/10.2/10.3) and [RFC3864] (10.4-10.9) sections
each use different section header formats in the version of the
document I'm reviewing, which seems unfortunate, but I guess that each
agency has defined its own style.
----

>   accept connections from non-WebSocket clients (e.g.  HTTP clients)

s/  HTTP/ HTTP/ -- "e.g." is not a sentence terminator.

>   set of protocol-level extensions to use during the connection.

"during" seems odd. Perhaps "for" or "for the duration of" or ...?
-- to me, "during" almost implies "during [set up of] the connection"