Re: Last Call: <draft-ietf-httpbis-h2-websockets-05.txt> (Bootstrapping WebSockets with HTTP/2) to Proposed Standard

[Patrick McManus <[e-mail]>]

Hi Julian, I apologize this got a little bit forgotten. I will issue a new
draft with the last call comments included.

On Fri, May 11, 2018 at 6:31 AM, Julian Reschke <[e-mail]>
wrote:

>
> The terminology "HTTP/2 CONNECT method" is a bit confusing, as it make it
> sound as if there are HTTP methods specific to a protocol version.
>
> Maybe "the HTTP CONNECT method (as specified for HTTP/2 in Section 8.7 of
> [RFC7540])"?
>
>
sure.




> 4.  The Extended CONNECT Method
>
>    ...
>
>    o  A new pseudo-header :protocol MAY be included on request HEADERS
>       indicating the desired protocol to be spoken on the tunnel created
>       by CONNECT.  The pseudo-header is single valued and contains a
>       value from the HTTP Upgrade Token Registry defined by [RFC7230].
>
> I believe it would be good to add a short explanation why it is ok to
> defined a new pseudo header field despite this not being an HTTP/2
> extension point.
>
> I realize that using the SETTINGS mechanism one can make incompatible
> changes (and this should be mentioned here), but I'm still unconfortable
> with the consequences, for instance: what if a different extension wants to
> define the same pseudo header field with different semantics?
>
>
The latest iteration of that discussion is here: https://www.ietf.org/mail-
archive/web/secdir/current/msg08134.html .. the wg also considered the
question.

It is an interesting question for everyone reviewing the documents, but I
don't really think its a very interesting question for someone implementing
this protocol. The general question of semantic conflict is something the
WG has to handle for all proposed extensions (and registries are never
going to cover the full semantic set of potential conflicts.. I would argue
that the change in interpretation of :authority is more significant than a
new header value) - nothing particular about this one. Maybe we should
consider a general document on the topic.

Based on these comments I added a bit more detail to the existing
discussion in section 3:

  The use of a SETTINGS Parameter to opt-in to an otherwise
   incompatible protocol change is a use of "Extending HTTP/2" defined
   by Section 5.5 of [RFC7540].  Specifically, the addition a new
   pseudo-header ":protocol" and the change in meaning of the
   ":authority" pseudo-header in Section 4 require opt-in negotiation.



>
> 8.  Security Considerations
>
>    [RFC6455] ensures that non-WebSockets clients, especially
>    XMLHttpRequest based clients, cannot make a WebSocket connection.
>    Its primary mechanism for doing that is the use of Sec- prefixed
>    request headers that cannot be created by XMLHttpRequest-based
>    clients.  This specification addresses that concern in two ways:
>
>    o  The CONNECT method is prohibited from being used by XMLHttpRequest
>
> Clarify that XMLHTTPRequest defines this.
>

ok


>    o  The use of a pseudo-header is something that is connection
>       specific and HTTP/2 does not ever allow to be created outside of
>       the protocol stack.
>
> It doesn't? Where? (I agree it would be bad but does the spec really
> prohibit it???)
>
>
"pseudo-header fields are not http header fields" in 7540.. as http header
fields are semantically part of the api, that's something you need to worry
about being generated outside the stack and gatewayed. but not in this case.


>
>
> - move first citation of RFC7230 up to first paragraph
>
> ok

- potentially make the citation for Upgrade more specific (6.7)
>
>
ok

>
> 4.  The Extended CONNECT Method
>
>    ...
>
>    o  A new pseudo-header :protocol MAY be included on request HEADERS
>       indicating the desired protocol to be spoken on the tunnel created
>       by CONNECT.  The pseudo-header is single valued and contains a
>       value from the HTTP Upgrade Token Registry defined by [RFC7230].
>
> It would probably better to refer to the registry instead (<
> https://www.iana.org/assignments/http-upgrade-tokens/http-
> upgrade-tokens.xhtml>).
>
>
>
ok


> 5.  Using Extended CONNECT To Bootstrap The WebSocket Protocol
>
> s/The/the/
>
>
ok

   ...
>
>    The scheme of the Target URI [RFC7230] MUST be "https" for "wss"
>    schemed WebSockets and "http" for "ws" schemed WebSockets.  The
>    websocket URI is still used for proxy autoconfiguration.
>
> Maybe cite Section 5.1 of RFC 7230 specifically.
>
>
ok


>    ...
>
>    The Sec-WebSocket-Version, Origin [RFC6454], Sec-WebSocket-Protocol,
>    and Sec-WebSocket-Extensions headers are used on the CONNECT request
>    and response headers in the same way as defined in [RFC6455].  Note
>    that HTTP/1 header names were case-insensitive and HTTP/2 requires
>    they be encoded as lower case.
>
> ...sort the field names so that "Origin" is at the start or the end.
>
>
ok


>    ...
>
>    After successfully processing the opening handshake, the peers should
>    proceed with The WebSocket Protocol [RFC6455] using the HTTP/2 stream
>
> s/The/the/
>
> ok


>    ...
>
>    5.1.  Example
>
> (Example still is too wide for the old RFC format)
>
>
I'm going to let the rfc editor help with the whitespace issues here.


>
> 6.  Design Considerations
>
> (maybe move to an appendix?)
>
>
Kinda meh here - and hearing no other comments will leave alone


>
> Best regards, Julian
>
>
>