Skip to content

Fixed a bug in websocket receive, when a compressed frame with no payload is received.#52621

Merged
CarnaViire merged 2 commits into
dotnet:mainfrom
zlatanov:websocket-deflate-bug
May 12, 2021
Merged

Fixed a bug in websocket receive, when a compressed frame with no payload is received.#52621
CarnaViire merged 2 commits into
dotnet:mainfrom
zlatanov:websocket-deflate-bug

Conversation

@zlatanov
Copy link
Copy Markdown
Contributor

@zlatanov zlatanov commented May 11, 2021

There are extremely rare cases where it is possible to receive an empty frame, part of compressed a message.

If such a frame happened to be the last frame of the message, and there was leftover data available in the inflater, the receive
operation would ignore the data and think that there is nothing and return 0 bytes received to the user.

resultHeader.Processed = header.PayloadLength == 0;

if (header.Processed || payloadBuffer.Length == 0)
{
_lastReceiveHeader = header;
return GetReceiveResult<TResult>(
count: 0,
messageType: header.Opcode == MessageOpcode.Text ? WebSocketMessageType.Text : WebSocketMessageType.Binary,
endOfMessage: header.EndOfMessage);
}

One other condition for this case is that the receive buffer is small enough, so the inflate cannot flush everything to the user. This was caught by @BrennanConroy running Autobahn Testsuite with receive buffer size of 4096. The Testsuite sends such frames occasionally to make sure the implementation handles them correctly.

Our internal implementation however cannot produce such frame. When sending an empty compressed frame, the resulting payload will always be 1 byte - a zero.

The fix for this is to treat empty compressed frames as not processed, until inflation has been run.

@BrennanConroy could you try and run your test server with this branch, please?

Fixes #52551

//cc @CarnaViire

…rely marked as processed.

Compressed frames should always go through inflation in order to make sure
there is no lefover from previous receives.
@ghost ghost added the area-System.Net label May 11, 2021
@ghost
Copy link
Copy Markdown

ghost commented May 11, 2021

Tagging subscribers to this area: @dotnet/ncl
See info in area-owners.md if you want to be subscribed.

Issue Details

There are extremely rare cases where it is possible to receive an empty frame, part of compressed a message.

If such a frame happened to be the last frame of the message, and there was lefover data available in the inflater, the receive
operation would ignore the data and think that there is nothing and return 0 bytes received to the user.

resultHeader.Processed = header.PayloadLength == 0;

if (header.Processed || payloadBuffer.Length == 0)
{
_lastReceiveHeader = header;
return GetReceiveResult<TResult>(
count: 0,
messageType: header.Opcode == MessageOpcode.Text ? WebSocketMessageType.Text : WebSocketMessageType.Binary,
endOfMessage: header.EndOfMessage);
}

One other condition for this case is that the receive buffer is small enough, so the inflate cannot flush everything to the user. This was caught by @BrennanConroy running Autobahn Testsuite with receive buffer 4096. The Testsuite sends such frames occasionally to make sure the implementation handles them correctly.

Our internal implementation however cannot produce such frame. When sending an empty compressed frame, the resulting payload will always be 1 byte - a zero. This is why I couldn't write a unit test for this.

The fix for this is to treat empty compressed frames as not processed, until inflation has been run.

@BrennanConroy could you try and run your test server with this branch, please?

Fixes #52551

//cc @CarnaViire

Author: zlatanov
Assignees: -
Labels:

area-System.Net

Milestone: -

@halter73
Copy link
Copy Markdown
Member

Our internal implementation however cannot produce such frame. When sending an empty compressed frame, the resulting payload will always be 1 byte - a zero. This is why I couldn't write a unit test for this.

Can we produce a raw byte stream that simulates a WebSocket with such a frame?

@zlatanov
Copy link
Copy Markdown
Contributor Author

Can we produce a raw byte stream that simulates a WebSocket with such a frame?

I see no reason why not. I will give it a go in a moment.

@zlatanov
Copy link
Copy Markdown
Contributor Author

Thanks for the suggestion @halter73! Created a test and verified that with the implementation before the fix, the test fails.

@BrennanConroy
Copy link
Copy Markdown
Member

All 12.* and 13.* autobahn tests pass now!

@karelz karelz requested a review from CarnaViire May 12, 2021 06:59
@CarnaViire

This comment has been minimized.

@azure-pipelines

This comment has been minimized.

@CarnaViire CarnaViire merged commit 7ecb1eb into dotnet:main May 12, 2021
@zlatanov zlatanov deleted the websocket-deflate-bug branch May 12, 2021 15:38
@karelz karelz added this to the 6.0.0 milestone May 20, 2021
@ghost ghost locked as resolved and limited conversation to collaborators Jun 19, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

WebSocket compression issues when testing server-side with Autobahn

5 participants