search

libp2p / js-libp2p-webrtc-star

libp2p WebRTC transport that includes a discovery mechanism provided by the signalling-star
https://libp2p.io
Other
320 stars 96 forks source link

bug: make legacy protocol check more specific #369

Closed Zaba505 closed 2 years ago

Zaba505 commented 3 years ago

I'm currently deploying the star-signal server via google cloud. I don't currently have a custom domain for it so the default generated domain is being used in my project. I named the deployment libp2p-webrtc-star-signal which means dns based multiaddrs look something like this: /dnsaddr/libp2p-webrtc-star-signal.run.app/tcp/443/wss/p2p-webrtc-star, where the domain is the auto-generated one provided by google cloud. This causes an issue in cleanMultiaddr because it doesn't validate where the legacy /libp2p-webrtc-star substring it located in the multiaddr.

A solution (which I'm taking) is to simply make sure /libp2p-webrtc-star doesn't appear anywhere else in your multiaddr. I don't think this is ideal due to it restricting the user on how they're able to name things. More importantly, it also requires users to be aware of the legacy /libp2p-webrtc-star in the first place. A much better solution is to validate that /libp2p-webrtc-star is being used as a protocol and not just some random substring found elsewhere in the multiaddr.

I'm planning to contribute the fix for this over the next day or so, as long as, no one else raises any problems with fixing it.

mpetrunic commented 2 years ago

@Zaba505 Are you still interested in providing fix?

Zaba505 commented 2 years ago

@mpetrunic yeh I'd totally be willing to implement this fix

github-actions[bot] commented 2 years ago

:tada: This issue has been resolved in version @libp2p/webrtc-star-v1.0.11 :tada:

The release is available on:

Your semantic-release bot :package::rocket: