cryptocoinjs / base-x

Encode/decode any base
MIT License
312 stars 75 forks source link

base32 and base16 are incorrect #38

Closed daviddias closed 7 years ago

daviddias commented 7 years ago

We had to change for base16 on multibase, because base-x was not padding it correctly.

Now we found that base32 is also creating different bases than the ones other libraries generate. https://github.com/multiformats/js-multibase/issues/17

Anyone else experiencing these issues as well?

sublimator commented 7 years ago

It's by design, and mentioned in the readme. No padding is used. It's a README.md bug.

(I think ... read the README anyway ... you'll probably see what I mean)

We might want to make the fact that it encodes hex etc in a non obvious way more obvious in the README

sublimator commented 7 years ago

Haven't explored ur issue in depth ... so apologies if I'm ... off base?

jprichardson commented 7 years ago

Thoughts @dcousens?

sublimator commented 7 years ago

random sanity/fuzzing tests ...

Kubuxu commented 7 years ago

About base16, it isn't really about padding (as to represent a byte you need two encoded chars). For some reason baseX trims trailing or leading 0 and makes base16 (hex) look like it encodes just (n-0.5) bytes.

sublimator commented 7 years ago

As long as it decodes any encoding to the same input ... it works imo

Kubuxu commented 7 years ago

If you are using just base-X library yes, but it doesn't work with other decoders and applications. Golang's in our case but probably others too.

dcousens commented 7 years ago

Fast base encoding / decoding of any given alphabet using bitcoin style leading zero compression.

https://github.com/cryptocoinjs/base-x#how-it-works

It encodes octet arrays by doing long divisions on all significant digits in the array, creating a representation of that number in the new base. Then for every leading zero in the input (not significant as a number) it will encode as a single leader character. This is the first in the alphabet and will decode as 8 bits. The other characters depend upon the base. For example, a base58 alphabet packs roughly 5.858 bits per character.

This means the encoded string 000f (using a 0-f alphabet) will actually decode to 4 bytes unlike a typical hex codec which uniformly packs 4 bits into each character.

While unusual, this does mean that no padding is required and it works for bases like 43. If you need standard hex encoding or base64 encoding you probably don't want this.

Emphasis' mine.

sublimator commented 7 years ago

Maybe we xan add those em to the readme

dcousens commented 7 years ago

@sublimator add the emphasis? Maybe, PR's accepted :+1: .

sublimator commented 7 years ago

on mobile :) haha .. dat my exvuse and im sticking to it!!

Kubuxu commented 7 years ago

It would be good if you mentioned in tagline of readme that it doesn't provide RFC 4648 compatible encodings.

sublimator commented 7 years ago

PR with the README that woulda saved you the time !

daviddias commented 7 years ago

Not that although you might have a design decision to not put that last 0, base32 is still giving out different values from other base32 encoders.

sublimator commented 7 years ago

Readme bug

dcousens commented 7 years ago

base32 is still giving out different values from other base32 encoders.

And it will, always. As intended.