Skip to content
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

Websocket connection kept alive when Netty channel was closed by responding to pings #99

Open
hc42 opened this issue Feb 14, 2022 · 3 comments

Comments

@hc42
Copy link

hc42 commented Feb 14, 2022

Hello,

in the Websocket Undertow adoption when an underlying Netty channel is (half) closed it is still possible to receive Websocket messages.
This is a known problem (see https://issues.redhat.com/browse/UNDERTOW-617) and the solution for messages passed on is to drop them in this case (fixed in f145e9c).

However Ping messages are still responded to (See https://github.com/quarkusio/quarkus-http/blob/main/websocket/core/src/main/java/io/undertow/websockets/FrameHandler.java#L139). In this case the Websocket connection is kept alive by the response but the implementation here drops other messages leading to a broken inconsistent state.

To be consistent pings should not be responded to as well.
I would propose a similar private handler method, that uses the same drop message mechanism.
This way the behavior is consistent and a client may even detect broken connections.

@hc42
Copy link
Author

hc42 commented Feb 14, 2022

P.S.One way to reproduce such a broken state is to send a TCP Reset on an established WS connection. This may occur in real live when a firewall, an API gateway or proxy server closes a connection.

@astanik
Copy link

astanik commented Feb 15, 2022

hc42 pushed a commit to hc42/quarkus-http that referenced this issue Feb 15, 2022
Fix for quarkusio#99 where a Websocket ping was responded after session was marked closed and other messages were dropped.

Fix for unhandled exceptions in Websocket channel pipeline. FrameHandler catches them
and passes to onError instead of ignoring and running in default logger / stack trace printing by DefaultChannelPipeline.TailContext.
@hc42
Copy link
Author

hc42 commented Feb 15, 2022

Made a solution proposal in #100

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants