diff --git a/lib/_http_outgoing.js b/lib/_http_outgoing.js index 32a51d120bad2b..4aa1ef6c364016 100644 --- a/lib/_http_outgoing.js +++ b/lib/_http_outgoing.js @@ -47,6 +47,7 @@ const { ERR_METHOD_NOT_IMPLEMENTED, ERR_STREAM_CANNOT_PIPE, ERR_STREAM_ALREADY_FINISHED, + ERR_STREAM_DESTROYED, ERR_STREAM_WRITE_AFTER_END }, hideStackFrames @@ -57,6 +58,7 @@ const HIGH_WATER_MARK = getDefaultHighWaterMark(); const { CRLF, debug } = common; const kIsCorked = Symbol('isCorked'); +const kErrorEmitted = Symbol('errorEmitted'); const RE_CONN_CLOSE = /(?:^|\W)close(?:$|\W)/i; const RE_TE_CHUNKED = common.chunkExpression; @@ -284,17 +286,40 @@ OutgoingMessage.prototype._send = function _send(data, encoding, callback) { OutgoingMessage.prototype._writeRaw = _writeRaw; function _writeRaw(data, encoding, callback) { const conn = this.socket; - if (conn && conn.destroyed) { - // The socket was destroyed. If we're still trying to write to it, - // then we haven't gotten the 'close' event yet. - return false; - } if (typeof encoding === 'function') { callback = encoding; encoding = null; } + if (conn && conn.destroyed) { + const err = new ERR_STREAM_DESTROYED('write'); + if (callback) { + callback(err); + } + + // We check if there is a listener for 'error' as a backward-compatible fix. + // TODO(mcollina): remove in a follow-up, semver-major PR. + if (this.listenerCount('error') > 0 && + this[kErrorEmitted] === undefined && + !conn._writableState.errorEmitted && + !conn._hadError) { // _hadError could be set by ClientRequest. + + // TODO(mcollina): clean up kErrorEmitted and _hadError + // they can likely be the same state variable. + this[kErrorEmitted] = true; + + // If the message is a ClientRequest, we avoid emitting 'error' more + // than once. + conn._hadError = true; + + process.nextTick(writeAfterEndNT, this, err); + } + // The socket was destroyed. If we're still trying to write to it, + // then we haven't gotten the 'close' event yet. + return false; + } + if (conn && conn._httpMessage === this && conn.writable) { // There might be pending data in the this.output buffer. if (this.outputData.length) { diff --git a/lib/_http_server.js b/lib/_http_server.js index 6dc29c855e414c..41b287745c1148 100644 --- a/lib/_http_server.js +++ b/lib/_http_server.js @@ -46,14 +46,16 @@ const { } = require('internal/http'); const { defaultTriggerAsyncIdScope, - getOrSetAsyncId + getOrSetAsyncId, + symbols: { async_id_symbol } } = require('internal/async_hooks'); const { IncomingMessage } = require('_http_incoming'); const { ERR_HTTP_HEADERS_SENT, ERR_HTTP_INVALID_STATUS_CODE, ERR_INVALID_ARG_TYPE, - ERR_INVALID_CHAR + ERR_INVALID_CHAR, + ERR_STREAM_WRITE_AFTER_END } = require('internal/errors').codes; const Buffer = require('buffer').Buffer; const { @@ -216,11 +218,44 @@ ServerResponse.prototype.detachSocket = function detachSocket(socket) { }; ServerResponse.prototype.writeContinue = function writeContinue(cb) { + if (this.finished) { + const err = new ERR_STREAM_WRITE_AFTER_END(); + const triggerAsyncId = this.socket ? + this.socket[async_id_symbol] : undefined; + defaultTriggerAsyncIdScope(triggerAsyncId, + process.nextTick, + writeAfterEndNT, + this, + err, + cb); + + return true; + } + this._writeRaw(`HTTP/1.1 100 Continue${CRLF}${CRLF}`, 'ascii', cb); this._sent100 = true; }; +function writeAfterEndNT(msg, err, callback) { + msg.emit('error', err); + if (callback) callback(err); +} + ServerResponse.prototype.writeProcessing = function writeProcessing(cb) { + if (this.finished) { + const err = new ERR_STREAM_WRITE_AFTER_END(); + const triggerAsyncId = this.socket ? + this.socket[async_id_symbol] : undefined; + defaultTriggerAsyncIdScope(triggerAsyncId, + process.nextTick, + writeAfterEndNT, + this, + err, + cb); + + return true; + } + this._writeRaw(`HTTP/1.1 102 Processing${CRLF}${CRLF}`, 'ascii', cb); }; diff --git a/test/parallel/test-http-destroyed-socket-write2.js b/test/parallel/test-http-destroyed-socket-write2.js index 551cea19829d93..3cb2c793b23463 100644 --- a/test/parallel/test-http-destroyed-socket-write2.js +++ b/test/parallel/test-http-destroyed-socket-write2.js @@ -41,7 +41,10 @@ server.listen(0, function() { }); function write() { - req.write('hello', function() { + req.write('hello', function(err) { + if (err) { + return; + } setImmediate(write); }); } @@ -52,6 +55,10 @@ server.listen(0, function() { case 'ECONNRESET': break; + // This is also ok, depends on the operating system + case 'ERR_STREAM_DESTROYED': + break; + // On Windows, this sometimes manifests as ECONNABORTED case 'ECONNABORTED': break; diff --git a/test/parallel/test-http-outgoing-socket-destroyed.js b/test/parallel/test-http-outgoing-socket-destroyed.js new file mode 100644 index 00000000000000..61ede110a52e4f --- /dev/null +++ b/test/parallel/test-http-outgoing-socket-destroyed.js @@ -0,0 +1,90 @@ +'use strict'; + +const common = require('../common'); +const { createServer, request } = require('http'); + +{ + const server = createServer((req, res) => { + server.close(); + + req.socket.destroy(); + + res.write('hello', common.expectsError({ + code: 'ERR_STREAM_DESTROYED' + })); + }); + + server.listen(0, common.mustCall(() => { + const req = request({ + host: 'localhost', + port: server.address().port + }); + + req.on('response', common.mustNotCall()); + req.on('error', common.expectsError({ + code: 'ECONNRESET' + })); + + req.end(); + })); +} + +{ + const server = createServer((req, res) => { + server.close(); + + req.socket.destroy(); + + const onError = common.expectsError({ + code: 'ERR_STREAM_DESTROYED' + }); + + res.on('error', (err) => onError(err)); + res.write('hello'); + }); + + server.listen(0, common.mustCall(() => { + const req = request({ + host: 'localhost', + port: server.address().port + }); + + req.on('response', common.mustNotCall()); + req.on('error', common.mustCall()); + + req.end(); + })); +} + +{ + const server = createServer((req, res) => { + res.write('hello'); + req.resume(); + + const onError = common.expectsError({ + code: 'ERR_STREAM_DESTROYED' + }); + + res.on('close', () => { + res.write('world'); + }); + + res.on('error', (err) => { + onError(err); + server.close(); + }); + }); + + server.listen(0, common.mustCall(() => { + const req = request({ + host: 'localhost', + port: server.address().port + }); + + req.on('response', common.mustCall(() => { + req.socket.destroy(); + })); + + req.end(); + })); +} diff --git a/test/parallel/test-http-write-continue-write-after-end.js b/test/parallel/test-http-write-continue-write-after-end.js new file mode 100644 index 00000000000000..77d9351f57974f --- /dev/null +++ b/test/parallel/test-http-write-continue-write-after-end.js @@ -0,0 +1,28 @@ +'use strict'; + +const common = require('../common'); +const { createServer, request } = require('http'); + +{ + const server = createServer((req, res) => { + server.close(); + + res.end(); + res.writeContinue(); + + res.on('error', common.expectsError({ + code: 'ERR_STREAM_WRITE_AFTER_END' + })); + }); + + server.listen(0, common.mustCall(() => { + const req = request({ + host: 'localhost', + port: server.address().port + }); + + req.on('response', common.mustCall((res) => res.resume())); + + req.end(); + })); +} diff --git a/test/parallel/test-http-write-processing-write-after-end.js b/test/parallel/test-http-write-processing-write-after-end.js new file mode 100644 index 00000000000000..013268fdca4eae --- /dev/null +++ b/test/parallel/test-http-write-processing-write-after-end.js @@ -0,0 +1,28 @@ +'use strict'; + +const common = require('../common'); +const { createServer, request } = require('http'); + +{ + const server = createServer((req, res) => { + server.close(); + + res.end(); + res.writeProcessing(); + + res.on('error', common.expectsError({ + code: 'ERR_STREAM_WRITE_AFTER_END' + })); + }); + + server.listen(0, common.mustCall(() => { + const req = request({ + host: 'localhost', + port: server.address().port + }); + + req.on('response', common.mustCall((res) => res.resume())); + + req.end(); + })); +}