From 8f14d9c4a5eddfe0ba49c8fdad772a5513498ded Mon Sep 17 00:00:00 2001 From: Benjamin Chen Date: Tue, 28 Aug 2018 01:51:00 -0400 Subject: [PATCH 1/7] string_decoder: support typed array or data view Fixes: https://github.com/nodejs/node/issues/1826 --- lib/string_decoder.js | 7 ++++++- test/parallel/test-string-decoder.js | 6 +++--- 2 files changed, 9 insertions(+), 4 deletions(-) diff --git a/lib/string_decoder.js b/lib/string_decoder.js index b32249ad9e5830..a6699a015fa66c 100644 --- a/lib/string_decoder.js +++ b/lib/string_decoder.js @@ -73,7 +73,12 @@ StringDecoder.prototype.write = function write(buf) { return buf; if (!ArrayBuffer.isView(buf)) throw new ERR_INVALID_ARG_TYPE('buf', - ['Buffer', 'Uint8Array', 'ArrayBufferView'], + [ + 'Buffer', + 'TypedArray', + 'DataView', + 'ArrayBufferView' + ], buf); return decode(this[kNativeDecoder], buf); }; diff --git a/test/parallel/test-string-decoder.js b/test/parallel/test-string-decoder.js index 6e4f4b121d20d7..463630d5de6c86 100644 --- a/test/parallel/test-string-decoder.js +++ b/test/parallel/test-string-decoder.js @@ -91,7 +91,7 @@ decoder = new StringDecoder('utf8'); assert.strictEqual(decoder.write(Buffer.from('E1', 'hex')), ''); // A quick test for lastChar, lastNeed & lastTotal which are undocumented. -assert(decoder.lastChar.equals(new Uint8Array([0xe1, 0, 0, 0]))); +assert(decoder.lastChar.equals(Buffer.from([0xe1, 0, 0, 0]))); assert.strictEqual(decoder.lastNeed, 2); assert.strictEqual(decoder.lastTotal, 3); @@ -174,8 +174,8 @@ common.expectsError( { code: 'ERR_INVALID_ARG_TYPE', type: TypeError, - message: 'The "buf" argument must be one of type Buffer, Uint8Array, or' + - ' ArrayBufferView. Received type object' + message: 'The "buf" argument must be one of type Buffer, TypedArray,' + + ' DataView, or ArrayBufferView. Received type object' } ); From 7505b4854f403c6fd103ff52baf0c2b19342c0b0 Mon Sep 17 00:00:00 2001 From: Benjamin Chen Date: Tue, 28 Aug 2018 02:09:17 -0400 Subject: [PATCH 2/7] string_decoder: update code format for ERR_INVALID_ARG_TYPE code area Fixes: https://github.com/nodejs/node/issues/1826 --- lib/string_decoder.js | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/lib/string_decoder.js b/lib/string_decoder.js index a6699a015fa66c..f6682e2cceea01 100644 --- a/lib/string_decoder.js +++ b/lib/string_decoder.js @@ -72,14 +72,11 @@ StringDecoder.prototype.write = function write(buf) { if (typeof buf === 'string') return buf; if (!ArrayBuffer.isView(buf)) - throw new ERR_INVALID_ARG_TYPE('buf', - [ - 'Buffer', - 'TypedArray', - 'DataView', - 'ArrayBufferView' - ], - buf); + throw new ERR_INVALID_ARG_TYPE( + 'buf', + ['Buffer', 'TypedArray', 'DataView', 'ArrayBufferView'], + buf + ); return decode(this[kNativeDecoder], buf); }; From 3a25c6e9c103405f730f7429e6d5242f197c5630 Mon Sep 17 00:00:00 2001 From: Benjamin Chen Date: Sat, 1 Sep 2018 00:09:27 -0400 Subject: [PATCH 3/7] string_decoder: update source & tests for TypedArray & DataView per reviews Fixes: https://github.com/nodejs/node/issues/1826 --- lib/string_decoder.js | 8 +++----- test/parallel/test-string-decoder.js | 19 +++++++++++++++++-- 2 files changed, 20 insertions(+), 7 deletions(-) diff --git a/lib/string_decoder.js b/lib/string_decoder.js index f6682e2cceea01..e5f396cab30d3b 100644 --- a/lib/string_decoder.js +++ b/lib/string_decoder.js @@ -72,11 +72,9 @@ StringDecoder.prototype.write = function write(buf) { if (typeof buf === 'string') return buf; if (!ArrayBuffer.isView(buf)) - throw new ERR_INVALID_ARG_TYPE( - 'buf', - ['Buffer', 'TypedArray', 'DataView', 'ArrayBufferView'], - buf - ); + throw new ERR_INVALID_ARG_TYPE('buf', + ['Buffer', 'TypedArray', 'DataView'], + buf); return decode(this[kNativeDecoder], buf); }; diff --git a/test/parallel/test-string-decoder.js b/test/parallel/test-string-decoder.js index 463630d5de6c86..976f316ce29cc6 100644 --- a/test/parallel/test-string-decoder.js +++ b/test/parallel/test-string-decoder.js @@ -91,12 +91,27 @@ decoder = new StringDecoder('utf8'); assert.strictEqual(decoder.write(Buffer.from('E1', 'hex')), ''); // A quick test for lastChar, lastNeed & lastTotal which are undocumented. -assert(decoder.lastChar.equals(Buffer.from([0xe1, 0, 0, 0]))); +assert(decoder.lastChar.equals(new Uint8Array([0xe1, 0, 0, 0]))); assert.strictEqual(decoder.lastNeed, 2); assert.strictEqual(decoder.lastTotal, 3); assert.strictEqual(decoder.end(), '\ufffd'); +// TypedArray with Int8Array test +decoder = new StringDecoder('utf8'); +assert.strictEqual(decoder.write(new Int8Array([1, 2])), '\u0001\u0002'); +assert.strictEqual(decoder.end(), ''); + +// DataView +const buffer = new ArrayBuffer(16); +const dv = new DataView(buffer, 0); +dv.setInt16(1, 42); + +decoder = new StringDecoder('utf8'); +assert.strictEqual(decoder.write(dv), '\u0000\u0000*\u0000\u0000\u0000\u0000' + + '\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000'); +assert.strictEqual(decoder.end(), ''); + decoder = new StringDecoder('utf8'); assert.strictEqual(decoder.write(Buffer.from('E18B', 'hex')), ''); assert.strictEqual(decoder.end(), '\ufffd'); @@ -175,7 +190,7 @@ common.expectsError( code: 'ERR_INVALID_ARG_TYPE', type: TypeError, message: 'The "buf" argument must be one of type Buffer, TypedArray,' + - ' DataView, or ArrayBufferView. Received type object' + ' or DataView. Received type object' } ); From 90575750c305bba811f04e523a36ffdf9f627f5b Mon Sep 17 00:00:00 2001 From: Benjamin Chen Date: Sat, 1 Sep 2018 11:09:58 -0400 Subject: [PATCH 4/7] doc: update .end & .write string_decoder.md Refs: https://github.com/nodejs/node/pull/22562#issuecomment-417852320 --- doc/api/string_decoder.md | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/doc/api/string_decoder.md b/doc/api/string_decoder.md index 1311f372551c9c..c2e5434ac72c4a 100644 --- a/doc/api/string_decoder.md +++ b/doc/api/string_decoder.md @@ -59,7 +59,8 @@ Creates a new `StringDecoder` instance. added: v0.9.3 --> -* `buffer` {Buffer} A `Buffer` containing the bytes to decode. +* `buffer` {Buffer|TypedArray|DataView} A `Buffer`, or `TypedArray`, or + `DataView` containing the bytes to decode. * Returns: {string} Returns any remaining input stored in the internal buffer as a string. Bytes @@ -79,10 +80,11 @@ changes: character instead of one for each individual byte. --> -* `buffer` {Buffer} A `Buffer` containing the bytes to decode. +* `buffer` {Buffer|TypedArray|DataView} A `Buffer`, or `TypedArray`, or + `DataView` containing the bytes to decode. * Returns: {string} Returns a decoded string, ensuring that any incomplete multibyte characters at -the end of the `Buffer` are omitted from the returned string and stored in an -internal buffer for the next call to `stringDecoder.write()` or -`stringDecoder.end()`. + the end of the `Buffer`, or `TypedArray`, or `DataView` are omitted from the + returned string and stored in an internal buffer for the next call to + `stringDecoder.write()` or `stringDecoder.end()`. From 62393c00ba0521388bb07cada43048d9636b32f6 Mon Sep 17 00:00:00 2001 From: Benjamin Chen Date: Tue, 4 Sep 2018 23:13:22 -0400 Subject: [PATCH 5/7] test: update string-decoder tests w. ArrayBufferView --- test/parallel/test-string-decoder.js | 28 ++++++++++++++-------------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/test/parallel/test-string-decoder.js b/test/parallel/test-string-decoder.js index 976f316ce29cc6..20d7ac096881cb 100644 --- a/test/parallel/test-string-decoder.js +++ b/test/parallel/test-string-decoder.js @@ -97,20 +97,20 @@ assert.strictEqual(decoder.lastTotal, 3); assert.strictEqual(decoder.end(), '\ufffd'); -// TypedArray with Int8Array test -decoder = new StringDecoder('utf8'); -assert.strictEqual(decoder.write(new Int8Array([1, 2])), '\u0001\u0002'); -assert.strictEqual(decoder.end(), ''); - -// DataView -const buffer = new ArrayBuffer(16); -const dv = new DataView(buffer, 0); -dv.setInt16(1, 42); - -decoder = new StringDecoder('utf8'); -assert.strictEqual(decoder.write(dv), '\u0000\u0000*\u0000\u0000\u0000\u0000' + - '\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000'); -assert.strictEqual(decoder.end(), ''); +// ArrayBufferView tests +const arrayBufferViewStr = 'String for ArrayBufferView tests\n'; +const inputBuffer = Buffer.from(arrayBufferViewStr.repeat(8), 'utf8'); +for (const expectView of common.getArrayBufferViews(inputBuffer)) { + console.log( + 'string-decoder test with TypedArray for', + expectView[Symbol.toStringTag] + ); + assert.strictEqual( + decoder.write(expectView), + inputBuffer.toString('utf8') + ); + assert.strictEqual(decoder.end(), ''); +} decoder = new StringDecoder('utf8'); assert.strictEqual(decoder.write(Buffer.from('E18B', 'hex')), ''); From 5fb319898b28d284d01ea2e7b120d63844c9dcf9 Mon Sep 17 00:00:00 2001 From: Benjamin Chen Date: Tue, 4 Sep 2018 23:41:00 -0400 Subject: [PATCH 6/7] test: shorten test message in string-decoder tests w. ArrayBufferView --- test/parallel/test-string-decoder.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/parallel/test-string-decoder.js b/test/parallel/test-string-decoder.js index 20d7ac096881cb..381194f9fd4f8f 100644 --- a/test/parallel/test-string-decoder.js +++ b/test/parallel/test-string-decoder.js @@ -102,7 +102,7 @@ const arrayBufferViewStr = 'String for ArrayBufferView tests\n'; const inputBuffer = Buffer.from(arrayBufferViewStr.repeat(8), 'utf8'); for (const expectView of common.getArrayBufferViews(inputBuffer)) { console.log( - 'string-decoder test with TypedArray for', + 'string-decoder test for', expectView[Symbol.toStringTag] ); assert.strictEqual( From 510ac79d85b9c95446d7732beda9b8c9cab6aa26 Mon Sep 17 00:00:00 2001 From: Benjamin Chen Date: Tue, 11 Sep 2018 00:09:40 -0400 Subject: [PATCH 7/7] test: remove console log message from string_decoder's ArrayBufferView tests Refs: https://github.com/nodejs/node/pull/22562#discussion_r216391676 --- test/parallel/test-string-decoder.js | 4 ---- 1 file changed, 4 deletions(-) diff --git a/test/parallel/test-string-decoder.js b/test/parallel/test-string-decoder.js index 381194f9fd4f8f..c4607672b0420c 100644 --- a/test/parallel/test-string-decoder.js +++ b/test/parallel/test-string-decoder.js @@ -101,10 +101,6 @@ assert.strictEqual(decoder.end(), '\ufffd'); const arrayBufferViewStr = 'String for ArrayBufferView tests\n'; const inputBuffer = Buffer.from(arrayBufferViewStr.repeat(8), 'utf8'); for (const expectView of common.getArrayBufferViews(inputBuffer)) { - console.log( - 'string-decoder test for', - expectView[Symbol.toStringTag] - ); assert.strictEqual( decoder.write(expectView), inputBuffer.toString('utf8')