From 68193ee1917b442914ef6830c6eae5404eb85bf2 Mon Sep 17 00:00:00 2001 From: Marco Ippolito Date: Mon, 23 Jan 2023 22:13:03 +0100 Subject: [PATCH 1/4] stream: validate writable defaultEncoding --- lib/internal/streams/writable.js | 8 ++++++- .../test-stream-writable-decoded-encoding.js | 24 +++++++++++++++++++ 2 files changed, 31 insertions(+), 1 deletion(-) diff --git a/lib/internal/streams/writable.js b/lib/internal/streams/writable.js index c4d95df9ac7153..4c1452120d4fc9 100644 --- a/lib/internal/streams/writable.js +++ b/lib/internal/streams/writable.js @@ -122,7 +122,13 @@ function WritableState(options, stream, isDuplex) { // Crypto is kind of old and crusty. Historically, its default string // encoding is 'binary' so we have to make this configurable. // Everything else in the universe uses 'utf8', though. - this.defaultEncoding = (options && options.defaultEncoding) || 'utf8'; + const defaultEncoding = options?.defaultEncoding; + + if (defaultEncoding && !Buffer.isEncoding(defaultEncoding)) { + throw new ERR_UNKNOWN_ENCODING(defaultEncoding); + } + + this.defaultEncoding = defaultEncoding || 'utf8'; // Not an actual buffer we keep track of, but a measurement // of how much we're waiting to get pushed to some underlying diff --git a/test/parallel/test-stream-writable-decoded-encoding.js b/test/parallel/test-stream-writable-decoded-encoding.js index 578208a84bdec0..b2caef11d01d9f 100644 --- a/test/parallel/test-stream-writable-decoded-encoding.js +++ b/test/parallel/test-stream-writable-decoded-encoding.js @@ -56,3 +56,27 @@ class MyWritable extends stream.Writable { m.write('some-text', 'utf8'); m.end(); } + +{ + assert.throws(() => { + const m = new MyWritable(null, { + defaultEncoding: 'my invalid encoding', + }); + m.end(); + }, { + code: 'ERR_UNKNOWN_ENCODING', + }); +} + +{ + const w = new MyWritable(function(isBuffer, type, enc) { + assert(!isBuffer); + assert.strictEqual(type, 'string'); + assert.strictEqual(enc, 'hex'); + }, { + defaultEncoding: 'hex', + decodeStrings: false + }); + w.write('asd'); + w.end(); +} From f9a2d212e29ff4c94f6279a5f0ab5312bc3039f5 Mon Sep 17 00:00:00 2001 From: Marco Ippolito Date: Tue, 24 Jan 2023 09:32:37 +0100 Subject: [PATCH 2/4] stream: test falsy defaultEncoding --- .../test-stream-writable-decoded-encoding.js | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/test/parallel/test-stream-writable-decoded-encoding.js b/test/parallel/test-stream-writable-decoded-encoding.js index b2caef11d01d9f..90fea7059a8ea4 100644 --- a/test/parallel/test-stream-writable-decoded-encoding.js +++ b/test/parallel/test-stream-writable-decoded-encoding.js @@ -80,3 +80,16 @@ class MyWritable extends stream.Writable { w.write('asd'); w.end(); } + +{ + const w = new MyWritable(function(isBuffer, type, enc) { + assert(!isBuffer); + assert.strictEqual(type, 'string'); + assert.strictEqual(enc, 'utf8'); + }, { + defaultEncoding: null, + decodeStrings: false + }); + w.write('asd'); + w.end(); +} From 1b962519f3c9658809638f4f4f80307ad30b72eb Mon Sep 17 00:00:00 2001 From: Marco Ippolito Date: Tue, 24 Jan 2023 10:07:00 +0100 Subject: [PATCH 3/4] stream: test object mode --- test/parallel/test-stream-writable-decoded-encoding.js | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/test/parallel/test-stream-writable-decoded-encoding.js b/test/parallel/test-stream-writable-decoded-encoding.js index 90fea7059a8ea4..e3caa9928fda8c 100644 --- a/test/parallel/test-stream-writable-decoded-encoding.js +++ b/test/parallel/test-stream-writable-decoded-encoding.js @@ -93,3 +93,13 @@ class MyWritable extends stream.Writable { w.write('asd'); w.end(); } + +{ + const m = new MyWritable(function(isBuffer, type, enc) { + assert.strictEqual(type, 'object'); + assert.strictEqual(enc, 'utf8'); + }, { defaultEncoding: 'hex', + objectMode: true }); + m.write({ foo: 'bar' }, 'utf8'); + m.end(); +} From 958633dd2ad99e170a38fec0df09264c9f94328c Mon Sep 17 00:00:00 2001 From: Marco Ippolito Date: Wed, 25 Jan 2023 10:27:25 +0100 Subject: [PATCH 4/4] stream: refactor code style --- lib/internal/streams/writable.js | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/lib/internal/streams/writable.js b/lib/internal/streams/writable.js index 4c1452120d4fc9..5d01c854fbd16d 100644 --- a/lib/internal/streams/writable.js +++ b/lib/internal/streams/writable.js @@ -123,13 +123,14 @@ function WritableState(options, stream, isDuplex) { // encoding is 'binary' so we have to make this configurable. // Everything else in the universe uses 'utf8', though. const defaultEncoding = options?.defaultEncoding; - - if (defaultEncoding && !Buffer.isEncoding(defaultEncoding)) { + if (defaultEncoding == null) { + this.defaultEncoding = 'utf8'; + } else if (Buffer.isEncoding(defaultEncoding)) { + this.defaultEncoding = defaultEncoding; + } else { throw new ERR_UNKNOWN_ENCODING(defaultEncoding); } - this.defaultEncoding = defaultEncoding || 'utf8'; - // Not an actual buffer we keep track of, but a measurement // of how much we're waiting to get pushed to some underlying // socket or file.