Skip to content
This repository has been archived by the owner on Apr 22, 2023. It is now read-only.

Commit

Permalink
Fix double free of parser on error in http.Agent.
Browse files Browse the repository at this point in the history
Thanks to Stéphan Kochen for the fix and Maurice Fonk for reproducing the
bug.

Closes GH-784.
Closes GH-803.
  • Loading branch information
ry committed Mar 18, 2011
1 parent 975d020 commit 66570c1
Show file tree
Hide file tree
Showing 2 changed files with 136 additions and 1 deletion.
1 change: 0 additions & 1 deletion lib/http.js
Original file line number Diff line number Diff line change
Expand Up @@ -1199,7 +1199,6 @@ Agent.prototype._establishNewConnection = function() {
parser.finish();
socket.destroy();
self._removeSocket(socket);
parsers.free(parser);
self._cycle();
});

Expand Down
136 changes: 136 additions & 0 deletions test/simple/test-regress-GH-784.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,136 @@
// Regression test for GH-784
// https://github.com/joyent/node/issues/784
//
// The test works by making a total of 8 requests to the server. The first
// two are made with the server off - they should come back as ECONNREFUSED.
// The next two are made with server on - they should come back successful.
// The next two are made with the server off - and so on. Without the fix
// we were experiencing parse errors and instead of ECONNREFUSED.
var common = require('../common');
var http = require('http');
var assert = require('assert');


var server = http.createServer(function(req, res) {
var body = '';

req.setEncoding('utf8');
req.on('data', function(chunk) {
body += chunk;
});

req.on('end', function() {
assert.equal('PING', body);
res.writeHead(200);
res.end('PONG');
});
});


server.on('listening', pingping);


function serverOn() {
console.error("Server ON");
server.listen(common.PORT);
}


function serverOff() {
console.error("Server OFF");
server.close();
pingping();
}

var responses = []


function afterPing(result) {
responses.push(result);
console.error('afterPing. responses.length = ' + responses.length);
switch (responses.length) {
case 2:
assert.ok(/ECONNREFUSED/.test(responses[0]));
assert.ok(/ECONNREFUSED/.test(responses[1]));
serverOn();
break;

case 4:
assert.ok(/success/.test(responses[2]));
assert.ok(/success/.test(responses[3]));
serverOff();
break;

case 6:
assert.ok(/ECONNREFUSED/.test(responses[4]));
assert.ok(/ECONNREFUSED/.test(responses[5]));
serverOn();
break;

case 8:
assert.ok(/success/.test(responses[6]));
assert.ok(/success/.test(responses[7]));
server.close();
// we should go to process.on('exit') from here.
break;
}
}


function ping() {
console.error("making req");

var opt = {
port: common.PORT,
path: '/ping',
method: 'POST'
};

var req = http.request(opt, function(res) {
var body = '';

res.setEncoding('utf8');
res.on('data', function(chunk) {
body += chunk;
});

res.on('end', function() {
assert.equal('PONG', body);
assert.ok(!hadError);
gotEnd = true;
afterPing("success");
});
});

req.end('PING');

var gotEnd = false;
var hadError = false;

req.on('error', function(error) {
console.log("Error making ping req: " + error);
hadError = true;
assert.ok(!gotEnd);
afterPing(error.message);
});
}



function pingping() {
ping();
ping();
}


pingping();




process.on('exit', function() {
console.error("process.on('exit')");
console.error(responses);

assert.equal(8, responses.length);
});

0 comments on commit 66570c1

Please sign in to comment.