Skip to content

Commit

Permalink
http: do not replace .read() in IncomingMessage
Browse files Browse the repository at this point in the history
Remove the req._consumed property, as its use is completely
superseded and not needed anymore. This was being set in the
overridden .read().

PR-URL: #18939
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: James M Snell <jasnell@gmail.com>
  • Loading branch information
mcollina committed Feb 27, 2018
1 parent 0bff955 commit 29be1e5
Show file tree
Hide file tree
Showing 2 changed files with 54 additions and 19 deletions.
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
54 changes: 54 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,54 @@
'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.resume();

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

0 comments on commit 29be1e5

Please sign in to comment.