Open agilenc opened 1 year ago
I really want to swap to ws
, however there are real users out there who uses the supply-headers-capability in a Node.js environment. So that will break their code if a solution is not provided. I have honestly not dug into the internals of the ws
package for a long time, so there may be solutions that appeared in the last 18-24 months for that functionality.
EDIT: Indeed, it actually does take header in the options, https://github.com/websockets/ws/blob/d412358ccb5320bc00c8993ecd5d9b992df0753e/lib/websocket.js#L717
Not sure how the rest of the options ca/cert/key/... etc fits into that lib.
For ca/cert/key those would land here in the lib as options.cert, options.key, options.ca
. Please find an example of using cert/key options here. Using ca
would follow the same pattern, just adding ca next to cert/key properties.
From the types we see the options
parameter is of type WebSocket.ClientOptions
which extends SecureContextOptions
. This is why it can contain ca/cert/key properties.
Good to hear there wouldn't be any blocker if swapping to ws
👍
The underlying ws library has been swapped in the last linked PR. As for extending options here, this is completely unlikely to be done by me, so I'll mark it as such. (There is just no way I can reliably setup the testing infrastructure alongside the coding to ensure everything is working, and not quite fond of putting up half-baked code and then playing whack-a-mole)
So, with the fact that the PR won't be done by me, this is what is required -
Now on point 2 - just tacking on more options is horrible. I'm guessing past-me didn't want to break compat so just kept tacking them on. Current and future me still doesn't want to break compat, but certainly doesn't want to tack on more.
So with that in mind, we want to keep the old signature, but add a new signature that takes an options object with headers
, timeout
, the new ca stuff, etc. On use detect which is use and apply correctly.
So something like (untested, spitballing...) -
interface WsOptions {
autoConnectMs?: number | false;
headers?: Record<string, string>;
timeout?: number;
// .. additional new ca options go here...
}
/** @deprecated Use constructor(endpoint, options) instead */
constructor (endpoint: string | string[] = defaults.WS_URL, autoConnectMs?: number | false, headers?: Record<string, string>, timeout?: number)
constructor (endpoint: string | string[] = defaults.WS_URL, options: WsOptions = {}) {
if (typeof options === 'object') {
// extract stuff from the options object
} else {
// old-style, deprecated
// NOTE Ensure that we still use the defaults as we have in the current code... (they have been removed
// from the signature above)
}
}
Please tell us about your environment:
[x] Node.js
[ ] Browser
[ ] Other (limited support for other environments)
What is the current behavior?
On Node when creating a new WsProvider there is no way to specify ca/cert/key (for mTLS).
Similar to the ws package where we can do:
It would be great to be able to specify a SecureContextOptions object to the constructor when creating a new WsProvider.
Support more use-cases, currently we are unable to use polkadot-js api because the ws client cannot connect to a secure endpoint with mTLS. Using mTLS is a requirement in our case. We only need this on Node (not browser or other env).
As a general rule client APIs that connect to nodes should provide security guarantees when possible. This initiative would support this.
Talked to @TarikGul who shared this issue https://github.com/polkadot-js/api/issues/5279. One comment is:
To allow a
SecureContextOptions
object to be passed to theWsProvider
constructor then here when using Node we'd first need to swap the polyfill from websocket to ws. Maybe some work has already been done in that regard?