Skip to content

Buffer bug fix #6775

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
jspri wants to merge 6 commits into from
Closed

Buffer bug fix #6775

Changes from 1 commit
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
Prev Previous commit
Next Next commit
Cleaned up test comment and added regression test
  • Loading branch information
@jspri
jspri committed May 18, 2016
commit c1d79eb6fbdd3aaa378e85b2c1e2aa5b9726c452
6 changes: 5 additions & 1 deletion test/parallel/test-buffer.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -749,11 +749,15 @@ for (let i = 0; i < 256; i++) {
assert.equal(hexb2[i], hexb[i]);
}

//#6770 Test single hex character throws TypeError
// Test single hex character throws TypeError
// - https://github.com/nodejs/node/issues/6770
assert.throws(function() {
Buffer.from('A', 'hex');
}, TypeError);
Copy link
Copy Markdown
Contributor

@trevnorris trevnorris May 16, 2016

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
Copy Markdown
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 single base64 char encodes as 0
assert.equal(Buffer.from('A', 'base64'), 0);
Copy link
Copy Markdown
Member

@addaleax addaleax May 18, 2016

Choose a reason for hiding this comment

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

This should use assert.strictEqual and should test the length of the buffer rather than the buffer itself.

Copy link
Copy Markdown
Contributor Author

@jspri jspri May 22, 2016

Choose a reason for hiding this comment

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

Fixed now


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