Skip to content

Commit

Permalink
http: remove bodyHead from 'upgrade' events
Browse files Browse the repository at this point in the history
Streams2 makes this unnecessary.
An empty buffer is provided for compatibility.
  • Loading branch information
edef1c authored and isaacs committed May 24, 2013
1 parent 007e63b commit a40133d
Show file tree
Hide file tree
Showing 6 changed files with 28 additions and 17 deletions.
20 changes: 8 additions & 12 deletions doc/api/http.markdown
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -102,16 +102,14 @@ 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
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
Expand All @@ -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`
Expand Down Expand Up @@ -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
Expand All @@ -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);
});
Expand All @@ -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
Expand All @@ -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
Expand All @@ -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' +
Expand All @@ -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);
Expand Down
10 changes: 8 additions & 2 deletions lib/http.js
Original file line number Diff line number Diff line change
Expand Up @@ -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); };
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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();
Expand Down
4 changes: 3 additions & 1 deletion test/simple/test-http-upgrade-agent.js
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand All @@ -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();
Expand Down
4 changes: 3 additions & 1 deletion test/simple/test-http-upgrade-client.js
Original file line number Diff line number Diff line change
Expand Up @@ -56,14 +56,16 @@ 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',
'connection': 'upgrade',
'upgrade': 'websocket' };
assert.deepEqual(expectedHeaders, res.headers);

assert.equal(socket.read(), 'nurtzo');

socket.end();
srv.close();

Expand Down
5 changes: 5 additions & 0 deletions test/simple/test-http-upgrade-client2.js
Original file line number Diff line number Diff line change
Expand Up @@ -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();
});
Expand All @@ -50,6 +53,7 @@ server.listen(common.PORT, function() {
wasUpgrade = true;

request.removeListener('upgrade', onUpgrade);
socket.unref();
socket.end();
}
request.on('upgrade', onUpgrade);
Expand All @@ -75,6 +79,7 @@ server.listen(common.PORT, function() {
successCount++;
// Test pass
console.log('Pass!');
server.unref();
server.close();
});
});
Expand Down
2 changes: 1 addition & 1 deletion test/simple/test-http-upgrade-server.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down

0 comments on commit a40133d

Please sign in to comment.