[Interface] Return C-compatible result when Rust FFI function fails (#…

…3047)

rust/coverage.stats

@@ -1 +1 @@
-83.9
+93.3

rust/tw_encoding/Cargo.toml

@@ -4,7 +4,7 @@ version = "0.1.0"
 edition = "2021"
 
 [dependencies]
-base64 = "0.21.0"
 bs58 = "0.4.0"
+data-encoding = "2.3.3"
 hex = "0.4.3"
 tw_memory = { path = "../tw_memory" }

rust/tw_encoding/src/base32.rs

@@ -4,83 +4,64 @@
 // terms governing use, modification, and redistribution, is contained in the
 // file LICENSE at the root of the source code distribution tree.
 
+use crate::{EncodingError, EncodingResult};
+use data_encoding::{Encoding, Specification};
+use std::cell::RefCell;
+use std::collections::hash_map::Entry;
 use std::collections::HashMap;
 
-const ALPHABET_RFC4648: &[u8] = b"ABCDEFGHIJKLMNOPQRSTUVWXYZ234567";
+const ALPHABET_RFC4648: &str = "ABCDEFGHIJKLMNOPQRSTUVWXYZ234567";
 
-pub fn encode(input: &[u8], alphabet: Option<&[u8]>, padding: bool) -> Result<String, String> {
-    let alphabet = alphabet.unwrap_or(ALPHABET_RFC4648);
-    if alphabet.len() != 32 {
-        return Err("Invalid alphabet: must contain 32 characters".to_string());
-    }
-
-    let mut result = String::new();
-    let mut buffer: u32 = 0;
-    let mut buffer_size = 0;
-
-    for &byte in input {
-        buffer = (buffer << 8) | u32::from(byte);
-        buffer_size += 8;
+type EncodingMap = HashMap<EncodingParams, Encoding>;
 
-        while buffer_size >= 5 {
-            let value = alphabet[(buffer >> (buffer_size - 5)) as usize & 31];
-            result.push(char::from(value));
-            buffer_size -= 5;
-        }
-    }
+thread_local! {
+    /// Cache the encodings to avoid parsing already handled alphabets on each operation.
+    static ENCODINGS: RefCell<EncodingMap> = RefCell::new(HashMap::new());
+}
 
-    if buffer_size > 0 {
-        let value = alphabet[(buffer << (5 - buffer_size)) as usize & 31];
-        result.push(char::from(value));
-    }
+#[derive(Debug, Clone, Eq, Hash, PartialEq)]
+struct EncodingParams {
+    alphabet: String,
+    padding: bool,
+}
 
-    if padding {
-        let padding = 8 - (result.len() % 8);
-        result.extend(std::iter::repeat('=').take(padding));
-    }
+pub fn encode(input: &[u8], alphabet: Option<String>, padding: bool) -> EncodingResult<String> {
+    let encoding = get_encoding(alphabet, padding)?;
+    Ok(encoding.encode(input))
+}
 
-    Ok(result)
+pub fn decode(input: &str, alphabet: Option<String>, padding: bool) -> EncodingResult<Vec<u8>> {
+    let encoding = get_encoding(alphabet, padding)?;
+    encoding
+        .decode(input.as_bytes())
+        .map_err(|_| EncodingError::InvalidInput)
 }
 
-/// TODO `base64::decode` requires for padding bytes to be present if `padding = true`.
-/// This leads to an inconsistent behaviour.
-pub fn decode(input: &str, alphabet: Option<&[u8]>, padding: bool) -> Result<Vec<u8>, String> {
-    let alphabet = alphabet.unwrap_or(ALPHABET_RFC4648);
-    let mut output = Vec::new();
-    let mut buffer: u32 = 0;
-    let mut bits_left = 0;
-    let alphabet_map: HashMap<u8, u32> = alphabet
-        .iter()
-        .enumerate()
-        .map(|(i, &c)| (c, i as u32))
-        .collect();
-    let input = if padding {
-        input.trim_end_matches('=')
-    } else {
-        input
-    };
-
-    for c in input.bytes() {
-        let val = match alphabet_map.get(&c) {
-            Some(val) => *val,
-            None => return Err("Invalid character in input".to_string()),
-        };
-        buffer = (buffer << 5) | val;
-        bits_left += 5;
-        if bits_left >= 8 {
-            output.push((buffer >> (bits_left - 8)) as u8);
-            bits_left -= 8;
-        }
-    }
+fn get_encoding(alphabet: Option<String>, padding: bool) -> EncodingResult<Encoding> {
+    let alphabet = alphabet.unwrap_or_else(|| ALPHABET_RFC4648.to_string());
+    let encoding_params = EncodingParams { alphabet, padding };
+
+    ENCODINGS.with(
+        |encodings| match encodings.borrow_mut().entry(encoding_params.clone()) {
+            Entry::Occupied(ready) => Ok(ready.get().clone()),
+            Entry::Vacant(vacant) => {
+                let new_encoding = create_encoding(encoding_params)?;
+                Ok(vacant.insert(new_encoding).clone())
+            },
+        },
+    )
+}
 
-    if padding && bits_left >= 5 {
-        return Err("Invalid padding in input".to_string());
+fn create_encoding(params: EncodingParams) -> EncodingResult<Encoding> {
+    let mut specification = Specification::new();
+    specification.symbols = params.alphabet;
+    if params.padding {
+        specification.padding = Some('=');
     }
 
-    if output == vec![0] {
-        return Ok(vec![]);
-    }
-    Ok(output)
+    specification
+        .encoding()
+        .map_err(|_| EncodingError::InvalidAlphabet)
 }
 
 #[cfg(test)]
@@ -110,12 +91,12 @@ mod tests {
         let alphabet = "abcdefghijklmnopqrstuvwxyz234567";
         let data = b"7uoq6tp427uzv7fztkbsnn64iwotfrristwpryy";
         let expected = "g52w64jworydimrxov5hmn3gpj2gwyttnzxdmndjo5xxiztsojuxg5dxobzhs6i";
-        let result = encode(data, Some(alphabet.as_bytes()), false).unwrap();
+        let result = encode(data, Some(alphabet.to_string()), false).unwrap();
         assert_eq!(result, expected);
 
         let invalid_alphabet = "invalidalphabet";
-        let result = encode(data, Some(invalid_alphabet.as_bytes()), false);
-        assert_eq!(result.is_err(), true);
+        let result = encode(data, Some(invalid_alphabet.to_string()), false);
+        assert_eq!(result, Err(EncodingError::InvalidAlphabet));
     }
 
     #[test]
@@ -127,22 +108,16 @@ mod tests {
         assert_eq!(result.as_slice(), expected);
     }
 
-    #[test]
-    fn test_base32_decode_abc() {
-        let data = "ABC";
-        let expected = b"";
-
-        let result = decode(data, None, false).unwrap();
-        assert_eq!(result.as_slice(), expected);
-    }
-
     #[test]
     fn test_base32_decode_padding() {
         let data = "JBSWY3DPFQQHO33SNRSCC===";
         let expected = b"Hello, world!";
 
         let result = decode(data, None, true).unwrap();
         assert_eq!(result.as_slice(), expected);
+
+        let data = "JBSWY3DPFQQHO33SNRSCC";
+        decode(data, None, true).expect_err("No padding while it's required");
     }
 
     #[test]
@@ -151,7 +126,22 @@ mod tests {
         let data = "g52w64jworydimrxov5hmn3gpj2gwyttnzxdmndjo5xxiztsojuxg5dxobzhs6i";
         let expected = b"7uoq6tp427uzv7fztkbsnn64iwotfrristwpryy";
 
-        let result = decode(data, Some(alphabet.as_bytes()), false).unwrap();
+        let result = decode(data, Some(alphabet.to_string()), false).unwrap();
         assert_eq!(result.as_slice(), expected);
     }
+
+    #[test]
+    fn test_decode_invalid() {
+        const BASE32_ALPHABET_FILECOIN: &str = "abcdefghijklmnopqrstuvwxyz234567";
+
+        decode("+-", None, false).unwrap_err();
+        decode("A", None, false).unwrap_err();
+        decode("ABC", None, false).unwrap_err();
+        decode(
+            "rw6wy7w6sbsguyn3yzeygg34fgf72n5ao5sxykz",
+            Some(BASE32_ALPHABET_FILECOIN.to_string()),
+            false,
+        )
+        .unwrap_err();
+    }
 }

rust/tw_encoding/src/base58.rs

@@ -4,14 +4,24 @@
 // terms governing use, modification, and redistribution, is contained in the
 // file LICENSE at the root of the source code distribution tree.
 
-use bs58::{decode::Result, Alphabet};
+use crate::{EncodingError, EncodingResult};
+use bs58::{decode::Error, Alphabet};
+
+impl From<Error> for EncodingError {
+    fn from(_: Error) -> Self {
+        EncodingError::InvalidInput
+    }
+}
 
 pub fn encode(input: &[u8], alphabet: &Alphabet) -> String {
     bs58::encode(input).with_alphabet(alphabet).into_string()
 }
 
-pub fn decode(input: &str, alphabet: &Alphabet) -> Result<Vec<u8>> {
-    bs58::decode(input).with_alphabet(alphabet).into_vec()
+pub fn decode(input: &str, alphabet: &Alphabet) -> EncodingResult<Vec<u8>> {
+    bs58::decode(input)
+        .with_alphabet(alphabet)
+        .into_vec()
+        .map_err(EncodingError::from)
 }
 
 #[cfg(test)]

rust/tw_encoding/src/base64.rs

@@ -4,20 +4,21 @@
 // terms governing use, modification, and redistribution, is contained in the
 // file LICENSE at the root of the source code distribution tree.
 
-use base64::{engine::general_purpose, DecodeError, Engine as _};
+use crate::{EncodingError, EncodingResult};
 
 pub fn encode(data: &[u8], is_url: bool) -> String {
     if is_url {
-        general_purpose::URL_SAFE.encode(data)
+        data_encoding::BASE64URL.encode(data)
     } else {
-        general_purpose::STANDARD.encode(data)
+        data_encoding::BASE64.encode(data)
     }
 }
 
-pub fn decode(data: &str, is_url: bool) -> Result<Vec<u8>, DecodeError> {
+pub fn decode(data: &str, is_url: bool) -> EncodingResult<Vec<u8>> {
     if is_url {
-        general_purpose::URL_SAFE.decode(data)
+        data_encoding::BASE64URL.decode(data.as_bytes())
     } else {
-        general_purpose::STANDARD.decode(data)
+        data_encoding::BASE64.decode(data.as_bytes())
     }
+    .map_err(|_| EncodingError::InvalidInput)
 }