0.21.0: decode_slice: Unexpected behavior for exact-size output slice

[cjpatton]

I noticed some behavior with STANDARD_NO_PAD that I didn't expect (similarly for URL_SAFE_NO_PAD). In the code below, I encode a 32 byte string, then use decode_slice() to read it into a buffer.

use rand::prelude::*;
use base64::{
    DecodeSliceError,
    engine::{Engine, general_purpose::STANDARD_NO_PAD as ENC}
}; // 0.21.0

fn main() {
    let x = thread_rng().gen::<[u8; 32]>();
    
    // Decode into a 32 byte buffer. I would expect this to succeed, since the
    // encoding is not padded.
    let mut buf = [0_u8; 32];
    assert!(matches!(
        ENC.decode_slice(ENC.encode(x), &mut buf[..]),
        Err(DecodeSliceError::OutputSliceTooSmall),
    ));
    
    // Decode into a 64 byte buffer. I would expect this to be longer than
    // necessary.
    let mut buf = [0_u8; 64];
    ENC.decode_slice(ENC.encode(x), &mut buf[..]).unwrap();
    assert_eq!(x, buf[..32]);
    
    // For what it's worth, the unchecked version does not panic if the buffer
    // holds only 32 bytes.
    let mut buf = [0_u8; 32];
    ENC.decode_slice_unchecked(ENC.encode(x), &mut buf[..]).unwrap(); // ok
    assert_eq!(x, buf);
}

Is this behavior expected? If so, is it documented anywhere?

[cjpatton]

I observe the same behavior when I construct the engine so that it requires no padding:

const ENC: base64::engine::GeneralPurpose =
    base64::engine::general_purpose::GeneralPurpose::new(
        &base64::alphabet::URL_SAFE,
        base64::engine::general_purpose::GeneralPurposeConfig::new()
            .with_encode_padding(false)
            .with_decode_padding_mode(base64::engine::DecodePaddingMode::RequireNone),
    );

[Nugine]

It is documented while it seems surprising.

rust-base64/src/engine/mod.rs

Line 307 in d7fb31c

/// Returns an error if `output` is smaller than the estimated decoded length.

[marshallpierce]

Yes, it is expected. There's #210 to track having a precise decode size check, but in the meantime if you want the old behavior (no error, just panic), use decode_slice_unchecked.

[cjpatton]

Well, to be on the safe side, I think perhaps it makes sense to check the input slice actually decodes to the desired length. Is there any way to compute the length of the decoded value from the length of the encoded value?

[cjpatton]

Anywho, thanks for linking the relevant issue. I'll close this as duplicate :)

[marshallpierce]

Is there any way to compute the length of the decoded value from the length of the encoded value?

If there was, it'd already be done. :) Unfortunately it requires doing at least a partial decode to get a precise answer, which adds overhead and complexity.

[cjpatton]

Boy Base64 is a hairy spec.

[marshallpierce]

Surprisingly so, for such a simple thing... padding ruins everything.

[marshallpierce]

Closing as a dup of #210.

[EvanCarroll]

For me I'm trying to decode 16 bytes of base64. When I use .decode_slice I get OutputSliceTooSmall. I migratedd to use STANDARD_NO_PAD thinking I was over the 16 byte buffer because of padding. This didn't work.

What I wanted was STANDARD. I wanted padding with with .decode_slice_unchecked . This is really complex. I would at least make a mention on .decode_slice that it will return an error if the internal estimate of buffer size is off. At least until that's fixed.

I imagine everyone decoding into a slice runs into this problem.

[marshallpierce]

Hmm... can you show more details about what you're trying to do? Ideally in the form of a branch with a failing test. :)

[EvanCarroll]

How am I going to show a failing test for a confusing UX?

fn base64decode(encoded: String) -> [u8; 16] {
	assert!( encoded.len() < 32, "Encoded must be len 32 {:?}", encoded.len() );
	use base64::{Engine as _, engine::general_purpose};
	let mut decoded: [u8; 16] = [0;16];
	general_purpose::STANDARD.decode_slice_unchecked(encoded, &mut decoded).unwrap();
	decoded
}

That's the code I was trying to write. By critique here is that getting to .decode_slice_unchecked is FAR less likely to be something the user finds then .decode_slice with STANDARD_NO_PAD. They'll find .decode_slice easily because "(a) I want to decode, (b) to a slice, and (c) that signature works" is how we evaluate in Rust. The error that tells them "hey you idiot you want .decode_slice_unchecked reads, OutputSliceTooSmall and the way I understand that and likely others is that you need to disable padding. Not that you're failing an arbitrary input check.

So potential solutions,

• Differentiate or rename entirely OutputSliceTooSmall to FailedInputCheck
• Document .decode_slice_unchecked under STANDARD_NO_PAD so people know that STANDARD_NO_PAD is likely not going to solve the problem they're experiencing.
• Rename .decode_slice to .decode_slice_checked, and clarify the difference in the docs.

But I can't see creating a failing test for any of these. It's just a rough area in the UX.