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.

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: inconsistent string validation #3770

Closed
vkurchatkin opened this issue Nov 11, 2015 · 2 comments
Closed

buffer: inconsistent string validation #3770

vkurchatkin opened this issue Nov 11, 2015 · 2 comments
Labels
buffer Issues and PRs related to the buffer subsystem.

Comments

@vkurchatkin
Copy link
Contributor

new Buffer('x', 'hex') // TypeError: Invalid hex string
new Buffer('xx', 'hex') // <Buffer >

This seems confusing

@thefourtheye thefourtheye added the buffer Issues and PRs related to the buffer subsystem. label Nov 11, 2015
thefourtheye added a commit to thefourtheye/io.js that referenced this issue Jan 30, 2016
As it is, if an invalid HEX string is passed to `Buffer` constructor,
it will only use the valid HEX values and ignore the rest. But, it also
throws an error when the length of the string is odd in length. This
patch throws an error if the string is not a valid HEX string.

Fixes: nodejs#3770
@Trott
Copy link
Member

Trott commented Jun 6, 2016

If it matters, the underlying issue is in Buffer.prototype.write() (which is called by the constructor in this case) and not specific to the constructor.

buf = new Buffer(64);
buf.write('x', 'hex'); // TypeError: Invalid hex string
buf.write('xx', 'hex'); // 0

@Trott
Copy link
Member

Trott commented Mar 23, 2017

Additionally, it seems to me that TypeError is the wrong error to throw here. A TypeError might be if something that was not a string was supplied. But an incorrectly formatted string is not a TypeError, or at least it seems to me.

Trott added a commit to Trott/io.js that referenced this issue Mar 25, 2017
Remove error message when a hex string of an incorrect length is sent
to .write() or .fill().

Fixes: nodejs#3770
@Trott Trott closed this as completed in 682573c Mar 28, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
buffer Issues and PRs related to the buffer subsystem.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants