libp2p / rust-libp2p

The Rust Implementation of the libp2p networking stack.
https://libp2p.io
MIT License
4.31k stars 893 forks source link

websocket: What is the use-case for paths in WS multiaddresses? #2708

Closed elenaf9 closed 1 year ago

elenaf9 commented 2 years ago

Description

In #1093 we enabled paths for ws / wss multiaddresses. We are currently ignoring this path in our websocket transport WsConfig, and just care about whether it's an ws or wss address. (Edit: I phrased this a bit misleading. I am talking about ws listeners. For dialing we do handle the ws path, and will continue to do so even after the listener changes described below.) But while WsConfig listeners itself does not handle the path, it still remembers for inner listeners the Protocol::Ws(s) suffix (that includes the path) and appends it back to all addresses reported by the inner listener.

From what I understand, this is rust-libp2p specific and not part of the multiaddr specs. The specs specify the ws / wss protocol with size 0: https://github.com/multiformats/multiaddr/blob/master/protocols.csv#L28-L29. (cc @aschmahmann - is IPFS using paths in websocket addresses?). @tomaka what was the use-case for paths in websocket multiaddresses? Is it still used somewhere or could it be ignored completely?

The motivation for this question is that as part of the listener refactoring in #2652 and elenaf9#4, "remembering" the ws / wss protocol for inner listeners gets more complicated. To simplify this, @mxinden proposed in https://github.com/elenaf9/rust-libp2p/pull/4#issuecomment-1153663146 to make an instance of WsConfig supporting either ws or wss, and to not store the original ws / wss protocol suffix that the user provided in the listen_on call. For addresses from the inner transport, we would then just append the ws or wss protocol with default path (which is just "/") back to the address. But this would mean that we loose the path information. Hence the question if it is even relevant in the first place.

tomaka commented 2 years ago

We're using libp2p-websocket in Substrate to connect to a good old HTTP server against a specific URL. It was the easiest way to implement this at the time.

I've always had the intention to add this feature to the multiaddr spec, but it's always been blocked due to no agreement on how to escape / characters.

rkuhn commented 2 years ago

I concur: a WebSocket is established by connecting to an URL, which always includes a path (that may just be / but very often isn’t). The point of WS is to use HTTP for the purpose of offering multiple service endpoints on a single host:port, differentiated by their path.

mxinden commented 2 years ago

The motivation for this question is that as part of the listener refactoring in #2652 and elenaf9#4, "remembering" the ws / wss protocol for inner listeners gets more complicated. To simplify this, @mxinden proposed in elenaf9#4 (comment) to make an instance of WsConfig supporting either ws or wss, and to not store the original ws / wss protocol suffix that the user provided in the listen_on call. For addresses from the inner transport, we would then just append the ws or wss protocol with default path (which is just "/") back to the address.

With the above replies in mind, I am fine with either of our two options, namely to:

I would go with the option that simplifies the most across the code base.

elenaf9 commented 2 years ago

We're using libp2p-websocket in Substrate to connect to a good old HTTP server against a specific URL. It was the easiest way to implement this at the time.

I've always had the intention to add this feature to the multiaddr spec, but it's always been blocked due to no agreement on how to escape / characters.

@tomaka so do I understand it correctly that for substrate the ws path is only relevant as a dialer?

I edited the issue description as it was misleading. I am only talking about the ws listeners side, not the dialing part. Sorry for the confusion.

tomaka commented 2 years ago

for substrate the ws path is only relevant as a dialer?

Yes

thomaseizinger commented 1 year ago

Considering this as resolved.