Re: [go-nuts] base32 DecodeLen() miscalculation when no padding

[Jeff Goldberg]

On Wednesday, December 13, 2017 at 5:06:01 AM UTC-6, Caleb Spare wrote:
>
> And in fact, Tv has already done that. You want 
> https://github.com/tv42/zbase32. 
>

Yep. That solves my immediate needs. (For what it is worth, I'm playing with
different ways of displaying public key fingerprints in the least 
user-hostile way.) 

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.

Re: [go-nuts] base32 DecodeLen() miscalculation when no padding

[Jeff Goldberg]

On Wednesday, December 13, 2017 at 5:02:40 AM UTC-6, rog wrote:
>
>
> Looking at 
> http://philzimmermann.com/docs/human-oriented-base-32-encoding.txt, 
> it seems that zbase32 allows the encoder and decoder to agree on the 
> number of bits transmitted, so if you're encoding 5 bits or less, you 
> can use a single encoded character.
>

Yep. I discovered this after making my for NoPadding. zbase32 uses a 
different
packing scheme, so there is no real hope of piggy backing of of 
encoding/base32.

ISTM that this difference is probably justification enough to make a 
> new zbase32 package rather than using encoding/base32 directly. 
>

Indeed, as Caleb Spare has since pointed out, that has indeed been done.

  https://github.com/tv42/zbase32 


I had actually seen that before starting, but thought why should I import 
all of that
when I can just use base32.NewEncoding() ?  Well, now I know.

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.

Re: [go-nuts] base32 DecodeLen() miscalculation when no padding

[Caleb Spare]

And in fact, Tv has already done that. You want https://github.com/tv42/zbase32.

On Wed, Dec 13, 2017 at 3:01 AM, roger peppe  wrote:
> On 13 December 2017 at 01:23, Ian Lance Taylor  wrote:
>> On Tue, Dec 12, 2017 at 3:13 PM, Jeffrey 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.
>>
>> Yes.  But even with the no padding case, it is impossible to have an
>> encoded length of a single byte (I tried to say that in a bit of the
>> message that you didn't quote).  And I hope we can all agree that
>> `DecodedLen` returns the correct value when called with an argument of
>> 2 when `padChar == NoPadding`.  So we are only talking about the value
>> 1, which I assert is impossible in a valid base32 encoded string with
>> no padding.
>
> Looking at http://philzimmermann.com/docs/human-oriented-base-32-encoding.txt,
> it seems that zbase32 allows the encoder and decoder to agree on the
> number of bits transmitted, so if you're encoding 5 bits or less, you
> can use a single encoded character.
>
> ISTM that this difference is probably justification enough to make a
> new zbase32 package rather than using encoding/base32 directly.
>
> --
> 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.

-- 
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.

Re: [go-nuts] base32 DecodeLen() miscalculation when no padding

[roger peppe]

On 13 December 2017 at 01:23, Ian Lance Taylor  wrote:
> On Tue, Dec 12, 2017 at 3:13 PM, Jeffrey 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.
>
> Yes.  But even with the no padding case, it is impossible to have an
> encoded length of a single byte (I tried to say that in a bit of the
> message that you didn't quote).  And I hope we can all agree that
> `DecodedLen` returns the correct value when called with an argument of
> 2 when `padChar == NoPadding`.  So we are only talking about the value
> 1, which I assert is impossible in a valid base32 encoded string with
> no padding.

Looking at http://philzimmermann.com/docs/human-oriented-base-32-encoding.txt,
it seems that zbase32 allows the encoder and decoder to agree on the
number of bits transmitted, so if you're encoding 5 bits or less, you
can use a single encoded character.

ISTM that this difference is probably justification enough to make a
new zbase32 package rather than using encoding/base32 directly.

-- 
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.

Re: [go-nuts] base32 DecodeLen() miscalculation when no padding

[Ian Lance Taylor]

On Tue, Dec 12, 2017 at 3:13 PM, Jeffrey 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.

Yes.  But even with the no padding case, it is impossible to have an
encoded length of a single byte (I tried to say that in a bit of the
message that you didn't quote).  And I hope we can all agree that
`DecodedLen` returns the correct value when called with an argument of
2 when `padChar == NoPadding`.  So we are only talking about the value
1, which I assert is impossible in a valid base32 encoded string with
no padding.

Ian

-- 
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.

Re: [go-nuts] base32 DecodeLen() miscalculation when no padding

[Jeff Goldberg]

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.go2017-08-24 
> 16:50:22.0 -0500 
> +++ base32.go2017-12-12 11:26:34.0 -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.

Re: [go-nuts] base32 DecodeLen() miscalculation when no padding

[Jeffrey Goldberg]

> 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.0 
-0500
+++ base32.go   2017-12-12 11:26:34.0 -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.


smime.p7s
Description: S/MIME cryptographic signature

Re: [go-nuts] base32 DecodeLen() miscalculation when no padding

[Ian Lance Taylor]

On Tue, Dec 12, 2017 at 12:06 AM, Jeff Goldberg  wrote:
>
> In encoding/base32 there is, I believe, an off by one error in the
> calculation of the size of the buffer needed for DecodeLen() when padding is
> turned off.
>
> // DecodedLen returns the maximum length in bytes of the decoded data
>   // corresponding to n bytes of base32-encoded data.
>   func (enc *Encoding) DecodedLen(n int) int {
>   if enc.padChar == NoPadding {
>   return n * 5 / 8
>   }
>
>   return n / 8 * 5
>   }
>
>
> Note that when n is 1, that leads to a DecodeLen() returning zero. Likewise,
> when n is 2, we get a DecodeLen of 1.
>
> This leads to incorrect decoding, as the size of dbuf is wrong in
> DecodeString().
>
> If needed, I can construct tests showing this, but what I have at the moment
> (how I stumbled across this) isn't going to
> be very useful.

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.  So when using padding, values of 1 or 2 should
never happen.  When not using padding, a minimum of two encoded bytes
is needed to represent a single decoded byte.  So a value of 1 should
never happen.

Perhaps DecodedLen should have had an error return, but it's too late
now.  There is no correct value to return to for an impossible input,
so returning 0 seems as good as anything.

Ian

-- 
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.

[go-nuts] base32 DecodeLen() miscalculation when no padding

[Jeff Goldberg]

In encoding/base32 there is, I believe, an off by one error in the 
calculation of the size of the buffer needed for DecodeLen() when padding 
is turned off.

// DecodedLen returns the maximum length in bytes of the decoded data  // 
corresponding to n bytes of base32-encoded data.  func (enc *Encoding) 
DecodedLen(n int) int {if enc.padChar == NoPadding {   return n 
* 5 / 8}   return n / 8 * 5  }  

Note that when n is 1, that leads to a DecodeLen() returning zero. 
Likewise, when n is 2, we get a DecodeLen of 1.

This leads to incorrect decoding, as the size of dbuf is wrong in 
DecodeString().

If needed, I can construct tests showing this, but what I have at the 
moment (how I stumbled across this) isn't going to
be very useful.

-- 
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.