Re: Call for Adoption: Bootstrapping WebSockets with HTTP/2

On 11/24/2017 11:58 AM, Andy Green wrote:
> 
> 
> On 11/24/2017 05:52 PM, Loïc Hoguin wrote:
>> Hello,
>>
>> On 11/22/2017 08:35 AM, Mark Nottingham wrote:
>>> As discussed in Singapore, this is a Call for Adoption of 
>>> Bootstrapping WebSockets with HTTP/2:
>>>    https://tools.ietf.org/html/draft-mcmanus-httpbis-h2-websockets-02
>>>
>>> So far there seems to be strong interest in this draft; please 
>>> forward any further thoughts (both for or against) on list in the 
>>> next week, when we'll make a decision.
>>
>> I've been putting some thoughts into implementing this and there's one 
>> difference betwwen h1-websocket and h2-websocket that I'm uneasy 
>> about: in h1 the server could receive a Websocket upgrade request, 
>> ignore it and send a normal 200 response with a representation of the 
>> resource. In h2 it can only error out.
>>
>> I feel that this difference in behavior ought to be pointed out in the 
>> RFC because depending on how the server is implemented you need to be 
>> careful about this.
>>
>> For example my server currently receives a Websocket upgrade request 
>> and starts processing it like any normal request (h1 or h2): by 
>> passing it to user code and letting the user decide what to do: 
>> upgrade or not. The user could very well send a 200 response here.
> 
> libwebsockets generally has the same approach to pass things to a 
> user-provided callback to decide what to do.  But eg, we have different 
> callback reasons coming according to the connection upgrade state; HTTP 
> connections send different callback reasons for established / rx / 
> writable / close than do connections in the ws state for example.

An h1 Websocket upgrade is first and foremost an HTTP request so it's 
treated as such in my server. Only after the 101 response has been sent 
does it stop being treated as a request and becomes a Websocket 
connection. There's server logic and callbacks specific to Websocket at 
that point.

My concerns are about the part of the logic where we are still dealing 
with HTTP (before accepting and performing the upgrade) where h1 and h2 
behaviors are subtly different.

>> Now with h2 I would like to keep this same interface however I cannot 
>> allow sending a 200 response anymore because a 200 response to CONNECT 
>> means we accept the Websocket connection. If I see the user try to 
>> handle the request as a normal HTTP request I have to fail the 
>> CONNECT, otherwise proceed with the upgrade. This is in contrast with 
>> h1 where the user can do what they want without such restrictions.
> 
> To implement the CONNECT processing for this, lws parses it internally 
> and filters on the upgrades it understands, so lws will fail it out if 
> it's not exactly upgrading to "websocket" and lws will send the 200 if 
> it likes it, and adapt the connection state to ws-over-h2.

My first implementation will be specific to Websocket and handle it 
automatically, therefore if it's not a CONNECT for Websocket or is 
missing required headers it can immediately fail.

However if it's a correct Websocket CONNECT I still need to route the 
request to the resource, call the user code and see if it accepts an 
upgrade to Websocket. Anything could happen in the user code (which uses 
the same interface for both h1 and h2) and I need to be ready for it.

>> To summarize:
> 
> Step zero: this implementation that doesn't actually understand and 
> handle CONNECT upgrades wrongly sent the nondefault SETTING that says 
> that it does understand them.

The implementation does understand CONNECT upgrades as stated in the 
document. What it does not do properly is handle edge cases that exist 
because of a difference in behavior between h1 and h2 Websocket 
upgrades. Having these differences clearly listed in the document can 
help avoid implementation issues.

To be clear I don't have an issue with the way the upgrade is performed. 
I just think adding an extra section would help implementers avoid 
falling into pitfalls.

-- 
Loïc Hoguin
https://ninenines.eu

Received on Friday, 24 November 2017 12:31:33 UTC