feat: custom address filter #116
Conversation
BREAKING CHANGE: Only DNS addresses are now returned on filter by default. WSS when in the browser and WS+WSS for Node.js. This can be overritten by the filter option.
| @@ -35,7 +35,25 @@ | |||
| > npm i libp2p-websockets | |||
| ``` | |||
|
|
|||
| ### Example | |||
| ### Constructor properties | |||
There was a problem hiding this comment.
Perhaps this is a good time to just replace this example with how to use it in a libp2p configuration with the passed options. That's likely much more useful than the existing examples as most people won't use this standalone.
README.md
Outdated
| const WS = require('libp2p-websockets') | ||
| const filters = require('libp2p-websockets/src/filters') | ||
|
|
||
| const ws = new WS({ upgrader, filter: filters.all }) |
There was a problem hiding this comment.
Yeah I think the libp2p transport configuration example will be ideal here, https://github.com/libp2p/js-libp2p/blob/master/doc/CONFIGURATION.md#customizing-transports.
There was a problem hiding this comment.
I added the libp2p usage example, but kept the other. They could still be relevant, but the Readme might get too noisy. What's your opinion?
src/index.js
Outdated
| return filters.dnsWss(multiaddrs) | ||
| } | ||
|
|
||
| return filters.dnsWsOrWss(multiaddrs) |
There was a problem hiding this comment.
This can probably be all in Node, because a TCP dial on ws is valid. We don't need crypto because we use Noise.
We might need to double check that node doesn't block this though. I think our tests are only using loopback addresses right now.
There was a problem hiding this comment.
Added a non loopback addr test to a local addr
README.md
Outdated
| const transportKey = Websockets.prototype[Symbol.toStringTag] | ||
| const node = await Libp2p.create({ | ||
| modules: { | ||
| transport: [WebRTCStar], |
There was a problem hiding this comment.
| transport: [WebRTCStar], | |
| transport: [Websockets], |
README.md
Outdated
| const node = await Libp2p.create({ | ||
| modules: { | ||
| transport: [WebRTCStar], | ||
| // ... other mandatory modules |
There was a problem hiding this comment.
I would just add NOISE and mplex in here. There aren't other options right now, so having a config people can just use would be helpful imo.
README.md
Outdated
|
|
||
| For more information see [libp2p/js-libp2p/doc/CONFIGURATION.md#customizing-transports](https://github.com/libp2p/js-libp2p/blob/master/doc/CONFIGURATION.md#customizing-transports). | ||
|
|
||
| ## Base Example |
There was a problem hiding this comment.
I would just delete this whole section, nobody should use it like this.
|
Released this with beta tag until |
This PR adds a custom address filter to websockets. Long story short,
libp2p-websocketshas allowed TCP and DNS addresses, both with ws or wss.Considering libp2p/js-libp2p#796 . Taking into account security (and browser policies), we should be restricting addresses to DNS only (unless in development), as well as supporting
wssonly in the browser (Node.js still permitsws).This PR aims to set the defaults as specified above, but also adding support for a custom address filter. This is super useful for development, but also if anyone wants to connect to a local TCP loopback peer.
This PR also makes some filters available for users and the dependencies were updated. Libp2p will be able to custom this via its config per transport.
BREAKING CHANGE: Only DNS addresses are now returned on filter by default. WSS when in the browser and WS+WSS for Node.js. This can be overriden by the filter option.