diff --git a/lib/_http_outgoing.js b/lib/_http_outgoing.js index b85e3c54074d82..7b4eb81a084e5d 100644 --- a/lib/_http_outgoing.js +++ b/lib/_http_outgoing.js @@ -83,6 +83,10 @@ function OutgoingMessage() { this._headers = null; this._headerNames = {}; + // When this is written to, it will cork for a tick. This represents + // whether the socket is corked for that reason or not. + this._corkedForWrite = false; + this._onPendingData = null; } util.inherits(OutgoingMessage, Stream); @@ -112,33 +116,26 @@ OutgoingMessage.prototype.setTimeout = function setTimeout(msecs, callback) { // any messages, before ever calling this. In that case, just skip // it, since something else is destroying this connection anyway. OutgoingMessage.prototype.destroy = function destroy(error) { - if (this.socket) + if (this.socket) { + // If the socket is corked from a write, uncork it before destroying it. + if (this._corkedForWrite) + this.socket.uncork(); + this.socket.destroy(error); - else + } else { this.once('socket', function(socket) { socket.destroy(error); }); + } }; // This abstract either writing directly to the socket or buffering it. OutgoingMessage.prototype._send = function _send(data, encoding, callback) { - // This is a shameful hack to get the headers and first body chunk onto - // the same packet. Future versions of Node are going to take care of - // this at a lower level and in a more general way. + // Send the headers before the body. OutgoingMessage#write will cork + // the stream before calling #_send, batching them in the same packet. if (!this._headerSent) { - if (typeof data === 'string' && - encoding !== 'hex' && - encoding !== 'base64') { - data = this._header + data; - } else { - this.output.unshift(this._header); - this.outputEncodings.unshift('latin1'); - this.outputCallbacks.unshift(null); - this.outputSize += this._header.length; - if (typeof this._onPendingData === 'function') - this._onPendingData(this._header.length); - } + this._writeRaw(this._header, 'latin1', null); this._headerSent = true; } return this._writeRaw(data, encoding, callback); @@ -158,10 +155,10 @@ function _writeRaw(data, encoding, callback) { connection.writable && !connection.destroyed) { // There might be pending data in the this.output buffer. - var outputLength = this.output.length; - if (outputLength > 0) { - this._flushOutput(connection); - } else if (data.length === 0) { + this._flushOutput(connection); + + // Avoid writing empty messages, but trigger the callback. + if (data.length === 0) { if (typeof callback === 'function') process.nextTick(callback); return true; @@ -465,31 +462,26 @@ OutgoingMessage.prototype.write = function write(chunk, encoding, callback) { // signal the user to keep writing. if (chunk.length === 0) return true; + // By corking the socket and uncorking after a tick, we manage to + // batch writes together, i.e. the header with the body. + if (this.connection && !this._corkedForWrite) { + this.connection.cork(); + this._corkedForWrite = true; + process.nextTick(connectionCorkNT, this); + } + var len, ret; if (this.chunkedEncoding) { - if (typeof chunk === 'string' && - encoding !== 'hex' && - encoding !== 'base64' && - encoding !== 'latin1') { + if (typeof chunk === 'string') { len = Buffer.byteLength(chunk, encoding); - chunk = len.toString(16) + CRLF + chunk + CRLF; - ret = this._send(chunk, encoding, callback); } else { - // buffer, or a non-toString-friendly encoding - if (typeof chunk === 'string') - len = Buffer.byteLength(chunk, encoding); - else - len = chunk.length; - - if (this.connection && !this.connection.corked) { - this.connection.cork(); - process.nextTick(connectionCorkNT, this.connection); - } - this._send(len.toString(16), 'latin1', null); - this._send(crlf_buf, null, null); - this._send(chunk, encoding, null); - ret = this._send(crlf_buf, null, callback); + len = chunk.length; } + + this._send(len.toString(16), 'latin1', null); + this._send(crlf_buf, null, null); + this._send(chunk, encoding, null); + ret = this._send(crlf_buf, null, callback); } else { ret = this._send(chunk, encoding, callback); } @@ -505,8 +497,9 @@ function writeAfterEndNT(self, err, callback) { } -function connectionCorkNT(conn) { - conn.uncork(); +function connectionCorkNT(msg) { + msg.connection.uncork(); + msg._corkedForWrite = false; } diff --git a/test/parallel/test-http-write-write-destroy.js b/test/parallel/test-http-write-write-destroy.js new file mode 100644 index 00000000000000..ccc556efcedf37 --- /dev/null +++ b/test/parallel/test-http-write-write-destroy.js @@ -0,0 +1,57 @@ +'use strict'; + +const common = require('../common'); +const assert = require('assert'); +const http = require('http'); + +// Ensure that, in a corked response, calling .destroy() will flush what was +// previously written completely rather than partially or none at all. It +// also makes sure it's written in a single packet. + +var hasFlushed = false; + +var server = http.createServer(common.mustCall((req, res) => { + res.write('first.'); + res.write('.second', function() { + // Set the flag to prove that all data has been written. + hasFlushed = true; + }); + + res.destroy(); + + // If the second callback from the .write() calls hasn't executed before + // the next tick, then the write has been buffered and was sent + // asynchronously. This means it wouldn't have been written regardless of + // corking, making the test irrelevant, so skip it. + process.nextTick(function() { + if (hasFlushed) return; + + common.skip('.write() executed asynchronously'); + process.exit(0); + return; + }); +})); + +server.listen(0); + +server.on('listening', common.mustCall(() => { + // Send a request, and assert the response. + http.get({ + port: server.address().port + }, (res) => { + var data = ''; + + // By ensuring that the 'data' event is only emitted once, we ensure that + // the socket was correctly corked and the data was batched. + res.on('data', common.mustCall(function(chunk) { + data += chunk.toString('latin1'); + }, 2)); + + res.on('end', common.mustCall(function() { + assert.equal(data, 'first..second'); + + res.destroy(); + server.close(); + })); + }); +}));