Return errors instead of panicking on _slice methods

Author: marshallpierce

RELEASE-NOTES.md

@@ -10,8 +10,16 @@
   usually expect from a `from` call.
 - `encode*` and `decode*` top level functions are now methods on `Engine`.
 - `DEFAULT_ENGINE` was replaced by `engine::general_purpose::STANDARD`
-- Predefined engine consts `engine::general_purpose::{STANDARD, URL_SAFE, URL_SAFE_NO_PAD}`
+- Predefined engine consts `engine::general_purpose::{STANDARD, STANDARD_NO_PAD, URL_SAFE, URL_SAFE_NO_PAD}`
     - These are `pub use`d into `engine` as well
+- The `*_slice` decode/encode functions now return an error instead of panicking when the output slice is too small
+    - As part of this, there isn't now a public way to decode into a slice _exactly_ the size needed for inputs that
+      aren't multiples of 4 tokens. If adding up to 2 bytes to always be a multiple of 3 bytes for the decode buffer is
+      a problem, file an issue.
+
+## Other changes
+
+- `decoded_len_estimate()` is provided to make it easy to size decode buffers correctly.
 
 ## Migration
 
@@ -32,8 +40,10 @@ The short-lived 0.20 functions were the 0.13 functions with `config` replaced wi
 
 ### Padding
 
-Where possible, use `engine::STANDARD`, `engine::URL_SAFE`, or `engine::URL_SAFE_NO_PAD`. The first two requires that
-canonical padding is present when decoding, and the last requires that padding is absent.
+If applicable, use the preset engines `engine::STANDARD`, `engine::STANDARD_NO_PAD`, `engine::URL_SAFE`,
+or `engine::URL_SAFE_NO_PAD`.
+The `NO_PAD` ones require that padding is absent when decoding, and the others require that
+canonical padding is present .
 
 If you need the < 0.20 behavior that did not care about padding, or want to recreate < 0.20.0's predefined `Config`s
 precisely, see the following table.

benches/benchmarks.rs

@@ -99,9 +99,7 @@ fn do_encode_bench_slice(b: &mut Bencher, &size: &usize) {
     let mut buf = Vec::new();
     // conservative estimate of encoded size
     buf.resize(v.len() * 2, 0);
-    b.iter(|| {
-        STANDARD.encode_slice(&v, &mut buf);
-    });
+    b.iter(|| STANDARD.encode_slice(&v, &mut buf).unwrap());
 }
 
 fn do_encode_bench_stream(b: &mut Bencher, &size: &usize) {

src/chunked_encoder.rs

@@ -41,7 +41,7 @@ impl<'e, E: Engine + ?Sized> ChunkedEncoder<'e, E> {
 
             let chunk = &bytes[input_index..(input_index + input_chunk_len)];
 
-            let mut b64_bytes_written = self.engine.inner_encode(chunk, &mut encode_buf);
+            let mut b64_bytes_written = self.engine.internal_encode(chunk, &mut encode_buf);
 
             input_index += input_chunk_len;
             let more_input_left = input_index < bytes.len();

src/decode.rs

@@ -1,6 +1,4 @@
-use crate::engine::Engine;
-#[cfg(any(feature = "alloc", feature = "std", test))]
-use crate::engine::STANDARD;
+use crate::engine::{DecodeEstimate, Engine, STANDARD};
 #[cfg(any(feature = "alloc", feature = "std", test))]
 use alloc::vec::Vec;
 use core::fmt;
@@ -44,17 +42,44 @@ impl fmt::Display for DecodeError {
 
 #[cfg(any(feature = "std", test))]
 impl error::Error for DecodeError {
-    fn description(&self) -> &str {
-        match *self {
-            Self::InvalidByte(_, _) => "invalid byte",
-            Self::InvalidLength => "invalid length",
-            Self::InvalidLastSymbol(_, _) => "invalid last symbol",
-            Self::InvalidPadding => "invalid padding",
+    fn cause(&self) -> Option<&dyn error::Error> {
+        None
+    }
+}
+
+/// Errors that can occur while decoding into a slice.
+#[derive(Clone, Debug, PartialEq, Eq)]
+pub enum DecodeSliceError {
+    /// A [DecodeError] occurred
+    DecodeError(DecodeError),
+    /// The provided slice _may_ be too small.
+    ///
+    /// The check is conservative (assumes the last triplet of output bytes will all be needed).
+    OutputSliceTooSmall,
+}
+
+impl fmt::Display for DecodeSliceError {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        match self {
+            Self::DecodeError(e) => write!(f, "DecodeError: {}", e),
+            Self::OutputSliceTooSmall => write!(f, "Output slice too small"),
         }
     }
+}
 
+#[cfg(any(feature = "std", test))]
+impl error::Error for DecodeSliceError {
     fn cause(&self) -> Option<&dyn error::Error> {
-        None
+        match self {
+            DecodeSliceError::DecodeError(e) => Some(e),
+            DecodeSliceError::OutputSliceTooSmall => None,
+        }
+    }
+}
+
+impl From<DecodeError> for DecodeSliceError {
+    fn from(e: DecodeError) -> Self {
+        DecodeSliceError::DecodeError(e)
     }
 }
 
@@ -101,10 +126,39 @@ pub fn decode_engine_slice<E: Engine, T: AsRef<[u8]>>(
     input: T,
     output: &mut [u8],
     engine: &E,
-) -> Result<usize, DecodeError> {
+) -> Result<usize, DecodeSliceError> {
     engine.decode_slice(input, output)
 }
 
+/// Returns a conservative estimate of the decoded size of `encoded_len` base64 symbols (rounded up
+/// to the next group of 3 decoded bytes).
+///
+/// The resulting length will be a safe choice for the size of a decode buffer, but may have up to
+/// 2 trailing bytes that won't end up being needed.
+///
+/// # Examples
+///
+/// ```
+/// use base64::decoded_len_estimate;
+///
+/// assert_eq!(3, decoded_len_estimate(1));
+/// assert_eq!(3, decoded_len_estimate(2));
+/// assert_eq!(3, decoded_len_estimate(3));
+/// assert_eq!(3, decoded_len_estimate(4));
+/// // start of the next quad of encoded symbols
+/// assert_eq!(6, decoded_len_estimate(5));
+/// ```
+///
+/// # Panics
+///
+/// Panics if decoded length estimation overflows.
+/// This would happen for sizes within a few bytes of the maximum value of `usize`.
+pub fn decoded_len_estimate(encoded_len: usize) -> usize {
+    STANDARD
+        .internal_decoded_len_estimate(encoded_len)
+        .decoded_len_estimate()
+}
+
 #[cfg(test)]
 mod tests {
     use super::*;
@@ -235,42 +289,6 @@ mod tests {
         }
     }
 
-    #[test]
-    fn decode_into_slice_fits_in_precisely_sized_slice() {
-        let mut orig_data = Vec::new();
-        let mut encoded_data = String::new();
-        let mut decode_buf = Vec::new();
-
-        let input_len_range = Uniform::new(0, 1000);
-        let mut rng = rand::rngs::SmallRng::from_entropy();
-
-        for _ in 0..10_000 {
-            orig_data.clear();
-            encoded_data.clear();
-            decode_buf.clear();
-
-            let input_len = input_len_range.sample(&mut rng);
-
-            for _ in 0..input_len {
-                orig_data.push(rng.gen());
-            }
-
-            let engine = random_engine(&mut rng);
-            engine.encode_string(&orig_data, &mut encoded_data);
-            assert_encode_sanity(&encoded_data, engine.config().encode_padding(), input_len);
-
-            decode_buf.resize(input_len, 0);
-
-            // decode into the non-empty buf
-            let decode_bytes_written = engine
-                .decode_slice(&encoded_data, &mut decode_buf[..])
-                .unwrap();
-
-            assert_eq!(orig_data.len(), decode_bytes_written);
-            assert_eq!(orig_data, decode_buf);
-        }
-    }
-
     #[test]
     fn decode_engine_estimation_works_for_various_lengths() {
         let engine = GeneralPurpose::new(&alphabet::STANDARD, general_purpose::NO_PAD);
@@ -284,4 +302,32 @@ mod tests {
             }
         }
     }
+
+    #[test]
+    fn decode_slice_output_length_errors() {
+        for num_quads in 1..100 {
+            let input = "AAAA".repeat(num_quads);
+            let mut vec = vec![0; (num_quads - 1) * 3];
+            assert_eq!(
+                DecodeSliceError::OutputSliceTooSmall,
+                STANDARD.decode_slice(&input, &mut vec).unwrap_err()
+            );
+            vec.push(0);
+            assert_eq!(
+                DecodeSliceError::OutputSliceTooSmall,
+                STANDARD.decode_slice(&input, &mut vec).unwrap_err()
+            );
+            vec.push(0);
+            assert_eq!(
+                DecodeSliceError::OutputSliceTooSmall,
+                STANDARD.decode_slice(&input, &mut vec).unwrap_err()
+            );
+            vec.push(0);
+            // now it works
+            assert_eq!(
+                num_quads * 3,
+                STANDARD.decode_slice(&input, &mut vec).unwrap()
+            );
+        }
+    }
 }

src/encode.rs

@@ -1,5 +1,8 @@
 #[cfg(any(feature = "alloc", feature = "std", test))]
 use alloc::string::String;
+use core::fmt;
+#[cfg(any(feature = "std", test))]
+use std::error;
 
 #[cfg(any(feature = "alloc", feature = "std", test))]
 use crate::engine::STANDARD;
@@ -49,9 +52,10 @@ pub fn encode_engine_slice<E: Engine, T: AsRef<[u8]>>(
     input: T,
     output_buf: &mut [u8],
     engine: &E,
-) -> usize {
+) -> Result<usize, EncodeSliceError> {
     engine.encode_slice(input, output_buf)
 }
+
 /// B64-encode and pad (if configured).
 ///
 /// This helper exists to avoid recalculating encoded_size, which is relatively expensive on short
@@ -70,7 +74,7 @@ pub(crate) fn encode_with_padding<E: Engine + ?Sized>(
 ) {
     debug_assert_eq!(expected_encoded_size, output.len());
 
-    let b64_bytes_written = engine.inner_encode(input, output);
+    let b64_bytes_written = engine.internal_encode(input, output);
 
     let padding_bytes = if engine.config().encode_padding() {
         add_padding(input.len(), &mut output[b64_bytes_written..])
@@ -88,7 +92,8 @@ pub(crate) fn encode_with_padding<E: Engine + ?Sized>(
 /// Calculate the base64 encoded length for a given input length, optionally including any
 /// appropriate padding bytes.
 ///
-/// Returns `None` if the encoded length can't be represented in `usize`.
+/// Returns `None` if the encoded length can't be represented in `usize`. This will happen for
+/// input lengths in approximately the top quarter of the range of `usize`.
 pub fn encoded_len(bytes_len: usize, padding: bool) -> Option<usize> {
     let rem = bytes_len % 3;
 
@@ -128,6 +133,28 @@ pub(crate) fn add_padding(input_len: usize, output: &mut [u8]) -> usize {
     bytes_written
 }
 
+/// Errors that can occur while encoding into a slice.
+#[derive(Clone, Debug, PartialEq, Eq)]
+pub enum EncodeSliceError {
+    /// The provided slice is too small.
+    OutputSliceTooSmall,
+}
+
+impl fmt::Display for EncodeSliceError {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        match self {
+            Self::OutputSliceTooSmall => write!(f, "Output slice too small"),
+        }
+    }
+}
+
+#[cfg(any(feature = "std", test))]
+impl error::Error for EncodeSliceError {
+    fn cause(&self) -> Option<&dyn error::Error> {
+        None
+    }
+}
+
 #[cfg(test)]
 mod tests {
     use super::*;
@@ -304,7 +331,7 @@ mod tests {
 
             assert_eq!(
                 encoded_size,
-                engine.encode_slice(&orig_data, &mut encoded_data)
+                engine.encode_slice(&orig_data, &mut encoded_data).unwrap()
             );
 
             assert_encode_sanity(
@@ -325,51 +352,6 @@ mod tests {
         }
     }
 
-    #[test]
-    fn encode_engine_slice_fits_into_precisely_sized_slice() {
-        let mut orig_data = Vec::new();
-        let mut encoded_data = Vec::new();
-        let mut decoded = Vec::new();
-
-        let input_len_range = Uniform::new(0, 1000);
-
-        let mut rng = rand::rngs::SmallRng::from_entropy();
-
-        for _ in 0..10_000 {
-            orig_data.clear();
-            encoded_data.clear();
-            decoded.clear();
-
-            let input_len = input_len_range.sample(&mut rng);
-
-            for _ in 0..input_len {
-                orig_data.push(rng.gen());
-            }
-
-            let engine = random_engine(&mut rng);
-
-            let encoded_size = encoded_len(input_len, engine.config().encode_padding()).unwrap();
-
-            encoded_data.resize(encoded_size, 0);
-
-            assert_eq!(
-                encoded_size,
-                engine.encode_slice(&orig_data, &mut encoded_data)
-            );
-
-            assert_encode_sanity(
-                str::from_utf8(&encoded_data[0..encoded_size]).unwrap(),
-                engine.config().encode_padding(),
-                input_len,
-            );
-
-            engine
-                .decode_vec(&encoded_data[0..encoded_size], &mut decoded)
-                .unwrap();
-            assert_eq!(orig_data, decoded);
-        }
-    }
-
     #[test]
     fn encode_to_slice_random_valid_utf8() {
         let mut input = Vec::new();
@@ -400,7 +382,7 @@ mod tests {
 
             let orig_output_buf = output.clone();
 
-            let bytes_written = engine.inner_encode(&input, &mut output);
+            let bytes_written = engine.internal_encode(&input, &mut output);
 
             // make sure the part beyond bytes_written is the same garbage it was before
             assert_eq!(orig_output_buf[bytes_written..], output[bytes_written..]);