src: remove BINARY encoding

[indutny]

Pull Request check-list

Please make sure to review and check all of these items:

• Does make -j8 test (UNIX) or vcbuild test nosign (Windows) pass with
  this change (including linting)?
• Is the commit message formatted according to CONTRIBUTING.md?
• If this change fixes a bug (or a performance problem), is a regression
  test (or a benchmark) included?
• Is a documentation update included (if this change modifies
  existing APIs, or introduces new ones)?

NOTE: these things are not required to open a PR and can be done
afterwards / while the PR is open.

Affected core subsystem(s)

Please provide affected core subsystem(s) (like buffer, cluster, crypto, etc)

Description of change

It was fun,
now it is gone.

---

I decided to follow @chrisdickinson footsteps and submit provocative PR 😉 Hopefully this will help to spin of a useful discussion!

[indutny]

CI: https://ci.nodejs.org/job/node-test-pull-request/1789/

[indutny]

cc @nodejs/ctc @nodejs/collaborators @nodejs/crypto

[Fishrock123]

Could you explain what benefit this is to us? I don't really understand.

[indutny]

Of course, binary encoding causes significant problems in crypto, and less often in other places. In general, it looks like people don't really understand how it works, and even if they do - other people that are using their code don't understand the limitations of binary.

As it is right now, it does not provide much use IMO, and there are lots of code to handle it. Therefore I propose removing it.

[mscdex]

-100. The binary encoding is still very useful in many situations, for example: when you need/want to take advantage of existing string or other functions (e.g. regexps) that cannot handle Buffers.

[indutny]

@mscdex example?

[indutny]

What if I will change defaults for crypto module instead? How should I approach it?

[mscdex]

@indutny Regexps was one such example. Anything from String.prototype.

[indutny]

@mscdex why utf8 won't work in your case?

[mscdex]

@indutny I think the reason that the default encoding is binary for crypto is that the typical use case is binary data. Yes most people will probably be using Buffers, but the binary encoding keeps with that theme, but for non-Buffer inputs.

@mscdex why utf8 won't work in your case?

I'm not sure what you mean here. Converting binary to utf8 is not safe.

[seishun]

+1 it was meant to be removed long ago.

@mscdex why would you use regexp for non-textual data?

[indutny]

@mscdex it is not safe to pass utf8 as binary. Also binary encoding may produce some unexpected results. What is unsafe about utf8?

@mscdex I would really appreciate if you could share a link with me, demonstrating a use case in some project. From what I heard, it does not look like binary is a requirement.

[mscdex]

@seishun Because it's useful if you need something more than a simple .indexOf()? For example, even needing to do a simple case-insensitive .indexOf() is impossible with Buffers.

[seishun]

@mscdex as @indutny said, a real-world usage example would be welcome.

For example, even needing to do a simple case-insensitive .indexOf() is impossible with Buffers.

Binary data doesn't have a concept of case, it's just a byte sequence.

[mscdex]

@indutny My point is that if you change the default encoding to utf8, strings containing binary bytes can get misinterpreted:

> new Buffer('\xFF\x7F', 'utf8')
<Buffer c3 bf 7f>

[indutny]

Well, same thing with binary?

> new Buffer('привет', 'binary').toString()
'?@825B'

[mscdex]

@indutny I don't see that as the same thing. Once you have imported the binary string with the 'binary' encoding there is no need to do .toString(). If you remove the 'binary' encoding you can no longer import the string safely (or generate a 'binary' string for manipulation when Buffer methods aren't enough as I previously described).

[indutny]

@mscdex what is "import the string safely" in your definition?

[micnic]

Is there any performance impact when using utf8 as default encoding?

[mscdex]

@indutny "safely" meaning the data/content is not modified

[vitaliylag]

Examples when Binary is useful:

var obj = {};
function wasExecuted(buf) {
    return buf.toString('binary') in obj;
    //Here we could use hex but it works slower and requires more memory
}

function addNumberToString(s, num) {
    var isBuf = Buffer.isBuffer(s) && (s = s.toString('binary'), true);

    //Here we can work with s like usual string. We don't need write individual
    //code for Buffer case. Of course we shouldn't add to string big char codes.
    s = s + num;

    return isBuf ? new Buffer(s, 'binary') : s;
}

I guess I can not use utf8 in these examples because not all bytes sequences can be safely converted to utf8, am I right?

[trevnorris]

@vitaliylag The majority of utf8 byte sequences can't be converted.

A useful usage I've used in the past is when I want to hold many open file contents that are relatively large in size (>= 1MB). Done this while parsing data sets. If the file text can be assumed as ascii or latin1 then the string can be referenced out side of the heap. Example script:

const buf = Buffer.alloc(1024 * 1024, 'a');
const arr = [];
for (var i = 0; i < 1e3; i++) {
  arr.push(buf.toString('binary'));
  //arr.push(buf.toString('utf8'));
}
process._rawDebug(require('v8').getHeapStatistics().used_heap_size / 0x100000);

Using 'binary' encoding output is ~3MB. Whereas 'utf8' sits just over 1GB.