WebSocket Cookies

[Nerixyz]

Since Deno 1.4, there's the Browser's WebSocket api in Deno which is now the preferred way of using WebSockets. Now in Browsers, it's possible for a site to set cookies which are sent (if valid) with the WebSocket request. In Deno, there's no way of setting these cookies (as there's no "cookie store"), so one has to use the std/ws module which isn't designed for clients. It would be appreciated to have some kind of way to set cookies with the request. I know this isn't straight forward as for example adding a constructor field would break the standard and purpose of implementing the api.

[crowlKats]

Now in Browsers, it's possible for a site to set cookies which are sent (if valid) with the WebSocket request

How does that work? The spec does not mention anything of the sort.

[lucacasonato]

I think in browsers your standard origin cookie jar is sent along.

[Nerixyz]

How does that work? The spec does not mention anything of the sort.

There are cookies sent with the request:

The headers to send appropriate cookies must be a Cookie header whose value is the cookie-string computed from the user's cookie store and the URL url; for these purposes this is not a "non-HTTP" API.

Source

[Nerixyz]

To remain compatible with the web, the WebSocket constructor could accept options as a third (or second) parameter. This it at least how it's done in the Node ws library (headers is an option on http.request().

The options could contain a headers field. This would allow users to set the Cookie header and possibly additional ones.

[Pangoraw]

Is there a consensus on what the API should look like? I would like to create an authenticated WebSocket client from deno but the current API provides no way to modify the underlying initial HTTP request.

I can submit a patch to propose the changes if the API has been validated :+1:

[Nerixyz]

Is there a consensus on what the API should look like?

No. The WebSocket API builds on the fetch API and it will take the cookies of the current context into consideration (that's at least what I think is meant by credentials mode is "include"). As Deno isn't a Browser, the cookies (that aren't present) aren't sent.

I think the most acceptable thing would be the method I proposed. But this would mean, Deno wouldn't have the same implementation as the Browsers. But due to the nature of the WebSocket constructor (it will immediately connect), there's no other step, where the configuration could happen.

[crowlKats]

This is somewhat related to whatwg/html#5862 i guess

[Pangoraw]

With fetch, you can at least workaround it by providing your own Cookie header in the ConnectionInit parameter. Whereas for the current websocket implementation there is nothing you can do. Using a shared cookie jar, it is not possible to make parallel requests as multiple users for example unlike with passing the cookie explicitely.

[crowlKats]

Found this whatwg/websockets#16

[scientific-dev]

Well. In this case you can either use the deno's 0.68.0 standard library where you can send cookies in Headers. https://deno.land/std@0.68.0/ws (I am not sure is it stable but it works fine). I attempted to make a similar implementation to it: https://deno.land/x/custom_socket (I am not sure is it stable but it works fine).

[iacore]

It's already supported in unstable API as WebSocketStream: https://github.com/denoland/deno/pull/11887.

[e3dio]

3 years and 1 line of code later, we now have working Websocket headers ;) running this in production

const headers = new Headers({ 'Set-Cookie': 'Hello=123' });

const { socket, response } = Deno.upgradeWebSocket(request, { headers });
return response;

[e3dio]

I opened different issue to address specifically Server setting headers for Websocket requests. There is no debate that the Server should be allowed to set headers, but there is still debate whether Client should be allowed to set headers, which I don't have an opinion on. I just need the basic functionality of the Server setting headers: https://github.com/denoland/deno/issues/19277

[e3dio]

Regarding allowing the client to set headers on new connections, you should probably do the same as https://github.com/websockets/ws and https://github.com/oven-sh/bun which is allow a 2nd/3rd options parameter new WebSocket(url, { headers })

[crowlKats]

Regarding allowing the client to set headers on new connections, you should probably do the same as https://github.com/websockets/ws and https://github.com/oven-sh/bun which is allow a 2nd/3rd options parameter new WebSocket(url, { headers }). The client can manually send cookies this way if they want to.

This would be going against the spec.

[josephrocca]

@crowlKats Maybe I misunderstand, but why is that going against the spec any more than this is?

• https://github.com/denoland/deno/pull/11887

Surely if WebSocket were to ever get headers, the second parameter (currently an array, protocols) will be allowed to be an options bag with protocols and headers keys?

There's abundent precedent in JS for changing a parameter to an options bag while still allowing the original for backwards-compatibility - e.g.

addEventListener(type, listener)
addEventListener(type, listener, useCapture) // old
addEventListener(type, listener, options) // new

I think Bun has taken the practical approach here that is most helpful for users while being least likely to cause any future problems.

As @lucacasonato said RE WebSocketStream:

This makes sense to me. It feels like a natural extension to the spec. If the spec ever adds header support, it would likely be in this format.

Would that not apply to WebSocket too? Again, I might have misunderstood your point here, so apologies if so.

[crowlKats]

@josephrocca if you currently do new WebSocket("ws://localhost:8000", {foo: "bar"}) in a browser, this will throw an error. On the case of WebSocketStream, object are naturally extensible, and additional fields are ignored, making it not a problem, but changing the parameter type itself does cause issues.

[josephrocca]

if you currently do new WebSocket("ws://localhost:8000", {foo: "bar"}) in a browser this will throw an error

This is actually a good thing - because it means that the Deno/Bun (vs browser) difference can't lead to weird problems where e.g. the indices/keys of a protocol array are treated as keys of the option object, or something strange like that.

If that code didn't throw an error in browsers, it could be a more dangerous extension to add, because it could lead to weird backwards-incompatible behaviour that browsers would never support. The explicit/up-front error makes us safe from that.

For context: I have a bunch of WebSocket code and have suddenly learned that a container platform that I'm moving to requires an authentication header on every request to get through the gateway. This situation is a bit more painful than I thought it'd be due to Deno's choices here. Bun's choice just seems plainly correct to me.

@bartlomieju if you have any thoughts here it'd be good to hear them - if not apologies for ping and please ignore.