Kestrel considers Connection: keep-alive, upgrade as non-upgrade request

[analogrelay]

Firefox sends it's WebSocket upgrade request with a Connection header value of keep-alive, Upgrade. Unfortunately MessageBody.For does a strict equality check (https://github.com/aspnet/KestrelHttpServer/blob/dev/src/Microsoft.AspNetCore.Server.Kestrel/Internal/Http/MessageBody.cs#L237) which means the WebSocket request is assigned the ForContentLength version, with a content length of 0. As a result, the connection is considered terminated before it is even established and things go haywire (specifically the first attempt to Receive a message causes an exception because the underlying connection has been terminated).

We'll need to switch to either a Contains check or multiple Equals checks in order to ensure we're getting things right.

/cc @halter73

[Tratcher]

WebListener probably has the same bug.

[analogrelay]

This is pretty high priority since it means Firefox can't establish a WebSocket connection to Kestrel. /cc @Eilon @muratg

[Tratcher]

Wow, this broke all the servers:
https://bugzilla.mozilla.org/show_bug.cgi?id=730742
nodejs/node-v0.x-archive#14191
Lawouach/WebSocket-for-Python#3
https://bugs.eclipse.org/bugs/show_bug.cgi?id=414167

[analogrelay]

Yeah, it definitely seems like we're in good company :). Best to be willing to accept it on the server though.

[Tratcher]

Same issue in WebSockets:
https://github.com/aspnet/WebSockets/blob/4e2ecf8a63b9fc78175d1ef62bf2b1918b8b3986/src/Microsoft.AspNetCore.WebSockets/Internal/HandshakeHelpers.cs#L38

[analogrelay]

Yep, saw that in my work porting that over to Sockets. I'll make the fix there as well.

[shirhatti]

@jhkimnew Can you check if this breaks IIS/ANCM as well?

[BrennanConroy]

No, firefox works fine with the server behind IIS/ANCM

[analogrelay]

In that case, because IIS is establishing the connection to Kestrel, it is fine because IIS uses Connection: Upgrade on the outbound request.