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

docs: Buffer.toString(): add note about invalid data #30706

Closed
wants to merge 2 commits into from

Conversation

jgehrcke
Copy link
Contributor

@jgehrcke jgehrcke commented Nov 28, 2019

New to the ecosystem. I looked up the docs to see how Buffer.toString() behaves upon invalid input data (for building solid code it's of course critically important to know whether it throws an error, or if it performs a surrogate escape instead). Is not documented as of today.

Once we modify the documentation we can close #23280.

There is an existing patch: #28249 -- but it's probably too controversial as of the amount of information I think.

I tried to stay minimal here, so that we at least don't ignore the topic in the docs.

#6075 is about the same topic, I think nobody questions that we need to have the behavior of toString() documented w.r.t. invalid byte sequences in the input.

Checklist

@nodejs-github-bot nodejs-github-bot added buffer Issues and PRs related to the buffer subsystem. doc Issues and PRs related to the documentations. labels Nov 28, 2019
@jgehrcke
Copy link
Contributor Author

Future potential addition: we might want to say

More strict data decoding into text can be done with TextDecoder [1]

[1] https://developer.mozilla.org/en-US/docs/Web/API/TextDecoder

doc/api/buffer.md Outdated Show resolved Hide resolved
Co-Authored-By: Denys Otrishko <[email protected]>
@lundibundi
Copy link
Member

@lundibundi lundibundi added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Nov 28, 2019
addaleax pushed a commit that referenced this pull request Dec 3, 2019
PR-URL: #30706
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Denys Otrishko <[email protected]>
Reviewed-By: Trivikram Kamat <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: David Carlier <[email protected]>
@addaleax
Copy link
Member

addaleax commented Dec 3, 2019

Landed in c0905b7 🎉

@addaleax addaleax closed this Dec 3, 2019
targos pushed a commit that referenced this pull request Dec 9, 2019
PR-URL: #30706
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Denys Otrishko <[email protected]>
Reviewed-By: Trivikram Kamat <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: David Carlier <[email protected]>
@MylesBorins MylesBorins mentioned this pull request Dec 13, 2019
targos pushed a commit that referenced this pull request Jan 13, 2020
PR-URL: #30706
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Denys Otrishko <[email protected]>
Reviewed-By: Trivikram Kamat <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: David Carlier <[email protected]>
BethGriggs pushed a commit that referenced this pull request Feb 6, 2020
PR-URL: #30706
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Denys Otrishko <[email protected]>
Reviewed-By: Trivikram Kamat <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: David Carlier <[email protected]>
@MylesBorins MylesBorins mentioned this pull request Feb 8, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
author ready PRs that have at least one approval, no pending requests for changes, and a CI started. buffer Issues and PRs related to the buffer subsystem. doc Issues and PRs related to the documentations.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Buffer.toString('utf8') appears to use wtf-8
7 participants