Incomplete documentation re handling the HTTP "upgrade" event

[8chanAnon]

Affected URL(s)

https://nodejs.org/api/http.html#event-upgrade

Description of the problem

The sample code fails to show the expected action in response to the event:

req.on('upgrade', (res, socket, upgradeHead) => {
    console.log('got upgraded!');
    socket.end();
    process.exit(0);
  });

What is anyone supposed to do with this? I have searched the Internet and have not found any working samples. After much trial and tribulation, I have come up with this (cribbed from my actual code):

req.on ("upgrade", function (res, socket, upgradeHead)
{
  socket.pipe (response.socket); response.socket.pipe (socket);
  response.writeHead (res.statusCode, res.statusMessage, res.headers);
  response.end ("");
});

I have no idea what upgradeHead is supposed to be or what should be done with it. Anyway, my code is working fine so the above should work as is (though I have not tried it).

[8chanAnon]

I should also mention that my example code is meant for a proxy. The problem that I was trying to solve is how to proxy an HTTP or HTTPS connection that suddenly goes websocket. My proxy has no interest in the communication taking place over the socket. That would be a totally different matter altogether. In which case, the sample code in the Nodejs doc would be even less helpful.

What I have seen is that there is a lot of confusion about the correct action that a proxy should take in the case of a status 101. It appears that many devs have taken to installing ws or some other package as a solution when all that needs to be done is to connect the client and server TCP sockets together. That is, using a shotgun to kill the bug instead of a fly swatter. That's why I felt the importance of mentioning this.

[bnoordhuis]

Pull request welcome.

[parmishh]

req.on('upgrade', (res, socket, upgradeHead) => { // Send a response with a '101 Switching Protocols' status code res.writeHead(101, { 'Upgrade': 'websocket', 'Connection': 'Upgrade' }); console.log('Connection upgraded to WebSocket.'); res.end(""); });

How is this? @bnoordhuis @VoltrexKeyva @8chanAnon Should I do a PR?

[8chanAnon]

@parmishh

All sorts of wrong there. Why create a new server object? "writeStatus" is not a native Node function. And calling "socket.end" will close your websocket.

Anyway, I have finally grokked what to do with upgradeHead. It is a buffer containing a segment of the content following the response headers (you may find that the buffer is usually empty). What to do with this buffer? I've scoured the source code for various websocket implementations and the consensus is that an "unshift" operation needs to be done. Documented here:

https://nodejs.org/api/stream.html#readableunshiftchunk-encoding

What we need to do is call "unshift" on the socket reference passed by the upgrade event (which is just a reference to res.socket). So the correct response would look like this:

req.on ("upgrade", function (res, socket, upgradeHead)
{
  if (upgradeHead.length) socket.unshift (upgradeHead);
  socket.pipe (response.socket); response.socket.pipe (socket);
  response.writeHead (res.statusCode, res.statusMessage, res.headers);
  response.end ("");
});

This is how a proxy should react to the upgrade event. Obviously, if the client intends to participate in the websocket session, the interaction would be more involved and, I guess, may be outside of the intended scope of the documentation.

I'm wondering whether the streams need to be piped before or after "response.end". Maybe it doesn't matter?

[8chanAnon]

@parmishh

What are you doing? You've edited your comment several times. Your sample code is all sorts of wrong, including the fact that you are incorrectly responding to the server's acknowledgement of the upgrade request. The server has terminated the HTTP session and now expects to proceed with a websocket session. The sample code in the Node documentation is itself all sorts of wrong. I've copied the code here for reference:

const http = require('node:http');

// Create an HTTP server
const server = http.createServer((req, res) => {
  res.writeHead(200, { 'Content-Type': 'text/plain' });
  res.end('okay');
});
server.on('upgrade', (req, socket, head) => {
  socket.write('HTTP/1.1 101 Web Socket Protocol Handshake\r\n' +
               'Upgrade: WebSocket\r\n' +
               'Connection: Upgrade\r\n' +
               '\r\n');

  socket.pipe(socket); // echo back
});

// Now that server is running
server.listen(1337, '127.0.0.1', () => {

  // make a request
  const options = {
    port: 1337,
    host: '127.0.0.1',
    headers: {
      'Connection': 'Upgrade',
      'Upgrade': 'websocket',
    },
  };

  const req = http.request(options);
  req.end();

  req.on('upgrade', (res, socket, upgradeHead) => {
    console.log('got upgraded!');
    socket.end();
    process.exit(0);
  });
});

What's wrong with this code is that no websocket session ever takes place. The server does nothing and the client does nothing. This is placeholder code that should never have been included. There is no support for websockets in Node except for this upgrade event.

I'm not sure what to do with this. It is plainly a simple matter from the perspective of a proxy which wants only to connect the two parties with each other (this is my interest in the matter). Anything deeper than that gets us into a mess of weeds.

What's missing is some indication of how a websocket session should work but that is difficult without native support from Node. The sample code would need to plainly indicate where black boxes would exist. Then maybe it can be useful.

I'll mull this over. My own sample code is confusing as well since it references a "response" object which is not in the Node sample. More context is needed.

[parmishh]

@8chanAnon Actually it seems I have not understood the question properly in the first place. But now I got your point. Apologies for editing the comment many times. I just didn't wanted to add extra comments.

[8chanAnon]

@parmishh

No need for apology. What is your interest in the matter? Are you doing any work with websockets? I'm presently researching how to implement a basic websocket server. I need to do that in order to understand the problem and propose an update to the Node documentation.

[8chanAnon]

I have a basic websocket server up and running. Very basic but it's enough for proof of concept. The source code is below. I know that my coding style does not comply with the style handbook so maybe somebody would like to take a hatchet to this.

This is a server example that echoes plain text and prints it on the console. I don't have a client example. The server was tested with a web browser.

Note that this does not use the "upgrade" event so I'm not sure where this should fit in the HTTP documentation.

const http = require ('http');
const crypto = require ('crypto');

var http_server = http.createServer (open_websocket).listen (8082);
// disable timeouts to keep websocket open
http_server.timeout = http_server.keepAliveTimeout = 0;

function open_websocket (request, response)
{
  var key, headers = { Upgrade: 'websocket', Connection: 'upgrade' };

  if (request.method != "GET" || request.headers ["upgrade"] != "websocket")
  {
    headers = { 'content-type': 'text/plain', 'content-length': 4 }
    response.writeHead (400, "Bad Request", headers);
    response.end ("oops");
    return;
  }

  // web browsers send a security key
  if (key = request.headers ["sec-websocket-key"])
  {
    key += "258EAFA5-E914-47DA-95CA-C5AB0DC85B11";
    key = crypto.createHash('sha1').update(key).digest('base64');
    headers ["Sec-WebSocket-Accept"] = key;
  }

  var sock = response.socket;
  console.log ("@ websocket opened");
  response.writeHead (101, "Switching Protocols", headers);
  response.end ("");

  sock.on ("close", function() { console.log ("@ websocket closed"); });

  sock.on ("data", function (buf)
  {
    // reject frames larger than 5000 bytes
    // accept only opcodes 0 (continuation) or 1 (text)
    // close the socket if the input is not acceptable

    var start = 2, opcode = buf [0] & 15, size = buf [1] & 127;
    if (size == 127 || (opcode != 0 && opcode != 1)) start = 0;
    if (size == 126) { size = buf.readUInt16BE (2); start = 4; }

    if (!start || size > 5000) sock.destroy(); else
    {
      sock.write (buf); print_message (buf, start, buf [1] & 128);
    }
  });
}

function print_message (buf, start, mask)
{
  if (mask)
  {
    mask = buf.readUInt32LE (start);
    start += 4; var i = start, j = buf.length;
    for (; i < j - 3; i += 4) buf.writeUInt32LE (buf.readUInt32LE (i) ^ mask, i);
    for (; i < j; i++) { buf [i] ^= mask & 255; mask = mask >> 8; }
  }
  console.log (buf.toString ('utf8', start));
}

I'm not familiar with doing a pull request. Can somebody handle that?

[8chanAnon]

Found an issue with the previous code sample. Some web browsers (Chrome-based) don't like getting a masked frame. Much ado about nothing. This updated code includes a routine for generating an unmasked text frame for the echo. Plus a few other improvements.

Seems like activity around here dies on the weekend. Is that true?

const http = require ('http');
const crypto = require ('crypto');

var http_server = http.createServer (open_websocket).listen (8082);
// disable timeouts to keep websocket open
http_server.timeout = http_server.keepAliveTimeout = 0;

console.log ("Demo websocket server on port 8082");

// write a text frame
function send_message (sock, msg)
{
  var m = Buffer.from (msg), n = m.length;  // length may not exceed 16 bits
  var x = Buffer.from (n < 126 ? [129, n] : [129, 126, n >> 8, n & 255]);
  if (!sock.destroyed) { sock.write (x); sock.write (m); }
}

// unmask a text frame
function take_message (buf, start, mask)
{
  if (mask)
  {
    mask = buf.readUInt32LE (start); start += 4; var i = start, j = buf.length;
    for (; i < j - 3; i += 4) buf.writeUInt32LE (buf.readUInt32LE (i) ^ mask, i);
    for (; i < j; i++) { buf [i] ^= mask & 255; mask = mask >> 8; }
  }
  return (buf.toString ('utf8', start));
}

function open_websocket (request, response)
{
  var key, headers = { Upgrade: 'websocket', Connection: 'upgrade' };

  if (request.method != "GET" || request.headers ["upgrade"] != "websocket")
  {
    response.writeHead (400, "Bad Request", {});
    response.end ("");
    return;
  }

  // compute a security key on request
  if (key = request.headers ["sec-websocket-key"])
  {
    key += "258EAFA5-E914-47DA-95CA-C5AB0DC85B11";
    key = crypto.createHash('sha1').update(key).digest('base64');
    headers ["Sec-WebSocket-Accept"] = key;
  }

  var sock = response.socket;
  response.writeHead (101, "Switching Protocols", headers);
  response.end ("");

  console.log ("@ websocket opened");
  sock.on ("close", function() { console.log ("@ websocket closed"); });
  send_message (sock, "Super-simple websocket echo server.");

  sock.on ("data", function (buf)
  {
    // reject frames larger than 5000 bytes
    // accept only opcodes 0 (continuation) or 1 (text)
    // close the socket if the input is not acceptable

    var start = 2, opcode = buf [0] & 15, size = buf [1] & 127;
    if (size == 127 || (opcode != 0 && opcode != 1)) size = -1;
    if (size == 126) { start = 4; size = buf.readUInt16BE(2); }

    if (size < 0 || size > 5000) sock.destroy(); else
    {
      // the server receives a masked frame but the echo must be unmasked
      var msg = take_message (buf, start, buf [1] & 128);
      send_message (sock, msg);
      console.log (msg);
    }
  });
}

[bnoordhuis]

Samples in the reference documentation should be shorter than ^. Rule of thumb is 10-20 lines max and as little "ceremony" (stuff not directly related to the problem at hand) as possible.

[bnoordhuis]

I'll close this out but please go ahead if you still want to send a pull request.

[8chanAnon]

I'll close this out but please go ahead if you still want to send a pull request.

Doesn't seem like anybody cares. I really have no idea where to take this. I wrote a functioning websocket server in my last post but apparently it's too big to qualify as an example in the Node docs. I have no words.

For the record, there is an issue with this piece of code:

function take_message (buf, start, mask)
{
  if (mask)
  {
    mask = buf.readUInt32LE (start); start += 4; var i = start, j = buf.length;
    for (; i < j - 3; i += 4) buf.writeUInt32LE (buf.readUInt32LE (i) ^ mask, i);
    for (; i < j; i++) { buf [i] ^= mask & 255; mask = mask >> 8; }
  }
  return (buf.toString ('utf8', start));
}

Node will crash because the XOR op can generate a negative result (supposed to be an unsigned integer). That's what I get for trying to be clever. This is the corrected version:

function take_message (buf, start, mask)
{
  if (mask)
  {
    var i = start + 4, j = buf.length, k = 0;
    for (; i < j; i++, k++) buf [i] ^= buf [(k & 3) + start];
  }
  return (buf.toString ('utf8', start + 4));
}