I apologize for being overly snide in my previous message (below). It's
been a rough day. I also failed to focus on an important distinction that
may be underlying a point of disagreement.
Ian is absolutely correct tools and implementations should be extremely
wary of (implicitly) expanding standards. If I were demanding an RFC 4648
encoder that also allowed for things that are disallowed by the the RFC,
then the correct behavior is to fail in some way or another. But I am
trying to use the advertised to features of encoding/base32 to create a
non-standard encoding. (In particular, zbase32, which is meant for very
different uses than the standard encoding.)
Encode/base32 does more than just give us the standard encoders. It offers
itself as a way to construct some *non*-standard encoders. This is what
NewEncoding() is all about. But with the math error in DecodedLen, it will
construct encoders for which the decode(encode(x)) does not equal x.
So as long as encode/base32 offers us NewEncoding() instead of merely
offering StdEnconding along with allowing some of these new encoders to
specify no padding, then it should handle the cases where a non-padded
non-standard base32 works as an encoder should.
Anyway, now that I've done more testing and have had this conversation, I'm
confident enough that this is a bug that I will file a bug for it.
Cheers,
-j
On Tuesday, December 12, 2017 at 5:13:41 PM UTC-6, Jeff Goldberg wrote:
>
>
>
> > On Dec 12, 2017, at 9:00 AM, Ian Lance Taylor wrote:
>
> > DecodedLen is supposed to be applied to the length of the encoded
> > data. RFC 4648 says that the encoded data must be padded to be a
> > multiple of 8 bytes.
>
> Yet encoding/hash32 defines a NoPadding constant, the code is filled
> with tests for whether the padding has been set to NoPadding, and the
> package
> docs make reference to setting things with NoPadding.
>
> There is a lot of logic for that and it can all be made to work with
>
> --- /usr/local/go/src/encoding/base32/base32.go 2017-08-24
> 16:50:22.000000000 -0500
> +++ base32.go 2017-12-12 11:26:34.000000000 -0600
> @@ -499,8 +499,11 @@
> // corresponding to n bytes of base32-encoded data.
> func (enc *Encoding) DecodedLen(n int) int {
> if enc.padChar == NoPadding {
> - return n * 5 / 8
> + r := n * 5 / 8
> + if (n*5)%8 != 0 {
> + return r + 1
> + }
> + return r
> }
> -
> return n / 8 * 5
> }
>
> > Perhaps DecodedLen should have had an error return, but it's too late
> > now.
>
> If you are going to go that route, then it isn’t just DecodedLen that
> needs to change. Every block of code like
>
> if enc.padChar == NoPadding { … }
>
> should be removed; NoPadding should not be defined; and the package
> documentation must stop claiming this can be set with NoPadding.
>
> Note that encoding without padding works as expected, but that would
> have to be removed as well.
>
> Or, we could simply fix the math in that length calculation. It is already
> part
> of a if enc.padChar == NoPadding { } block. It just has a math error.
>
> > There is no correct value to return to for an impossible input,
> > so returning 0 seems as good as anything.
>
> I would really hate to have to fork encoding/hash32 for the stuff
> that I’m trying to build. Building a zbase32 encoder/decoder was
> just a call to base32.NewEncoder(). But now it will have to
> be a call to a forked version.
>
> Or a math error could be corrected to get the package to behave
> as clearly intended.
>
> Cheers,
>
> -j
>
--
You received this message because you are subscribed to the Google Groups
"golang-nuts" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to golang-nuts+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.
