From a40133d10cdb911b27fe8d46d67a835b0103bbf1 Mon Sep 17 00:00:00 2001 From: Nathan Zadoks Date: Fri, 24 May 2013 19:34:38 +0200 Subject: [PATCH] http: remove bodyHead from 'upgrade' events Streams2 makes this unnecessary. An empty buffer is provided for compatibility. --- doc/api/http.markdown | 20 ++++++++------------ lib/http.js | 10 ++++++++-- test/simple/test-http-upgrade-agent.js | 4 +++- test/simple/test-http-upgrade-client.js | 4 +++- test/simple/test-http-upgrade-client2.js | 5 +++++ test/simple/test-http-upgrade-server.js | 2 +- 6 files changed, 28 insertions(+), 17 deletions(-) diff --git a/doc/api/http.markdown b/doc/api/http.markdown index e93aa2d2dd..7204690e34 100644 --- a/doc/api/http.markdown +++ b/doc/api/http.markdown @@ -93,7 +93,7 @@ not be emitted. ### Event: 'connect' -`function (request, socket, head) { }` +`function (request, socket) { }` Emitted each time a client requests a http CONNECT method. If this event isn't listened for, then clients requesting a CONNECT method will have their @@ -102,8 +102,6 @@ connections closed. * `request` is the arguments for the http request, as it is in the request event. * `socket` is the network socket between the server and client. -* `head` is an instance of Buffer, the first packet of the tunneling stream, - this may be empty. After this event is emitted, the request's socket will not have a `data` event listener, meaning you will need to bind to it in order to handle data @@ -111,7 +109,7 @@ sent to the server on that socket. ### Event: 'upgrade' -`function (request, socket, head) { }` +`function (request, socket) { }` Emitted each time a client requests a http upgrade. If this event isn't listened for, then clients requesting an upgrade will have their connections @@ -120,7 +118,6 @@ closed. * `request` is the arguments for the http request, as it is in the request event. * `socket` is the network socket between the server and client. -* `head` is an instance of Buffer, the first packet of the upgraded stream, this may be empty. After this event is emitted, the request's socket will not have a `data` @@ -595,7 +592,7 @@ Emitted after a socket is assigned to this request. ### Event: 'connect' -`function (response, socket, head) { }` +`function (response, socket) { }` Emitted each time a server responds to a request with a CONNECT method. If this event isn't being listened for, clients receiving a CONNECT method will have @@ -612,14 +609,13 @@ A client server pair that show you how to listen for the `connect` event. res.writeHead(200, {'Content-Type': 'text/plain'}); res.end('okay'); }); - proxy.on('connect', function(req, cltSocket, head) { + proxy.on('connect', function(req, cltSocket) { // connect to an origin server var srvUrl = url.parse('http://' + req.url); var srvSocket = net.connect(srvUrl.port, srvUrl.hostname, function() { cltSocket.write('HTTP/1.1 200 Connection Established\r\n' + 'Proxy-agent: Node-Proxy\r\n' + '\r\n'); - srvSocket.write(head); srvSocket.pipe(cltSocket); cltSocket.pipe(srvSocket); }); @@ -639,7 +635,7 @@ A client server pair that show you how to listen for the `connect` event. var req = http.request(options); req.end(); - req.on('connect', function(res, socket, head) { + req.on('connect', function(res, socket) { console.log('got connected!'); // make a request over an HTTP tunnel @@ -658,7 +654,7 @@ A client server pair that show you how to listen for the `connect` event. ### Event: 'upgrade' -`function (response, socket, head) { }` +`function (response, socket) { }` Emitted each time a server responds to a request with an upgrade. If this event isn't being listened for, clients receiving an upgrade header will have @@ -673,7 +669,7 @@ A client server pair that show you how to listen for the `upgrade` event. res.writeHead(200, {'Content-Type': 'text/plain'}); res.end('okay'); }); - srv.on('upgrade', function(req, socket, head) { + srv.on('upgrade', function(req, socket) { socket.write('HTTP/1.1 101 Web Socket Protocol Handshake\r\n' + 'Upgrade: WebSocket\r\n' + 'Connection: Upgrade\r\n' + @@ -698,7 +694,7 @@ A client server pair that show you how to listen for the `upgrade` event. var req = http.request(options); req.end(); - req.on('upgrade', function(res, socket, upgradeHead) { + req.on('upgrade', function(res, socket) { console.log('got upgraded!'); socket.end(); process.exit(0); diff --git a/lib/http.js b/lib/http.js index ab36cd7491..3c3a56c50b 100644 --- a/lib/http.js +++ b/lib/http.js @@ -28,6 +28,9 @@ var FreeList = require('freelist').FreeList; var HTTPParser = process.binding('http_parser').HTTPParser; var assert = require('assert').ok; +// an empty buffer for UPGRADE/CONNECT bodyHead compatibility +var emptyBuffer = new Buffer(0); + var debug; if (process.env.NODE_DEBUG && /http/.test(process.env.NODE_DEBUG)) { debug = function(x) { console.error('HTTP: %s', x); }; @@ -1579,7 +1582,9 @@ function socketOnData(d, start, end) { socket.removeListener('close', socketCloseListener); socket.removeListener('error', socketErrorListener); - req.emit(eventName, res, socket, bodyHead); + socket.unshift(bodyHead); + + req.emit(eventName, res, socket, emptyBuffer); req.emit('close'); } else { // Got Upgrade header or CONNECT method, but have no handler. @@ -1952,7 +1957,8 @@ function connectionListener(socket) { var eventName = req.method === 'CONNECT' ? 'connect' : 'upgrade'; if (EventEmitter.listenerCount(self, eventName) > 0) { - self.emit(eventName, req, req.socket, bodyHead); + socket.unshift(bodyHead); + self.emit(eventName, req, req.socket, emptyBuffer); } else { // Got upgrade header or CONNECT method, but have no handler. socket.destroy(); diff --git a/test/simple/test-http-upgrade-agent.js b/test/simple/test-http-upgrade-agent.js index 1077a983dc..ff8e540556 100644 --- a/test/simple/test-http-upgrade-agent.js +++ b/test/simple/test-http-upgrade-agent.js @@ -67,7 +67,7 @@ srv.listen(common.PORT, '127.0.0.1', function() { req.on('upgrade', function(res, socket, upgradeHead) { // XXX: This test isn't fantastic, as it assumes that the entire response // from the server will arrive in a single data callback - assert.equal(upgradeHead, 'nurtzo'); + assert.equal(upgradeHead, ''); console.log(res.headers); var expectedHeaders = { 'hello': 'world', @@ -78,6 +78,8 @@ srv.listen(common.PORT, '127.0.0.1', function() { // Make sure this request got removed from the pool. assert(!http.globalAgent.sockets.hasOwnProperty(name)); + assert.equal(socket.read(), 'nurtzo'); + req.on('close', function() { socket.end(); srv.close(); diff --git a/test/simple/test-http-upgrade-client.js b/test/simple/test-http-upgrade-client.js index 3bf5beccf5..9361e0b503 100644 --- a/test/simple/test-http-upgrade-client.js +++ b/test/simple/test-http-upgrade-client.js @@ -56,7 +56,7 @@ srv.listen(common.PORT, '127.0.0.1', function() { req.on('upgrade', function(res, socket, upgradeHead) { // XXX: This test isn't fantastic, as it assumes that the entire response // from the server will arrive in a single data callback - assert.equal(upgradeHead, 'nurtzo'); + assert.equal(upgradeHead, ''); console.log(res.headers); var expectedHeaders = {'hello': 'world', @@ -64,6 +64,8 @@ srv.listen(common.PORT, '127.0.0.1', function() { 'upgrade': 'websocket' }; assert.deepEqual(expectedHeaders, res.headers); + assert.equal(socket.read(), 'nurtzo'); + socket.end(); srv.close(); diff --git a/test/simple/test-http-upgrade-client2.js b/test/simple/test-http-upgrade-client2.js index fa39f2a572..53f15d2939 100644 --- a/test/simple/test-http-upgrade-client2.js +++ b/test/simple/test-http-upgrade-client2.js @@ -30,6 +30,9 @@ server.on('upgrade', function(req, socket, head) { socket.write('HTTP/1.1 101 Ok' + CRLF + 'Connection: Upgrade' + CRLF + 'Upgrade: Test' + CRLF + CRLF + 'head'); + socket.on('readable', function() { + socket.read(); + }); socket.on('end', function() { socket.end(); }); @@ -50,6 +53,7 @@ server.listen(common.PORT, function() { wasUpgrade = true; request.removeListener('upgrade', onUpgrade); + socket.unref(); socket.end(); } request.on('upgrade', onUpgrade); @@ -75,6 +79,7 @@ server.listen(common.PORT, function() { successCount++; // Test pass console.log('Pass!'); + server.unref(); server.close(); }); }); diff --git a/test/simple/test-http-upgrade-server.js b/test/simple/test-http-upgrade-server.js index 6a14d39aa5..88b83ca85e 100644 --- a/test/simple/test-http-upgrade-server.js +++ b/test/simple/test-http-upgrade-server.js @@ -55,7 +55,7 @@ function testServer() { 'Connection: Upgrade\r\n' + '\r\n\r\n'); - request_upgradeHead = upgradeHead; + request_upgradeHead = socket.read(); socket.ondata = function(d, start, end) { var data = d.toString('utf8', start, end);