Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
elenaf9 opened this issue Jun 15, 2022 · 6 comments
Closed

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

elenaf9 opened this issue Jun 15, 2022 · 6 comments

Comments

@elenaf9
Copy link
Contributor

elenaf9 commented Jun 15, 2022

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 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.
But this would mean that we loose the path information. Hence the question if it is even relevant in the first place.

@tomaka
Copy link
Member

tomaka commented Jun 15, 2022

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
Copy link
Contributor

rkuhn commented Jun 16, 2022

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
Copy link
Member

mxinden commented Jun 20, 2022

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:

  • Remove the concept of ListenerId and follow *: Remove ListenerId elenaf9/rust-libp2p#4 (comment), thus not supporting WebSocket paths. It is not supported today. I am not aware of anyone planning to implement it. In case someone does, we can re-introduce ListenerIds at a later point in time.

  • Keep the concept of ListenerIds, but generate them randomly in the lowest Transport implementations, thus allowing the WebSocket Transport to add the right path and ws / wss to each address.

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

@elenaf9
Copy link
Contributor Author

elenaf9 commented Jun 20, 2022

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
Copy link
Member

tomaka commented Jun 20, 2022

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

Yes

@thomaseizinger
Copy link
Contributor

Considering this as resolved.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants