bytes(8).encode() doesn't return an error when supplied a 7B buffer about scodec HOT 10 CLOSED

Note: instead it silently pads with zeroes.

from scodec.

Hm, I think the padding behavior is desirable in many cases (and folks are likely relying on this behavior as coded). We could add a bytesStrict method which does what the documentation claims though. What do you think?

from scodec.

I think the padding behavior is definitely needed in some cases, adding a bytesStrict method would certainly be acceptable.

But on a related note I think the naming of fixedSizeBytes and paddedFixedSizeBytes is a little bit misleading because both do add padding. I realized it because I expected to bypass the issue I had with bytes by composing it with fixedSizeBytes but the result was the same. So another possibility would be to just fix bytes documentation, and change the behavior of fixedSizeBytes so that it does what its name advertises?

Cheers,
Pierre

edit: of course 2nd proposition would break backward compatibility and that is a big deal

from scodec.

Yeah, the compatibility breakage is my concern -- specifically that it is a silent breakage that folks would only notice if they have explicit tests which cover the behavior change. If not for that, I'd say let's fix the implementation and leave docs as-is.

from scodec.

Agreed, I guess bytesStrict is the way to go then. Should we still fix the def sizeBound = SizeBound.exact(size * 8L) or leave it as is?

If you give me some insight on how you'd do it I can submit a PR.

from scodec.

On the other hand, not fixing fixedSizeBytes is a hard decision because its misleading name can lead to bugs. Actually I am pretty sure the documentation bug in bytes is related to its implementation relying upon fixedSizeBytes 😁

One option could be to:

• add bytesStrict
• remove fixedSizeBytes
• maybe add a zeroPaddedFixedSizeBytes (or defaulting paddedFixedSizeBytes with a zero padder)

This would break compatibility, but in a non-silent way and maybe prevent future bugs in implementations.

from scodec.

Hm, I think I'd rather keep fixedSizeBytes and fixedSizeBits as they are -- the docs on fixedSizeBits correctly define what happens if the encoded value is less than the specified size. Plus, these methods are used a lot in implementations of existing protocols, so renaming w/ a deprecation cycle adds busywork for those downstream projects. I don't think the potential confusion over the name is enough to offset the downstream impacts.

So I guess that means we just need to add bytesStrict and bitsStrict.

Should we still fix the def sizeBound = SizeBound.exact(size * 8L) or leave it as is?

I think the existing implementation is correct -- SizeBound.exact(size * 8L) -- which says that no matter how many bytes you encode with bytes(n), you are guaranteed to get n bytes in the output. Right?

from scodec.

Should we still fix the def sizeBound = SizeBound.exact(size * 8L) or leave it as is?

I think the existing implementation is correct -- SizeBound.exact(size * 8L) -- which says that no matter how many bytes you encode with bytes(n), you are guaranteed to get n bytes in the output. Right?

Right!

from scodec.

@pm47 Were you still planning on opening a PR for addingbytesStrict and bitsStrict?

from scodec.

Fixed in #99

from scodec.