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

[bug] Cannot Add Sec-Websocket-Extensions header field as a client #674

Open
navintestapi opened this issue Mar 17, 2021 · 12 comments
Open

Comments

@navintestapi
Copy link

Describe the bug

The library doesn't allow adding "Sec-Websocket-Extensions" header.
However, the RFC says that this header can be set multiple times in the Request, but only once in the response.

Get Error : duplicate header not allowed: Sec-Websocket-Extensions

@navintestapi navintestapi added the bug Something isn't working label Mar 17, 2021
@ghost
Copy link

ghost commented Mar 17, 2021

I assume that the package disallows setting the extension headers because the package must include code specific to any extension.

What is the Sec-Websocket-Extensions header that you are trying to set?

@navintestapi
Copy link
Author

Well that's very limiting then. If the server wants some capabilities to be specified in the extensions, then there is no way I can use the package.

@ghost
Copy link

ghost commented Mar 17, 2021

The Sec-Websocket-Extensions header is used to negotiate protocol-level extensions to the Websocket protocol. If a protocol-level extension can be implemented by the application, then the extension is probably not at the protocol level.

What is the Sec-Websocket-Extensions header that you are trying to set? Without a concrete example showing otherwise, I think the code is correct as is.

If you are trying to negotiate features between your client application and your server application, then take a look at subprotocols. Unlike extensions, most libraries (including this library and web browsers) offer subprotocol negotiation.

Edit: The registered extensions to the protocol are listed here.

@navintestapi
Copy link
Author

The server needs this to support multiplexing.
https://tools.ietf.org/html/draft-ietf-hybi-websocket-multiplexing-11
"A client that supports this extension will
advertise support for it in the client's opening handshake using the
"Sec-WebSocket-Extensions" header."

@ghost
Copy link

ghost commented Mar 18, 2021

I see. The proposed mux extension can be implemented by an application.

The tricky issue is working out how package and application implemented extensions are listed in the extension header.

Do you actually have a server that implements the mux proposal? The proposal has been inactive for years.

@navintestapi
Copy link
Author

Yes, I think just allowing the header to be modified should work, right? The application will then take care of handling the multiplexing, would the package need to do anything different than passing on the received data to the application?

@ghost
Copy link

ghost commented Mar 18, 2021

There are two problems to resolve in this feature request:

  • How does the application specify the extensions to add to the header. The header argument to dial is one option, but see the next point.
  • The order of extensions is significant. How does the application specify whether an extension comes before or after package supplied extensions (currently per message compression extensions).

This strikes me as the type of issue that will be tagged "waiting on new maintainer". I am not a maintainer of the project.

@navintestapi
Copy link
Author

May be at a minimum, allow the header to be either used by the application or the package, but not both.....so that whoever decided to use this handles everything within the application and one who wants to use package functionality does not specify the header.

@ghost
Copy link

ghost commented Mar 19, 2021

Just to confirm, are you implementing the client part of the mux spec to work with a server with mux support? I am double checking because the mux extension was abandoned, I am surprised that there are extant implementations of the spec.

The extension implemented by the package (per message deflate) cannot be implemented by an application. The all or nothing approach kills support for that extension.

@navintestapi
Copy link
Author

That's correct, that's what I'm doing.

@bartekpacia
Copy link

I also need this feature. Why can't this library just let me set what I want?

@ghost
Copy link

ghost commented May 31, 2021

@bartekpacia I explained in a previous comment why the library cannot just use the header you want.

It will be helpful if you explain why you need the feature. More use cases will strengthen the argument for implementing the feature.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: No status
Development

No branches or pull requests

4 participants