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

Allows establishing of websocket with web optional protocols parameter #108

Closed
wants to merge 2 commits into from

Conversation

jadedevin13
Copy link

The browser won't autoclose the connection since it will now see the header it requires.

#107

@agalakhov
Copy link
Member

Sorry, the code you just pushed is not compileable.

Copy link
Member

@daniel-abramov daniel-abramov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do we need protocol variable set to an empty header value if get() returns an optional value?

It may be rewritten much simpler as:

    let mut builder = Response::builder()
        .status(StatusCode::SWITCHING_PROTOCOLS)
        .version(request.version())
        .header("Connection", "Upgrade")
        .header("Upgrade", "websocket")
        .header("Sec-WebSocket-Accept", convert_key(key.as_bytes())?);

    const PROTOCOL: &'static str = "Sec-WebSocket-Protocol";
    if let Some(protocol) = request.headers().get(PROTOCOL) {
        builder = builder.header(PROTOCOL, protocol);
    }

or something like this.

@daniel-abramov
Copy link
Member

There seems to be no activity in this PR, so I'm going to close it. We could re-introduce the changes if we decide to use this approach. Please check #145 (comment) for further details.

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.

3 participants