Reactor-netty: onCloseState in WebsocketServerOperations#sendCloseNow should emit 1006 if the session is closed without a close frame

[JamesChenX]

Expected Behavior

onCloseState in reactor.netty.http.server.WebsocketServerOperations#sendCloseNow should emit 1006 (NO_CLOSE_FRAME) instead of -1 if no close frame.

    void sendCloseNow(@Nullable CloseWebSocketFrame frame, ChannelFutureListener listener) {
        if (frame != null && !frame.isFinalFragment()) {
            channel().writeAndFlush(frame);
            return;
        }
        if (CLOSE_SENT.getAndSet(this, 1) == 0) {
            if (frame != null) {
                onCloseState.tryEmitValue(new WebSocketCloseStatus(frame.statusCode(), frame.reasonText()));
                channel().writeAndFlush(frame)
                         .addListener(listener);
            } else {
                onCloseState.tryEmitValue(new WebSocketCloseStatus(-1, "")); // should emit 1006 instead of -1
                channel().writeAndFlush(new CloseWebSocketFrame())
                         .addListener(listener);
            }
        }
        else if (frame != null) {
            frame.release();
        }
    }

Actual Behavior

org.springframework.web.reactive.socket.CloseStatus throw "java.lang.IllegalArgumentException: Invalid status code" because reactor.netty.http.server.WebsocketServerOperations#sendCloseNow tries to create an WebSocketCloseStatus with the status code -1.

Steps to Reproduce

The bug happens when a WebSocket session is closed without a close frame.

Possible Solution

onCloseState in reactor.netty.http.server.WebsocketServerOperations#sendCloseNow should emit 1006 (NO_CLOSE_FRAME) instead of -1 if no close frame.

Your Environment

reactor-netty-http-1.0.1
spring-webflux-5.3.1

[violetagg]

@JamesChenX Is it possible to provide a reproducible example?

[JamesChenX]

I can create a new project to reproduce the bug but it's exaggerated.
The bug can always happen in the scenario when a TCP connection is closed by the peer without the WebSocket close frame and it's easy to understand.

The key point for the issue is: reactor-netty should not create a new WebSocketCloseStatus with the status code "-1" in any scenario because WebFlux will convert the WebSocketCloseStatus instance to CloseStatus while "-1" is an illegal close status code in org.springframework.web.reactive.socket.CloseStatus.

WebsocketServerOperations should use 1006 instead -1 for the disconnection without the WebSocket close frame as mentioned in RFC 6455: The WebSocket Protocol.

  1006 is a reserved value and MUST NOT be set as a status code in a
  Close control frame by an endpoint.  It is designated for use in
  applications expecting a status code to indicate that the
  connection was closed abnormally, e.g., without sending or
  receiving a Close control frame.

package org.springframework.web.reactive.socket;

public final class CloseStatus {
    public CloseStatus(int code, @Nullable String reason) {
        Assert.isTrue((code >= 1000 && code < 5000), "Invalid status code"); // So -1 is an illegal status code
        this.code = code;
        this.reason = reason;
    }
}

[violetagg]

@JamesChenX I was asking because we invoke that method with NULL only in two cases, so most probably your use case is related to onInboundCancel

https://github.com/reactor/reactor-netty/blob/95b7abb684eba610d52b13f75de62b33cacb756d/reactor-netty-http/src/main/java/reactor/netty/http/server/WebsocketServerOperations.java#L185

https://github.com/reactor/reactor-netty/blob/95b7abb684eba610d52b13f75de62b33cacb756d/reactor-netty-http/src/main/java/reactor/netty/http/server/HttpServerOperations.java#L742

[JamesChenX]

I know you want to know why it happened. And I have mentioned the reason: The bug can always happen in the scenario when a TCP connection is closed by the peer without the WebSocket close frame and it's easy to understand.

I can reproduce the bug every time if I want by the following steps:

1. Set up a WebSocket server based on reactor-netty
2. Open a browser to connect it.
3. Then, close the browser forcibly (by Task Manager on Windows) to make sure the TCP is closed without sending the WebSocket close frame.

And the onInboundCancel in WebsocketServerOperations will be triggered. In this case, reactor-netty should create a WebSocket status code with 1006 instead of -1.

BTW: We cannot control the behavior of WebSocket clients because they are running on our customers' device (Android, iOS, etc)

[violetagg]

@JamesChenX In order to proceed with a fix for this issue we need a new version of Netty - https://github.com/netty/netty/pull/10846

[JamesChenX]

Very appreciate your support. I will keep an eye on the issue and test it again once the new version of reactor-netty published. Thanks!

[violetagg]

@JamesChenX PTAL #1416

[violetagg]

@JamesChenX This change is available from 1.0.3-SNAPSHOT and Spring's maven repository https://repo.spring.io/snapshot . If you can test it, it will be great. You need also to update Netty to version 4.1.55.Final.

[JamesChenX]

@violetagg I have tested it with 1.0.3-SNAPSHOT and it works as expected now. Thanks for your support.