From 5303db79470084bdac196f1d824d721fd05d54b0 Mon Sep 17 00:00:00 2001 From: Roee Kasher Date: Fri, 30 Jun 2017 23:45:12 +0300 Subject: [PATCH] http: OutgoingMessage change writable after end When an OutgoingMessage is closed (for example, using the `end` method), its 'writable' property should be changed to false - since it is not writable anymore. The 'writable' property should have the opposite value of the 'finished' property. PR-URL: https://github.com/nodejs/node/pull/14024 Fixes: https://github.com/nodejs/node/issues/14023 Reviewed-By: Matteo Collina Reviewed-By: Gireesh Punathil --- lib/_http_outgoing.js | 1 + .../test-http-outgoing-finish-writable.js | 31 +++++++++++++++ ...tgoing-message-data-emitted-after-ended.js | 39 +++++++++++++++++++ 3 files changed, 71 insertions(+) create mode 100644 test/parallel/test-http-outgoing-finish-writable.js create mode 100644 test/parallel/test-pipe-outgoing-message-data-emitted-after-ended.js diff --git a/lib/_http_outgoing.js b/lib/_http_outgoing.js index 5bf27a2e71d1d6..ae0c46b77f3d5a 100644 --- a/lib/_http_outgoing.js +++ b/lib/_http_outgoing.js @@ -606,6 +606,7 @@ OutgoingMessage.prototype.end = function(data, encoding, callback) { this.connection.uncork(); this.finished = true; + this.writable = false; // There is the first message on the outgoing queue, and we've sent // everything to the socket. diff --git a/test/parallel/test-http-outgoing-finish-writable.js b/test/parallel/test-http-outgoing-finish-writable.js new file mode 100644 index 00000000000000..23b456b4354e4d --- /dev/null +++ b/test/parallel/test-http-outgoing-finish-writable.js @@ -0,0 +1,31 @@ +'use strict'; +const common = require('../common'); +const assert = require('assert'); +const http = require('http'); + +// Verify that after calling end() on an `OutgoingMessage` (or a type that +// inherits from `OutgoingMessage`), its `writable` property is set to false. + +const server = http.createServer(common.mustCall(function(req, res) { + assert.strictEqual(res.writable, true); + assert.strictEqual(res.finished, false); + res.end(); + assert.strictEqual(res.writable, false); + assert.strictEqual(res.finished, true); + + server.close(); +})); + +server.listen(0); + +server.on('listening', common.mustCall(function() { + const clientRequest = http.request({ + port: server.address().port, + method: 'GET', + path: '/' + }); + + assert.strictEqual(clientRequest.writable, true); + clientRequest.end(); + assert.strictEqual(clientRequest.writable, false); +})); diff --git a/test/parallel/test-pipe-outgoing-message-data-emitted-after-ended.js b/test/parallel/test-pipe-outgoing-message-data-emitted-after-ended.js new file mode 100644 index 00000000000000..6a5c170e7121cb --- /dev/null +++ b/test/parallel/test-pipe-outgoing-message-data-emitted-after-ended.js @@ -0,0 +1,39 @@ +'use strict'; +const common = require('../common'); +const http = require('http'); +const util = require('util'); +const stream = require('stream'); + +// Verify that when piping a stream to an `OutgoingMessage` (or a type that +// inherits from `OutgoingMessage`), if data is emitted after the +// `OutgoingMessage` was closed - no `write after end` error is raised (this +// should be the case when piping - when writing data directly to the +// `OutgoingMessage` this error should be raised). + +function MyStream() { + stream.call(this); +} +util.inherits(MyStream, stream); + +const server = http.createServer(common.mustCall(function(req, res) { + const myStream = new MyStream(); + myStream.pipe(res); + + process.nextTick(common.mustCall(() => { + res.end(); + myStream.emit('data', 'some data'); + + // If we got here - 'write after end' wasn't raised and the test passed. + process.nextTick(common.mustCall(() => server.close())); + })); +})); + +server.listen(0); + +server.on('listening', common.mustCall(function() { + http.request({ + port: server.address().port, + method: 'GET', + path: '/' + }).end(); +}));