Bug 32575 - Bugs and suggestions for improving System.Net.ClientWebSocket
Summary: Bugs and suggestions for improving System.Net.ClientWebSocket
Status: RESOLVED FIXED
Alias: None
Product: Class Libraries
Classification: Mono
Component: System ()
Version: master
Hardware: All All
: --- normal
Target Milestone: Untriaged
Assignee: Bugzilla
URL:
Depends on:
Blocks:
 
Reported: 2015-07-29 10:19 UTC by Jahmai
Modified: 2017-07-27 12:28 UTC (History)
8 users (show)

Tags:
Is this bug a regression?: ---
Last known good build:


Attachments
Debug inspector showing incorrectly parsed incoming message (85.19 KB, image/png)
2016-02-09 11:26 UTC, Adam
Details


Notice (2018-05-24): bugzilla.xamarin.com is now in read-only mode.

Please join us on Visual Studio Developer Community and in the Xamarin and Mono organizations on GitHub to continue tracking issues. Bugzilla will remain available for reference in read-only mode. We will continue to work on open Bugzilla bugs, copy them to the new locations as needed for follow-up, and add the new items under Related Links.

Our sincere thanks to everyone who has contributed on this bug tracker over the years. Thanks also for your understanding as we make these adjustments and improvements for the future.


Please create a new report on GitHub or Developer Community with your current version information, steps to reproduce, and relevant error messages or log files if you are hitting an issue that looks similar to this resolved bug and you do not yet see a matching new report.

Related Links:
Status:
RESOLVED FIXED

Description Jahmai 2015-07-29 10:19:34 UTC
After attempting to use the ClientWebSocket class on iOS/Android I was stumbled with so many issues that I ended up needing to re-write the class myself to fix the issues, and just use reflection hacks to access the internal stuff of HttpWebRequest and WebConnection classes.

I'm referring to these classes:

https://github.com/mono/mono/blob/master/mcs/class/System/System.Net.WebSockets/ClientWebSocket.cs
https://github.com/mono/mono/blob/master/mcs/class/System/System.Net/WebConnection.cs

Here are the issues I faced:

- ClientWebSocket.ReceiveAsync and ClientWebSocket.SendAsync both use Task.Run to call synchronous methods on WebConnection when WebConnection offers async (Begin/End) methods that would do the job just fine. This causes unnecessary spinning up of Threadpool threads instead of using IO completion routines.

- WebConnection.BeginRead sometimes returns an instance of IAsyncResult who's concrete type is WebAsyncResult, depending on if chunking reads are enabled, but WebConnection.EndRead assumes WebAsyncResult is always passed in. This causes InvalidCastException's when calling EndRead when chunked reads are not enabled.

- ClientWebSocket.ReceiveAsync uses WebConnection.Read to get input which goes via the NetworkStream / SslStream inside WebConnection, but ClientWebSocket.SendAsync directly uses the raw Socket from WebConnection. This means that if SSL/TLS is enabled on the socket (e.g. for all wss:// connections) then writing raw unencrypted data to the socket will break the connection.

- ClientWebSocket.ReceiveAsync doesn't handle the "Ping" and "Pong" op-codes, and instead maps them to a "Close" message type resulting in the connection being disconnected. Since both "Ping" and "Pong" can be sent by the server unsolicited, the WebSocket connection will reliably drop whenever a keep alive Ping/Pong are sent by the server which is generally every 30 seconds or so.

I have no intention of writing any tests to assert these issues. They are too numerous and setting up a test project that hosts a secure websocket server isn't something I have time to do.

As I have worked around each of these issues already, this is more of a courtesy to the Mono team to take a serious look at just how broken this class is.
Comment 1 Nicholas Ventimiglia 2015-08-17 14:11:03 UTC
I would like to second this. I was filing a similar bug report when I found this one. 

I am the developer of Websocket.Portable on Github / nuget and the Realtime.co websocket platfrom api for Xamarin. Currently the ClientWebSocket is sending close messages at incorrect times. Moreover, I have identical code running on standard dotnet which runs fine.

Websocket.Portable :
https://github.com/NVentimiglia/WebSocket.Portable

Realtime Xamarin 
https://github.com/NVentimiglia/Realtime.Xamarin

To replicate you should :

- Download Realtime.Xamarin
https://accounts.realtime.co/signup/

(Sorry my bug relates to chunking, and its easier to just test against a free hosted service)

- Acquire a realtime application key

-Add an application key here (free)
 https://github.com/NVentimiglia/Realtime.Xamarin/blob/master/Samples/Xamarin/Realtime.Xamarin/MainView.xaml.cs#L40

- Replace the Message with a call to BigChunk()
https://github.com/NVentimiglia/Realtime.Xamarin/blob/master/Samples/Xamarin/Realtime.Xamarin/MainView.xaml.cs#L394


- Run the application (I debug on Droid)

Explanation :

When sending numerous large messages the WebSocketClient will issue a 'close frame' and end the websocket connection. This sucks.

I have identical code using non-mono (which works) here:

https://github.com/NVentimiglia/Realtime.Xamarin/tree/master/Tests/RealtimeDotNetTests
Comment 2 Adam 2016-02-09 11:25:37 UTC
I third this!
I have just been porting the WebSocket transport for SignalR to Xamarin, and I'm almost there, but incoming messages just get chewed up. See the screenshot that I attached.
You can see that on multi-part messages, MessageType is set to "Closed" when it should be "Text", and the text content is dumped into the "CloseStatusDescription" field of the receive result! Argh!!

My code works perfectly on MS .net.
Comment 3 Adam 2016-02-09 11:26:29 UTC
Created attachment 14958 [details]
Debug inspector showing incorrectly parsed incoming message
Comment 4 Adam 2016-02-10 01:22:22 UTC
I've had some feedback from Xamarin. They have said that they're aware that the WebSocket implementation is broken - and is more of a 'proof of concept' than a 'full implementation'.
Personally I think proof of concept code should be provided as source only, and not distributed in the framework for us dev's to stumble painfully across... but there it is. Don't expect a fix any time soon as it doesn't appear to be a priority.

On the flip-side, I got my SignalR Websocket transport working using WebSocket-Sharp. Yay.
Comment 5 DigitalNut 2016-09-04 00:15:57 UTC
I can provide sample code that show 2 issues.

I also entered this info here (Xamarin Cross Platform form)

http://forums.xamarin.com/categories/cross-platform

1) End Of Message not being handled correctly

2) Websocket Ping / Pong (Hearbeat) not being handled correctly

The sample code is here. 1 project is a simple WebSocket Android app and the other is a console WebSocket Server to demonstrate these two issues. Note to see issue #2 you must make sure to set the EndOfMessage flag to true.

This is blocking my Android app...I may have to try to use another library.
Comment 6 DigitalNut 2016-09-04 00:17:14 UTC
Oops, forgot to link my GitHub sample code for comment #5:

https://github.com/DigitalNut/MonoWebSocketIssue
Comment 7 Marek Safar 2017-04-03 12:47:19 UTC
Should be fixed in master
Comment 8 Eric Liu 2017-06-09 03:04:07 UTC
"ClientWebSocket.ReceiveAsync uses WebConnection.Read to get input which goes via the NetworkStream / SslStream inside WebConnection, but ClientWebSocket.SendAsync directly uses the raw Socket from WebConnection. This means that if SSL/TLS is enabled on the socket (e.g. for all wss:// connections) then writing raw unencrypted data to the socket will break the connection."

this still representing for me on MonoAndroid 7.1 (mono 5.0.1.1, xamarin 4.5.0.481)

Scenario:
SSL Connection -> Connect -> SendAsync -> ReceiveAsync -/> Exception

System.ObjectDisposedException: Cannot access a disposed object.
Object name: 'System.Net.Sockets.NetworkStream'.
  at System.Net.WebConnection.Read (System.Net.HttpWebRequest request, System.Byte[] buffer, System.Int32 offset, System.Int32 size) [0x0002b] in <a547bd0d78184f26ab08d022f013c1e1>:0 
  at System.Net.WebSockets.ClientWebSocket+<>c__DisplayClass35_0.<ReceiveAsync>b__0 () [0x0016b] in <a547bd0d78184f26ab08d022f013c1e1>:0 
  at System.Threading.Tasks.Task`1[TResult].InnerInvoke () [0x0000f] in <3fd174ff54b146228c505f23cf75ce71>:0 
  at System.Threading.Tasks.Task.Execute () [0x00010] in <3fd174ff54b146228c505f23cf75ce71>:0
Comment 9 Eric Liu 2017-06-10 17:56:33 UTC
I ported the corefx implementation of ClientWebSocket and that's working fine. (tested on Android and desktop console)

Will mono adopt the corefx implementation in the future?
Comment 10 Marek Safar 2017-06-11 06:48:21 UTC
Mono is using CoreFX WebSocket's already
Comment 11 Ammar Mheir 2017-07-15 01:12:33 UTC
We seem to still be experiencing this issue in a Xamarin.iOS application. It has been pointed that it seems to be occurring at this specific line of code:
https://github.com/ably/ably-dotnet/blob/dotnet-standard/src/IO.Ably.Shared/Transport/MsWebSocketConnection.cs#L135

Additionally, the below is the error message we’re observing:
>ObjectDisposedExceptionCannot access a disposed object. Object name: 'System.Net.Sockets.Socket'.
>Raw
>null
>System.ObjectDisposedExceptionCannot access a disposed object. Object name: 'System.Net.Sockets.Socket'.
>System.Net.Sockets.Socket.ThrowIfDisposedAndClosed()
>System.Net.Sockets.Socket.Send(byte[] buffer, int offset, int size, SocketFlags socketFlags, ref SocketError errorCode)
>System.Net.Sockets.Socket.Send(byte[] buffer, int offset, int size, SocketFlags socketFlags)
>System.Net.WebSockets.ClientWebSocket.<>c__DisplayClass28_0.<SendAsync>b__0()
>System.Threading.Tasks.Task.InnerInvoke()
>System.Threading.Tasks.Task.Execute()
>System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
>System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess(Task task)
>System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
>System.Runtime.CompilerServices.TaskAwaiter.ValidateEnd(Task task)
>System.Runtime.CompilerServices.TaskAwaiter.GetResult()
>at IO.Ably.Transport.MsWebSocketConnection+<<StartSenderQueueConsumer>b__17_0>d.MoveNext () <0x1015def00 + 0x00267> in <c4e321e6fb8e41ab8e5d206c4e805416#dcddafa70bae31ffa9f620e4fc2a0a0b>:0

#Version Informaiton

Visual Studio Enterprise 2017 for Mac
Version 7.0.1 (build 24)
Installation UUID: 5dd7b690-fda1-4097-af00-2c233739dd55
Runtime:
	Mono 5.0.1.1 (2017-02/5077205) (64-bit)
	GTK+ 2.24.23 (Raleigh theme)

	Package version: 500010001

Apple Developer Tools
Xcode 8.3.2 (12175)
Build 8E2002

Xamarin.iOS
Version: 10.10.0.36 (Visual Studio Enterprise)
Hash: d2270eec
Branch: d15-2
Build date: 2017-05-22 16:30:53-0400


Operating System
Mac OS X 10.12.3
Darwin 16.4.0 Darwin Kernel Version 16.4.0
    Thu Dec 22 22:53:21 PST 2016
    root:xnu-3789.41.3~3/RELEASE_X86_64 x86_64
Comment 12 Marek Safar 2017-07-27 12:28:39 UTC
This is different issue. Even if you can see WebSockets on the stack the issue look to be in Socket class. If you have a repro we can use to track this down, please fill a new bug report.