Skip to content

Commit

Permalink
http_client: ensure empty socket on error
Browse files Browse the repository at this point in the history
Read all pending data out of the socket on `error` event and ensure that
no `data`/`end` handlers will be invoked on `socket.destroy()`.
Otherwise following assertion happens:

    AssertionError: null == true
        at TLSSocket.socketOnData (_http_client.js:308:3)
        at TLSSocket.emit (events.js:107:17)
        at TLSSocket.Readable.read (_stream_readable.js:373:10)
        at TLSSocket.socketCloseListener (_http_client.js:229:10)
        at TLSSocket.emit (events.js:129:20)
        at TCP.close (net.js:476:12)

Fix: nodejs/node-v0.x-archive#9348
PR-URL: #1103
Reviewed-By: Rod Vagg <rod@vagg.org>
Reviewed-By: Nicu Micleușanu <micnic90@gmail.com>
  • Loading branch information
indutny committed Mar 9, 2015
1 parent 8670613 commit 1a3ca82
Show file tree
Hide file tree
Showing 2 changed files with 50 additions and 1 deletion.
9 changes: 8 additions & 1 deletion lib/_http_client.js
Expand Up @@ -247,7 +247,6 @@ function socketCloseListener() {

function socketErrorListener(err) {
var socket = this;
var parser = socket.parser;
var req = socket._httpMessage;
debug('SOCKET ERROR:', err.message, err.stack);

Expand All @@ -258,10 +257,18 @@ function socketErrorListener(err) {
req.socket._hadError = true;
}

// Handle any pending data
socket.read();

var parser = socket.parser;
if (parser) {
parser.finish();
freeParser(parser, req, socket);
}

// Ensure that no further data will come out of the socket
socket.removeListener('data', socketOnData);
socket.removeListener('end', socketOnEnd);
socket.destroy();
}

Expand Down
42 changes: 42 additions & 0 deletions test/parallel/test-http-client-read-in-error.js
@@ -0,0 +1,42 @@
var net = require('net');
var http = require('http');
var util = require('util');

function Agent() {
http.Agent.call(this);
}
util.inherits(Agent, http.Agent);

Agent.prototype.createConnection = function() {
var self = this;
var socket = new net.Socket();

socket.on('error', function() {
socket.push('HTTP/1.1 200\r\n\r\n');
});

socket.on('newListener', function onNewListener(name) {
if (name !== 'error')
return;
socket.removeListener('newListener', onNewListener);

// Let other listeners to be set up too
process.nextTick(function() {
self.breakSocket(socket);
});
});

return socket;
};

Agent.prototype.breakSocket = function breakSocket(socket) {
socket.emit('error', new Error('Intentional error'));
};

var agent = new Agent();

http.request({
agent: agent
}).once('error', function() {
console.log('ignore');
});

0 comments on commit 1a3ca82

Please sign in to comment.