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

From: Julian Reschke <[email-masked]>
Date: Fri, 11 May 2018 12:31:31 +0200

On 2018-05-07 19:44, The IESG wrote:
> 
> The IESG has received a request from the Hypertext Transfer Protocol WG
> (httpbis) to consider the following document: - 'Bootstrapping WebSockets
> with HTTP/2'
>    <draft-ietf-httpbis-h2-websockets-05.txt> as Proposed Standard
> ...

Comments:

1.  Introduction

    ...

    This document extends the HTTP/2 CONNECT method.  The extension
    allows the substitution of a new protocol name to connect to rather
    than the external host normally used by CONNECT.  The result is a
    tunnel on a single HTTP/2 stream that can carry data for WebSockets
    (or any other protocol).  The other streams on the connection may
    carry more extended CONNECT tunnels, traditional HTTP/2 data, or a
    mixture of both.

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

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?

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.

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

Editorial nits:

1.  Introduction

    The Hypertext Transfer Protocol (HTTP) provides compatible resource-
    level semantics across different versions but it does not offer
    compatibility at the connection management level.  Other protocols,
    such as WebSockets, that rely on connection management details of
    HTTP must be updated for new versions of HTTP.

    The WebSocket Protocol [RFC6455] uses the HTTP/1.1 [RFC7230] Upgrade
    mechanism to transition a TCP connection from HTTP into a WebSocket
    connection.  A different approach must be taken with HTTP/2
    [RFC7540].  HTTP/2 does not allow connection-wide headers and status
    codes such as the Upgrade and Connection request headers or the 101
    response code due to its multiplexing nature.  These are all required
    by the [RFC6455] opening handshake.

- move first citation of RFC7230 up to first paragraph

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

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

5.  Using Extended CONNECT To Bootstrap The WebSocket Protocol

s/The/the/

    ...

    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.

    ...

    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.

    ...

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

s/The/the/

    ...

    5.1.  Example

(Example still is too wide for the old RFC format)

6.  Design Considerations

(maybe move to an appendix?)

Best regards, Julian