Skip to content

Commit

Permalink
Revert "http: remove bodyHead from 'upgrade' events"
Browse files Browse the repository at this point in the history
This reverts commit a40133d.

Unfortunately, this breaks socket.io.  Even though it's not strictly an
API change, it is too subtle and in too brittle an area of node, to be
done in a stable branch.

Conflicts:
	doc/api/http.markdown
  • Loading branch information
isaacs committed Jun 13, 2013
1 parent 4847627 commit e850027
Show file tree
Hide file tree
Showing 6 changed files with 18 additions and 28 deletions.
21 changes: 13 additions & 8 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) { }`
`function (request, socket, head) { }`

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,14 +102,16 @@ 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) { }`
`function (request, socket, head) { }`

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 @@ -118,6 +120,8 @@ 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`
event listener, meaning you will need to bind to it in order to handle data
Expand Down Expand Up @@ -591,7 +595,7 @@ Emitted after a socket is assigned to this request.

### Event: 'connect'

`function (response, socket) { }`
`function (response, socket, head) { }`

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 @@ -608,13 +612,14 @@ 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) {
proxy.on('connect', function(req, cltSocket, head) {
// 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 @@ -634,7 +639,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) {
req.on('connect', function(res, socket, head) {
console.log('got connected!');

// make a request over an HTTP tunnel
Expand All @@ -653,7 +658,7 @@ A client server pair that show you how to listen for the `connect` event.

### Event: 'upgrade'

`function (response, socket) { }`
`function (response, socket, head) { }`

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 @@ -668,7 +673,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) {
srv.on('upgrade', function(req, socket, head) {
socket.write('HTTP/1.1 101 Web Socket Protocol Handshake\r\n' +
'Upgrade: WebSocket\r\n' +
'Connection: Upgrade\r\n' +
Expand All @@ -693,7 +698,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) {
req.on('upgrade', function(res, socket, upgradeHead) {
console.log('got upgraded!');
socket.end();
process.exit(0);
Expand Down
10 changes: 2 additions & 8 deletions lib/http.js
Original file line number Diff line number Diff line change
Expand Up @@ -28,9 +28,6 @@ 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 @@ -1582,9 +1579,7 @@ function socketOnData(d, start, end) {
socket.removeListener('close', socketCloseListener);
socket.removeListener('error', socketErrorListener);

socket.unshift(bodyHead);

req.emit(eventName, res, socket, emptyBuffer);
req.emit(eventName, res, socket, bodyHead);
req.emit('close');
} else {
// Got Upgrade header or CONNECT method, but have no handler.
Expand Down Expand Up @@ -1957,8 +1952,7 @@ function connectionListener(socket) {

var eventName = req.method === 'CONNECT' ? 'connect' : 'upgrade';
if (EventEmitter.listenerCount(self, eventName) > 0) {
socket.unshift(bodyHead);
self.emit(eventName, req, req.socket, emptyBuffer);
self.emit(eventName, req, req.socket, bodyHead);
} else {
// Got upgrade header or CONNECT method, but have no handler.
socket.destroy();
Expand Down
4 changes: 1 addition & 3 deletions 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, '');
assert.equal(upgradeHead, 'nurtzo');

console.log(res.headers);
var expectedHeaders = { 'hello': 'world',
Expand All @@ -78,8 +78,6 @@ 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: 1 addition & 3 deletions test/simple/test-http-upgrade-client.js
Original file line number Diff line number Diff line change
Expand Up @@ -56,16 +56,14 @@ 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, '');
assert.equal(upgradeHead, 'nurtzo');

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

request.removeListener('upgrade', onUpgrade);
socket.unref();
socket.end();
}
request.on('upgrade', onUpgrade);
Expand All @@ -79,7 +75,6 @@ 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 = socket.read();
request_upgradeHead = upgradeHead;

socket.ondata = function(d, start, end) {
var data = d.toString('utf8', start, end);
Expand Down

0 comments on commit e850027

Please sign in to comment.