vlcn-io / js

Components to build apps in JavaScript atop cr-sqlite
MIT License
51 stars 6 forks source link

Unhandled exception in attachWebsocketServer #50

Open brianhv opened 4 months ago

brianhv commented 4 months ago

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