Permalink
Browse files

http: Upgrade/CONNECT request should detach its socket earlier

With Upgrade or CONNECT request, http.ClientRequest emits 'close' event
after its socket is closed. However, after receiving a response, the socket
is not under management by the request.

http.ClientRequest should detach the socket before 'upgrade'/'connect'
event is emitted to pass the socket to a user. After that, it should
emit 'close' event immediately without waiting for closing of the socket.
  • Loading branch information...
1 parent 8abb73e commit 97b46ab6e5cc5eadb0030c15d6254cb64c8da46a @koichik committed Jan 11, 2012
Showing with 35 additions and 11 deletions.
  1. +8 −2 lib/http.js
  2. +23 −5 test/simple/test-http-connect.js
  3. +4 −4 test/simple/test-http-upgrade-agent.js
View
@@ -1195,8 +1195,14 @@ ClientRequest.prototype.onSocket = function(socket) {
var eventName = req.method === 'CONNECT' ? 'connect' : 'upgrade';
if (req.listeners(eventName).length) {
req.upgradeOrConnect = true;
- req.emit(eventName, res, socket, bodyHead);
+
+ // detach the socket
socket.emit('agentRemove');
+ socket.removeListener('close', closeListener);
+ socket.removeListener('error', errorListener);
+
+ req.emit(eventName, res, socket, bodyHead);
+ req.emit('close');
} else {
// Got Upgrade header or CONNECT method, but have no handler.
socket.destroy();
@@ -1316,7 +1322,7 @@ ClientRequest.prototype._deferToConnect = function(method, arguments_, cb) {
}
if (cb) { cb(); }
} else {
- self.socket.on('connect', function() {
+ self.socket.once('connect', function() {
if (method) {
self.socket[method].apply(self.socket, arguments_);
}
@@ -53,16 +53,39 @@ server.listen(common.PORT, function() {
}, function(res) {
assert(false);
});
+
+ var clientRequestClosed = false;
+ req.on('close', function() {
+ clientRequestClosed = true;
+ });
+
req.on('connect', function(res, socket, firstBodyChunk) {
common.debug('Client got CONNECT request');
clientGotConnect = true;
+ // Make sure this request got removed from the pool.
+ var name = 'localhost:' + common.PORT;
+ assert(!http.globalAgent.sockets.hasOwnProperty(name));
+ assert(!http.globalAgent.requests.hasOwnProperty(name));
+
+ // Make sure this socket has detached.
+ assert(!socket.ondata);
+ assert(!socket.onend);
+ assert.equal(socket.listeners('connect').length, 0);
+ assert.equal(socket.listeners('data').length, 0);
+ assert.equal(socket.listeners('end').length, 0);
+ assert.equal(socket.listeners('free').length, 0);
+ assert.equal(socket.listeners('close').length, 0);
+ assert.equal(socket.listeners('error').length, 0);
+ assert.equal(socket.listeners('agentRemove').length, 0);
+
var data = firstBodyChunk.toString();
socket.on('data', function(buf) {
data += buf.toString();
});
socket.on('end', function() {
assert.equal(data, 'HeadBody');
+ assert(clientRequestClosed);
server.close();
});
socket.write('Body');
@@ -79,9 +102,4 @@ server.listen(common.PORT, function() {
process.on('exit', function() {
assert.ok(serverGotConnect);
assert.ok(clientGotConnect);
-
- // Make sure this request got removed from the pool.
- var name = 'localhost:' + common.PORT;
- assert(!http.globalAgent.sockets.hasOwnProperty(name));
- assert(!http.globalAgent.requests.hasOwnProperty(name));
});
@@ -74,11 +74,11 @@ srv.listen(common.PORT, '127.0.0.1', function() {
'connection': 'upgrade',
'upgrade': 'websocket' };
assert.deepEqual(expectedHeaders, res.headers);
- assert.equal(http.globalAgent.sockets[name].length, 1);
- process.nextTick(function() {
- // Make sure this request got removed from the pool.
- assert(!http.globalAgent.sockets.hasOwnProperty(name));
+ // Make sure this request got removed from the pool.
+ assert(!http.globalAgent.sockets.hasOwnProperty(name));
+
+ req.on('close', function() {
socket.end();
srv.close();

0 comments on commit 97b46ab

Please sign in to comment.