websocket: proposal for a new core module #49478
Conversation
This is a proposal to add websockets as a new module for Node.js. This implementation attempts to adhere to RFC 6455. If differences are identified I will supply the necessary changes. This implementation does not attempt to parse connection header values, such as extensions or sub-protocols values. This proposal focuses exclusively on Node's APIs from its core modules and then supplies additional options as necessary to populate callbacks and RFC 6455 connection header values. Some notes about performance. Everybody that writes a websocket library wants to measure performance in terms of message sent speed, which seems to be a red herring. Message send speed is trivial compared to message receive speed. Message send speed appears to be a memory bound operation. Using this approach to websockets I send at about 180,000 messages per second on my old desktop computer with slow DDR3 memory. On my newer laptop with faster DDR4 memory I can send at about 460,000 - 480,000 messages per second. This speed is largely irrelevant though because at around 460,000 messages garbage collection kicks in and message send speed slows to a crawl at around 5,000 per second. Message receive speed is much slower and far more dependent upon the speed of the CPU. On my old desktop I can receive messages at a speed of around 18,000 messages per second while on my laptop its a bit slower at around 12,000-15,000 messages per second because the desktop has a more powerful CPU. The speed penalty on message reception appears to be due to analysis for message fragmentation. This approach accounts for 4 types of message fragmentation.
|
FYI, there is some ongoing discussion about exposing websocket into Node.js from undici itself - nodejs/undici#2217 |
|
@RaisinTen Thank you. I was not aware of that project. It seems associating WebSockets with HTTP imposes too much work and crushes performance while greatly complicating scale for future enhancements. I updated the pull request readme to be more clear on this matter. I understand the browser executes WebSockets in that manner, but Node is not a web browser. Browsers, aside from things like Chrome DevTools Protocol, are never servers in any capacity and browsers must accept anonymous connections to external servers. Those constraints, and many more, do not apply to Node, and so Node should not be restricted in the manner of a web browser. |
|
TODO list:
|
Can you clarify what you mean when you say this? I see this PR also includes http machinery. |
|
I like this pure protocol approach, at a similar level to the |
Any WebSocket implementation has nothing to do with HTTP apart from the initial handshake request. Using a plain |
But also, process the HTTP |
|
@devsnek More precisely HTTP is an application layer protocol (OSI layer 7). What happens typically with HTTP is that a socket is opened for each request. A keep alive header modifies that behavior and HTTP/2 and QUIK work very differently. Even still HTTP always imposes a connection roundtrip request/response and HTTP is both sessionless and anonymous. There are advantages to sessionless communication, most primarily initiating other communication conventions. There are also conveniences to the HTTP round trip process. TCP sockets do not make these distinctions. TCP sockets can be session-oriented or not, and anonymous or not. They are too low level to care. In most use cases WebSockets are session-oriented and long lived to push multiple messages across a network over some undefined length of time. TCP sockets are also full-duplex, at least with regard to WebSockets, which means both sides can send messages at any time. That further means that once the handshake completes the sockets send and listen for messages in identical ways irrespective of whether the socket occurs in the role of client or server. RFC 6455 provides a distinction there though that client's must mask their messages and servers must not. My personal opinion is that message masking is completely unnecessary so I provided an option to force it on or off for both the client and the server. This proposal sends a text message across a network that looks like an HTTP heading. It is just a text message sent using socket.write. The server responds with a confirmation text message. Just about everything in those two messages that make up the handshake process is decoration. The required pieces are the Something that will demand further examination is piping a stream directly through the internal messageSend function. |
It means reimplementing the whole headers parsing which ironically has nothing to do with WebSocket. It might make sense on the client but it is not so easy on the server if you want a server that handles both normal and upgrade requests and with things like slowloris. Why reinventing the wheel? Also the API should be discussed and there is work to do. I see no validation (invalid method, subprotol header, sec-websocket-extensions header, invalid frames, UTF8), no closing handshake, no way to send a fragmented message, etc. This is far from ready and usable. I would convert it to draft. |
|
The way to send a fragmented message is through the third argument of Closing occurs through sending a control frame with opcode 8. When the client receives that opcode it will destroy the socket from its side if its not already destroyed by the server. There is also support for sub-protocol and extension headers. This implementation makes no attempt to parse the values internal from those headers as that is best left to user land code. The sub-protocol and extension headers are populated by the client user into the options argument for the |
I might be missing something but the I would expect an api that would allow me to do
That's not how the closing handshake work. Each peer should have both sent and received a close frame before closing the TCP socket.
The RFC mandates that the connection must be closed during the opening handshake if the those headers are invalid. |
|
There are many existing WebSocket implementations for Node.js. I assume folks would want to see an implementation following the WebSockets Standard in core. |
That is handled internally by the
The spec does say that for extensions in section 9.1. I can add that logic, but its pretty draconian. According to the spec values must be applied according to the conventions of 2616, which is super old and also means you cannot apply a JSON based value. The extensions header is the primary place to add custom header data without adding additional custom headers which would also exceed the scope of the specification. |
|
@tniessen that is the API standard for the browser. This implementation takes no position on API and will change to whatever is best for sending messages. Since the WHATWG spec is for the browser it provides no API definition for a WebSocket server. |
|
Closing process updated to better comply with RFC 6455 and status codes updated to better align with the WHATWG status codes. |
|
|
I have spent a little time running and reviewing this code, including running the server and connecting my own client implementations to it. I have found a number of correctness issues that ought to be addressed before this is considered production-ready.
In addition to these issues there seems to be some confusion about the purpose of extensions and the
This is not what the As such, this header requires a standard format for how extension names and parameters are represented, you cannot put arbitrary data in there. Though it may be "super old", that syntax is defined in the spec with reference to grammar from RFC 2616. This is also why the HTTP handshake and binary framing protocol are not fully separable -- the HTTP handshake establishes state that affects the operation of the socket for the rest of its lifetime. If you want to keep extensions but discard the HTTP handshake then you need to specify a different mechanism for extension negotiation; RFC 6455 simply leans on existing HTTP conventions to achieve this. For example, the Compression Extensions allow for messages to be compressed; specifically This indicates the client wants to use the Handling of extensions in general imposes a bunch of extra requirements on an implementation:
This is a non-trivial set of requirements and I have a library dedicated just to handling them. This is used in faye-websocket, websocket-driver and permessage-deflate. All this behaviour also carries security implications and we have experienced two denial-of-service vulnerabilities, one due to a breaking change in zlib and one due to a regex used in parsing the header. This implementation lets the client provide a That covers everything I have noticed in terms of correctness issues. While not strictly issues of correctness, I also noticed the following places where the code's efficiency could be substantially improved:
Finally, some notes on the proposed API:
|
|
@jcoglan thank you for taking the time to review and write all of that. From a quick glance, in addition to what you wrote, many conditions are not handled according to RFC 6455. This is what I meant in my previous comment with "missing validation". I'll copy paste some here. |
|
I will apply these changes, but it will take several days. Tomorrow morning I move across the country. |
mcollina
added
the
tsc-agenda
|
By the way we already have a C++ websocket server implementation in core for the inspector protocol: https://github.com/nodejs/node/blob/main/src/inspector_socket.cc it's just not exposed to users (and probably quite specific to the inspector protocol, but can surely be tweaked to be more general) |
I think that could be the best aproach, or one of them. |
If we want something that is significantly faster than userland implementations, then yes, but it that case I'm not sure how hard it would be to integrate it well with the |
There was a problem hiding this comment.
I feel strongly that we should prefer an existing battle tested implementation or a very fast one instead of "rolling our brand new one", especially given the spec compliance concerns outlined by others.
Thank you for the effort it would be great if you'd be involved in the efforts to add websockets to Node.js 🙇
|
@benjamingr A production tested implementation is a better idea. My approach is only tested within my personal application, which is a self-contained environment. Either way there will need to be a checklist of conformance checks and corresponding test units. Performance checks, if those should be included, must test message send speed and message receive speed independently. One final consideration of note, and perhaps the most important distinction from HTTP, is that once the socket is established at both ends it sends and receives in an identical manner irrespective of whether the socket initiated as a client or server role (aside from default message masking). For stability any test automation validating message transfer or message integrity must be applied to both server and client sockets identically. |
There was a problem hiding this comment.
While I appreciate the work here and agree that adding WebSocket support in core is a good thing, I would rather avoid adding a new, bespoke, non-standard WebSocket API. While the Web platform standard WebSocket API leaves lots to be desired, we should focus on implementing the standard API and work with the ecosystem to improve the standard.
tniessen
removed
the
tsc-agenda
|
Thank you very much for your contribution! Unfortunately, It seems like the discussion of WebSockets is at https://github.com/nodejs/undici, and it's unlikely this PR will receive any action. I'm closing this to reduce the number of open PRs and issues in Node.js. Please let me know if you disagree or have a team member reopen it. Nonetheless, thank you for your contribution to NodeJS! |
|
Does this means built-in support for WebSockets will be discarded? I was really interested on that... :-( |
Where can I get more details about this? |
This is a proposal to add websockets as a new module for Node.js.
Standards Conformance
This implementation attempts to adhere to RFC 6455. If differences are identified I will supply the necessary changes. This implementation does not attempt to parse connection header values, such as extensions or sub-protocols values.
API
This proposal focuses exclusively on Node's APIs from its core modules and then supplies additional options as necessary to populate callbacks and RFC 6455 connection header values.
This proposal does not attempt to emulate any WHJATWG specification or execute with regards to HTTP. Node is not a web browser. The operating constraints are wildly different, therefore in the best interests of design a WebSocket implementation must adhere to the protocol standard closely, but otherwise isolated from other API concerns. All other API concerns represent additional constraints not necessary for the transport of messages over this protocol.
According to RFC 6455 section 1.7: The WebSocket Protocol is an independent TCP-based protocol. Its only relationship to HTTP is that its handshake is interpreted by HTTP servers as an Upgrade request. That means so long as the as the handshake completes according to the syntax provided HTTP remains completely unrelated. HTTP is a layer 7 (application) technology where WebSocket is defined as a TCP transport mechanism (layer 4). Executing WebSockets over HTTP then creates unnecessary overhead dramatically impacting maintenance, scale, and performance.
Performance
Some notes about performance.
Everybody that writes a websocket library wants to measure performance in terms of message sent speed, which seems to be a red herring. Message send speed is trivial compared to message receive speed.
Message send speed appears to be a memory bound operation. Using this approach to websockets I send at about 180,000 messages per second on my old desktop computer with slow DDR3 memory. On my newer laptop with faster DDR4 memory I can send at about 460,000 - 480,000 messages per second. This speed is largely irrelevant though because at around 460,000 messages garbage collection kicks in and message send speed slows to a crawl at around 5,000 per second.
Message receive speed is much slower and far more dependent upon the speed of the CPU. On my old desktop I can receive messages at a speed of around 18,000 messages per second while on my laptop its a bit slower at around 12,000-15,000 messages per second because the desktop has a more powerful CPU. The speed penalty on message reception appears to be due to analysis for message fragmentation. This approach accounts for 4 types of message fragmentation.
Origin
This implementation is based upon my own logic for my personal application. I have been experimenting with a faster native Node WebSocket implementation for more than 2 years. Origin code at https://github.com/prettydiff/share-file-systems/blob/master/lib/terminal/server/transmission/transmit_ws.ts
Test logic
A means to test and experiment with this proposal with minimal overhead.