ashtuchkin / iconv-lite

Convert character encodings in pure javascript.
MIT License
3.08k stars 282 forks source link

decoder/encoder swapped for hex and base64 #231

Open gynvael opened 4 years ago

gynvael commented 4 years ago

It seems that encoder for hex/base64 actually decodes hex/base64. And at the same time, the decoder for hex/base64 actually encodes input as hex/base64.

> iconv.encode('41', 'hex')
<Buffer 41>
> iconv.decode('41', 'hex')
'3431'
> iconv.decode('A', 'base64')
'QQ=='
> iconv.encode('QQ==', 'base64')
<Buffer 41>

Since this is counter-intuitive it makes sense to swap encoder/decoder pairs in these cases.

ashtuchkin commented 4 years ago

Hmm yep, you're right about that. It's confusing. I guess the reason is that 'hex' and 'base64' are not actually string encodings, they are binary encodings. String encodings are representations of character sequences as bytes; binary encodings represent bytes as character sequences.

It's not easy to "fix" this as all the types will have to be changed. Currently it's assumed that iconv.encode always converts a string to a Buffer and iconv.decode always converts a Buffer to a string (it also supports string as its input for backwards compatibility, but this is deprecated).

The only way to make it less confusing is probably to remove both of these encodings from iconv-lite, with the reasoning that they are not string encodings and you can always use Node.js directly to do these conversions. Not sure about that, what do you think?

gynvael commented 4 years ago

Good points.

I personally do not see a reason for iconv-lite to support them for the exact reasons you have mentioned ("'hex' and 'base64' are not actually string encodings, they are binary encodings").

The one small doubt in my head is that there might be projects out there actually relying on this behavior. On the flip side - as you've mentioned - node supports hex/base64 out of the box for buffers (Buffer.from('...', 'hex')) anyway, and on the browser side base64 is supported as well (btoa, atob), with only the hex part on the browser side missing.

So... deprecate message & then remove perhaps?

yosion-p commented 3 years ago

@ashtuchkin i saw it in wiki, may i filter it and return a msg?

ashtuchkin commented 3 years ago

@yosion-p I'll remove these encodings in a next major version, together with other breaking changes.