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.

Sign up for GitHub

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

Jump to bottom

Make websocket/not websocket choice client-based instead of advertising wss:// RelayUrls #2442

Closed
matheus23 opened this issue Jul 2, 2024 · 3 comments
Closed

Make websocket/not websocket choice client-based instead of advertising wss:// RelayUrls #2442

matheus23 opened this issue Jul 2, 2024 · 3 comments
Assignees
@matheus23
Labels
c-api iroh-api crate issues c-iroh
Milestone
v0.20.0

Comments

@matheus23
Copy link
Member

@flub correctly pointed out on discord:

  • Websocket transport is selected based on wss:// vs. https:// scheme in the RelayUrl
  • This means clients would advertise whether they want to use websockets or not in their tickets, even though clients should decide that for themselves only!
  • We achieve better compatibility if we announce a single URL
  • It's weird to have the wss:// prefix, given there's also other endpoints like https://relay.example.org/generate_204

I think the fix is:

  • Make wss or https selection based on a client-side flag (probably in the ClientBuilder)
  • Always advertise http(s) relay urls.
@matheus23 matheus23 added c-api iroh-api crate issues c-iroh labels Jul 2, 2024
@matheus23 matheus23 self-assigned this Jul 2, 2024
@dignifiedquire
Copy link
Contributor

I think the proposed solution is the right one

@matheus23 matheus23 added this to the v0.20.0 milestone Jul 2, 2024
@matheus23
Copy link
Member Author

I'll put up a fix today, so it'll hopefully make it in before the release on Monday.

@matheus23 matheus23 mentioned this issue Jul 2, 2024
Merged
4 tasks
github-merge-queue bot pushed a commit that referenced this issue Jul 3, 2024
@matheus23
feat(iroh-net)!: Make relay protocol configurable on ClientBuilder …
ab2c7ea 
…instead of defined by the relay url scheme (#2446)

## Description

Details in #2442 

## Breaking Changes

- `iroh_net::relay::http::ClientBuilder`: Added `.protocol()` function
to the builder for choosing whether to connect to the relay via
websockets or not

## Change checklist

- [X] Self-review.
- [X] Documentation updates if relevant.
- [X] Tests if relevant.
- [X] All breaking changes documented.
@ramfox ramfox moved this to 📋 Backlog in iroh Jul 3, 2024
@github-project-automation github-project-automation bot moved this from 📋 Backlog to ✅ Done in iroh Jul 3, 2024
@dignifiedquire
Copy link
Contributor

Fixed in #2446

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@matheus23 matheus23

Labels
c-api iroh-api crate issues c-iroh
Projects
iroh
Archived in project
Milestone
v0.20.0
Development

No branches or pull requests
2 participants
@dignifiedquire @matheus23