Bootstrapping WebSockets with HTTP/2
draft-ietf-httpbis-h2-websockets-07

Thanks for the well-written document!

I only have one comment: should we explicitly mention that the security considerations of RFC 6455
continue to apply?

Rich version of this review at:
https://mozphab-ietf.devsvcdev.mozaws.net/D5180



COMMENTS
S 4.
>         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].
>   
>      o  On requests bearing the :protocol pseudo-header, the :scheme and
>         :path pseudo-header fields MUST be included.

You should say what the path means.


S 4.
>   
>      o  On requests bearing the :protocol pseudo-header, the :authority
>         pseudo-header field is interpreted according to Section 8.1.2.3 of
>         [RFC7540] instead of Section 8.3 of [RFC7540].  In particular the
>         server MUST NOT make a new TCP connection to the host and port
>         indicated by the :authority.

I was sort of able to make sense of this, but it's kind of confusing.
Perhaps you could say a word about it.




S 5.
>      the GET-based request in [RFC6455] and is used to process the
>      WebSockets opening handshake.
>   
>      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.

Just to be clear, you are saying ":scheme" must be https or http?

One high level question: You say in the intro that a mechanism to transition from http to TCP is needed while the proposed mechanism does actually not "transition" but use websockets OVER http. Was just opening a second TCP connection considered as an alternative?

This was mostly clear and easy to understand. I wanted to pass on a few places where I struggled.

This is a nit:

  This tunneled stream will be multiplexed with other regular streams
   on the connection and enjoys the normal priority, cancellation, and
   flow control features of HTTP/2.

"streams enjoying HTTP/2 features" seems odd to me. Perhaps "has" or even "shares"? But do the right thing.

It was really easy for me to read 

  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. 

without realizing that ":protocol" WAS the pseudo-header - it took me two or three passes to realize that I could stop looking for the pseudo-header, because I'd already seen it. Maybe no one else will be confused? I notice that Section 5 puts scheme names in quotes, and they are more noticeable, so maybe ":protocol" (and the other pseudo-headers in the section) would be helpful to the reader. But do the right thing.

I had trouble parsing 

  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.

somewhere around "does not ever allow to be created" - the combination of passive tense and negation was probably my problem. Even then, "does not allow the creation of" - what? matching text seems to say "the use of a pseudo-header is not allowed to be created". Is that what is intended? If I'm tripping over well-understood HTTP/2 terminology, my apologies, of course.

Is there a pointer you could include with this text?

  The 2017 HTTP Workshop had a very productive discussion that helped
   determine the key problem and acceptable level of solution
   complexity.