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

Added configurable maxFramePayloadLength to ReactorNettyWebSocketClient #22367

Closed

Conversation

envious
Copy link
Contributor

@envious envious commented Feb 6, 2019

Currently the ReactorNettyWebSocketClient (RNWSC) doesn't allow for a configurable maxFramePayloadLength. The HttpClient used by the RNWSC allows for a configurable maxFramePayloadLength however, the method .websocket() used by the RNWSC ignores whatever is set by the HttpClient and uses a default value of 65536. As shown below:

ReactorNettyWebSocketClient invokes .websocket()

.websocket(StringUtils.collectionToCommaDelimitedString(handler.getSubProtocols()))

The method invoked on the HttpClient ignores the value you may have set in your HttpClient

public final WebsocketSender websocket(String subprotocols) {
	return websocket(subprotocols, 65536);
}

I've solved the issue by using the overloaded .websocket() method on the HttpClient which takes a maxFramePayloadLength.

.websocket(StringUtils.collectionToCommaDelimitedString(handler.getSubProtocols()), 
getMaxFramePayloadLength())

The maxFramePayloadLength is now configurable in the RNWSC.

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged or decided on label Feb 6, 2019
@rstoyanchev rstoyanchev added in: web Issues in web modules (web, webmvc, webflux, websocket) type: enhancement A general enhancement and removed status: waiting-for-triage An issue we've not yet triaged or decided on labels Feb 8, 2019
@rstoyanchev rstoyanchev added this to the 5.2 M1 milestone Feb 8, 2019
@philsttr
Copy link

I noticed that this PR did not change ReactorNettyWebSocketClient to call the constructor for ReactorNettyWebSocketSession that takes the maxFramePayloadLength

It seems odd that the ReactorNettyWebSocketSession.maxFramePayloadLength is not set on the client side, but is set on the server-side. i.e. ReactorNettyWebSocketSession.maxFramePayloadLength will keep the default value (64k) on the client side regardless of how ReactorNettyWebSocketClient.maxFramePayloadLength is configured.

Is this intended?

Will something bad happen if ReactorNettyWebSocketClient.maxFramePayloadLength has a different value than ReactorNettyWebSocketSession.maxFramePayloadLength ?

@rstoyanchev
Copy link
Contributor

Good catch @philsttr, thanks.

@eliquinox
Copy link

Is it possible to use this feature now or do we have to wait for the milestone?

@benitocm

This comment has been minimized.

@snicoll

This comment has been minimized.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: web Issues in web modules (web, webmvc, webflux, websocket) type: enhancement A general enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants