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

WebSockets: support passing through sec-websocket headers #6094

Merged
merged 3 commits into from
Mar 5, 2025

Conversation

SteffenDE
Copy link
Contributor

Supersedes #5904.

@josevalim
Copy link
Member

Ideally connect_info is not specific to any transport. What if we just include all websocket headers alongside x-headers?

@SteffenDE
Copy link
Contributor Author

@josevalim well, :x_headers is also not transport agnostic. If I build a transport based on telnet, there are no x_headers. I don't think it makes sense to mix x_headers + sec_websocket. If we didn't have x_headers yet, I'd propose a generic :headers key that one would configure with a list of headers / prefixes:

socket "/socket", AppWeb.UserSocket,
  websocket: [
    connect_info: [:uri, headers: [prefix: ["x-", "sec-websocket-"]]]
  ]

But since we already have x_headers 🤷🏻‍♂️

@josevalim
Copy link
Member

@SteffenDE x-headers work for websockets too, except in the browser. Another interpretation is to say that x_headers means "extra headers", and that would fit both x- and websocket prefixes. However, if you prefer to go with this PR, I am ok with it. Your call.

@SteffenDE SteffenDE force-pushed the sd-sec-websocket-headers branch from 7f3540d to 7a99c9c Compare March 5, 2025 12:44
@SteffenDE
Copy link
Contributor Author

Chris approves, let's go!

@SteffenDE SteffenDE merged commit 16bfa91 into main Mar 5, 2025
10 checks passed
@SteffenDE SteffenDE deleted the sd-sec-websocket-headers branch March 5, 2025 14:14
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

Successfully merging this pull request may close these issues.

4 participants