Skip to content
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

Buffer bug fix #6775

Closed
wants to merge 6 commits into from
Closed

Buffer bug fix #6775

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
f998347
Buffer: Fixes #6770, Single digit hex strings not raising TypeError o…
jspri May 15, 2016
61a4129
Adds test
jspri May 15, 2016
bada963
Fixed linting
jspri May 15, 2016
e4b8572
Refactored to improve performance
jspri May 18, 2016
c1d79eb
Cleaned up test comment and added regression test
jspri May 18, 2016
2db93e5
Changed base64 test to use length
jspri May 19, 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
2 changes: 1 addition & 1 deletion lib/buffer.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -227,7 +227,7 @@ function fromString(string, encoding) {

var length = byteLength(string, encoding);

if (length === 0)
if (length === 0 && string.length === 0)
return Buffer.alloc(0);

if (length >= (Buffer.poolSize >>> 1))
Expand Down
5 changes: 5 additions & 0 deletions test/parallel/test-buffer.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -749,6 +749,11 @@ for (let i = 0; i < 256; i++) {
assert.equal(hexb2[i], hexb[i]);
}

//#6770 Test single hex character throws TypeError
Copy link
Member

Choose a reason for hiding this comment

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

You can reference github issues in comments, but it is usually preferred to give the full URL for things like that so that it’s easier to look up and there is no ambiguity wrt the old issue tracker at https://github.com/nodejs/node-v0.x-archive/

assert.throws(function() {
Buffer.from('A', 'hex');
}, TypeError);
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you put a test to verify there's no regression to 'base64' since it follows the same logic path in this case? Something simple like assert.equal(Buffer.from('A', 'base64'), 0).

Copy link
Contributor Author

@jspri jspri May 17, 2016 •
edited
Loading

Choose a reason for hiding this comment

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

Sure! Is there a reason why hex throws an error for odd lengths but base64 doesn't?

Edit: Don't worry, the final comment for #6107 goes into a bit.


{
// test an invalid slice end.
console.log('Try to slice off the end of the buffer');
Expand Down