-
Notifications
You must be signed in to change notification settings - Fork 29.6k
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
Update buffer.md #29792
Update buffer.md #29792
Conversation
updated Buffer.from(string[, encoding]) in documentation Documented an issue when hex encoding a string using Buffer.from(string, [,encoding]) with an odd number of characters string. Ref: #29786
doc/api/buffer.md
Outdated
@@ -100,7 +100,7 @@ to one of these new APIs.* | |||
* [`Buffer.from(buffer)`][] returns a new `Buffer` that *contains a copy* of the | |||
contents of the given `Buffer`. | |||
* [`Buffer.from(string[, encoding])`][`Buffer.from(string)`] returns a new | |||
`Buffer` that *contains a copy* of the provided string. | |||
`Buffer` that *contains a copy* of the provided string. (When `'hex'` encoding a string, using odd number of characters may result in an unpredictable behaviour). |
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.
Given that this affects every Node.js API that takes an encoding
argument (which is many), describing the behaviour of invalid input would be better placed at https://nodejs.org/api/buffer.html#buffer_buffers_and_character_encodings
Also, the phrase added isn't so useful, I can't think of a predictable behaviour for providing a hex string that has half-bytes. Maybe describe what it actually does, ignore input from the first invalid or incomplete hex pair?
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.
Got you, on it.
@@ -203,7 +203,8 @@ The character encodings currently supported by Node.js include: | |||
|
|||
* `'binary'` - Alias for `'latin1'`. | |||
|
|||
* `'hex'` - Encode each byte as two hexadecimal characters. | |||
* `'hex'` - Encode each byte as two hexadecimal characters. If the number of | |||
hexadecimal characters is odd, the first incomplete hexadecimal pair is ignored. |
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.
hexadecimal characters is odd, the first incomplete hexadecimal pair is ignored. | |
hexadecimal characters is odd, the last character is ignored. |
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.
I think this might be better moving it down into it's own one line paragraph with a simple example
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.
If we move it to its own paragraph, we can also add information (if it's not already there) about what happens if the string contains characters that are not hexadecimal (e.g., '1a7g'
). That can also be in a separate PR, but wouldn't be out of place in this one!
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.
@Trott "hexadecimal characters is odd, the last character is ignored." adding this line is enough ?
ping @tr0id - are you available to make the suggested changes? thanks! |
Ping @tr0id |
fixes: nodejs#29786 refs: nodejs#29792 refs: nodejs#24491
The required changes are actually realized through #31352. This PR can be closed. |
updated Buffer.from(string[, encoding]) in documentation
Documented an issue when hex encoding a string using
Buffer.from(string, [,encoding]) with an odd number of characters string.
Ref: #29786
Checklist