[
https://issues.apache.org/jira/browse/TINKERPOP-2717?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17504515#comment-17504515
]
ASF GitHub Bot commented on TINKERPOP-2717:
-------------------------------------------
olivertowers opened a new pull request #1584:
URL: https://github.com/apache/tinkerpop/pull/1584
### Issue:
https://issues.apache.org/jira/browse/TINKERPOP-2717
### Problem:
Per [RFC 6455
WebSocket](https://datatracker.ietf.org/doc/html/rfc6455#section-1.4), either
peer in a websocket an initiate a close handshake by sending a close message
frame. In practical terms, a Gremlin server can send a close message to the
client and the client should notify in-flight requests and acknowledge the
closure.
Currently, Gremln.NET does not check for a `WebSocketMessageType.Close`,
resulting in a `NullReferenceException` when attempting to deserialize an empty
message body.
### Fix:
Change addresses this in the following way:
1. Adds `ConnectionClosedException` that includes CloseStatus and
CloseDescription properties that a WebSocket close message can return.
2. Checks for `WebSocketMessageType.Close` in
WebSocketConnection.ReceiveMessageAsync` and throws the exception.
From there, `Connection.ReceiveMessagesAsync` already handles catching any
exception thrown by the `WebSocketConnection` and will complete the close
handshake and notify all in-flight requests of the exception.
### Additional Changes:
- Added `IClientWebSocket`/`ProxyClientWebSocket` to enable unit testing on
interactions with `System.Net.Websockets.ClientWebSocket`. This included adding
internal property `WebSocketSettings.WebSocketFactoryCallback` so that a mocked
version of `IClientWebSocket` can be injected.
- Added `ConnectionTests` to validate proper handling of close message and
`ConnectionClosedException` propagation to in-flight requests.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
> Gremlin.NET : WebSocketConnection does not check for MessageType.Close,
> causing error InvalidOperationException: "Received data deserialized into
> null object message. Cannot operate on it."
> ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
>
> Key: TINKERPOP-2717
> URL: https://issues.apache.org/jira/browse/TINKERPOP-2717
> Project: TinkerPop
> Issue Type: Bug
> Components: dotnet
> Affects Versions: 3.4.13, 3.5.2
> Reporter: Oliver Towers
> Priority: Minor
>
> h2. Issue:
> If the server sends a valid Close websocket message to the client, the client
> will throw {{InvalidOperationException}} rather than acknowledge/cleanup the
> connection gracefully.
> See relevant point where exception is thrown.
> [here|https://github.com/apache/tinkerpop/blob/7835e7a2c3b19a14f196525da844a2e35ba5a105/gremlin-dotnet/src/Gremlin.Net/Driver/Connection.cs#L123]
> Close messages from the server can be expected when the server is doing a
> graceful shutdown, and clients would be expected to reconnect.
> h2. Details:
> A Close websocket message being received by ClientWebSocket,
> [WebSocketReceiveResult.MessageType|https://docs.microsoft.com/en-us/dotnet/api/system.net.websockets.websocketreceiveresult.messagetype?view=net-6.0]
> will return {{MessageType.Close}}. In this case, the message buffer will be
> zero-bytes, see these
> [remarks|https://docs.microsoft.com/en-us/dotnet/api/system.net.websockets.websocketreceiveresult.count?view=net-6.0#remarks].
> Currently, Gremlin.NET does not check for this message type in
> [WebSocketConnection.ReceiveMessageAsync|https://github.com/apache/tinkerpop/blob/7835e7a2c3b19a14f196525da844a2e35ba5a105/gremlin-dotnet/src/Gremlin.Net/Driver/WebSocketConnection.cs#L92]
> and return a zero-length buffer. In
> [Connection.HandleReceiveAsync|https://github.com/apache/tinkerpop/blob/7835e7a2c3b19a14f196525da844a2e35ba5a105/gremlin-dotnet/src/Gremlin.Net/Driver/Connection.cs#L118],
> the attempt to deserialize the empty buffer returns `null` and the check
> fails.
> h2. Potential Fix
> # If `MessageType == Close` Throw a new connection closed exception from
> {{WebSocketConnection.ReceiveMessageAsync}} which includes
> {{WebSocketReceiveResult.CloseStatus}} and
> {{WebSocketReceiveResult.CloseDescription}} in the exception message and as
> properties.
> # This will be caught in {{Connection.HandleRecieveAsync}} and treated as
> fatal and will call {{CloseConnectionBecauseOfFailureAsync}} and will handle
> completing the close handshake, and notifies pending requests on the
> connection with the closed connection exception.
--
This message was sent by Atlassian Jira
(v8.20.1#820001)
