Skip to content

Commit 48b5097

Browse files
lpincaMylesBorins
authored andcommitted
http: make request.abort() destroy the socket
`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: #10812 PR-URL: #10818 Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
1 parent f9e121e commit 48b5097

4 files changed

+84
-1
lines changed

lib/_http_client.js

+5-1
Original file line numberDiff line numberDiff line change
@@ -530,7 +530,11 @@ ClientRequest.prototype.onSocket = function(socket) {
530530
function onSocketNT(req, socket) {
531531
if (req.aborted) {
532532
// If we were aborted while waiting for a socket, skip the whole thing.
533-
socket.emit('free');
533+
if (req.socketPath || !req.agent) {
534+
socket.destroy();
535+
} else {
536+
socket.emit('free');
537+
}
534538
} else {
535539
tickOnSocket(req, socket);
536540
}
+36
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
'use strict';
2+
const common = require('../common');
3+
const assert = require('assert');
4+
const http = require('http');
5+
6+
let socketsCreated = 0;
7+
8+
class Agent extends http.Agent {
9+
createConnection(options, oncreate) {
10+
const socket = super.createConnection(options, oncreate);
11+
socketsCreated++;
12+
return socket;
13+
}
14+
}
15+
16+
const server = http.createServer((req, res) => res.end());
17+
18+
server.listen(0, common.mustCall(() => {
19+
const port = server.address().port;
20+
const agent = new Agent({
21+
keepAlive: true,
22+
maxSockets: 1
23+
});
24+
25+
http.get({agent, port}, (res) => res.resume());
26+
27+
const req = http.get({agent, port}, common.fail);
28+
req.abort();
29+
30+
http.get({agent, port}, common.mustCall((res) => {
31+
res.resume();
32+
assert.strictEqual(socketsCreated, 1);
33+
agent.destroy();
34+
server.close();
35+
}));
36+
}));
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
'use strict';
2+
const common = require('../common');
3+
const http = require('http');
4+
const net = require('net');
5+
6+
const server = http.createServer(common.fail);
7+
8+
server.listen(0, common.mustCall(() => {
9+
const req = http.get({
10+
createConnection(options, oncreate) {
11+
const socket = net.createConnection(options, oncreate);
12+
socket.once('close', () => server.close());
13+
return socket;
14+
},
15+
port: server.address().port
16+
});
17+
18+
req.abort();
19+
}));
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
'use strict';
2+
const common = require('../common');
3+
const http = require('http');
4+
5+
const server = http.createServer(common.fail);
6+
7+
class Agent extends http.Agent {
8+
createConnection(options, oncreate) {
9+
const socket = super.createConnection(options, oncreate);
10+
socket.once('close', () => server.close());
11+
return socket;
12+
}
13+
}
14+
15+
common.refreshTmpDir();
16+
17+
server.listen(common.PIPE, common.mustCall(() => {
18+
const req = http.get({
19+
agent: new Agent(),
20+
socketPath: common.PIPE
21+
});
22+
23+
req.abort();
24+
}));

0 commit comments

Comments
 (0)