refactor(http): [torrust#160] extract functions for percent decoding

src/http/filters.rs

@@ -6,6 +6,7 @@ use std::sync::Arc;
 use warp::{reject, Filter, Rejection};
 
 use super::error::Error;
+use super::percent_encoding::{percent_decode_info_hash, percent_decode_peer_id};
 use super::{request, WebResult};
 use crate::protocol::common::MAX_SCRAPE_TORRENTS;
 use crate::protocol::info_hash::InfoHash;
@@ -77,9 +78,11 @@ fn info_hashes(raw_query: &String) -> WebResult<Vec<InfoHash>> {
 
     for v in split_raw_query {
         if v.contains("info_hash") {
+            // get raw percent encoded infohash
             let raw_info_hash = v.split('=').collect::<Vec<&str>>()[1];
-            let info_hash_bytes = percent_encoding::percent_decode_str(raw_info_hash).collect::<Vec<u8>>();
-            let info_hash = InfoHash::from_str(&hex::encode(info_hash_bytes));
+
+            let info_hash = percent_decode_info_hash(raw_info_hash);
+
             if let Ok(ih) = info_hash {
                 info_hashes.push(ih);
             }
@@ -106,22 +109,15 @@ fn peer_id(raw_query: &String) -> WebResult<peer::Id> {
     for v in split_raw_query {
         // look for the peer_id param
         if v.contains("peer_id") {
-            // get raw percent_encoded peer_id
+            // get raw percent encoded peer id
             let raw_peer_id = v.split('=').collect::<Vec<&str>>()[1];
 
-            // decode peer_id
-            let peer_id_bytes = percent_encoding::percent_decode_str(raw_peer_id).collect::<Vec<u8>>();
-
-            // peer_id must be 20 bytes
-            if peer_id_bytes.len() != 20 {
+            if let Ok(id) = percent_decode_peer_id(raw_peer_id) {
+                peer_id = Some(id);
+            } else {
                 return Err(reject::custom(Error::InvalidPeerId));
             }
 
-            // clone peer_id_bytes into fixed length array
-            let mut byte_arr: [u8; 20] = Default::default();
-            byte_arr.clone_from_slice(peer_id_bytes.as_slice());
-
-            peer_id = Some(peer::Id(byte_arr));
             break;
         }
     }

src/http/mod.rs

@@ -15,6 +15,7 @@ pub mod axum;
 pub mod error;
 pub mod filters;
 pub mod handlers;
+pub mod percent_encoding;
 pub mod request;
 pub mod response;
 pub mod routes;

src/http/percent_encoding.rs

@@ -0,0 +1,68 @@
+use std::str::FromStr;
+
+use crate::protocol::info_hash::InfoHash;
+use crate::tracker::peer::{self, IdConversionError};
+
+/// # Errors
+///
+/// Will return `Err` if if the decoded bytes do not represent a valid `InfoHash`.
+pub fn percent_decode_info_hash(raw_info_hash: &str) -> Result<InfoHash, binascii::ConvertError> {
+    let bytes = percent_encoding::percent_decode_str(raw_info_hash).collect::<Vec<u8>>();
+    InfoHash::from_str(&hex::encode(bytes))
+}
+
+/// # Errors
+///
+/// Will return `Err` if if the decoded bytes do not represent a valid `peer::Id`.
+pub fn percent_decode_peer_id(raw_peer_id: &str) -> Result<peer::Id, IdConversionError> {
+    let bytes = percent_encoding::percent_decode_str(raw_peer_id).collect::<Vec<u8>>();
+    peer::Id::try_from(bytes)
+}
+
+#[cfg(test)]
+mod tests {
+    use std::str::FromStr;
+
+    use crate::http::percent_encoding::{percent_decode_info_hash, percent_decode_peer_id};
+    use crate::protocol::info_hash::InfoHash;
+    use crate::tracker::peer;
+
+    #[test]
+    fn it_should_decode_a_percent_encoded_info_hash() {
+        let encoded_infohash = "%3B%24U%04%CF%5F%11%BB%DB%E1%20%1C%EAjk%F4Z%EE%1B%C0";
+
+        let info_hash = percent_decode_info_hash(encoded_infohash).unwrap();
+
+        assert_eq!(
+            info_hash,
+            InfoHash::from_str("3b245504cf5f11bbdbe1201cea6a6bf45aee1bc0").unwrap()
+        );
+    }
+
+    #[test]
+    fn it_should_fail_decoding_an_invalid_percent_encoded_info_hash() {
+        let invalid_encoded_infohash = "invalid percent-encoded infohash";
+
+        let info_hash = percent_decode_info_hash(invalid_encoded_infohash);
+
+        assert!(info_hash.is_err());
+    }
+
+    #[test]
+    fn it_should_decode_a_percent_encoded_peer_id() {
+        let encoded_peer_id = "%2DqB00000000000000000";
+
+        let peer_id = percent_decode_peer_id(encoded_peer_id).unwrap();
+
+        assert_eq!(peer_id, peer::Id(*b"-qB00000000000000000"));
+    }
+
+    #[test]
+    fn it_should_fail_decoding_an_invalid_percent_encoded_peer_id() {
+        let invalid_encoded_peer_id = "invalid percent-encoded peer id";
+
+        let peer_id = percent_decode_peer_id(invalid_encoded_peer_id);
+
+        assert!(peer_id.is_err());
+    }
+}

src/tracker/peer.rs

@@ -91,6 +91,45 @@ impl Peer {
 #[derive(PartialEq, Eq, Hash, Clone, Debug, PartialOrd, Ord, Copy)]
 pub struct Id(pub [u8; 20]);
 
+#[derive(Debug)]
+pub enum IdConversionError {
+    NotEnoughBytes,
+    TooManyBytes,
+}
+
+impl Id {
+    /// # Panics
+    ///
+    /// Will panic if byte slice does not contains the exact amount of bytes need for the `Id`.
+    #[must_use]
+    pub fn from_bytes(bytes: &[u8]) -> Self {
+        assert_eq!(bytes.len(), 20);
+        let mut ret = Id([0u8; 20]);
+        ret.0.clone_from_slice(bytes);
+        ret
+    }
+}
+
+impl From<[u8; 20]> for Id {
+    fn from(bytes: [u8; 20]) -> Self {
+        Id(bytes)
+    }
+}
+
+impl TryFrom<Vec<u8>> for Id {
+    type Error = IdConversionError;
+
+    fn try_from(bytes: Vec<u8>) -> Result<Self, Self::Error> {
+        if bytes.len() < 20 {
+            return Err(IdConversionError::NotEnoughBytes);
+        }
+        if bytes.len() > 20 {
+            return Err(IdConversionError::TooManyBytes);
+        }
+        Ok(Self::from_bytes(&bytes))
+    }
+}
+
 impl std::fmt::Display for Id {
     fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
         match self.to_hex_string() {
@@ -239,6 +278,75 @@ mod test {
     mod torrent_peer_id {
         use crate::tracker::peer;
 
+        #[test]
+        fn should_be_instantiated_from_a_byte_slice() {
+            let id = peer::Id::from_bytes(&[
+                0, 159, 146, 150, 0, 159, 146, 150, 0, 159, 146, 150, 0, 159, 146, 150, 0, 159, 146, 150,
+            ]);
+
+            let expected_id = peer::Id([
+                0, 159, 146, 150, 0, 159, 146, 150, 0, 159, 146, 150, 0, 159, 146, 150, 0, 159, 146, 150,
+            ]);
+
+            assert_eq!(id, expected_id);
+        }
+
+        #[test]
+        #[should_panic]
+        fn should_fail_trying_to_instantiate_from_a_byte_slice_with_less_than_20_bytes() {
+            let less_than_20_bytes = [0; 19];
+            let _ = peer::Id::from_bytes(&less_than_20_bytes);
+        }
+
+        #[test]
+        #[should_panic]
+        fn should_fail_trying_to_instantiate_from_a_byte_slice_with_more_than_20_bytes() {
+            let more_than_20_bytes = [0; 21];
+            let _ = peer::Id::from_bytes(&more_than_20_bytes);
+        }
+
+        #[test]
+        fn should_be_converted_from_a_20_byte_array() {
+            let id = peer::Id::from([
+                0, 159, 146, 150, 0, 159, 146, 150, 0, 159, 146, 150, 0, 159, 146, 150, 0, 159, 146, 150,
+            ]);
+
+            let expected_id = peer::Id([
+                0, 159, 146, 150, 0, 159, 146, 150, 0, 159, 146, 150, 0, 159, 146, 150, 0, 159, 146, 150,
+            ]);
+
+            assert_eq!(id, expected_id);
+        }
+
+        #[test]
+        fn should_be_converted_from_a_byte_vector() {
+            let id = peer::Id::try_from(
+                [
+                    0, 159, 146, 150, 0, 159, 146, 150, 0, 159, 146, 150, 0, 159, 146, 150, 0, 159, 146, 150,
+                ]
+                .to_vec(),
+            )
+            .unwrap();
+
+            let expected_id = peer::Id([
+                0, 159, 146, 150, 0, 159, 146, 150, 0, 159, 146, 150, 0, 159, 146, 150, 0, 159, 146, 150,
+            ]);
+
+            assert_eq!(id, expected_id);
+        }
+
+        #[test]
+        #[should_panic]
+        fn should_fail_trying_to_convert_from_a_byte_vector_with_less_than_20_bytes() {
+            let _ = peer::Id::try_from([0; 19].to_vec()).unwrap();
+        }
+
+        #[test]
+        #[should_panic]
+        fn should_fail_trying_to_convert_from_a_byte_vector_with_more_than_20_bytes() {
+            let _ = peer::Id::try_from([0; 21].to_vec()).unwrap();
+        }
+
         #[test]
         fn should_be_converted_to_hex_string() {
             let id = peer::Id(*b"-qB00000000000000000");

tests/http/mod.rs

@@ -1,7 +1,28 @@
 pub mod asserts;
-pub mod bencode;
 pub mod client;
 pub mod connection_info;
 pub mod requests;
 pub mod responses;
 pub mod server;
+
+use percent_encoding::NON_ALPHANUMERIC;
+
+pub type ByteArray20 = [u8; 20];
+
+pub fn percent_encode_byte_array(bytes: &ByteArray20) -> String {
+    percent_encoding::percent_encode(bytes, NON_ALPHANUMERIC).to_string()
+}
+
+pub struct InfoHash(ByteArray20);
+
+impl InfoHash {
+    pub fn new(vec: &[u8]) -> Self {
+        let mut byte_array_20: ByteArray20 = Default::default();
+        byte_array_20.clone_from_slice(vec);
+        Self(byte_array_20)
+    }
+
+    pub fn bytes(&self) -> ByteArray20 {
+        self.0
+    }
+}

tests/http/requests/announce.rs

@@ -2,12 +2,11 @@ use std::fmt;
 use std::net::{IpAddr, Ipv4Addr};
 use std::str::FromStr;
 
-use percent_encoding::NON_ALPHANUMERIC;
 use serde_repr::Serialize_repr;
 use torrust_tracker::protocol::info_hash::InfoHash;
 use torrust_tracker::tracker::peer::Id;
 
-use crate::http::bencode::ByteArray20;
+use crate::http::{percent_encode_byte_array, ByteArray20};
 
 pub struct Query {
     pub info_hash: ByteArray20,
@@ -211,11 +210,11 @@ impl QueryParams {
         let compact = announce_query.compact.as_ref().map(std::string::ToString::to_string);
 
         Self {
-            info_hash: Some(percent_encoding::percent_encode(&announce_query.info_hash, NON_ALPHANUMERIC).to_string()),
+            info_hash: Some(percent_encode_byte_array(&announce_query.info_hash)),
             peer_addr: Some(announce_query.peer_addr.to_string()),
             downloaded: Some(announce_query.downloaded.to_string()),
             uploaded: Some(announce_query.uploaded.to_string()),
-            peer_id: Some(percent_encoding::percent_encode(&announce_query.peer_id, NON_ALPHANUMERIC).to_string()),
+            peer_id: Some(percent_encode_byte_array(&announce_query.peer_id)),
             port: Some(announce_query.port.to_string()),
             left: Some(announce_query.left.to_string()),
             event,

tests/http/requests/scrape.rs

@@ -1,10 +1,9 @@
 use std::fmt;
 use std::str::FromStr;
 
-use percent_encoding::NON_ALPHANUMERIC;
 use torrust_tracker::protocol::info_hash::InfoHash;
 
-use crate::http::bencode::ByteArray20;
+use crate::http::{percent_encode_byte_array, ByteArray20};
 
 pub struct Query {
     pub info_hash: Vec<ByteArray20>,
@@ -111,7 +110,7 @@ impl QueryParams {
         let info_hashes = scrape_query
             .info_hash
             .iter()
-            .map(|info_hash_bytes| percent_encoding::percent_encode(info_hash_bytes, NON_ALPHANUMERIC).to_string())
+            .map(percent_encode_byte_array)
             .collect::<Vec<String>>();
 
         Self { info_hash: info_hashes }

tests/http/responses/scrape.rs

@@ -4,7 +4,7 @@ use std::str;
 use serde::{self, Deserialize, Serialize};
 use serde_bencode::value::Value;
 
-use crate::http::bencode::{ByteArray20, InfoHash};
+use crate::http::{ByteArray20, InfoHash};
 
 #[derive(Debug, PartialEq, Default)]
 pub struct Response {