Skip to content

Commit

Permalink
buffer: fix single-character string filling
Browse files Browse the repository at this point in the history
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: #9836
PR-URL: #9837
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Trevor Norris <trev.norris@gmail.com>
  • Loading branch information
addaleax committed Dec 12, 2016
1 parent 9ee915b commit d4f00fe
Show file tree
Hide file tree
Showing 2 changed files with 42 additions and 12 deletions.
26 changes: 15 additions & 11 deletions lib/buffer.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down
28 changes: 27 additions & 1 deletion test/parallel/test-buffer-fill.js
Original file line number Diff line number Diff line change
Expand Up @@ -395,11 +395,37 @@ assert.throws(() => {
buf.fill('');
}, /^RangeError: out of range index$/);


assert.deepStrictEqual(
Buffer.allocUnsafeSlow(16).fill('ab', 'utf16le'),
Buffer.from('61006200610062006100620061006200', 'hex'));

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));

0 comments on commit d4f00fe

Please sign in to comment.