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

Unhandled exception in attachWebsocketServer #50

Open
brianhv opened this issue Feb 25, 2024 · 0 comments
Open

Unhandled exception in attachWebsocketServer #50

brianhv opened this issue Feb 25, 2024 · 0 comments

Comments

@brianhv
Copy link

brianhv commented Feb 25, 2024

Minimal reproduction at https://github.com/brianhv/vlcnbug

Something (perhaps in Vite?) seems to be opening a websocket connection to /, which gets handled by attachWebsocketServer even though it should be constrained to /sync. Regardless of how it's happening, this probably shouldn't take down the node process like it does.

Here's a curl request that kills the server:

curl 'http://localhost:8080/' -H 'Sec-WebSocket-Version: 13'\
    -H 'Origin: http://localhost:8080'\
    -H 'Sec-WebSocket-Protocol: vite-hmr'\
    -H 'Sec-WebSocket-Extensions: permessage-deflate'\
    -H 'Connection: keep-alive, Upgrade'\
    -H 'Sec-Fetch-Dest: empty'\
    -H 'Sec-Fetch-Mode: websocket'\
    -H 'Sec-Fetch-Site: same-origin'\
    -H 'Pragma: no-cache'\
    -H 'Cache-Control: no-cache'\
    -H 'Upgrade: websocket'

There are other possible failure modes. For example, not sending the Sec-WebSocket-Protocol header also crashes the server with a different error.

I was able to make things work for me by explicitly exempting / in attachWebsocketServer, but there's probably a more elegant solution.

server.on("upgrade", (request, socket, head) => {
  logger.info("upgrading to ws connection");
  // Added these three lines
  if (request.url === "/") {
    return;
  }
  // ...
}
image image

The output I get:

No logger set! Using default logger.
info: Watching ./dbs/* for changes using polling: false {"service":"ws-server"}
2:32:11 PM [vite-express] Running in development mode
info listening on http://localhost:8080!
2:32:11 PM [vite-express] Using Vite to resolve the config file
info: upgrading to ws connection {"service":"ws-server"}

node:buffer:1343
      throw lazyDOMException('Invalid character', 'InvalidCharacterError');
      ^
DOMException [InvalidCharacterError]: Invalid character
    at new DOMException (node:internal/per_context/domexception:53:5)
    at __node_internal_ (node:internal/util:520:10)
    at atob (node:buffer:1343:13)
    at parseSecHeader (file:///Users/bhv1/Development/vlcnbug/.yarn/cache/@vlcn.io-ws-server-npm-0.2.3-c78b352b98-699685742a.zip/node_modules/@vlcn.io/ws-server/dist/index.js:80:21)
    at pullSecHeaders (file:///Users/bhv1/Development/vlcnbug/.yarn/cache/@vlcn.io-ws-server-npm-0.2.3-c78b352b98-699685742a.zip/node_modules/@vlcn.io/ws-server/dist/index.js:77:12)
    at Server.<anonymous> (file:///Users/bhv1/Development/vlcnbug/.yarn/cache/@vlcn.io-ws-server-npm-0.2.3-c78b352b98-699685742a.zip/node_modules/@vlcn.io/ws-server/dist/index.js:35:25)
    at Server.emit (node:events:526:35)
    at onParserExecuteCommon (node:_http_server:915:14)
    at onParserExecute (node:_http_server:809:3)

Node.js v18.17.0
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

No branches or pull requests

1 participant