From b99a372e911efefa9a092b694b3db936cdc9e432 Mon Sep 17 00:00:00 2001 From: Anna Henningsen Date: Tue, 29 Nov 2016 05:03:24 -0600 Subject: [PATCH] buffer: fix single-character string filling Fix the fast path for `buffer.fill()` with a single-character string. The fast path only works for strings that are equivalent to a single-byte buffer, but that condition was not checked properly for the `utf8` or `utf16le` encodings and is always true for the `latin1` encoding. This change fixes these problems. Fixes: https://github.com/nodejs/node/issues/9836 PR-URL: https://github.com/nodejs/node/pull/9837 Reviewed-By: James M Snell Reviewed-By: Ben Noordhuis Reviewed-By: Trevor Norris --- lib/buffer.js | 26 +++++++++++++++----------- test/parallel/test-buffer-fill.js | 28 +++++++++++++++++++++++++++- 2 files changed, 42 insertions(+), 12 deletions(-) diff --git a/lib/buffer.js b/lib/buffer.js index 3453f5fe8cd6ae..557ac867e2e0fc 100644 --- a/lib/buffer.js +++ b/lib/buffer.js @@ -672,23 +672,27 @@ Buffer.prototype.fill = function fill(val, start, end, encoding) { encoding = end; end = this.length; } - if (val.length === 1) { - var code = val.charCodeAt(0); - if (code < 256) - val = code; - } - if (val.length === 0) { - // Previously, if val === '', the Buffer would not fill, - // which is rather surprising. - val = 0; - } + if (encoding !== undefined && typeof encoding !== 'string') { throw new TypeError('encoding must be a string'); } - if (typeof encoding === 'string' && !Buffer.isEncoding(encoding)) { + var normalizedEncoding = internalUtil.normalizeEncoding(encoding); + if (normalizedEncoding === undefined) { throw new TypeError('Unknown encoding: ' + encoding); } + if (val.length === 0) { + // Previously, if val === '', the Buffer would not fill, + // which is rather surprising. + val = 0; + } else if (val.length === 1) { + var code = val.charCodeAt(0); + if ((normalizedEncoding === 'utf8' && code < 128) || + normalizedEncoding === 'latin1') { + // Fast path: If `val` fits into a single byte, use that numeric value. + val = code; + } + } } else if (typeof val === 'number') { val = val & 255; } diff --git a/test/parallel/test-buffer-fill.js b/test/parallel/test-buffer-fill.js index 68b8bbd69c4566..eecb14abb06001 100644 --- a/test/parallel/test-buffer-fill.js +++ b/test/parallel/test-buffer-fill.js @@ -395,7 +395,6 @@ assert.throws(() => { buf.fill(''); }, /^RangeError: out of range index$/); - assert.deepStrictEqual( Buffer.allocUnsafeSlow(16).fill('ab', 'utf16le'), Buffer.from('61006200610062006100620061006200', 'hex')); @@ -403,3 +402,30 @@ assert.deepStrictEqual( assert.deepStrictEqual( Buffer.allocUnsafeSlow(15).fill('ab', 'utf16le'), Buffer.from('610062006100620061006200610062', 'hex')); + +assert.deepStrictEqual( + Buffer.allocUnsafeSlow(16).fill('ab', 'utf16le'), + Buffer.from('61006200610062006100620061006200', 'hex')); +assert.deepStrictEqual( + Buffer.allocUnsafeSlow(16).fill('a', 'utf16le'), + Buffer.from('61006100610061006100610061006100', 'hex')); + +assert.strictEqual( + Buffer.allocUnsafeSlow(16).fill('a', 'utf16le').toString('utf16le'), + 'a'.repeat(8)); +assert.strictEqual( + Buffer.allocUnsafeSlow(16).fill('a', 'latin1').toString('latin1'), + 'a'.repeat(16)); +assert.strictEqual( + Buffer.allocUnsafeSlow(16).fill('a', 'utf8').toString('utf8'), + 'a'.repeat(16)); + +assert.strictEqual( + Buffer.allocUnsafeSlow(16).fill('Љ', 'utf16le').toString('utf16le'), + 'Љ'.repeat(8)); +assert.strictEqual( + Buffer.allocUnsafeSlow(16).fill('Љ', 'latin1').toString('latin1'), + '\t'.repeat(16)); +assert.strictEqual( + Buffer.allocUnsafeSlow(16).fill('Љ', 'utf8').toString('utf8'), + 'Љ'.repeat(8));