Http request with Connection : "keep-alive, Upgrade" does not upgrade connection.

[Hyvis]

Firefox (version 36.0.1) seems to send WebSocket http requests with header Connection: "keep-alive, Upgrade". Earlier version (0.10.x) of Nodes http-module worked and it emits 'upgrade'-event. At version 0.12.0 it doesn't and http responce has only Connection:'keep-alive' because request is handled like normal GET request with wrong url (ws:// -protocol). Removing 'keep-alive' from request header seem to work and produces 'upgrade'-event. At least node-module: theturtle32/WebSocket-Node (https://github.com/theturtle32/WebSocket-Node) is affected by this and doesn't get 'upgrade'-event.

Sabayon-linux, Node 0.12.0, Firefox 36.0.1

[misterdjules]

It seems that this problem was fixed by nodejs/node@88aaff9. I haven't had the time to reproduce it though.

So my suggestion would be to:

1. Write a node program that doesn't use any external module and that performs the upgrade dance.
2. Make sure it breaks with the current code in the v0.12 branch.
3. Make sure that nodejs/node@88aaff9 fixes the problem.
4. Put nodejs/node@88aaff9 and the node program as a new test in test/simple in the same PR as two separate commits.

[Hyvis]

Yes, problem is in LWS, even single space characher causes issue. nodejs/node@88aaff9 fixes problem. However, current source version 0.13-pre has no problem. Do you still want test for it (copy/paste from nodes api-page http 'upgrade'-event + 'keep-alive, '-string) or let it be because problem is solved.

[misterdjules]

@Hyvis I tried to reproduce the problem with the following sample code, borrowed from the API docs:

var http = require('http');

var SERVER_PORT = 4242;

// Create an HTTP server
var srv = http.createServer(function (req, res) {
  res.writeHead(200, {'Content-Type': 'text/plain'});
  res.end('okay');
});

srv.on('upgrade', function(req, socket, head) {
  console.log('got upgraded on server!')
  socket.write('HTTP/1.1 101 Web Socket Protocol Handshake\r\n' +
               'Upgrade: WebSocket\r\n' +
               'Connection: keep-alive, Upgrade\r\n' +
               '\r\n');

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

// now that server is running
srv.listen(SERVER_PORT, '127.0.0.1', function() {

  // make a request
  var options = {
    port: SERVER_PORT,
    hostname: '127.0.0.1',
    headers: {
      'Connection': 'keep-alive, Upgrade',
      'Upgrade': 'websocket'
    }
  };

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

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

Here's the output I get with node v0.12.0:

➜  v0.12 git:(v0.12.0-release) ✗ ./node --version
v0.12.0
➜  v0.12 git:(v0.12.0-release) ✗ ./node ~/dev/repros/http-upgrade-keepalive.js
got upgraded on server!
got upgraded on client!
➜  v0.12 git:(v0.12.0-release) ✗

Am I missing something?

[misterdjules]

@Hyvis node v0.12 uses http_parser 2.3, which doesn't seem to have this issue. I ran the tests added by nodejs/node@88aaff9 against the version of http_parser in deps/http_parser for node v0.12, and they pass.

Also, the regression described in this issue seems like it may have been introduced by nodejs/http-parser@091ebb8, which is included in http_parser versions 2.4 and later.

Please let me know if I'm missing something. Without any more input I'll have to close this issue as I can't reproduce it. Thank you!

[misterdjules]

@Hyvis Closing because I can't reproduce it, but please feel free to comment further if you think it's still an issue.

[Hyvis]

Sorry that I didn't read email last week and answered to you earlier.

I confirm that, my package manager has installed http_parser version
2.4.1~0, and it has installed at same the time than be problems with node
started in early March. And I now downloaded 0.12.0 and build it from
source. It does not have the problem I reported. I'm sorry I didn't do it
earlier I have something to learn with package manager and open source
projects.

2015-04-22 1:06 GMT+03:00 Julien Gilli [email protected]:

Closed #14191 #14191.

—
Reply to this email directly or view it on GitHub
#14191 (comment).

[misterdjules]

@Hyvis No worries! Out of curiosity, what package manager are you using?

[Hyvis]

I think it's Entropy, but it has Rigo as GUI.

2015-04-22 19:55 GMT+03:00 Julien Gilli [email protected]:

@Hyvis https://github.com/Hyvis No worries! Out of curiosity, what
package manager are you using?

—
Reply to this email directly or view it on GitHub
#14191 (comment).

[misterdjules]

@Hyvis Thanks for the info!

[mikethejet]

Still having this issues in firefox 52.0.2. What node packages i need to upgrade?