`DecoderReader` does not respect `with_decode_allow_trailing_bits`

[chanced]

Before you file an issue

• Did you read the docs? yes
• Did you read the README? yes

The problem

• DecoderReader does not respect with_decode_allow_trailing_bits. Decoding input with a trailing newline results in the error Invalid byte 10, offset 224.

https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=6f1faeb41bee1e2b3bab8b5a510dd9f3

use base64::{
    engine::{DecodePaddingMode,  GeneralPurpose, GeneralPurposeConfig},
    read::DecoderReader,
};
use std::io::Read;

fn main() {
    let engine = GeneralPurpose::new(
        &base64::alphabet::STANDARD,
        GeneralPurposeConfig::new()
            .with_encode_padding(false)
            .with_decode_padding_mode(DecodePaddingMode::Indifferent)
            .with_decode_allow_trailing_bits(true),
    );
    let input = b"aGVsbG8gd29ybGQ=\n";
    let mut r = DecoderReader::new(input.as_slice(), &engine);
    let mut data = vec![];
    r.read_to_end(&mut data).unwrap();
}

This is somewhat problematic with a CLI which can pipe back into itself. Adding a newline at the end of the output avoids the shell partial line indicator %.

How I, the issue filer, am going to help solve it

• I'm willing to implement it.

[chanced]

Also, thanks for the crate. It is incredibly clean and impressive.

[marshallpierce]

Thanks for the bug report, and I'm glad you're enjoying the crate -- though apparently it's not clean enough to reach the goal of 0 bugs ;)

It's troubling that the first legit incorrect behavior bugs that I can recall in a long time (this and #226) are both in DecoderReader. My primary aim in fixing both of those is not so much the fixes (which are pretty small) but rather greatly improving the test coverage in DecoderReader so that any future error handling corner cases are addressed. This also will be a big help in avoiding regressions with #231.

[chanced]

Thanks for the bug report, and I'm glad you're enjoying the crate -- though apparently it's not clean enough to reach the goal of 0 bugs ;)

Oh, what fun would this profession be without them? :) In all seriousness, the crate has worked flawlessly otherwise. I really appreciate the architecture too; it is incredibly configurable.

Cool, let me know if I can help. I have a workaround in place. It's just a nice to have.

[marshallpierce]

OK, finally had some time to take a look at this. I don't think it's actually a bug -- with_decode_allow_trailing_bits is not for ignoring trailing whitespace, etc. Whitespace can be removed in various ways, like this (or the equivalent with Vec.retain):

let mut input = String::from("aGVsbG8gd29ybGQ=\n");
input.retain(|c| !c.is_whitespace());
let mut r = DecoderReader::new(input.as_bytes(), &engine);

The goal of with_decode_allow_trailing_bits is to handle erroneous output from buggy encoders. The trailing symbol Q corresponds to 16 in the standard alphabet, aka 0x10. If we change it to R = 0x11, the trailing 1 is useless since the last few bits of that symbol aren't needed, and that's exposed as an error unless with_decode_allow_trailing_bits(false) is set on the relevant config.

I think ultimately the issue is that the line-oriented CLI stuff you're doing need newlines as framing, so to speak, but base64 doesn't know about newlines, or TCP headers, or any other similar concept -- it only does base64, so you'll need to remove your particular framing before decoding.