base64::decode_config_slice should not panic

[snyball]

I think the panicking behavior of base64::decode_config_slice is a mistake, this is an error that an application receiving untrusted input should be able to deal with. The size of decoded data can be calculated from the base64 input, before running base64::decode_config_slice; but I think this goes against separation of concerns. The API user shouldn't have to know about internals like that.

There are situations where one wants to decode some untrusted base64 input, with a known maximum/absolute size, where the size is small enough that heap-allocation is unnecessary. A classic example of this would be a session ID.

Suggested implementation

I think DecodeError should have a BufferOverflow variant, which base64::decode_config_slice should return when it overflows a given mutable slice.

I foresee two use-cases for this error:

1. Intended: Abort handling whatever request submitted base64 with an invalid decode-size, and log/report the error without having to unwind.
2. Unintended: Increasing the size of the output buffer until the decoding succeeds

Use-case number 2 should be strongly discouraged by the API docs, with a reference to base64::decode for decoding base64 of unknown length.

Backwards-compatibility

The suggested implementation is not backwards compatible because it changes DecodeError, and is therefore unrealistic given the widespread use of this crate in the ecosystem.

The implementation should therefore add a new function basse64::decode_config_fixed_slice_bikeshed with a new error-type SliceDecodeErrorBikeshed with variants BufferOverflow, and DecodeError(DecodeError) while deprecating base64::decode_config_slice.

[marshallpierce]

• If we add another enum variant, it need only apply to decode_config_slice(now decode_engine_slice). That is perhaps an acceptable burden rather than introducing yet another function.
• Assuming we don't change the existing function: decode_engine_slice_checked to evoke checked_add and the like?
• In the meantime of course you can write your own wrapper function, but I realize that's not satisfying.
• To be useful, does the error have to be precise? That is, is it ok to require a size-3 decode buffer for input "AA==" that could decode to 3 bytes, but in fact only decodes to 1? This affects whether all write paths need to be aware of the slice size vs relying on panicking, or if a cheaper pre-calculation is acceptable.

[snyball]

I don't think the error has to be precise beyond "doesn't fit," but then there could be cases where an API-user is not looking for "should be exactly this many bytes" but instead "can be maximum this amount of bytes." I haven't looked in to that option and have never been in that situation myself.

Adding the new enum variant just to the decode_engine_slice function would be ideal I think, with the second best option being to add a decode_engine_slice_checked (similarity to checked_add is a good idea!)

[marshallpierce]

I wonder if encode_engine_slice should get a similar treatment (either changing in place, or adding a _checked version). Unlike decoding, there is an easy way to tell what the encoded size of an input is (encoded_len()), so it is feasible for users to size the buffer correctly, but it is nice to offer something safer than "don't hold it wrong"...

[marshallpierce]

Take a look at #207. I've released that as 0.21.0-beta.1 also to make it easy to try out.

[marshallpierce]

Fixed in 0.21.0.