Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

Commit 4c6bfbd

Browse filesBrowse files
ronagaddaleax
authored andcommitted
http: fix client response close & aborted
Fixes: #20102 Fixes: #20101 Fixes: #1735 - Response should always emit close. - Response should always emit aborted if aborted. - Response should always emit close after request has emitted close. PR-URL: #20075 Fixes: #17352 Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
1 parent 30aceed commit 4c6bfbd
Copy full SHA for 4c6bfbd

File tree

Expand file treeCollapse file tree

2 files changed

+69
-41
lines changed
Open diff view settings
Filter options
Expand file treeCollapse file tree

2 files changed

+69
-41
lines changed
Open diff view settings
Collapse file

‎lib/_http_client.js‎

Copy file name to clipboardExpand all lines: lib/_http_client.js
+23-16Lines changed: 23 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -328,26 +328,33 @@ function socketCloseListener() {
328328

329329
// NOTE: It's important to get parser here, because it could be freed by
330330
// the `socketOnData`.
331-
var parser = socket.parser;
332-
if (req.res && req.res.readable) {
331+
const parser = socket.parser;
332+
const res = req.res;
333+
if (res) {
333334
// Socket closed before we emitted 'end' below.
334-
if (!req.res.complete) {
335-
req.res.aborted = true;
336-
req.res.emit('aborted');
335+
if (!res.complete) {
336+
res.aborted = true;
337+
res.emit('aborted');
337338
}
338-
var res = req.res;
339-
res.on('end', function() {
339+
req.emit('close');
340+
if (res.readable) {
341+
res.on('end', function() {
342+
this.emit('close');
343+
});
344+
res.push(null);
345+
} else {
340346
res.emit('close');
341-
});
342-
res.push(null);
343-
} else if (!req.res && !req.socket._hadError) {
344-
// This socket error fired before we started to
345-
// receive a response. The error needs to
346-
// fire on the request.
347-
req.socket._hadError = true;
348-
req.emit('error', createHangUpError());
347+
}
348+
} else {
349+
if (!req.socket._hadError) {
350+
// This socket error fired before we started to
351+
// receive a response. The error needs to
352+
// fire on the request.
353+
req.socket._hadError = true;
354+
req.emit('error', createHangUpError());
355+
}
356+
req.emit('close');
349357
}
350-
req.emit('close');
351358

352359
// Too bad. That output wasn't getting written.
353360
// This is pretty terrible that it doesn't raise an error.
Collapse file

‎test/parallel/test-http-response-close.js‎

Copy file name to clipboardExpand all lines: test/parallel/test-http-response-close.js
+46-25Lines changed: 46 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -23,29 +23,50 @@
2323
const common = require('../common');
2424
const http = require('http');
2525

26-
const server = http.createServer(common.mustCall(function(req, res) {
27-
res.writeHead(200);
28-
res.write('a');
26+
{
27+
const server = http.createServer(
28+
common.mustCall((req, res) => {
29+
res.writeHead(200);
30+
res.write('a');
31+
})
32+
);
33+
server.listen(
34+
0,
35+
common.mustCall(() => {
36+
http.get(
37+
{ port: server.address().port },
38+
common.mustCall((res) => {
39+
res.on('data', common.mustCall(() => {
40+
res.destroy();
41+
}));
42+
res.on('close', common.mustCall(() => {
43+
server.close();
44+
}));
45+
})
46+
);
47+
})
48+
);
49+
}
2950

30-
req.on('close', common.mustCall(function() {
31-
console.error('request aborted');
32-
}));
33-
res.on('close', common.mustCall(function() {
34-
console.error('response aborted');
35-
}));
36-
}));
37-
server.listen(0);
38-
39-
server.on('listening', function() {
40-
console.error('make req');
41-
http.get({
42-
port: this.address().port
43-
}, function(res) {
44-
console.error('got res');
45-
res.on('data', function(data) {
46-
console.error('destroy res');
47-
res.destroy();
48-
server.close();
49-
});
50-
});
51-
});
51+
{
52+
const server = http.createServer(
53+
common.mustCall((req, res) => {
54+
res.writeHead(200);
55+
res.end('a');
56+
})
57+
);
58+
server.listen(
59+
0,
60+
common.mustCall(() => {
61+
http.get(
62+
{ port: server.address().port },
63+
common.mustCall((res) => {
64+
res.on('close', common.mustCall(() => {
65+
server.close();
66+
}));
67+
res.resume();
68+
})
69+
);
70+
})
71+
);
72+
}

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.