From 1f73c8a954c08eeda6f126fde3612b1c5c8c2da5 Mon Sep 17 00:00:00 2001 From: Robert Nagy Date: Sun, 8 Nov 2020 19:56:56 +0100 Subject: [PATCH 1/8] net: Fix invalid write after end error Don't error if not ended. Fixes: https://github.com/nodejs/node/issues/36029 --- lib/net.js | 5 ++++- test/parallel/test-net-writable.js | 16 ++++++++++++++++ test/parallel/test-stream-transform-end.js | 20 ++++++++++++++++++++ 3 files changed, 40 insertions(+), 1 deletion(-) create mode 100644 test/parallel/test-net-writable.js create mode 100644 test/parallel/test-stream-transform-end.js diff --git a/lib/net.js b/lib/net.js index 45598114770646..b22306596ac906 100644 --- a/lib/net.js +++ b/lib/net.js @@ -434,6 +434,10 @@ function afterShutdown() { // of the other side sending a FIN. The standard 'write after end' // is overly vague, and makes it seem like the user's code is to blame. function writeAfterFIN(chunk, encoding, cb) { + if (!this.writabledEnded) { + return stream.Duplex.prototype.write.call(this, chunk, encoding, cb); + } + if (typeof encoding === 'function') { cb = encoding; encoding = null; @@ -947,7 +951,6 @@ Socket.prototype.connect = function(...args) { this._unrefTimer(); this.connecting = true; - this.writable = true; if (pipe) { validateString(path, 'options.path'); diff --git a/test/parallel/test-net-writable.js b/test/parallel/test-net-writable.js new file mode 100644 index 00000000000000..f293a645617954 --- /dev/null +++ b/test/parallel/test-net-writable.js @@ -0,0 +1,16 @@ +'use strict'; +const common = require('../common'); +const assert = require('assert'); + +const net = require('net'); + +net.createServer(function(s) { + this.close(); + s.end(); +}).listen(0, 'localhost', function() { + const socket = net.connect(this.address().port, 'localhost'); + socket.on('end', common.mustCall(() => { + assert.strictEqual(socket.writable, true); + socket.write('hello world'); + })); +}); diff --git a/test/parallel/test-stream-transform-end.js b/test/parallel/test-stream-transform-end.js new file mode 100644 index 00000000000000..453eaa34b08cba --- /dev/null +++ b/test/parallel/test-stream-transform-end.js @@ -0,0 +1,20 @@ +'use strict'; +const common = require('../common'); +const { Readable, Transform } = require('stream'); + +const src = new Readable({ + read() { + this.push(Buffer.alloc(20000)); + } +}); + +const dst = new Transform({ + transform(chunk, output, fn) { + this.push(null); + fn(); + } +}); + +src.pipe(dst); +dst.resume(); +dst.once('end', common.mustCall()); From 819b441042aef8b775333068beb0862a0409f892 Mon Sep 17 00:00:00 2001 From: Robert Nagy Date: Sun, 8 Nov 2020 23:27:05 +0100 Subject: [PATCH 2/8] fixup --- test/parallel/test-net-writable.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/test/parallel/test-net-writable.js b/test/parallel/test-net-writable.js index f293a645617954..44e4de494e611d 100644 --- a/test/parallel/test-net-writable.js +++ b/test/parallel/test-net-writable.js @@ -4,13 +4,13 @@ const assert = require('assert'); const net = require('net'); -net.createServer(function(s) { +net.createServer(common.mustCall(function(s) { this.close(); s.end(); -}).listen(0, 'localhost', function() { +})).listen(0, 'localhost', common.mustCall(function() { const socket = net.connect(this.address().port, 'localhost'); socket.on('end', common.mustCall(() => { assert.strictEqual(socket.writable, true); socket.write('hello world'); })); -}); +})); From fef1845dabf584d3e3020fb328758bea8f0b9572 Mon Sep 17 00:00:00 2001 From: Robert Nagy Date: Sun, 8 Nov 2020 23:27:56 +0100 Subject: [PATCH 3/8] Update lib/net.js Co-authored-by: Antoine du Hamel --- lib/net.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/net.js b/lib/net.js index b22306596ac906..b7ca11bba1fc99 100644 --- a/lib/net.js +++ b/lib/net.js @@ -435,7 +435,7 @@ function afterShutdown() { // is overly vague, and makes it seem like the user's code is to blame. function writeAfterFIN(chunk, encoding, cb) { if (!this.writabledEnded) { - return stream.Duplex.prototype.write.call(this, chunk, encoding, cb); + return ReflectApply(stream.Duplex.prototype.write, this, [chunk, encoding, cb]); } if (typeof encoding === 'function') { From d1bce40e55c335bc41e2bc04df89c3e5113aa5cb Mon Sep 17 00:00:00 2001 From: Robert Nagy Date: Mon, 9 Nov 2020 08:43:41 +0100 Subject: [PATCH 4/8] fixup --- lib/net.js | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/lib/net.js b/lib/net.js index b7ca11bba1fc99..f5a6567eae0d97 100644 --- a/lib/net.js +++ b/lib/net.js @@ -30,6 +30,7 @@ const { NumberParseInt, ObjectDefineProperty, ObjectSetPrototypeOf, + ReflectApply, Symbol, } = primordials; @@ -435,7 +436,8 @@ function afterShutdown() { // is overly vague, and makes it seem like the user's code is to blame. function writeAfterFIN(chunk, encoding, cb) { if (!this.writabledEnded) { - return ReflectApply(stream.Duplex.prototype.write, this, [chunk, encoding, cb]); + return ReflectApply( + stream.Duplex.prototype.write, this, [chunk, encoding, cb]); } if (typeof encoding === 'function') { From d9d5b77094e56eff2b4e90c57ffb4cd9d80cef1d Mon Sep 17 00:00:00 2001 From: Robert Nagy Date: Mon, 9 Nov 2020 20:52:45 +0100 Subject: [PATCH 5/8] fixup --- test/parallel/test-net-writable.js | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/test/parallel/test-net-writable.js b/test/parallel/test-net-writable.js index 44e4de494e611d..dcbb64e272b830 100644 --- a/test/parallel/test-net-writable.js +++ b/test/parallel/test-net-writable.js @@ -1,11 +1,10 @@ 'use strict'; const common = require('../common'); const assert = require('assert'); - const net = require('net'); -net.createServer(common.mustCall(function(s) { - this.close(); +const server = net.createServer(common.mustCall(function(s) { + server.close(); s.end(); })).listen(0, 'localhost', common.mustCall(function() { const socket = net.connect(this.address().port, 'localhost'); From 1fbd2eba705bf4689fc8cf0a015bac4c6a417eda Mon Sep 17 00:00:00 2001 From: Robert Nagy Date: Mon, 9 Nov 2020 21:05:43 +0100 Subject: [PATCH 6/8] fixup --- test/parallel/test-stream-transform-end.js | 20 -------------------- 1 file changed, 20 deletions(-) delete mode 100644 test/parallel/test-stream-transform-end.js diff --git a/test/parallel/test-stream-transform-end.js b/test/parallel/test-stream-transform-end.js deleted file mode 100644 index 453eaa34b08cba..00000000000000 --- a/test/parallel/test-stream-transform-end.js +++ /dev/null @@ -1,20 +0,0 @@ -'use strict'; -const common = require('../common'); -const { Readable, Transform } = require('stream'); - -const src = new Readable({ - read() { - this.push(Buffer.alloc(20000)); - } -}); - -const dst = new Transform({ - transform(chunk, output, fn) { - this.push(null); - fn(); - } -}); - -src.pipe(dst); -dst.resume(); -dst.once('end', common.mustCall()); From a0ea1b0b97c5136a7a4677e1ca17c8366cde5e26 Mon Sep 17 00:00:00 2001 From: Robert Nagy Date: Mon, 9 Nov 2020 23:51:33 +0100 Subject: [PATCH 7/8] fixup --- lib/net.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/net.js b/lib/net.js index f5a6567eae0d97..4b3b96c8cd2736 100644 --- a/lib/net.js +++ b/lib/net.js @@ -435,7 +435,7 @@ function afterShutdown() { // of the other side sending a FIN. The standard 'write after end' // is overly vague, and makes it seem like the user's code is to blame. function writeAfterFIN(chunk, encoding, cb) { - if (!this.writabledEnded) { + if (!this.writableEnded) { return ReflectApply( stream.Duplex.prototype.write, this, [chunk, encoding, cb]); } From e34e3bc8a46698a6ac2708607d72aeebf0e9bbc1 Mon Sep 17 00:00:00 2001 From: Robert Nagy Date: Tue, 10 Nov 2020 10:13:38 +0100 Subject: [PATCH 8/8] fixup --- test/parallel/test-socket-write-after-fin-error.js | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/test/parallel/test-socket-write-after-fin-error.js b/test/parallel/test-socket-write-after-fin-error.js index 4626729c0598a0..64c8143abebbd9 100644 --- a/test/parallel/test-socket-write-after-fin-error.js +++ b/test/parallel/test-socket-write-after-fin-error.js @@ -26,8 +26,10 @@ const server = net.createServer(function(sock) { }); sock.on('end', function() { gotServerEnd = true; - sock.write(serverData); - sock.end(); + setImmediate(() => { + sock.write(serverData); + sock.end(); + }); }); server.close(); });