-
Notifications
You must be signed in to change notification settings - Fork 29.8k
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.
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
Buffer bug fix #6775
Conversation
…rror on Buffer creation
@nodejs/buffer |
@@ -749,6 +749,11 @@ for (let i = 0; i < 256; i++) { | |||
assert.equal(hexb2[i], hexb[i]); | |||
} | |||
|
|||
//#6770 Test single hex character throws TypeError |
There was a problem hiding this comment.
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/
Slightly related #3773 |
Should be good to go I think |
}, TypeError); | ||
|
||
// Test single base64 char encodes as 0 | ||
assert.equal(Buffer.from('A', 'base64'), 0); |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed now
LGTM… CI: https://ci.nodejs.org/job/node-test-commit/3437/ @Crazometer btw, Github doesn’t send out notifications when somebody pushes new commits to a PR, so posting a quick comment helps to make sure noething gets lost :) |
@addaleax Thanks for that, you could read my mind |
CI is green. You author name in this commit is given as “Crazometer”. Is that intended or do you prefer to be listed (changelog, git log, AUTHORS file) with some other name? People typically prefer their full name, but ultimately it’s up to you. |
"Justin Sprigg" if you could thanks. I'll make sure to change that next time. |
Landed in 05e2acb. Thanks for fixing this! |
I'm assuming this doesn't affect the v4.x buffer system /cc @jasnell |
Checklist
master
Affected core subsystem(s)
Buffer
Description of change
Fixes #6770, Single digit hex strings not raising TypeError on Buffer creation