Clean up FastPortableConfig creation

also a few other Config tweaks

Author: marshallpierce

Cargo.toml

@@ -20,8 +20,8 @@ criterion = "0.3.4"
 rand = "0.6.1"
 structopt = "0.3.21"
 # test fixtures for engine tests
-rstest = "0.6.4"
-rstest_reuse = "0.1.1"
+rstest = "0.11.0"
+rstest_reuse = "0.1.3"
 
 [features]
 default = ["std"]

src/chunked_encoder.rs

@@ -27,7 +27,7 @@ impl<'e, E: Engine> ChunkedEncoder<'e, E> {
     pub fn from(engine: &'e E) -> ChunkedEncoder<'e, E> {
         ChunkedEncoder {
             engine,
-            max_input_chunk_len: max_input_length(BUF_SIZE, engine.config().padding()),
+            max_input_chunk_len: max_input_length(BUF_SIZE, engine.config().encode_padding()),
         }
     }
 
@@ -46,7 +46,7 @@ impl<'e, E: Engine> ChunkedEncoder<'e, E> {
             input_index += input_chunk_len;
             let more_input_left = input_index < bytes.len();
 
-            if self.engine.config().padding() && !more_input_left {
+            if self.engine.config().encode_padding() && !more_input_left {
                 // no more input, add padding if needed. Buffer will have room because
                 // max_input_length leaves room for it.
                 b64_bytes_written += add_padding(bytes.len(), &mut encode_buf[b64_bytes_written..]);

src/decode.rs

@@ -240,7 +240,7 @@ mod tests {
 
             let engine = random_engine(&mut rng);
             encode_engine_string(&orig_data, &mut encoded_data, &engine);
-            assert_encode_sanity(&encoded_data, engine.config().padding(), input_len);
+            assert_encode_sanity(&encoded_data, engine.config().encode_padding(), input_len);
 
             let prefix_len = prefix_len_range.sample(&mut rng);
 
@@ -295,7 +295,7 @@ mod tests {
 
             let engine = random_engine(&mut rng);
             encode_engine_string(&orig_data, &mut encoded_data, &engine);
-            assert_encode_sanity(&encoded_data, engine.config().padding(), input_len);
+            assert_encode_sanity(&encoded_data, engine.config().encode_padding(), input_len);
 
             // fill the buffer with random garbage, long enough to have some room before and after
             for _ in 0..5000 {
@@ -347,7 +347,7 @@ mod tests {
 
             let engine = random_engine(&mut rng);
             encode_engine_string(&orig_data, &mut encoded_data, &engine);
-            assert_encode_sanity(&encoded_data, engine.config().padding(), input_len);
+            assert_encode_sanity(&encoded_data, engine.config().encode_padding(), input_len);
 
             decode_buf.resize(input_len, 0);
 

src/encode.rs

@@ -54,7 +54,7 @@ pub fn encode<T: AsRef<[u8]>>(input: T) -> String {
 ///```
 #[cfg(any(feature = "alloc", feature = "std", test))]
 pub fn encode_engine<E: Engine, T: AsRef<[u8]>>(input: T, engine: &E) -> String {
-    let encoded_size = encoded_len(input.as_ref().len(), engine.config().padding())
+    let encoded_size = encoded_len(input.as_ref().len(), engine.config().encode_padding())
         .expect("integer overflow when calculating buffer size");
     let mut buf = vec![0; encoded_size];
 
@@ -148,7 +148,7 @@ pub fn encode_engine_slice<E: Engine, T: AsRef<[u8]>>(
 ) -> usize {
     let input_bytes = input.as_ref();
 
-    let encoded_size = encoded_len(input_bytes.len(), engine.config().padding())
+    let encoded_size = encoded_len(input_bytes.len(), engine.config().encode_padding())
         .expect("usize overflow when calculating buffer size");
 
     let mut b64_output = &mut output_buf[0..encoded_size];
@@ -178,7 +178,7 @@ fn encode_with_padding<E: Engine>(
 
     let b64_bytes_written = engine.encode(input, output);
 
-    let padding_bytes = if engine.config().padding() {
+    let padding_bytes = if engine.config().encode_padding() {
         add_padding(input.len(), &mut output[b64_bytes_written..])
     } else {
         0
@@ -350,12 +350,12 @@ mod tests {
             );
             assert_encode_sanity(
                 &encoded_data_no_prefix,
-                engine.config().padding(),
+                engine.config().encode_padding(),
                 input_len,
             );
             assert_encode_sanity(
                 &encoded_data_with_prefix[prefix_len..],
-                engine.config().padding(),
+                engine.config().encode_padding(),
                 input_len,
             );
 
@@ -401,7 +401,7 @@ mod tests {
 
             let engine = random_engine(&mut rng);
 
-            let encoded_size = encoded_len(input_len, engine.config().padding()).unwrap();
+            let encoded_size = encoded_len(input_len, engine.config().encode_padding()).unwrap();
 
             assert_eq!(
                 encoded_size,
@@ -410,7 +410,7 @@ mod tests {
 
             assert_encode_sanity(
                 std::str::from_utf8(&encoded_data[0..encoded_size]).unwrap(),
-                engine.config().padding(),
+                engine.config().encode_padding(),
                 input_len,
             );
 
@@ -447,7 +447,7 @@ mod tests {
 
             let engine = random_engine(&mut rng);
 
-            let encoded_size = encoded_len(input_len, engine.config().padding()).unwrap();
+            let encoded_size = encoded_len(input_len, engine.config().encode_padding()).unwrap();
 
             encoded_data.resize(encoded_size, 0);
 
@@ -458,7 +458,7 @@ mod tests {
 
             assert_encode_sanity(
                 std::str::from_utf8(&encoded_data[0..encoded_size]).unwrap(),
-                engine.config().padding(),
+                engine.config().encode_padding(),
                 input_len,
             );
 
@@ -490,7 +490,7 @@ mod tests {
             let engine = random_engine(&mut rng);
 
             // fill up the output buffer with garbage
-            let encoded_size = encoded_len(input_len, config.padding()).unwrap();
+            let encoded_size = encoded_len(input_len, config.encode_padding()).unwrap();
             for _ in 0..encoded_size {
                 output.push(rng.gen());
             }
@@ -529,7 +529,7 @@ mod tests {
             let engine = random_engine(&mut rng);
 
             // fill up the output buffer with garbage
-            let encoded_size = encoded_len(input_len, engine.config().padding()).unwrap();
+            let encoded_size = encoded_len(input_len, engine.config().encode_padding()).unwrap();
             for _ in 0..encoded_size + 1000 {
                 output.push(rng.gen());
             }

src/engine/fast_portable/decode.rs

@@ -20,7 +20,7 @@ const INPUT_BLOCK_LEN: usize = CHUNKS_PER_FAST_LOOP_BLOCK * INPUT_CHUNK_LEN;
 const DECODED_BLOCK_LEN: usize =
     CHUNKS_PER_FAST_LOOP_BLOCK * DECODED_CHUNK_LEN + DECODED_CHUNK_SUFFIX;
 
-/// Estimate with metadata for FastPortable's decode logic
+#[doc(hidden)]
 pub struct FastPortableEstimate {
     /// Total number of decode chunks, including a possibly partial last chunk
     num_chunks: usize,

src/engine/fast_portable/mod.rs

@@ -176,8 +176,8 @@ impl super::Engine for FastPortable {
         )
     }
 
-    fn config(&self) -> Self::Config {
-        self.config
+    fn config(&self) -> &Self::Config {
+        &self.config
     }
 }
 
@@ -238,43 +238,61 @@ fn read_u64(s: &[u8]) -> u64 {
     u64::from_be_bytes(s[..8].try_into().unwrap())
 }
 
-/// Contains miscellaneous configuration parameters for base64 encoding and decoding.
+/// Contains configuration parameters for base64 encoding and decoding.
+///
+/// ```
+/// # use base64::engine::fast_portable::FastPortableConfig;
+/// let config = FastPortableConfig::new()
+///     .with_encode_padding(false);
+///     // further customize using `.with_*` methods as needed
+/// ```
+///
+/// The constants [PAD] and [NO_PAD] cover most use cases.
 ///
 /// To specify the characters used, see [crate::alphabet::Alphabet].
 #[derive(Clone, Copy, Debug)]
 pub struct FastPortableConfig {
-    /// `true` to pad output with `=` characters
-    padding: bool,
-    /// `true` to ignore excess nonzero bits in the last few symbols, otherwise an error is returned
+    encode_padding: bool,
     decode_allow_trailing_bits: bool,
 }
 
 impl FastPortableConfig {
-    /// Create a new config.
+    /// Create a new config with `padding` = `true` and `decode_allow_trailing_bits` = `false`.
     ///
-    /// - `padding`: if `true`, encoding will append `=` padding characters to produce an
-    /// output whose length is a multiple of 4. Padding is not needed for decoding and
-    /// only serves to waste bytes but it's in the spec. For new applications, consider
-    /// not using padding.
-    /// - `decode_allow_trailing_bits`: If unsure, use `false`.
-    /// Useful if you need to decode base64 produced by a buggy encoder that
-    /// has bits set in the unused space on the last base64 character as per
-    /// [forgiving-base64 decode](https://infra.spec.whatwg.org/#forgiving-base64-decode).
-    /// If invalid trailing bits are present and this `true`, those bits will
-    /// be silently ignored, else `DecodeError::InvalidLastSymbol` will be emitted.
-    pub const fn from(padding: bool, decode_allow_trailing_bits: bool) -> FastPortableConfig {
+    /// This probably matches most people's expectations, but consider disabling padding to save
+    /// a few bytes unless you specifically need it for compatibility with some legacy system.
+    pub const fn new() -> FastPortableConfig {
         FastPortableConfig {
-            padding,
-            decode_allow_trailing_bits,
+            // RFC states that padding must be applied by default
+            encode_padding: true,
+            decode_allow_trailing_bits: false,
         }
     }
 
-    /// Create a new `Config` based on `self` with an updated `padding` parameter.
-    pub const fn with_padding(self, padding: bool) -> FastPortableConfig {
-        FastPortableConfig { padding, ..self }
+    /// Create a new config based on `self` with an updated `padding` parameter.
+    ///
+    /// If `true`, encoding will append either 1 or 2 `=` padding characters to produce an
+    /// output whose length is a multiple of 4.
+    ///
+    /// Padding is not needed for correct decoding and only serves to waste bytes, but it's in the
+    /// [spec](https://datatracker.ietf.org/doc/html/rfc4648#section-3.2).
+    ///
+    /// For new applications, consider not using padding if the decoders you're using don't require
+    /// padding to be present.
+    pub const fn with_encode_padding(self, padding: bool) -> FastPortableConfig {
+        FastPortableConfig {
+            encode_padding: padding,
+            ..self
+        }
     }
 
-    /// Create a new `Config` based on `self` with an updated `decode_allow_trailing_bits` parameter.
+    /// Create a new config based on `self` with an updated `decode_allow_trailing_bits` parameter.
+    ///
+    /// Most users will not need to configure this. It's useful if you need to decode base64
+    /// produced by a buggy encoder that has bits set in the unused space on the last base64
+    /// character as per [forgiving-base64 decode](https://infra.spec.whatwg.org/#forgiving-base64-decode).
+    /// If invalid trailing bits are present and this is `true`, those bits will
+    /// be silently ignored, else `DecodeError::InvalidLastSymbol` will be emitted.
     pub const fn with_decode_allow_trailing_bits(self, allow: bool) -> FastPortableConfig {
         FastPortableConfig {
             decode_allow_trailing_bits: allow,
@@ -283,20 +301,21 @@ impl FastPortableConfig {
     }
 }
 
+impl Default for FastPortableConfig {
+    /// Delegates to [FastPortableConfig::new].
+    fn default() -> Self {
+        FastPortableConfig::new()
+    }
+}
+
 impl Config for FastPortableConfig {
-    fn padding(&self) -> bool {
-        self.padding
+    fn encode_padding(&self) -> bool {
+        self.encode_padding
     }
 }
 
 /// Include padding bytes when encoding.
-pub const PAD: FastPortableConfig = FastPortableConfig {
-    padding: true,
-    decode_allow_trailing_bits: false,
-};
+pub const PAD: FastPortableConfig = FastPortableConfig::new();
 
 /// Don't add padding when encoding.
-pub const NO_PAD: FastPortableConfig = FastPortableConfig {
-    padding: false,
-    decode_allow_trailing_bits: false,
-};
+pub const NO_PAD: FastPortableConfig = FastPortableConfig::new().with_encode_padding(false);

src/engine/mod.rs

@@ -10,8 +10,6 @@ mod naive;
 #[cfg(test)]
 mod tests;
 
-// TODO shared DecodeError or per-impl as an associated type?
-
 /// An `Engine` provides low-level encoding and decoding operations that all other higher-level parts of the API use.
 ///
 /// Different implementations offer different characteristics. The library currently ships with
@@ -25,7 +23,7 @@ mod tests;
 // - add an implementation of [engine::tests::EngineWrapper]
 // - add the implementation to the `all_engines` macro
 // All tests run on all engines listed in the macro.
-pub trait Engine {
+pub trait Engine: Send + Sync {
     /// The config type used by this engine
     type Config: Config;
     /// The decode estimate used by this engine
@@ -70,10 +68,10 @@ pub trait Engine {
     ) -> Result<usize, DecodeError>;
 
     /// Returns the config for this engine.
-    fn config(&self) -> Self::Config;
+    fn config(&self) -> &Self::Config;
 }
 
-/// The minimal level of configuration engines must expose.
+/// The minimal level of configuration that engines must support.
 pub trait Config {
     /// Returns `true` if padding should be added after the encoded output.
     ///
@@ -83,10 +81,11 @@ pub trait Config {
     // It could be provided as a separate parameter when encoding, but that feels like
     // leaking an implementation detail to the user, and it's hopefully more convenient
     // to have to only pass one thing (the engine) to any part of the API.
-    fn padding(&self) -> bool;
+    fn encode_padding(&self) -> bool;
 }
 
-/// The decode estimate used by an engine implementation.
+/// The decode estimate used by an engine implementation. Users do not need to interact with this;
+/// it is only for engine implementors.
 ///
 /// Implementors may want to store relevant calculations when constructing this to avoid having
 /// to calculate them again during actual decoding.
@@ -96,6 +95,6 @@ pub trait DecodeEstimate {
     fn decoded_length_estimate(&self) -> usize;
 }
 
-/// An engine that will work on all CPUs using the standard base64 alphabet.
+/// An engine that will work on all CPUs using the standard base64 alphabet and config.
 pub const DEFAULT_ENGINE: FastPortable =
     FastPortable::from(&alphabet::STANDARD, fast_portable::PAD);

src/engine/naive.rs

@@ -264,8 +264,8 @@ impl Engine for Naive {
         Ok(output_index)
     }
 
-    fn config(&self) -> Self::Config {
-        self.config
+    fn config(&self) -> &Self::Config {
+        &self.config
     }
 }
 
@@ -301,7 +301,7 @@ pub struct NaiveConfig {
 }
 
 impl Config for NaiveConfig {
-    fn padding(&self) -> bool {
+    fn encode_padding(&self) -> bool {
         self.padding
     }
 }

src/engine/tests.rs

@@ -14,10 +14,11 @@ use crate::{
     DecodeError, PAD_BYTE,
 };
 
+// the case::foo syntax includes the "foo" in the generated test method names
 #[template]
 #[rstest(engine_wrapper,
-case(FastPortableWrapper {}),
-case(NaiveWrapper {}),
+case::fast_portable(FastPortableWrapper {}),
+case::naive(NaiveWrapper {}),
 )]
 fn all_engines<E: EngineWrapper>(engine_wrapper: E) {}
 
@@ -830,7 +831,7 @@ impl EngineWrapper for FastPortableWrapper {
     fn standard_forgiving() -> Self::Engine {
         fast_portable::FastPortable::from(
             &STANDARD,
-            fast_portable::FastPortableConfig::from(true, true),
+            fast_portable::FastPortableConfig::new().with_decode_allow_trailing_bits(true),
         )
     }
 
@@ -841,7 +842,9 @@ impl EngineWrapper for FastPortableWrapper {
     }
 
     fn random_alphabet<R: Rng>(rng: &mut R, alphabet: &Alphabet) -> Self::Engine {
-        let config = fast_portable::FastPortableConfig::from(rng.gen(), rng.gen());
+        let config = fast_portable::FastPortableConfig::new()
+            .with_encode_padding(rng.gen())
+            .with_decode_allow_trailing_bits(rng.gen());
 
         fast_portable::FastPortable::from(alphabet, config)
     }