WampSharp icon indicating copy to clipboard operation
WampSharp copied to clipboard

JTokenMessageParser.Parse exception may cause unhandled exception and crash process

Open bigbearzhu opened this issue 7 years ago • 3 comments

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)

bigbearzhu avatar Mar 02 '17 06:03 bigbearzhu

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

darkl avatar Mar 02 '17 07:03 darkl

You mean swallow it like in MethodInfoSubscriber.InnerEvent?

On 2 Mar 2017, at 5:28 pm, Elad Zelingher [email protected] 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.

bigbearzhu avatar Mar 02 '17 21:03 bigbearzhu

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

darkl avatar Mar 02 '17 22:03 darkl