Spring WebSocketMessage's methods fromMessage and toFrames ignore Netty WebSocketFrame metadata

Affects: LATEST (since ReactorNettyWebSocketSession introduced)

Expected Behavior

Add metadata property to the WebSocketMessage POJO. And by-pass Netty WebSocketFrame metadata in ReactorNettyWebSocketSession implementation.

Motivation

To allow proxy compressed WebSocket frames through Spring Cloud Gateway metadata of the WebSocketFrame (rsv and finalFragment) should be processed. This will open the possibility to proxy frames "as is" and avoid unwanted decompress\compress cycles inside the proxy service.

This will be also useful for the Undertow implementation soon since the migration to Netty was announced(http://undertow.io/blog/2019/04/15/Undertow-3.html) Metadata can be useful for other implementation in the future.

Possible Solution

Since internal Spring WebSocketMessage is the generic POJO for the multiple servers I'd propose adding some metadata to the WebSocketMessage which and use it for any metadata needs to be passed through the toMessage and toFrame methods.

For the specific ReactorNettyWebSocketSession implementation it may looks like:

//avoided constants similar extractions
    protected WebSocketMessage toMessage(WebSocketFrame frame) {
        DataBuffer payload = bufferFactory().wrap(frame.content());

        return new WebSocketMessage(messageTypes.get(frame.getClass()), payload,extractFrameMetadata(frame));
    }

    private void extractFrameMetadata(WebSocketFrame frame) {
        Map<String,Object> frameMetadata = new HashMap<>();
        frameMetadata.put("rsv",frame.rsv());
        frameMetadata.put("finalFragment", frame.isFinalFragment());
    }

    protected WebSocketFrame toFrame(WebSocketMessage message) {
        ByteBuf byteBuf = NettyDataBufferFactory.toByteBuf(message.getPayload());
        if (WebSocketMessage.Type.TEXT.equals(message.getType())) {
            return new TextWebSocketFrame(byteBuf,message.getMetadataEntry("rsv"), message.getMetadataEntry("finalFragment"));
        }
        else if (WebSocketMessage.Type.BINARY.equals(message.getType())) {
            return new BinaryWebSocketFrame(byteBuf,message.getMetadataEntry("rsv"), message.getMetadataEntry("finalFragment"));
        }
        else if (WebSocketMessage.Type.PING.equals(message.getType())) {
            return new PingWebSocketFrame(byteBuf,message.getMetadataEntry("rsv"), message.getMetadataEntry("finalFragment"));
        }
        else if (WebSocketMessage.Type.PONG.equals(message.getType())) {
            return new PongWebSocketFrame(byteBuf,message.getMetadataEntry("rsv"), message.getMetadataEntry("finalFragment"));
        }
        else {
            throw new IllegalArgumentException("Unexpected message type: " + message.getType());
        }
    }

Comment From: rstoyanchev

We could pass the entire frame into WebSocketMessage and expose it like so:

public Object getNativeMessage() { ... }

Comment From: Fetsivalen

Hi @rstoyanchev , First of all thanks for your response. It may be good in some cases to pass entire frame but from other perspective it can be, for example, a room to unefficient body consumption, or some other unefficient operations with frame data. Usually I prefer a wat which is used in netty/reactor netty projects, which allow to perform potentially unefficient operations only through their helper methods, and that's why i proposed to bypass relevant metadata only here. In this case in seems that frame do have only body and "metadata", (rsv and finalFragment properties) WDYT? In any case passing complete frame will solve my case completelly as well.

Comment From: rstoyanchev

Spring's WebSocketMessage already exposes the ByteBuf content of the Netty WebSocketFrame. So in regards to body consumption, I don't see anything that changes. It's more about providing access to the rest of what's in the WebSocketFrame in a way that is simple (without any indirection) and is also future proof in case new fields are exposed.

Comment From: Fetsivalen

Hi @rstoyanchev
Unfortunately, you've fixed only half of the original issue. since the method - toFrame does not get the data from the original message and rsv and isFinalFragment fragment is still missing when user uses toFrame method through WebSocketClient. and also in Spring Cloud Gateway (where I originally discovered this issue), the original problem will be still actual.

Comment From: rstoyanchev

So it should be something like this then?

protected WebSocketFrame toFrame(WebSocketMessage message) {
    if (message.getNativeMessage() != null) {
        return message.getNativeMessage();
    }
    // the rest of toFrame...
}

As for Spring Cloud Gateway I think that would have to be changed there in a similar way I think.