search

Code-Sharp / WampSharp

A C# implementation of WAMP (The Web Application Messaging Protocol)
http://wampsharp.net
Other
385 stars 84 forks source link

JTokenMessageParser.Parse exception may cause unhandled exception and crash process #175

Open bigbearzhu opened 7 years ago

bigbearzhu commented 7 years ago

This is the stack trace. The JTokenMessageParser.Parse does capture exception and log it, and, rethrow it. However, the callers to the function don't capture the exception and handle it. So it would end up as unhandled and crash the process. Would you think WebSocket4NetTextConnection.OnMessageReceived should capture exception and swallow it? Same thing happens on WebSocket4NetBinaryConnection.OnDataReceived. Thanks.


   at WampSharp.Newtonsoft.JTokenMessageParser.Parse(String text)
   at WampSharp.WebSocket4Net.WebSocket4NetTextConnection`1.OnMessageReceived(Object sender, MessageReceivedEventArgs e)
   at WebSocket4Net.WebSocket.OnDataReceived(Byte[] data, Int32 offset, Int32 length)
   at SuperSocket.ClientEngine.AsyncTcpSession.ProcessReceive(SocketAsyncEventArgs e)
   at System.Net.Sockets.SocketAsyncEventArgs.OnCompleted(SocketAsyncEventArgs e)
   at System.Threading.ExecutionContext.RunInternal(ExecutionContext executionContext, ContextCallback callback, Object state, Boolean preserveSyncCtx)
   at System.Threading.ExecutionContext.Run(ExecutionContext executionContext, ContextCallback callback, Object state, Boolean preserveSyncCtx)
   at System.Threading.ExecutionContext.Run(ExecutionContext executionContext, ContextCallback callback, Object state)
   at System.Net.Sockets.SocketAsyncEventArgs.FinishOperationSuccess(SocketError socketError, Int32 bytesTransferred, SocketFlags flags)
   at System.Net.Sockets.SocketAsyncEventArgs.CompletionPortCallback(UInt32 errorCode, UInt32 numBytes, NativeOverlapped* nativeOverlapped)
   at System.Threading._IOCompletionCallback.PerformIOCompletionCallback(UInt32 errorCode, UInt32 numBytes, NativeOverlapped* pOVERLAP)
darkl commented 7 years ago

Actually all connections need to terminate upon a serialization error (and a few other errors).

For now we should mimic the JSON/text behavior. I'll try to make serialization exceptions terminate the connection in a future release.

Elad

bigbearzhu commented 7 years ago

You mean swallow it like in MethodInfoSubscriber.InnerEvent?

On 2 Mar 2017, at 5:28 pm, Elad Zelingher notifications@github.com wrote:

Actually all connections need to terminate upon a serialization error (and a few other errors).

For now we should mimic the JSON/text behavior. I'll try to make serialization exceptions terminate the connection in a future release.

Elad

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub, or mute the thread.

darkl commented 7 years ago

I'm sorry, I misread the issue. I thought only the binary version has the issue.

In this case, I think we should catch the exception and close the connection. It would be even better to send a protocol violation error code (1002).

See also here https://github.com/crossbario/autobahn-python/blob/ea4842090a03cf57c09eb0c34359cff4d3cc2b44/autobahn/wamp/websocket.py#L100 .

The only problem is that this requires changes to all WampConnection classes which makes it a harder task.

Elad