WebSockets: frames with fragmented header are causing halt in websocket processing

[marius-klimantavicius]

In ParseWebSocketReceivedData if _webSocketReceiveState == WebSocketReceiveState.Header and there is not enough ReceiveDataLength to read full header (ParseHeader returns 0) the data is neither consumed nor additional receive is started. See autobahn test suite case "2.6.1" (fuzzingclient).

Before the latest changed I had a special case for ParseHeader(...) == 0 to simply call StartReciveDataIfNotAlreadyReceiving but now it causes a StackOverflowException because no data is consumed from Callback and StartReceiveData calls ProcessReceive if ReceiveDataLength > 0. The same might happen for Close frame due to: if (Callback.ReceiveDataLength < (int)_webSocketFrameLen) as it calls start receive without consuming data.

In my case (I am using somewhat refactored version of nowin thus a straight pull request is not possible :() I added additional buffers for header, control frame, ping frame and pong frames. I modified ParseWebSocketReceivedData in following ways:

1. If ReceiveDataLength == 0 I still have to check whether I have a complete frame (in body with frame length zero or a full control frame)
2. Even if there is no receive in progress try to read frame header (if state is Header) consume data always but start a new receive if the header is incomplete.
3. If the header is complete - check if the frame is for Ping/Pong - handle accordingly. Read Ping frame payload into ping buffer and start a send that copied ping buffer to pong buffer (within locks) and issues a send.
4. For both Ping/Close frames if there is only partial data available fill control frame buffer, consume data and request more.

[marius-klimantavicius]

During my testing I found that sometimes _webSocketReceiveTcs has SetResult called multiple times (this mostly happened when there were breakpoints with action to print some text in autobahn test suite case 9.7.1 [fuzzingclient]). In those cases I changed var tsc = _webSocketReceiveTcs; if (tsc != null) { _webSocketReceiveTcs = null; .. } to: var tsc = Interlocked.Exchange(ref _webSocketReceiveTcs, null); if (tsc != null) { .. } Though I am not sure whether this was a real fix or only fixing a symptom.

[Bobris]

Nice findings thanks a lot. Is ping support really so interesting when no browser send them?

[marius-klimantavicius]

Ping/Pong support probably is not that interesting though some libraries (i.e. WebSocket4Net and websocket-sharp) do have support for sending pings. I found this fragmentation issue when running a ping/pong test in autobahn test suite.