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

Added CustomHeaders to WebSocketClient #22

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open

Added CustomHeaders to WebSocketClient #22

wants to merge 6 commits into from

Conversation

dallonf
Copy link

@dallonf dallonf commented Oct 18, 2013

First off, thanks so much for this library! I was previously using WebSocket4Net which apparently doesn't handle large messages correctly.

This change adds a CustomHeaders property to WebSocketClient which will send additional HTTP headers on the handshake request. This isn't exactly per the WebSocket spec, but it was needed for a particular project and it could be useful for others as well.

@sta
Copy link
Owner

sta commented Oct 20, 2013

Hello, there.

Well, Your 'pull request' is similar to the issue #15.
I think it's possible to do too many and it's not safe.
So, I ask you, Which HTTP header did you add using that property?

@dallonf
Copy link
Author

dallonf commented Oct 21, 2013

I added an "Authorization" header, which is proprietary to the app I'm working on.

Someone didn't tell our server engineer that the WebSockets spec doesn't support custom headers :P

@sta
Copy link
Owner

sta commented Oct 22, 2013

websocket-sharp supports the HTTP Basic/Digest auth as a client.

ws.SetCredentials ("name", "password", true); // 'true' if Basic auth

Is your "Authorization" header used for neither Basic or Digest auth?

@dallonf
Copy link
Author

dallonf commented Oct 24, 2013

Nope, we have a proprietary format for the Authorization header. It consists of a keyword unique to the app, followed by a Base64 encoding of the user's credentials, followed by a Base64 encoding of the MD5 hash of a special string constructed from certain other HTTP headers... it's really a strange situation.

@sta
Copy link
Owner

sta commented Oct 25, 2013

I've understood your situation at your app.
(I think it's more usual to use Cookie... Well, There is unique situation at your app?)

But, I don't plan to merge your 'pull request'.
I think i should support for each necessary header. (e.g. WebSocket.Origin)

@maqdev
Copy link

maqdev commented May 28, 2014

"I think i should support for each necessary header." — this is wrong in so many ways. You can't support every possible header. It's against HTTP nature.

@sta
Copy link
Owner

sta commented May 28, 2014

@maqdev Huh? What do you mean?

Necessary means necessary for the WebSocket protocol defined in RFC 6455.

I've never said to support Every HTTP header.

@maqdev
Copy link

maqdev commented May 28, 2014

RFC 6455 defines that

... relationship to HTTP is that its handshake is interpreted by
HTTP servers as an Upgrade request

Also it says that

Additional header fields may also be
present, such as cookies [RFC6265].  The format and parsing of
headers is as defined in [RFC2616]

RFC 6455 doesn't restricts HTTP header possible values. And you can't strictly define what headers are necessary in a library designed for a public use.

Sure you can do it as an author of this great library. But I'm also sure that it doesn't make this library better.

@naqoyqatsi
Copy link

sta, what do you suggest for those who need custom headers? to not use your library?

@sta
Copy link
Owner

sta commented May 29, 2014

First of all, i don't plan to support user defined custom header.

what do you suggest for those who need custom headers? to not use your library?

I suggest to use SetCookie instead of custom header.

But I'm also sure that it doesn't make this library better.

I don't think so. The better, i mean, is to be more stable, and is not to be exaggerated specifications.

So, i don't think it's better to be able to add custom header or any header undefined in RFC 6455.

RFC 6455 doesn't restricts HTTP header possible values.

And also, RFC 6455 doesn't describe to use every HTTP header.

I (will) basically follow headers described in Section 1.3 of RFC 6455.

But i will not follow undescribed HTTP headers in RFC 6455.

@naqoyqatsi
Copy link

"I suggest to use SetCookie instead of custom header."

so... you claim that every possible WS server that requires custom headers from its clients is bad-designed - they all should use cookies... is it your thesis? )

@sta
Copy link
Owner

sta commented May 29, 2014

Hmm,,, It's an exaggerated story.

every possible WS server that requires custom headers from its clients is bad-designed

That's just user defined custom matter, and it's not related to the WebSocket protocol spec.

I think that to use cookie is better than to use custom header, because of closer to standard and easier.

But this is my idea, i don't claim that it's best.

@naqoyqatsi
Copy link

It's an interpretation problem. ) WS spec claims that "custom headers are ALLOWED", and it seems you interpret the word "ALLOWED" as "NOT NECESSARY" (not necessary to support custom headers), but imho it should be interpreted as "WS library implementation SHOULD ALLOW custom headers".

I think you'll agree that custom headers (in point of server-side developers' view) don't break any HTTP/WS rules and design guidelines - and no one can say that cookies are always better than custom headers.

And now suppose we have 3rd-party WS server (we cannot modify it) and it requires some custom headers from clients. In this normal(!) case your library becomes useless - is it OK for you?

@sta
Copy link
Owner

sta commented May 29, 2014

WS spec claims that "custom headers are ALLOWED"

Huh? Which section of RFC 6455 is it described in?

@dallonf
Copy link
Author

dallonf commented May 29, 2014

In my scenario (the scenario that prompted me to make this pull request), I don't have any control over the server. The server expects a custom header and rejects my connection if it doesn't receive it. Cookies are not a workaround for this.

I agree that it's generally bad form to require custom headers on the server-side, especially because the WebSocket API on browsers don't support this. But please don't punish client-side developers when server developers don't follow best practices.

@thomaslevesque
Copy link

I'm in the exact same case as @dallonf. The headers are often application dependent, and it's not a bad practice to use custom headers in HTTP... It should be possible to specify any custom header in the handshake request. @sta, I don't understand your position; by refusing this feature, you're making your library useless to many people...

@sta
Copy link
Owner

sta commented Jul 29, 2014

@thomaslevesque My position is very clear. I follow RFC 6455.

And RFC 6455 describes like the following in 4.1. Client Requirements:

The request MAY include any other header fields, for example,
cookies RFC6265 and/or authentication-related header fields
such as the |Authorization| header field RFC2616, which are
processed according to documents that define them.

Does your custom header have public document that defines that?

@thomaslevesque
Copy link

I think we disagree on how the specification should be understood...

The request MAY include any other header fields

I think that part is quite clear and unambiguous; what comes after is just examples of headers that can be included, not an exhaustive list ("for example", "such as"). The way I read it, it clearly means that you can include any header you like.

In any case, even if your interpretation of the specification were correct, for all practical purposes, support for custom headers is necessary; in many cases, the server will refuse the connection if they're not present, which makes this library unusable.

As for me, I know that I can't use it in its current state; perhaps I'll fork it at some point, but for now I'm using WebSocket4Net instead.

@dallonf
Copy link
Author

dallonf commented Jul 29, 2014

Emphasis mine:

The request may include ANY other header fields

@sta
Copy link
Owner

sta commented Jul 29, 2014

I think:

The request may include any other header fields, ..., WHICH ARE PROCESSED ACCORDING TO DOCUMENTS THAT DEFINE THEM.

And @dallonf wrote:

..., especially because the WebSocket API on browsers don't support this.

That's right.

So, to add custom headers is weak for me, under the WebSocket spec.

@thomaslevesque
Copy link

The request may include any other header fields, ..., WHICH ARE PROCESSED ACCORDING TO DOCUMENTS THAT DEFINE THEM.

Nothing says these documents have to be public RFCs. They can be internal design documents that describe the application's API.

And even if browsers don't support custom headers in the handshake request, the specification does say they're allowed, so it's not a valid argument against supporting them.

@sta
Copy link
Owner

sta commented Jul 29, 2014

Nothing says these documents have to be public RFCs. They can be internal design documents that describe the application's API.

How does many people see such INTERNAL design documents? Plz show me.

And even if browsers don't support custom headers in the handshake request, the specification does say they're allowed, so it's not a valid argument against supporting them.

The WebSocket spec allows the headers that have accessible documents (such as RFCs) that define them, not custom headers.

Which section of RFC 6455 is CUSTOM HEADER described in?

@dallonf
Copy link
Author

dallonf commented Jul 29, 2014

RFC 6455 Section 1.2 (emphasis mine):

An unordered set of header fields comes after the leading line in
both cases. The meaning of these header fields is specified in
Section 4 of this document. Additional header fields may also be
present,
such as cookies [RFC6265]. The format and parsing of
headers is as defined in [RFC2616].

And here's a link to RFC2616 for reference; it's just the HTTP spec for headers (HTTP allows for pretty much any header).

I'm curious why you are so opposed to this change.

It's not a difficult improvement, it took me maybe 15 minutes to submit this pull request the first time (it would probably have to be done again, since the library has changed significantly since then and this PR now doesn't merge cleanly).

It's not against the WebSocket spec to include arbitrary headers - at best, it's open for interpretation, which means it's a library's responsibility to enable as many use cases as possible.

And the lack of this feature has prevented more than one developer (that is, everybody in this thread except for you) from using this library without forking it.

I'm offering to make this library more useful for more people. All you have to do is click the Merge button. Is it that difficult?

@thomaslevesque
Copy link

How does many people see such INTERNAL design documents? Plz show me.

What I mean is that not all web APIs are public; a company might have a specific API that uses custom headers, and these headers would typically be described in the technical specifications of the API. That's what I meant by "internal": internal to the company.

But actually, it could also be public documents that just happen not to be a RFC: for instance, if a website offers a public API that uses websockets, it might define custom headers that would be described in the API documentation.

For instance, my company develops an app that uses a custom X-DeviceId header; this header is clearly defined in our documentation, and is necessary for the server to correctly identify the device.

The WebSocket spec allows the headers that have accessible documents (such as RFCs) that define them, not custom headers.

Which section of RFC 6455 is CUSTOM HEADER described in?

They don't specifically mention "custom headers", but RFC 6455 very clearly says that "any other header fields" are allowed, which obviously includes custom headers.

@mliberant
Copy link

+1 Custom headers. There are many 3rd party APIs that require customs headers. Wish I read this sooner.

@PsyonixThomas
Copy link

+1 Custom headers. I need to send "X-Forwarded-For" to simulate a reverse proxy set up in front of my app, but I'm unable to do that with the current codebase. I could set up a reverse proxy, but this would make it easier for me to debug on my development machine.

@cfehr247
Copy link

cfehr247 commented Sep 6, 2017

+1 Custom headers. This is to CONSUME web sockets, which definitely accepts any and all headers as per RFC and literally every single web socket server software in existence. Using this library does not change 3rd party proprietary servers to auto-magically read cookies instead of headers. This package would be used a lot more with custom headers.

@ericdolson
Copy link

+100 for custom headers.

It is required for a library such as this! Thanks btw for this library. I appreciate your time and everything. But, because ws protocol begins as an HTTP handshake, ws should support any and all HTTP(S) headers so that the handshake can be properly authorized or whatever. This should include all known and custom headers, so let the developers choose whatever they want (whitelisting headers would be impossible and pointless). There is no reason headers should be restricted.

@digitalpacman
Copy link

I need this because of the library I'm trying to connect to. I don't get a choice. They use Authorization bearer tokens. They don't use cookie. Literally impossible to integrate with their websocket without something like this.

@cloutiertyler
Copy link

cloutiertyler commented Jul 7, 2018

This is incorrect and borders on absurd. This library does not properly implement RFC 6455 as written. The document states:

The request MAY include any other header fields, for example,
cookies RFC6265 and/or authentication-related header fields
such as the |Authorization| header field RFC2616, which are
processed according to documents that define them.

It also states:

An unordered set of header fields comes after the leading line in
both cases. The meaning of these header fields is specified in
Section 4 of this document. Additional header fields may also be
present, such as cookies [RFC6265]. The format and parsing of
headers is as defined in [RFC2616].

The referenced RFC, RFC 2616, is the "Hypertext Transfer Protocol -- HTTP/1.1" specification. That document defines the format of a message-header to be:

   message-header = field-name ":" [ field-value ]

The format and parsing of a message header says NOTHING about it's contents. There is nothing in RFC 6455 that says the WebSocket library must process a header. Applications are allowed to create and process their own headers according to their own internal documents as they see fit.

This library clearly DOES NOT fully conform to the specification as defined in RFC 6455 and is in violation of its intent.

@Grey-DeMonstr
Copy link

Socket library without custom headers is useless. 7k users are aready uses fork with custom headers.

@clough360
Copy link

Yes, I need this feature too - for a test stub for a mobile app.
I'll use the fork instead.

@andrewmd5
Copy link

andrewmd5 commented Nov 25, 2018

For everyone who was on this thread: deniszykov is doing some amazing work with his WebSocket server/client. It's fast, scalable and has custom headers. https://github.com/deniszykov/WebSocketListener

@thomaslevesque
Copy link

@Codeusa thanks, but this thread is about the websocket client, not server ;)

@andrewmd5
Copy link

@thomaslevesque the library features a WebSocket client.

@thomaslevesque
Copy link

@thomaslevesque the library features a WebSocket client.

Ah, ok. Thanks!

@mirkokg
Copy link

mirkokg commented Apr 6, 2019

No customizable headers and no way to customize User Agent make this library completly unusable. Many sites restrict unknown user agents (connecting with "websocket-sharp/1.0" user agent is no-go), also many sites require custom authentication headers.

If Microsoft had this kind of attitude they would remove methods to modify HTTP Headers in WebRequest and also hardcode ".net Framework" as enforced user agent string. I don't have to add how absurd that is.

@Fenny
Copy link

Fenny commented Nov 2, 2019

@sta
websocket-sharp > 1,938 downloads
websocket-sharp-customheaders > 19,594 downloads

I think this speaks for itself, enjoy people!

https://www.nuget.org/packages/websocket-sharp-customheaders

@PathToLife
Copy link

Hey, can we stop the strict adherence to RFC 6455 in December 2011?

As a new generation of wonderful things come along (O Auth headers), I expected websocket sharp to have an add headers function.

  • trying to get a njs game server working with web sockets, learnt about the basics, spent 3 hours. then tried to add headers.. 00:57 blinks on my 24hr clock.

@thomaslevesque
Copy link

@PathToLife this project is dead. Use the fork ;)
https://www.nuget.org/packages/websocket-sharp-customheaders

@Sergio1C
Copy link

So, i have readed this as a real novell =)

@ericvanderwal
Copy link

Wow, here in 2020 and no custom headers have been implemented. Moving to the fork. Such a shame.

@jijohn14
Copy link

jijohn14 commented Nov 7, 2020

Use .net ClientWebSocket. Refer setrequestheader

@amedley
Copy link

amedley commented Dec 21, 2020

I am definitely going to be using that fork - nice.

@LaivlyCurt
Copy link

https://github.com/felixhao28/websocket-sharp fork is 2606 commits behind, but people still prefer it 10 to 1 just over this one feature! This is a bad hill to die on.

@gkapellmann
Copy link

Has this been resolved?
I really like this project, but will be moving to ClientWebSocket for this same reason.

@DavidZaludek
Copy link

Has this been resolved? I really like this project, but will be moving to ClientWebSocket for this same reason.

Still not resolved, and probably never will be. There are multiple wrappers for .net default ClientWebSocket use one of those.

Willis1776 added a commit to Willis1776/websocket-sharp that referenced this pull request Jan 31, 2024
sta#22 - Added CustomHeaders to WebSocketClient (Enhancement)
sta#198 - Wildcard Regex pattern for Service Paths
sta#730 - add_http_response_code_and_body_when_handshake_error
@realcoloride
Copy link

The fact this has not been resolved and implemented 11 years later is insane. How is that even possible?!

@KatDevsGames
Copy link

It's possible because this library is dead. Everyone switched to the fork.

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

Successfully merging this pull request may close these issues.