Skip to content

Commit 3d480dc

Browse files
committed
http: fix _dump regression
A recent set of changes removed _consuming tracking from server incoming messages which ensures that _dump only runs if the user has never attempted to read the incoming data. Fix by reintroducing _consuming which tracks whether _read was ever successfully called. PR-URL: #20088 Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Trivikram Kamat <[email protected]>
1 parent bbdb4af commit 3d480dc

File tree

3 files changed

+42
-1
lines changed

3 files changed

+42
-1
lines changed

lib/_http_incoming.js

+8
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,8 @@ function readStop(socket) {
3838
function IncomingMessage(socket) {
3939
Stream.Readable.call(this);
4040

41+
this._readableState.readingMore = true;
42+
4143
this.socket = socket;
4244
this.connection = socket;
4345

@@ -63,6 +65,7 @@ function IncomingMessage(socket) {
6365
this.statusMessage = null;
6466
this.client = socket;
6567

68+
this._consuming = false;
6669
// flag for when we decide that this message cannot possibly be
6770
// read by the user, so there's no point continuing to handle it.
6871
this._dumped = false;
@@ -79,6 +82,11 @@ IncomingMessage.prototype.setTimeout = function setTimeout(msecs, callback) {
7982

8083

8184
IncomingMessage.prototype._read = function _read(n) {
85+
if (!this._consuming) {
86+
this._readableState.readingMore = false;
87+
this._consuming = true;
88+
}
89+
8290
// We actually do almost nothing here, because the parserOnBody
8391
// function fills up our internal buffer directly. However, we
8492
// do need to unpause the underlying socket so that it flows.

lib/_http_server.js

+1-1
Original file line numberDiff line numberDiff line change
@@ -560,7 +560,7 @@ function resOnFinish(req, res, socket, state, server) {
560560
// if the user never called req.read(), and didn't pipe() or
561561
// .resume() or .on('data'), then we call req._dump() so that the
562562
// bytes will be pulled off the wire.
563-
if (!req._readableState.resumeScheduled)
563+
if (!req._consuming && !req._readableState.resumeScheduled)
564564
req._dump();
565565

566566
res.detachSocket(socket);
+33
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
'use strict';
2+
3+
const common = require('../common');
4+
const assert = require('assert');
5+
const http = require('http');
6+
7+
const server = http.createServer(common.mustCall(function(req, res) {
8+
req.once('data', common.mustCall(() => {
9+
req.pause();
10+
res.writeHead(200);
11+
res.end();
12+
res.on('finish', common.mustCall(() => {
13+
assert(!req._dumped);
14+
}));
15+
}));
16+
}));
17+
server.listen(0);
18+
19+
server.on('listening', common.mustCall(function() {
20+
const req = http.request({
21+
port: this.address().port,
22+
method: 'POST',
23+
path: '/'
24+
}, common.mustCall(function(res) {
25+
assert.strictEqual(res.statusCode, 200);
26+
res.resume();
27+
res.on('end', common.mustCall(() => {
28+
server.close();
29+
}));
30+
}));
31+
32+
req.end(Buffer.allocUnsafe(1024));
33+
}));

0 commit comments

Comments
 (0)