dotnet / runtime

.NET is a cross-platform runtime for cloud, mobile, desktop, and IoT apps.
https://docs.microsoft.com/dotnet/core/
MIT License
14.56k stars 4.55k forks source link

ClientWebSocket closes with Aborted exception when payload contains 0xFF characters #54360

Open tvone-timmoore opened 3 years ago

tvone-timmoore commented 3 years ago

We have a simple text based CLI over WebSockets and sending a command results in multiple packets being returned. It appears that when awaiting ClientWebSocket.ReceiveAsync packets that have 0xFF's in the payload are causing an WebSocketExecption Aborted exception.

Example code as not doing any special: 

               ArraySegment<byte> _buffer = new(new byte[2048]);
               var ws = new ClientWebSocket();

               await ws.ConnectAsync(uri, CancellationToken.None);

               byte[] data = Encoding.UTF8.GetBytes(line + delimiter);
               await ws.SendAsync(data, WebSocketMessageType.Text, true, CancellationToken.None);

               var result = await ws.ReceiveAsync(_buffer, CancellationToken.None);

From WireShark

Working packet:

Value = Slot2.Carddata.SubNo = 0000000000000\r\n

0000   90 b1 1c 7a 90 f2 00 16 9e dc 09 10 08 00 45 00   ...z..........E.
0010   00 50 c5 e7 00 00 40 06 32 67 ac 10 15 0e ac 10   .P....@.2g......
0020   15 2b 00 50 e7 63 00 98 ab 04 9e 44 79 a6 50 18   .+.P.c.....Dy.P.
0030   39 08 e3 8b 00 00 81 26 53 6c 6f 74 32 2e 43 61   9......&Slot2.Ca
0040   72 64 64 61 74 61 2e 53 75 62 4e 6f 20 3d 20 30   rddata.SubNo = 0
0050   30 30 30 30 30 30 30 30 30 30 30 30 0d 0a         000000000000..

Packet causing ClientWebSocket to throw WebSocketExecption

Value = Slot3.Carddata.SubNo = �������������\r\n

0000   90 b1 1c 7a 90 f2 00 16 9e dc 09 10 08 00 45 00   ...z..........E.
0010   00 50 c5 f4 00 00 40 06 32 5a ac 10 15 0e ac 10   .P....@.2Z......
0020   15 2b 00 50 e7 63 00 98 ac af 9e 44 79 c9 50 18   .+.P.c.....Dy.P.
0030   39 08 01 10 00 00 81 26 53 6c 6f 74 33 2e 43 61   9......&Slot3.Ca
0040   72 64 64 61 74 61 2e 53 75 62 4e 6f 20 3d 20 ff   rddata.SubNo = .
0050   ff ff ff ff ff ff ff ff ff ff ff ff 0d 0a         ..............

NOTE: The remaining packets after this one have been sent ok and I can see them in the WireShark.

Configuration

.NET 5 WPF application Windows 10 64bit - Version 10.0.19043 Build 19043 Microsoft Visual Studio Enterprise 2019 - Version 16.10.2

.NET SDK (reflecting any global.json): Version: 5.0.301 Commit: ef17233f86

Runtime Environment: OS Name: Windows OS Version: 10.0.19043 OS Platform: Windows RID: win10-x64 Base Path: C:\Program Files\dotnet\sdk\5.0.301\

Host (useful for support): Version: 5.0.7 Commit: 556582d964

StackTrace:

System.Net.WebSockets.WebSocketException
  HResult=0x80004005
  Message=An exception caused the WebSocket to enter the Aborted state. Please see the InnerException, if present, for more details.
  Source=System.Net.WebSockets
  StackTrace:
   at System.Net.WebSockets.ManagedWebSocket.<CloseWithReceiveErrorAndThrowAsync>d__71.MoveNext()

Other information

WebSocketSharp-core works correctly and does not abort with same payload.

Additional issue

Why Oh Why has ClientWebSocket been sealed it meant that I could not try fix this by pre-processing the data before it was processed by ClientWebSocket handlers!

ghost commented 3 years ago

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

Issue Details
When awaiting ClientWebSocket.ReceiveAsync packets that have 0xFF appear to be causing a Aborted exception. ``` ArraySegment _buffer = new(new byte[2048]); var ws = new ClientWebSocket(); await ws.ConnectAsync(uri, CancellationToken.None); var result = await _wsa.ReceiveAsync(_buffer, CancellationToken.None); ``` From WireShark Working packet: ``` Value = Slot2.Carddata.SubNo = 0000000000000\r\n 0000 90 b1 1c 7a 90 f2 00 16 9e dc 09 10 08 00 45 00 ...z..........E. 0010 00 50 c5 e7 00 00 40 06 32 67 ac 10 15 0e ac 10 .P....@.2g...... 0020 15 2b 00 50 e7 63 00 98 ab 04 9e 44 79 a6 50 18 .+.P.c.....Dy.P. 0030 39 08 e3 8b 00 00 81 26 53 6c 6f 74 32 2e 43 61 9......&Slot2.Ca 0040 72 64 64 61 74 61 2e 53 75 62 4e 6f 20 3d 20 30 rddata.SubNo = 0 0050 30 30 30 30 30 30 30 30 30 30 30 30 0d 0a 000000000000.. ``` Packet causing ClientWebSocket to throw WebSocketExecption ``` Value = Slot3.Carddata.SubNo = �������������\r\n 0000 90 b1 1c 7a 90 f2 00 16 9e dc 09 10 08 00 45 00 ...z..........E. 0010 00 50 c5 f4 00 00 40 06 32 5a ac 10 15 0e ac 10 .P....@.2Z...... 0020 15 2b 00 50 e7 63 00 98 ac af 9e 44 79 c9 50 18 .+.P.c.....Dy.P. 0030 39 08 01 10 00 00 81 26 53 6c 6f 74 33 2e 43 61 9......&Slot3.Ca 0040 72 64 64 61 74 61 2e 53 75 62 4e 6f 20 3d 20 ff rddata.SubNo = . 0050 ff ff ff ff ff ff ff ff ff ff ff ff 0d 0a .............. ``` **NOTE**: The remaining packets after this one have been sent ok and I can see them in the WireShark. ### Configuration .NET 5 WPF application Windows 10 64bit - Version 10.0.19043 Build 19043 Microsoft Visual Studio Enterprise 2019 - Version 16.10.2 .NET SDK (reflecting any global.json): Version: 5.0.301 Commit: ef17233f86 Runtime Environment: OS Name: Windows OS Version: 10.0.19043 OS Platform: Windows RID: win10-x64 Base Path: C:\Program Files\dotnet\sdk\5.0.301\ Host (useful for support): Version: 5.0.7 Commit: 556582d964 ###StackTrace: ``` System.Net.WebSockets.WebSocketException HResult=0x80004005 Message=An exception caused the WebSocket to enter the Aborted state. Please see the InnerException, if present, for more details. Source=System.Net.WebSockets StackTrace: at System.Net.WebSockets.ManagedWebSocket.d__71.MoveNext() ``` ### Other information WebSocketSharp-core **works correctly** and does not abort with same payload. ###Additional issue Why Oh Why has ClientWebSocket been sealed it meant that I could not try fix this by pre-processing the data before it was processed by ClientWebSocket handlers!
Author: tvone-timmoore
Assignees: -
Labels: `area-System.Net`, `untriaged`
Milestone: -
CarnaViire commented 3 years ago

That's because the data is sent with WebSocketMessageType.Text, for which we check UTF-8 validity. 0xFF is not a valid UTF-8 character. If you need to send 0xFF for some reason, you can do so with WebSocketMessageType.Binary

tvone-timmoore commented 3 years ago

Thanks CarnaVire, I thought as well that it might be a internal check but didn't want to jump to any conclusions.

We can't change or send as Binary as the data being returned from devices out in the field that have an unset value. It is still text but ASCII in some cases. We have to deal with older devices and code that is never going to be updated.

If that is the case then I believe the internal UTF-8 check should be optional, or is not required as:

  1. You don't know what kind of text is being sent could be ASCII, UTF-16, or something else encoded payload, ideally UTF-8. I am aware what the WebSocket protocol rfc6455 says about text encoding.
  2. Bytes are returned from the ReceiveAsync where I then have to do the UTF-8 decoding.
  3. None of the other WebSocket implementations enforce internal UTF-8 checks that I have looked at anyway.

At present I have to use WebSocketSharp-core as this works ok, I would like to use the native versions where possible. We are migrating our apps to .NET 5 then to multiple platforms with Uno and found this when trying to use the native ClientWebSocket.

stephentoub commented 3 years ago

the internal UTF-8 check should be optional, or is not required

It's actually required by the websocket specification: https://datatracker.ietf.org/doc/html/rfc6455

   Text

      The "Payload data" is text data encoded as UTF-8.  Note that a
      particular text frame might include a partial UTF-8 sequence;
      however, the whole message MUST contain valid UTF-8.  Invalid
      UTF-8 in reassembled messages is handled as described in
      Section 8.1.
8.1.  Handling Errors in UTF-8-Encoded Data

   When an endpoint is to interpret a byte stream as UTF-8 but finds
   that the byte stream is not, in fact, a valid UTF-8 stream, that
   endpoint MUST _Fail the WebSocket Connection_.  This rule applies
   both during the opening handshake and during subsequent data
   exchange.
tvone-timmoore commented 3 years ago

Thanks stephentoub, yes I've read the spec :) fun times, but that is not helpful in the real world, which is why I believe if the check could be made optional then that would be very helpful.

Rightly or wrongly other WebSocket implementations do not appear to be enforcing this check.

tvone-timmoore commented 3 years ago

I suppose it depends on who does the checks, as I have to decode the bytes with UTF-8 then it's kind of saying that I should be doing the check and closing the connection?

karelz commented 3 years ago

Triage: Might be a good idea to enable interop. It would be nice to see if there is higher demand for such feature / option.