Skip to content

buffer: fix single-character string filling #9837

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
addaleax wants to merge 2 commits into from
Closed

buffer: fix single-character string filling #9837

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
cea2194
buffer: handle UCS2 `.fill()` properly on BE
addaleax Dec 6, 2016
b0a80b7
buffer: fix single-character string filling
addaleax Nov 29, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
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: #9836
  • Loading branch information
@addaleax
addaleax committed Dec 12, 2016
commit b0a80b70488b03f258f58e4577ba0488dd240f30
26 changes: 15 additions & 11 deletions lib/buffer.js
View file
Open in desktop
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') {

@trevnorris trevnorris Dec 6, 2016

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For backwards compatibility do we still need to check for 'binary', or is that no longer supported? If this is back ported then it'll probably need to be checked there.

@addaleax addaleax Dec 6, 2016

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@trevnorris I switched the blocks around here so that this is after normalizeEncoding, so there’s no need to worry about alternate names

@trevnorris trevnorris Dec 9, 2016

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cool. thanks.

// 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
View file
Open in desktop
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));