Skip to content

Commit c329843

Browse files
lpincaevanlucas
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 <[email protected]> Reviewed-By: Matteo Collina <[email protected]>
1 parent 2bfd58a commit c329843

4 files changed

+84
-1
lines changed

lib/_http_client.js

+5-1
Original file line numberDiff line numberDiff line change
@@ -629,7 +629,11 @@ ClientRequest.prototype.onSocket = function onSocket(socket) {
629629
function onSocketNT(req, socket) {
630630
if (req.aborted) {
631631
// If we were aborted while waiting for a socket, skip the whole thing.
632-
socket.emit('free');
632+
if (req.socketPath || !req.agent) {
633+
socket.destroy();
634+
} else {
635+
socket.emit('free');
636+
}
633637
} else {
634638
tickOnSocket(req, socket);
635639
}
+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)