From d47c4f409c593c4b8e86783df95fc72d89679323 Mon Sep 17 00:00:00 2001 From: Daijiro Wachi Date: Thu, 7 Oct 2021 15:44:19 +0900 Subject: [PATCH 1/5] net: throw error to object mode in Socket Fixes: https://github.com/nodejs/node/issues/40336 --- lib/net.js | 15 ++++++++++ .../test-net-connect-options-invalid.js | 27 ++++++++++++++++++ test/parallel/test-socket-options-invalid.js | 28 +++++++++++++++++++ 3 files changed, 70 insertions(+) create mode 100644 test/parallel/test-net-connect-options-invalid.js create mode 100644 test/parallel/test-socket-options-invalid.js diff --git a/lib/net.js b/lib/net.js index ded48732c2e11e..666854a3ccff60 100644 --- a/lib/net.js +++ b/lib/net.js @@ -30,6 +30,7 @@ const { NumberIsNaN, NumberParseInt, ObjectDefineProperty, + ObjectKeys, ObjectSetPrototypeOf, Symbol, } = primordials; @@ -282,6 +283,20 @@ const kSetNoDelay = Symbol('kSetNoDelay'); function Socket(options) { if (!(this instanceof Socket)) return new Socket(options); + const invalidKeys = [ + 'objectMode', + 'readableObjectMode', + 'writableObjectMode', + ]; + invalidKeys.forEach((invalidKey) => { + if (ObjectKeys(options).includes(invalidKey)) { + throw new ERR_INVALID_ARG_VALUE( + `options.${invalidKey}`, + options[invalidKey], + 'is not supported' + ); + } + }); this.connecting = false; // Problem with this is that users can supply their own handle, that may not diff --git a/test/parallel/test-net-connect-options-invalid.js b/test/parallel/test-net-connect-options-invalid.js new file mode 100644 index 00000000000000..441e2583f3ee99 --- /dev/null +++ b/test/parallel/test-net-connect-options-invalid.js @@ -0,0 +1,27 @@ +'use strict'; +const common = require('../common'); +const assert = require('assert'); +const net = require('net'); + +{ + const invalidKeys = [ + 'objectMode', + 'readableObjectMode', + 'writableObjectMode', + ]; + invalidKeys.forEach((invalidKey) => { + const option = { + ...common.localhostIPv4, + [invalidKey]: true + }; + const message = `The property 'options.${invalidKey}' is not supported. Received true`; + + assert.throws(() => { + net.createConnection(option); + }, { + code: 'ERR_INVALID_ARG_VALUE', + name: 'TypeError', + message: new RegExp(message) + }); + }); +} diff --git a/test/parallel/test-socket-options-invalid.js b/test/parallel/test-socket-options-invalid.js new file mode 100644 index 00000000000000..96dc500cab8f08 --- /dev/null +++ b/test/parallel/test-socket-options-invalid.js @@ -0,0 +1,28 @@ +'use strict'; +const common = require('../common'); +const assert = require('assert'); +const net = require('net'); + +{ + const invalidKeys = [ + 'objectMode', + 'readableObjectMode', + 'writableObjectMode', + ]; + invalidKeys.forEach((invalidKey) => { + const option = { + ...common.localhostIPv4, + [invalidKey]: true + }; + const message = `The property 'options.${invalidKey}' is not supported. Received true`; + + assert.throws(() => { + const socket = new net.Socket(option); + socket.connect({ port: 8080 }); + }, { + code: 'ERR_INVALID_ARG_VALUE', + name: 'TypeError', + message: new RegExp(message) + }); + }); +} From 144fef23f3cf0320b2481c5707d40c5b73195dc9 Mon Sep 17 00:00:00 2001 From: Daijiro Wachi Date: Thu, 7 Oct 2021 16:42:49 +0900 Subject: [PATCH 2/5] net: check objectMode first and then readble || writable Co-authored-by: Luigi Pinca --- lib/net.js | 29 +++++++++++++++-------------- 1 file changed, 15 insertions(+), 14 deletions(-) diff --git a/lib/net.js b/lib/net.js index 666854a3ccff60..0250f7a585ffe6 100644 --- a/lib/net.js +++ b/lib/net.js @@ -283,20 +283,21 @@ const kSetNoDelay = Symbol('kSetNoDelay'); function Socket(options) { if (!(this instanceof Socket)) return new Socket(options); - const invalidKeys = [ - 'objectMode', - 'readableObjectMode', - 'writableObjectMode', - ]; - invalidKeys.forEach((invalidKey) => { - if (ObjectKeys(options).includes(invalidKey)) { - throw new ERR_INVALID_ARG_VALUE( - `options.${invalidKey}`, - options[invalidKey], - 'is not supported' - ); - } - }); + if (options.objectMode) { + throw new ERR_INVALID_ARG_VALUE( + 'options.objectMode', + options.objectMode, + 'is not supported' + ); + } else if (options.readableObjectMode || options.writableObjectMode) { + throw new ERR_INVALID_ARG_VALUE( + `options.${ + options.readableObjectMode ? 'readableObjectMode' : 'writableObjectMode' + }`, + options.readableObjectMode || options.writableObjectMode, + 'is not supported' + ); + } this.connecting = false; // Problem with this is that users can supply their own handle, that may not From 604700633dd71a9866679700e9751cef16ed79ce Mon Sep 17 00:00:00 2001 From: Daijiro Wachi Date: Thu, 7 Oct 2021 17:29:04 +0900 Subject: [PATCH 3/5] net: remove unused ObjectKeys Co-authored-by: Luigi Pinca --- lib/net.js | 1 - 1 file changed, 1 deletion(-) diff --git a/lib/net.js b/lib/net.js index 0250f7a585ffe6..c367eb4181c85c 100644 --- a/lib/net.js +++ b/lib/net.js @@ -30,7 +30,6 @@ const { NumberIsNaN, NumberParseInt, ObjectDefineProperty, - ObjectKeys, ObjectSetPrototypeOf, Symbol, } = primordials; From b9c41d378aea0df2470e1c425dfa06aee27525cb Mon Sep 17 00:00:00 2001 From: Daijiro Wachi Date: Thu, 7 Oct 2021 18:49:07 +0900 Subject: [PATCH 4/5] test: replace common port with specific number --- test/parallel/test-net-connect-options-invalid.js | 4 ++-- test/parallel/test-socket-options-invalid.js | 3 +-- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/test/parallel/test-net-connect-options-invalid.js b/test/parallel/test-net-connect-options-invalid.js index 441e2583f3ee99..05a565463099ec 100644 --- a/test/parallel/test-net-connect-options-invalid.js +++ b/test/parallel/test-net-connect-options-invalid.js @@ -1,5 +1,5 @@ 'use strict'; -const common = require('../common'); +require('../common'); const assert = require('assert'); const net = require('net'); @@ -11,7 +11,7 @@ const net = require('net'); ]; invalidKeys.forEach((invalidKey) => { const option = { - ...common.localhostIPv4, + port: 8080, [invalidKey]: true }; const message = `The property 'options.${invalidKey}' is not supported. Received true`; diff --git a/test/parallel/test-socket-options-invalid.js b/test/parallel/test-socket-options-invalid.js index 96dc500cab8f08..32bf0810c65692 100644 --- a/test/parallel/test-socket-options-invalid.js +++ b/test/parallel/test-socket-options-invalid.js @@ -1,5 +1,5 @@ 'use strict'; -const common = require('../common'); +require('../common'); const assert = require('assert'); const net = require('net'); @@ -11,7 +11,6 @@ const net = require('net'); ]; invalidKeys.forEach((invalidKey) => { const option = { - ...common.localhostIPv4, [invalidKey]: true }; const message = `The property 'options.${invalidKey}' is not supported. Received true`; From 43ba42e03247cbc025a81aca6c2314eb79d70f83 Mon Sep 17 00:00:00 2001 From: Daijiro Wachi Date: Fri, 8 Oct 2021 14:16:56 +0900 Subject: [PATCH 5/5] net: check if option is undefined --- lib/net.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/net.js b/lib/net.js index c367eb4181c85c..41ff284e1ec027 100644 --- a/lib/net.js +++ b/lib/net.js @@ -282,13 +282,13 @@ const kSetNoDelay = Symbol('kSetNoDelay'); function Socket(options) { if (!(this instanceof Socket)) return new Socket(options); - if (options.objectMode) { + if (options?.objectMode) { throw new ERR_INVALID_ARG_VALUE( 'options.objectMode', options.objectMode, 'is not supported' ); - } else if (options.readableObjectMode || options.writableObjectMode) { + } else if (options?.readableObjectMode || options?.writableObjectMode) { throw new ERR_INVALID_ARG_VALUE( `options.${ options.readableObjectMode ? 'readableObjectMode' : 'writableObjectMode'