Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

http: do not replace .read() in IncomingMessage #18939

Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 0 additions & 19 deletions lib/_http_incoming.js
Original file line number Diff line number Diff line change
Expand Up @@ -38,13 +38,6 @@ function readStop(socket) {
function IncomingMessage(socket) {
Stream.Readable.call(this);

// Set this to `true` so that stream.Readable won't attempt to read more
// data on `IncomingMessage#push` (see `maybeReadMore` in
// `_stream_readable.js`). This is important for proper tracking of
// `IncomingMessage#_consuming` which is used to dump requests that users
// haven't attempted to read.
this._readableState.readingMore = true;

this.socket = socket;
this.connection = socket;

Expand All @@ -70,9 +63,6 @@ function IncomingMessage(socket) {
this.statusMessage = null;
this.client = socket;

// flag for backwards compatibility grossness.
this._consuming = false;

// flag for when we decide that this message cannot possibly be
// read by the user, so there's no point continuing to handle it.
this._dumped = false;
Expand All @@ -88,15 +78,6 @@ IncomingMessage.prototype.setTimeout = function setTimeout(msecs, callback) {
};


IncomingMessage.prototype.read = function read(n) {
if (!this._consuming)
this._readableState.readingMore = false;
this._consuming = true;
this.read = Stream.Readable.prototype.read;
return this.read(n);
};


IncomingMessage.prototype._read = function _read(n) {
// We actually do almost nothing here, because the parserOnBody
// function fills up our internal buffer directly. However, we
Expand Down
60 changes: 60 additions & 0 deletions test/parallel/test-http-dump-req-when-res-ends.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
'use strict'

const common = require('../common');
const http = require('http');
const fs = require('fs');

const server = http.createServer(function(req, res) {
// this checks if the request gets dumped
req.on('resume', common.mustCall(function() {
console.log('resume called');

req.on('data', common.mustCallAtLeast(function(d) {
console.log('data', d)
}, 1));
}));

// end is not called as we are just exhausting
// the in-memory buffer
req.on('end', common.mustNotCall);

// this 'data' handler will be removed when dumped
req.on('data', common.mustNotCall);

// start sending the response
res.flushHeaders();

setTimeout(function () {
res.end('hello world');
}, common.platformTimeout(100));
});

server.listen(0, function() {
const req = http.request({
method: 'POST',
port: server.address().port
})

// Send the http request without waiting
// for the body
req.flushHeaders();

req.on('response', common.mustCall(function (res) {
// pipe the body as soon as we get the headers of the
// response back
fs.createReadStream(__filename).pipe(req);

res.setEncoding('utf8');

let data = ''

res.on('data', function(d) {
data += d;
});

// wait for the response
res.on('end', function() {
server.close();
});
}));
});