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: consider deprecated raws as valid encoding #2829

Closed
wants to merge 1 commit into from

Conversation

thefourtheye
Copy link
Contributor

Even though raws is a deprecated encoding, it is still valid as per
ParseEncoding in node.cc.

Refer https://github.com/nodejs/node/blob/v4.0.0/src/node.cc#L1214-L1218

cc @trevnorris

Even though `raws` is a deprecated encoding, it is still valid as per
`ParseEncoding` in `node.cc`.
@thefourtheye thefourtheye added the buffer Issues and PRs related to the buffer subsystem. label Sep 12, 2015
@brendanashworth
Copy link
Contributor

Or it could be removed, it's been deprecated forever: 07792af.

@mscdex
Copy link
Contributor

mscdex commented Sep 12, 2015

Are there any npm modules that use the raws encoding still?

@ChALkeR
Copy link
Member

ChALkeR commented Sep 12, 2015

A quick grep over 1/3 of npm packages showed no signs of 'raws' or "raws" (except for one mention in a wordlist file, but that's irrelevant).

Note that my package set includes only the last version of packages, and may be biased (it's actually packages that were published with npm2+). I have a new and complete set (still only the last versions, though), but I did not prepare a searchable form of those yet.

@thefourtheye
Copy link
Contributor Author

@ChALkeR Can you please grep raw as well? I'll remove them if that is also not used.

@ChALkeR
Copy link
Member

ChALkeR commented Sep 12, 2015

@thefourtheye 'raw' is a common word. What context should I check?
«Buffer» context:

j/jkurwa-0.4.20.tgz/lib/models/Priv.js:264:    bufZZ = new Buffer(pointZ.x.buf8(), 'raw');
j/jkurwa-0.4.20.tgz/examples/decrypt.js:11:        data = new Buffer(u8, 'raw');

I will prepare a searchable file of the current versions of all packages in the coming days and will publish it, so that everyone could perform those tests. The expected file size is around 3 GiB compressed with lzop, and around 15 GiB uncompressed, grep time around a minute.

@thefourtheye
Copy link
Contributor Author

Thanks @ChALkeR :-) Let's hear what @trevnorris thinks.

@trevnorris
Copy link
Contributor

Whatever change is made, make sure it's reflected in the native code.

I say just remove it (and raw). Been deprecated as long as I can remember, and never seen it used.

thefourtheye added a commit to thefourtheye/io.js that referenced this pull request Sep 14, 2015
As `raw` and `raws` encodings are deprecated for such a long time, and
it is an undocumented feature. This patch removes the support for those
encoding completely.

Previous discussion: nodejs#2829
@thefourtheye
Copy link
Contributor Author

Closing in favor of #2859

@thefourtheye thefourtheye deleted the add-missing-raws branch September 15, 2015 04:38
thefourtheye added a commit that referenced this pull request Sep 16, 2015
As `raw` and `raws` encodings are deprecated for such a long time, and
they both are undocumented, this patch removes the support for those
encodings completely.

Previous discussion: #2829

PR-URL: #2859
Reviewed-By: Brian White <[email protected]>
Reviewed-By: Trevor Norris <[email protected]>
Reviewed-By: Ben Noordhuis <[email protected]>
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 this pull request may close these issues.

5 participants