Extract src/websockets from Polykey to js-ws

[amydevs]

Description

This PR moves src/websockets from https://github.com/MatrixAI/Polykey to this repository.
It also implements backpressure and stream multiplexing according to the spec in #2.

Issues Fixed

• Fixes Extract src/websockets from Polykey to js-ws #1
• Fixed Support stream multiplexing for websockets #2

Tasks

• 1. Move source from src/websockets
• 2. Separate WebSocketConnection and WebSocketStream out of WebSocketConnection
• 3. StreamMessage Parser
  • parseVarInt
  • parseStreamId
  • parseStreamMessageType
  • parseStreamMessageAckPayload
  • parseStreamMessageClosePayload
  • parseStreamMessageErrorPayload
  • parseStreamMessage
• 4. StreamMessage Generator
  • generateVarInt
  • generateStreamId
  • generateStreamMessageType
  • generateStreamMessageAckPayload
  • generateStreamMessageClosePayload
  • generateStreamMessageErrorPayload
  • generateStreamMessage
• 5. WebSocketStreamQueue to buffer generated StreamMessages
• 6. Benchmarks
  • Over net.Socket (TCP)
  • Over ws.WebSocket
  • Over WebSocketConnection
  • Over WebSocketStream
  • iPerf3
• Rework Events
  • Rework Stopping of WebSocketConnection to rely on Events
  • Rework Destruction of WebSocketStream to rely on Events
• Injectable http/s Server for WebSocketServer
  • Allow https server to be injected
  • [ ] Allow for http server to be injected - no need for this because we are just going to focus on https only
  • Allow for server to be created and encapsulated in the lifecycle of WebSocketServer depending on TLS configuration
• Standardize TLS errors
• Hostname Resolution

Final checklist

• Domain specific tests
• Full tests
• Updated inline-comment documentation
• Lint fixed
• Squash and rebased
• Sanity check the final build

[ghost]

👇 Click on the image for a new way to code review

Legend

[CMCDragonkai]

Can this fix #2 and #4 at the same time?

[tegefaulkes]

We shouldn't kitchen sink PRs, Try do do them in a new PR if they're not required by changes in this one.

[amydevs]

When we are in a node environment, we are using the ws library. In these cases, the user is able to provide a verifyCallback to verify that the server-supplied peerCert is as expected. This is done by adding an event listener to the upgrade event on a ws.WebSocket event target. This facilitates connecting the PKE backend to PolyKey nodes (using self-signed certs).

However, this event is not exposed in the window.WebSocket object that exists within browsers. There is no way to allow a self-signed certificate without user intervention within browsers, hence we should throw an error is a verifyCallback if provided in a browser environment. This facilitates connecting the PKE frontend to the PKE backend (using certs issued by a valid CA).

[CMCDragonkai]

Yea the browser will be considered a deficient environment. The API should look the same but we should be strict on creation where we throw errors for unimplemented features because of browser limitations.

[amydevs]

There will need to be new classes created, I'm in the process of doing so at the moment.
Mostly WSSocket and WSConnection.

The reason for the first being that in QUIC, a QUICSocket has a one to many relationship with QUICConnection. Meanwhile, a WebSocketConnection has a one to one relationship with a ws.WebSocket or window.WebSocket.
Hence, a WSSocket class needs to be created to wrap and group ws.WebSockets, emulating how a QUICSocket can have multiple connections.

This will mean that the interfaces for WSServer and WSClient need to change. These classes originally contained WebSocketStream instances, which were one-to-one with ws.WebSocket. Essentially WebSocketStream is what really the WSConnection should be, barring the ReadableWritablePair, which should be sectioned off to WSStream instead.

WSConnection will have a one-to-many relationship with WSStream by implementing muxing.

[amydevs]

WSSocket will be simply a container for WSConnections and ws.WebSockets.

Furthermore, the creation of WSServer should no longer be asynchronous. The start should be the real binding of the https.Server instead. Instead of creating a WebSocketStream on connectionHandler, it should be now WSConnection instead. WSConnection should parse the the ws.WebSocket received messages for the creation of WSStreams

[tegefaulkes]

Feel free to ask me any questions about how the WebSockets and WebStreams work, I've worked with them a fair bit.

[amydevs]

Only the server should be pinging, and the client responds with pongs. We do this as ping/ponging is not available on the window.WebSocket instance. However, browsers will automatically respond with pongs to server-sent pings, rather than exposing the feature to the user. Since we cannot control this behaviour, we have to implement similar behaviour on our WebSocketClient in a nodejs context. Or we can implement application-level ping/ponging.

[CMCDragonkai]

Ignore the browser environment for a moment.

The websocket spec already defines how ping/pong works. I'm not actually sure which side is supposed to be sending the pings. Can you find out? Either way, keep alive mechanism must exist on both sides so that if the other side stops sending activity, we end up timing out.

According to a quick search, browser websockets might actually be doing the keep alive internally already, so when using browser websockets, you just throw an exception when the parameter is set. Similar to what we talked about already about configuration not available on the browser.

[CMCDragonkai]

@amydevs update the spec above with all the changes that is required. Including the spec on how the ping will work between client and server as drawn on the whiteboard.

[amydevs]

for client - server ping-pong keep alive here's what should happen:
client and server both start pinging initially
if the client receives a ping, stop pinging
if the server receives a ping, keep pinging
if the server receives two pings, stop pinging

this could be expanded upon by using the keepalive interval timer.
for example, if the keepalive interval timer on a server is higher than on a client, the client can eagerly ping to establish keepalive responsibility

[CMCDragonkai]

Regarding benchmarking:

iperf -s # server side
iperf -c 127.0.0.1 -l 1024 # client side

Do this on your local system to test your localhost. Use the -l 1024 to use a message buffer size of 1024 bytes or 1 KiB. (We always use KiB, MiB, GiB, never KB, MB, GB).

On the Linux OS, the kernel will set the underlying TCP buffer size. We don't need to change this.

Get a baseline for just regular TCP doing the same thing, you can compare against iperf3. That gives you the overhead of JS.

Then use ws in binary mode and do the same thing with 1024 bytes, now you know the overhead of ws.

Then use js-ws and benchmark only the connection, not the stream. Now you now the overhead of js-ws's connection.

Then use js-ws and benchmark a single stream on single connection. Now you know the overhead over the muxing/demuxing. In this mode, you now have 1 additional buffer to control, and that's the per-stream buffer, keep this consistent and try different per-stream buffer sizes, doubling each time.

[amydevs]

my iperf3 results performed with

iperf3 -s # server side
iperf3 -c 127.0.0.1 -l 1024 # client side

[amydevs]

Good idea to document how your TLS behaves btw with all the potential failure scenarios.

i think i'm going to have to change how the TLS errors to be more specific rather than just internal, though the problem is that i can't really tell a socket error is because of a peer TLS failure in the case where verifyCallback is null, because ws simply errors that the server has dropped the socket

[amydevs]

i'll probably standardize the TLS errors like QUIC first and then document them

[CMCDragonkai]

Some things to port over from js-quic:

• Events Refactoring (integrating js-events) js-quic#53 (comment) - generateCertificate and generateTLSConfig
• Events Refactoring (integrating js-events) js-quic#53 (comment) and Events Refactoring (integrating js-events) js-quic#53 (comment) - bring in validateTarget if it is relevant to you. (Note that I believe some of my transforms is actually relevant).

[amydevs]

I've added inline documentation to WebSocketConnection:

/**
 * Note that on TLS verification failure, {@link events.EventWebSocketConnectionError} is emitted with the following `event.detail`:
 * - If we failed to verify the peer, the `event.detail` will be an instance of {@link errors.ErrorWebSocketConnectionLocalTLS}.
 * - If the peer failed to verify us, the `event.detail` will be an instance of {@link errors.ErrorWebSocketConnectionPeer} with an error code of {@link utils.ConnectionErrorCode.AbnormalClosure}.
 *
 * The reason for this is that when the peer fails to verify us, Node only tells us that the TCP socket has been reset, but not why.
 */

[CMCDragonkai]

Hooray!