-
Notifications
You must be signed in to change notification settings - Fork 7
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[Fix] WebSocket Close Bugs #11
Conversation
Is it feasible to also get the changes upstream in |
I can make a PR on their repository with these changes. I think making the changes here would be the fastest solution in the interim. Update: I have submitted two PRs for these fixes upstream. rsocket/rsocket-js#278 and rsocket/rsocket-js#279 |
…. Closing a Duplex stream will close raw socket
Overview
Recent alerts have shown uncaught exceptions for clients using the WebSockets connection method. Two main issues were identified.
Write to closed WebSocket
In some circumstances the WebSocket might close unexpectedly. If the server tried to write data to this port an unhandled exception would be thrown.
An investigation led to this issue websockets/ws#1515.
This PR adds better error and closed socket handling to the
WebSocketDuplexConnection
andWebSocketServerTransport
implementations provided by RSocket.A unit test has been added to ensure no unhandled exceptions are thrown when the client closes the WebSocket.
Undefined frames
Some reports of the server failing to handle WebSocket frames were received.
This was reproducible by sending invalid data (not a valid RSocket frame buffer) over a WebSocket. Although not verified: this might be caused by a broken
Buffer
implementation on the client side which could happen in environments such as React Native.Additional guards have been added to the
WebSocketDuplexConnection
frame handling. Unit tests have also been added for invalid frames.Double close warning
Previously a double close warning for the RSocket server could be seen in logs. Currently we use a Fastify server which a WebSocket server manages
upgrade
events on. The standard RSocketWebSocketServerTransport
registers a listener for theclose
event of the WebSocketServer and calls theServerCloseable
close
wherein the server is again closed. This behaviour has been removed as the WebSocket server's closing is tied to the Fastify server'sclose
event.Additional Fixes
This PR includes some additional bug fixes:
path
references for theserver
project..probes
folder forservice
folder