nats.java
nats.java copied to clipboard
Websocket support
This is the simplest possible implementation of websockets with a net increase in code coverage.
- No alterations to DataPort.
- Two new options, one to support httpInterceptors and another to support proxy requests.
- Websockets is supported via a new schema ("ws" or "wss").
- Copy writer headers are at the top of every new file added.
- All websocket implementation details are encapsulated by extending Socket and implementing WebSocket with a WebSocketInputStream and WebSocketOutputStream.
I'm planning to use these changes in our production environment within the next few weeks.
Being critical of myself, these are the points that are lacking:
- Proper websocket closure handshake is not implemented, close() simply forwards to the underlying socket.
- The http interceptors don't allow filtering HTTP responses and are very primitive. They work for my use case, but not sure if it works for all use-cases.
- The handling of upgradeToSecure is a little awkward since websockets is typically performed over a TLS channel and thus there is no need for double encryption. A better solution would involve delegating this to the DataPort... or more specifically, when the secure() option is set, we should NOT throw an error if "wss" transport is used.
- Create URI for server now takes in an "is websocket" parameter, again this is a little awkward. A better solution would involve delegating this to the DataPort.
- No support for
Sec-WebSocket-Extensions: permessage-deflate(aka websocket level compression... which should really be combined withNats-No-Masking: TRUEto really get the full benefits). - The implementation of websocket framing strikes a balance between TCP fragmentation and extra buffer copying by using a 1440 sized buffer used for the initial frame header + the first ~1438 bytes. The channel based implementation mitigates this fragmentation by using the GatheringByteChannel and passing in an array of ByteBuffers.
Note that this PR replaces https://github.com/nats-io/nats.java/pull/426 which was posted before I had access to the nats-io/nats.java repository (and for some reason that other PR isn't working with coveralls).
For more information on websockets, I'd really recommend reading the RFC: https://datatracker.ietf.org/doc/html/rfc6455
A 4x more complex PR which involves a DataPort refactor was posted here:
- DataPort refactor: https://github.com/nats-io/nats.java/pull/494
- ByteBuffer based websocket support: https://github.com/nats-io/nats.java/pull/502
I've closed out the 4x more complex PRs, since the guiding principle that @scottf suggests is simplicity... and this is as simple as it gets.
The only issue with this so far is in regards to using proxies that require authentication. If a proxy requires authentication, you can call Authenticator.setDefault(), and override the getPasswordAuthentication() to return the username and password to use in the proxy.
However, this is also effectively a global variable, and thus impacts any usage of Proxy global to the JVM.
To mitigate this, we could define an option used to construct a new Socket object (effectively a socket factory). If that option was available, one could define a socket factory that creates a TCP socket and then connects to the proxy using CONNECT and the appropriate Authorization header. Note that the "real" hostname + port that we are trying to connect to would need to be provided as inputs to this socket factory.
Tried to review this. Checked out the branch, and the WebSocket tests were failing. There were 13 WebSocket tests that are failing.
Hi @RichardHightower what tests where failing? Last time I checked out the branch the tests succeeded, although some of the tests are unreliable (not due to the websocket code, just in general some of the tests have been unreliable in the past... and I even put together some PRs in the past to fix those).

@brimworks We are revisiting this as a review of open PR's with some work we are considering doing to add in Vert-x. I though your interface changes might be re-usable or at least a roadmap. I will try to get to the conflicts, they look to be relatively minor.
Closing this in favor of https://github.com/nats-io/nats.java/pull/826
...as that seems to be the path forward. Thanks for moving this along!