search

multiformats / go-multibase

Implementation of multibase parser in go
MIT License
33 stars 17 forks source link

go-base32 encoding is broken with no padding and an encoder to io.Writer #62

Open MichaelMure opened 1 month ago

MichaelMure commented 1 month ago

For example with b32.NewEncoder(b32.NewEncodingCI("abcdefghijklmnopqrstuvwxyz234567").WithPadding(b32.NoPadding), out), encoding Decentralize everything!!! yield birswgzloorzgc3djpjssazlwmvzhs5dinfxgoijbeeswgzlo instead of birswgzloorzgc3djpjssazlwmvzhs5dinfxgoijbee.

Note the swgzlo suffix, which is taken from the beginning of the encoding buffer, bytes 3 to 8.

This has been fixed upstream (in the go stdlib) with https://github.com/golang/go/commit/10529a01fd8b0d5cc07eb3f6aa00a0272597684b.

To be honest, I don't see the point why go-base32 exist anymore, as the original reason for the fork was to "add option for raw encoding", which exist in the stdlib since https://github.com/golang/go/commit/5f4f7519b6c038ab6771e6c7111bcd29967f2750

Maybe go-base32 should be retired entirely?

lidel commented 1 month ago

Retiring sounds sensible. Are you willing to open a PR that switches to stdlib? Or could it be a part of https://github.com/multiformats/go-multibase/issues/44?

We could then release and archive https://github.com/multiformats/go-base32

MichaelMure commented 1 month ago

@lidel FYI I tried, but the problem is that go-base32 has case-insensitive decoding, meaning that for example birswgzloorzgc3djpjssazlwmvzhs5dinfxgoijbeeswGZLO could be decoded, even if strictly speaking it's outside of the alphabet you expect for the multibase. I don't really see why it's important, but it seems like there has been some effort to have it that way.

The funny thing is that there is an argument to have it in encoding/base32: encoding/hex, which also has a case insensitive alphabet does support case insensitive decoding. Why not base32 then? It's a bit more than I'm willing to do on my detour or my detour here though.