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: https://github.com/nodejs/node/pull/20088 Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
This commit is contained in:
parent
bbdb4af0bd
commit
3d480dcf4c
@ -38,6 +38,8 @@ function readStop(socket) {
|
|||||||
function IncomingMessage(socket) {
|
function IncomingMessage(socket) {
|
||||||
Stream.Readable.call(this);
|
Stream.Readable.call(this);
|
||||||
|
|
||||||
|
this._readableState.readingMore = true;
|
||||||
|
|
||||||
this.socket = socket;
|
this.socket = socket;
|
||||||
this.connection = socket;
|
this.connection = socket;
|
||||||
|
|
||||||
@ -63,6 +65,7 @@ function IncomingMessage(socket) {
|
|||||||
this.statusMessage = null;
|
this.statusMessage = null;
|
||||||
this.client = socket;
|
this.client = socket;
|
||||||
|
|
||||||
|
this._consuming = false;
|
||||||
// flag for when we decide that this message cannot possibly be
|
// flag for when we decide that this message cannot possibly be
|
||||||
// read by the user, so there's no point continuing to handle it.
|
// read by the user, so there's no point continuing to handle it.
|
||||||
this._dumped = false;
|
this._dumped = false;
|
||||||
@ -79,6 +82,11 @@ IncomingMessage.prototype.setTimeout = function setTimeout(msecs, callback) {
|
|||||||
|
|
||||||
|
|
||||||
IncomingMessage.prototype._read = function _read(n) {
|
IncomingMessage.prototype._read = function _read(n) {
|
||||||
|
if (!this._consuming) {
|
||||||
|
this._readableState.readingMore = false;
|
||||||
|
this._consuming = true;
|
||||||
|
}
|
||||||
|
|
||||||
// We actually do almost nothing here, because the parserOnBody
|
// We actually do almost nothing here, because the parserOnBody
|
||||||
// function fills up our internal buffer directly. However, we
|
// function fills up our internal buffer directly. However, we
|
||||||
// do need to unpause the underlying socket so that it flows.
|
// do need to unpause the underlying socket so that it flows.
|
||||||
|
@ -560,7 +560,7 @@ function resOnFinish(req, res, socket, state, server) {
|
|||||||
// if the user never called req.read(), and didn't pipe() or
|
// if the user never called req.read(), and didn't pipe() or
|
||||||
// .resume() or .on('data'), then we call req._dump() so that the
|
// .resume() or .on('data'), then we call req._dump() so that the
|
||||||
// bytes will be pulled off the wire.
|
// bytes will be pulled off the wire.
|
||||||
if (!req._readableState.resumeScheduled)
|
if (!req._consuming && !req._readableState.resumeScheduled)
|
||||||
req._dump();
|
req._dump();
|
||||||
|
|
||||||
res.detachSocket(socket);
|
res.detachSocket(socket);
|
||||||
|
33
test/parallel/test-http-pause-no-dump.js
Normal file
33
test/parallel/test-http-pause-no-dump.js
Normal file
@ -0,0 +1,33 @@
|
|||||||
|
'use strict';
|
||||||
|
|
||||||
|
const common = require('../common');
|
||||||
|
const assert = require('assert');
|
||||||
|
const http = require('http');
|
||||||
|
|
||||||
|
const server = http.createServer(common.mustCall(function(req, res) {
|
||||||
|
req.once('data', common.mustCall(() => {
|
||||||
|
req.pause();
|
||||||
|
res.writeHead(200);
|
||||||
|
res.end();
|
||||||
|
res.on('finish', common.mustCall(() => {
|
||||||
|
assert(!req._dumped);
|
||||||
|
}));
|
||||||
|
}));
|
||||||
|
}));
|
||||||
|
server.listen(0);
|
||||||
|
|
||||||
|
server.on('listening', common.mustCall(function() {
|
||||||
|
const req = http.request({
|
||||||
|
port: this.address().port,
|
||||||
|
method: 'POST',
|
||||||
|
path: '/'
|
||||||
|
}, common.mustCall(function(res) {
|
||||||
|
assert.strictEqual(res.statusCode, 200);
|
||||||
|
res.resume();
|
||||||
|
res.on('end', common.mustCall(() => {
|
||||||
|
server.close();
|
||||||
|
}));
|
||||||
|
}));
|
||||||
|
|
||||||
|
req.end(Buffer.allocUnsafe(1024));
|
||||||
|
}));
|
Loading…
x
Reference in New Issue
Block a user