oven-sh / bun

Incredibly fast JavaScript runtime, bundler, test runner, and package manager – all in one
https://bun.sh
Other
71.29k stars 2.5k forks source link

`WebSocket` client is acting differently than `ws` module with Node, with the exact same code #4529

Open Vexcited opened 8 months ago

Vexcited commented 8 months ago

What version of Bun is running?

0.8.1+16b4bf341acc0f4804f0b6bdf5298c180cd00366

What platform is your computer?

Linux 5.15.90.1-microsoft-standard-WSL2 x86_64 x86_64

What steps can reproduce the bug?

const ws = new WebSocket("wss://remote-auth-gateway.discord.gg/?v=2", {
  headers: {
    "Origin": "https://discord.com"
  }
});

ws.onmessage = (e) => {
  console.log(e.data);
}

ws.onclose = (e) => {
  console.log("Closed:", e.code, e.reason);
}

What is the expected behavior?

When using, Node (v20) with the ws module from NPM, (so don't forget to add the import WebSocket from "ws"; at the top), with the exact same code, we get the WebSocket connection ready and working:

{"timeout_ms":144636,"op":"hello","heartbeat_interval":41250}

(message received from server on connection)

What do you see instead?

When running the following with Bun (bun run index.ts), we get

Closed: 1002 Expected 101 status code

In fact, we received a 403 status code.

Additional information

Appears to be an issue with the headers casing.

When using Origin: ... in Bun, it sends in reality origin: ... and as you can see, this is incorrect because the o is in lowercase.

HTTP 1.1 specification says that the casing is not sensitive but some servers decided that it is (just see my example above with the Discord Remote Auth Gateway)

This issue happens on every single headers, they're all getting lowercased and that's a major issue.

Vexcited commented 8 months ago

Also, I forgot to precise it, but using ws module in Bun results in the same output

sapter commented 8 months ago

Can confirm this issue. Given the following code in ws.ts:

import WebSocket from "ws";

function initWs() {
  const ws = new WebSocket("wss://remote-auth-gateway.discord.gg/?v=2", {
    headers: {
      Origin: "https://discord.com",
    },
  });

  ws.addEventListener("open", () => {
    console.log("discord client connected!!!");
  });

  ws.addEventListener("error", (e) => {
    console.log(e);
  });

  ws.onmessage = (e) => {
    console.log(e.data);
  };

  ws.onclose = (e) => {
    console.log("Closed:", e.code, e.reason);
  };
}

initWs();

Running Node 19 (via ts-node):

npx ts-node ws.ts
discord client connected!!!
{"timeout_ms":132156,"op":"hello","heartbeat_interval":41250}

Bun:

bun ws.ts
[0.53ms] ".env"
Closed: 1002 Expected 101 status code
Vexcited commented 8 months ago

Still happens on Bun v1.0.0 by the way

Vexcited commented 8 months ago

Found a way to tackle the issue using websocket-driver (github) and node:tls (from bun)

import tls from 'node:tls';
import websocket from 'websocket-driver';

const driver = websocket.client('wss://remote-auth-gateway.discord.gg/?v=2');
driver.setHeader('Origin', 'https://discord.com');

const tcp = tls.connect(443, 'remote-auth-gateway.discord.gg');

tcp.pipe(driver.io).pipe(tcp);

tcp.on('connect', function() {
  driver.start();
});

driver.on('open', function(event) {
  console.log("discord client connected!!!");
})

driver.on('error', function(event) {
  console.error(event);
})

driver.on('close', function(event) {
  console.log('Closed:', event);
})

driver.messages.on('data', function(message) {
  console.log(message);
});

This works! Hope that the global WebSocket gets fixed though...

niemal commented 8 months ago

Yeah I am facing some issues with WebSocketServer from ws package as well. It needs some fixing, hope it gets it!

Vexcited commented 8 months ago

Made a package to make all this easier, until Bun fixes it themselves :')

https://github.com/Vexcited/tcp-websocket

bun add tcp-websocket


import TCPWebSocket from "tcp-websocket";

const ws = new TCPWebSocket("wss://remote-auth-gateway.discord.gg/?v=2", {
  headers: {
    "Origin": "https://discord.com"
  }
});

ws.on("message", (e) => {
  console.log(e.data);
})

ws.on("close", (e) => {
  console.log("Closed:", e.code, e.reason);
});
jackyzha0 commented 7 months ago

Still observing this in 1.0.7 and this is preventing us from using Bun in production. ws.on('message', (evt) => console.log(evt.data.toString()) works as expected but ws.onmessage = (evt) => console.log(evt.data.toString()) for example does not

illusionTBA commented 5 months ago

Still happening after v1.0.15 which is a bummer seeing they fixed so much in the v1.0.15 changelog

paperdave commented 5 months ago

@cirospaciari any idea?

cirospaciari commented 5 months ago

The problem is that we got 403 error when trying to upgrade the connection Need to check if we support passing headers like this on our WebSocket but is probably related with Origin headers.

image

EDIT: Just checked and working on a fix for this

Vexcited commented 5 months ago

but is probably related with Origin headers.

Yes the issue is that: Origin header gets cased to origin and some servers don't support lowercased headers and it just returns an error.

Could happen with any other header to be honest like the Authorization header that gets lowercased to authorization when doing an HTTP request using fetch.

illusionTBA commented 5 months ago

any updates on this?

kerminek commented 2 months ago

v1.0.31 and that bug is still there.

foodornt commented 1 month ago

Solved in bun 1.1 :)

dargmuesli commented 1 month ago

Solved in bun 1.1 :)

not too sure about that. I tried and the connection still didn't persist for too long :sweat_smile: As far as I know there were no additional major changes regarding websockets for bun 1.1. The changes mentioned in the release notes are the ones that were added already in a patch level release, I think.

Vexcited commented 1 month ago

Solved in bun 1.1 :)

Still having issues with the headers casing (it's not applying the case from JS) so it's still bugged for me.

(ex: Some-Header -> some-header and the request to WS is not successful because it checks Some-Header server side and not some-header)

nik-webdevelop commented 1 month ago

Experiencing the same error while using graphql-ws

1002 Expected 101 status code

aayush-khatrii commented 2 weeks ago

i am working on bun version 1.0.29 in May 2024. I am making a request to the other server from BUN + Elysia, header contain the underscore and also capital case: "Auth_Token". when i send the api request through axios the api server responding that token data is not avalible. i think it might be an issue from axios but then i check the header of my request the bun converts the header to lowercase and added the delete unicode : auth\u007ftoken. after upgrading to 1.1.8 version only the underscore appears perfect but the casing issue is still there : auth_token😥@oven-sh/bun

aschafers commented 1 week ago

Hello plssssssss.

Anyone can take a coffe and fix this bug ?