Skip to content

Commit

Permalink
http: make request.abort() destroy the socket
Browse files Browse the repository at this point in the history
`request.abort()` did not destroy the socket if it was called
before a socket was assigned to the request and the request
did not use an `Agent` or a Unix Domain Socket was used.

Fixes: nodejs/node#10812
PR-URL: nodejs/node#10818
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
  • Loading branch information
lpinca committed Mar 9, 2017
1 parent 281999e commit b36e136
Show file tree
Hide file tree
Showing 4 changed files with 84 additions and 1 deletion.
6 changes: 5 additions & 1 deletion lib/_http_client.js
Original file line number Diff line number Diff line change
Expand Up @@ -530,7 +530,11 @@ ClientRequest.prototype.onSocket = function(socket) {
function onSocketNT(req, socket) {
if (req.aborted) {
// If we were aborted while waiting for a socket, skip the whole thing.
socket.emit('free');
if (req.socketPath || !req.agent) {
socket.destroy();
} else {
socket.emit('free');
}
} else {
tickOnSocket(req, socket);
}
Expand Down
36 changes: 36 additions & 0 deletions test/parallel/test-http-abort-queued-2.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
'use strict';
const common = require('../common');
const assert = require('assert');
const http = require('http');

let socketsCreated = 0;

class Agent extends http.Agent {
createConnection(options, oncreate) {
const socket = super.createConnection(options, oncreate);
socketsCreated++;
return socket;
}
}

const server = http.createServer((req, res) => res.end());

server.listen(0, common.mustCall(() => {
const port = server.address().port;
const agent = new Agent({
keepAlive: true,
maxSockets: 1
});

http.get({agent, port}, (res) => res.resume());

const req = http.get({agent, port}, common.fail);
req.abort();

http.get({agent, port}, common.mustCall((res) => {
res.resume();
assert.strictEqual(socketsCreated, 1);
agent.destroy();
server.close();
}));
}));
19 changes: 19 additions & 0 deletions test/parallel/test-http-client-abort-no-agent.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
'use strict';
const common = require('../common');
const http = require('http');
const net = require('net');

const server = http.createServer(common.fail);

server.listen(0, common.mustCall(() => {
const req = http.get({
createConnection(options, oncreate) {
const socket = net.createConnection(options, oncreate);
socket.once('close', () => server.close());
return socket;
},
port: server.address().port
});

req.abort();
}));
24 changes: 24 additions & 0 deletions test/parallel/test-http-client-abort-unix-socket.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
'use strict';
const common = require('../common');
const http = require('http');

const server = http.createServer(common.fail);

class Agent extends http.Agent {
createConnection(options, oncreate) {
const socket = super.createConnection(options, oncreate);
socket.once('close', () => server.close());
return socket;
}
}

common.refreshTmpDir();

server.listen(common.PIPE, common.mustCall(() => {
const req = http.get({
agent: new Agent(),
socketPath: common.PIPE
});

req.abort();
}));

0 comments on commit b36e136

Please sign in to comment.