5
\$\begingroup\$

The base64 decode algorithm is taken directly from Base64 wiki page. This exercise is meant to help me learn Rust, so any pointers in that general direction are very welcome. :)

fn main() {
    println!("{}",
         hex_to_base64("49276d206b696c6c696e6720796f757220627261696e206c696b65206120706f69736f6e6f7573206d757368726f6f6d"));
}

fn hex_to_base64(hex_string: &str) -> String {
    println!("Converting {} from hex to base64", hex_string);
    let alphabet: Vec<char> =
        "ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789+/=".chars().collect();

    // Decode the input string from hex into individual bytes
    let input_chars: Vec<char> = hex_string.chars().collect();
    let mut decoded_bytes: Vec<u8> = Vec::new();

    for i in 0..hex_string.len() {
        if i % 2 != 0 { continue }
        let first_byte = input_chars[i].to_digit(16).unwrap() as u8;
        let second_byte = input_chars[i + 1].to_digit(16).unwrap() as u8;
        println!("Decoding 0x{}{} to bytes:", input_chars[i], input_chars[i + 1]);
        let byte = (first_byte << 4) | second_byte;
        println!("    {}", byte);
        decoded_bytes.push(byte)
    }

    // Encode the decoded bytes into Base64
    let mut output = String::from("");

    let mut b: u8;
    for i in 0..decoded_bytes.len() {
        if i % 3 != 0 { continue }
        b = (decoded_bytes[i] & 0xFC) >> 2;
        output.push(alphabet[b as usize]);
        b = (decoded_bytes[i] & 0x03) << 4;
        if i + 1 < decoded_bytes.len() {
            b |= (decoded_bytes[i + 1] & 0xF0) >> 4;
            output.push(alphabet[b as usize]);
            b = (decoded_bytes[i + 1] & 0x0F) << 2;
            if i + 2 < decoded_bytes.len() {
                b |= (decoded_bytes[i + 2] & 0xC0) >> 6;
                output.push(alphabet[b as usize]);
                b = decoded_bytes[i + 2] & 0x3F;
                output.push(alphabet[b as usize]);
            } else {
                output.push(alphabet[b as usize]);
                output.push('=');
            }
        } else {
            output.push(alphabet[b as usize]);
            output.push_str("==");
        }
    }
    return output
}

#[test]
fn test_hex_to_base64() {
    assert!(hex_to_base64("49276d206b696c6c696e6720796f757220627261696e206c696b65206120706f69736f6e6f7573206d757368726f6f6d") == String::from("SSdtIGtpbGxpbmcgeW91ciBicmFpbiBsaWtlIGEgcG9pc29ub3VzIG11c2hyb29t"));
}
\$\endgroup\$
  • \$\begingroup\$ Matasano crypto challenge, by any chance? \$\endgroup\$ – Shepmaster Nov 21 '16 at 16:30
  • \$\begingroup\$ That's right :) Didn't want to name it explicitly, so that it a little harder to google the solution. \$\endgroup\$ – sYnfo Nov 21 '16 at 19:28
4
\$\begingroup\$
  1. I am glad to see your method takes a &str instead of a &String or String!

  2. Shouldn't be printing output inside function.

  3. The one function is doing too much. Instead, create two functions, one to convert hex -> bytes, andone to convert bytes -> base64. Note that your comments already declare where functions should be! You could promote those to doc comments, or the function name may be enough.

  4. Don't specify exact types when declaring variables, let type inference handle it. decoded_bytes: Vec<u8> = Vec::new(); should be decoded_bytes = Vec::new();. Note that changed code has very few foo: type lines, mostly for collect where the return type is up to the caller.

  5. Can collect into a Vec<_>, no need to specify item type.

  6. Use chunks instead of reimplementing it. Note that it returns a smaller chunk when not a multiple of the chunk size. hex_to_bytes will crash with odd count of values, I think the previous did as well.

  7. Once operating on an iterator, switch to map and add collect. Avoids need for mutation.

  8. Prefer expect over unwrap. When it crashes you will be thankful.

  9. String::new instead of String::from("").

  10. Traditionally, the Base64 conversion table is a hard-coded constant, not re-created for each call.

  11. The length of the bytes never changes, get it once before the loop and reuse it.

  12. Declare variables as near to usage as they can be. b shouldn't be outside the loop.

  13. Consider rebinding b when it's reset to a brand new value.

  14. Consider getting the result of indexing and storing it into a variable, since it shouldnt change

  15. Instead of checking the length and then indexing, use if let combined with get.

  16. Use chunks again.

  17. No explicit return at end of method.

  18. Use assert_eq!.

  19. Can compare string literal against String, no need to allocate


  1. Is there a better way to write debug statements?

You may be interested in the log crate, although I'd argue that these printing statements only had any value when the function was being originally developed, and probably don't deserve to stay in as logging either.

  1. I'm not sure what this one means?

Instead of

let mut b = 1;
b += 1;
b = 5;

You may want to try

let mut b = 1;
b += 1;
let b = 5;

This may help indicate that you deliberately reset the value.


fn main() {
    let b64 = hex_to_base64("49276d206b696c6c696e6720796f757220627261696e206c696b65206120706f69736f6e6f7573206d757368726f6f6d");
    println!("{}", b64);
}

/// Decode the input string from hex into individual bytes
fn hex_to_bytes(hex_string: &str) -> Vec<u8> {
    let input_chars: Vec<_> = hex_string.chars().collect();

    input_chars.chunks(2).map(|chunk| {
        let first_byte = chunk[0].to_digit(16).unwrap();
        let second_byte = chunk[1].to_digit(16).unwrap();
        ((first_byte << 4) | second_byte) as u8
    }).collect()
}

/// Encode the decoded bytes into Base64
fn bytes_to_base64(decoded_bytes: &[u8]) -> String {
    let mut output = String::new();
    let alphabet: Vec<_> =
        "ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789+/=".chars().collect();

    for chunk in decoded_bytes.chunks(3) {
        let c0 = chunk[0];
        let b = (c0 & 0xFC) >> 2;
        output.push(alphabet[b as usize]);

        let mut b = (c0 & 0x03) << 4;

        if let Some(c1) = chunk.get(1) {
            b |= (c1 & 0xF0) >> 4;
            output.push(alphabet[b as usize]);

            let mut b = (c1 & 0x0F) << 2;

            if let Some(c2) = chunk.get(2) {
                b |= (c2 & 0xC0) >> 6;
                output.push(alphabet[b as usize]);

                let b = c2 & 0x3F;
                output.push(alphabet[b as usize]);
            } else {
                output.push(alphabet[b as usize]);
                output.push('=');
            }
        } else {
            output.push(alphabet[b as usize]);
            output.push_str("==");
        }
    }

    output
}


fn hex_to_base64(hex_string: &str) -> String {
    bytes_to_base64(&hex_to_bytes(hex_string))
}

#[test]
fn test_hex_to_base64() {
    assert_eq!(
        hex_to_base64("49276d206b696c6c696e6720796f757220627261696e206c696b65206120706f69736f6e6f7573206d757368726f6f6d"),
        "SSdtIGtpbGxpbmcgeW91ciBicmFpbiBsaWtlIGEgcG9pc29ub3VzIG11c2hyb29t"
    );
}
\$\endgroup\$
  • \$\begingroup\$ Thank you! Great points. 2) Is there a better way to write debug statements? 3) That was me testing out number-none.com/blow/john_carmack_on_inlined_code.html, but since the function don't mutate state anyway, I do agree separate function are better 12) For whatever reason I thought that's would mean doing extra work, good to know it isn't. 13) I'm not sure what this one means? \$\endgroup\$ – sYnfo Nov 21 '16 at 19:57
  • \$\begingroup\$ @sYnfo I've updated with responses. I don't really agree with Carmack, but I haven't made a AAA video game either. \$\endgroup\$ – Shepmaster Nov 22 '16 at 0:24
  • 1
    \$\begingroup\$ Carmack's note at the top suggests that his major goal in inlining is avoiding surprise state changes. If he's talking about mutations in the parameter objects like I think he is, then I think Rust avoids the surprises by requiring mutation and ownership documentation. This obviates inlining, freeing you up to maintain a nicer separation of concerns. \$\endgroup\$ – Doug Bradshaw Nov 2 '17 at 18:00

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.