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

jetty-10 WebSocket configuration refactor #3660

Conversation

lachlan-roberts
Copy link
Contributor

review of the websocket configuration paths, added improved testing for jetty-websocket configuration which exposed errors in the implementation which have now been fixed

jetty-websocket configuration

  • maxMessageSizes can now be updated by configuring the Session
  • added testing for different client and server configuration paths

refactor of javax WebSocketContainer configuration

  • The JavaxWebSocketServerContainer now shares a configuration instance between the server and the client so that customising the ServerContainer will also apply that configuration to the client which it extends
  • wired up the JavaxWebSocketClientContainer to give its configuration to its CoreClient

Signed-off-by: Lachlan Roberts <lachlan@webtide.com>
the client and server subclasses of JavaxWebSocketContainer now
share a common configuration instance which is used as the default
configuration for both server and client endpoints

to do this a setter was added for the configuration on the CoreClient

Signed-off-by: Lachlan Roberts <lachlan@webtide.com>
correctly detect when a message has exceeded the max size in the sink

forward changes to the input buffer size to the connection in WSChannel

Signed-off-by: Lachlan Roberts <lachlan@webtide.com>
Signed-off-by: Lachlan Roberts <lachlan@webtide.com>
Signed-off-by: Lachlan Roberts <lachlan@webtide.com>
and set the configuration on supplied CoreClient to the Javax client container

Signed-off-by: Lachlan Roberts <lachlan@webtide.com>
@lachlan-roberts lachlan-roberts force-pushed the jetty-10.0.x-configuration-refactor branch from 4cbca1f to c40a303 Compare May 16, 2019 05:51
@lachlan-roberts lachlan-roberts force-pushed the jetty-10.0.x-configuration-refactor branch from a4dc88b to 3271564 Compare May 17, 2019 06:04
@lachlan-roberts lachlan-roberts force-pushed the jetty-10.0.x-configuration-refactor branch from 3271564 to beae4a7 Compare May 17, 2019 06:52
Signed-off-by: Lachlan Roberts <lachlan@webtide.com>
@lachlan-roberts lachlan-roberts force-pushed the jetty-10.0.x-configuration-refactor branch from beae4a7 to 0b41386 Compare May 17, 2019 06:54
configuration moved to ClientUpgradeRequest

Signed-off-by: Lachlan Roberts <lachlan@webtide.com>
Signed-off-by: Lachlan Roberts <lachlan@webtide.com>
@gregw
Copy link
Contributor

gregw commented May 22, 2019

@lachlan-roberts can you fix the merge issue and I'll rereview.

…-configuration-refactor

Signed-off-by: Lachlan Roberts <lachlan@webtide.com>
@lachlan-roberts lachlan-roberts force-pushed the jetty-10.0.x-configuration-refactor branch from 39738a9 to d6c68c4 Compare May 22, 2019 23:03
@gregw
Copy link
Contributor

gregw commented May 23, 2019

@lachlan-roberts so the forced push (grrrrr) was just the merge ??? Now you are a committer there is absolutely no need to force push into branches as you can squash and merge the PR yourself.

@lachlan-roberts
Copy link
Contributor Author

@gregw this force push was before the i got write access

but it was because the first merge i did was missing things, that stopped it from compiling so I just amended it, and it was only on a commit after the review which shouldn't not break the review on github

@lachlan-roberts lachlan-roberts merged commit 49356bb into jetty:jetty-10.0.x May 24, 2019
@lachlan-roberts lachlan-roberts deleted the jetty-10.0.x-configuration-refactor branch July 1, 2019 08:06
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

Successfully merging this pull request may close these issues.

3 participants