From 2b30adc7e03eb236007e8a481bc1a6248d005478 Mon Sep 17 00:00:00 2001
From: Darius Auding <Darius.auding@gmx.de>
Date: Mon, 29 May 2023 14:19:46 +0200
Subject: [PATCH] Cleaning up the codebase removing `unwraps()` and replacing
 them with propper error handling remove unnecessary `clone()` calls

---
 core/http/src/handlers/handlers.rs | 77 ++++++++++++++++++++----------
 core/http/src/handling/request.rs  | 24 +++++-----
 core/http/src/handling/routes.rs   |  2 +-
 core/http/src/setup.rs             | 28 ++++++++---
 site/src/main.rs                   | 27 ++++++-----
 5 files changed, 103 insertions(+), 55 deletions(-)

diff --git a/core/http/src/handlers/handlers.rs b/core/http/src/handlers/handlers.rs
index 0cf7d48..d9ba8df 100755
--- a/core/http/src/handlers/handlers.rs
+++ b/core/http/src/handlers/handlers.rs
@@ -15,29 +15,45 @@ pub async fn handle_connection(mut stream: TcpStream, mountpoints: Vec<MountPoin
     let mut http_request: Vec<String> = Vec::with_capacity(100);
     loop {
         let mut buffer = String::new();
-        let _ = buf_reader.read_line(&mut buffer).await.unwrap();
+        if let Err(_) = buf_reader.read_line(&mut buffer).await {
+            eprintln!("\x1b[31mAborting due to invalid UTF-8 in request header\x1b[0m");
+            return;
+        }
         if buffer == "\r\n" {
             break;
         }
         http_request.push(buffer);
     }
 
-    println!("{:#?}", http_request);
+    // println!("{:#?}", http_request);
 
-    let request_status_line = http_request.get(0);
-    if request_status_line == None {
+    let request_status_line = if let Some(status_line) = http_request.get(0).cloned() {
+        status_line
+    } else {
+        eprintln!("\x1b[31mAborting due to missing headers\x1b[0m");
         return;
-    }
-    let request_status_line = request_status_line.unwrap();
+    };
     let request = Request {
-        uri: &request_status_line.split(" ").nth(1).unwrap(),
-        headers: http_request.clone(),
-        method: request_status_line
+        uri: if let Some(uri) = &request_status_line.split(" ").nth(1) {
+            *uri
+        } else {
+            eprintln!("\x1b[31mAborting due to invalid status line\x1b[0m");
+            return;
+        },
+        headers: http_request,
+        method: if let Some(method) = request_status_line
             .split(" ")
-            .next()
-            .unwrap()
-            .parse()
-            .unwrap(),
+            .next() {
+            if let Ok(ok) = method.parse() {
+                ok
+            } else {
+                eprintln!("\x1b[31mAborting due to invalid request method\x1b[0m");
+                return;
+            }
+        } else {
+            eprintln!("\x1b[31mAborting due to invalid status line\x1b[0m");
+            return;
+        }
     };
 
     let mut data = Data {
@@ -48,8 +64,7 @@ pub async fn handle_connection(mut stream: TcpStream, mountpoints: Vec<MountPoin
         let length = if let Some(len) = request
             .headers
             .iter()
-            .filter(|header| header.contains("Content-Length: "))
-            .clone()
+            .filter(|header| header.starts_with("Content-Length: "))
             .map(|header| {
                 let header = header.strip_prefix("Content-Length: ").unwrap();
                 header.trim().parse::<usize>()
@@ -64,12 +79,17 @@ pub async fn handle_connection(mut stream: TcpStream, mountpoints: Vec<MountPoin
         } else {
             0
         };
+
         let mut buffer: Vec<u8> = vec![];
         buf_reader.read_buf(&mut buffer).await.unwrap();
         if buffer.len() != length {
-            let respone = len_not_defined(Status::LengthRequired).await;
-            stream.write_all(respone.0.as_bytes()).await.unwrap();
-            stream.write(&respone.1).await.unwrap();
+            let respone = len_not_defined(Status::LengthRequired);
+            let mut response = respone.0.as_bytes().to_vec();
+            response.extend_from_slice(&respone.1);
+            if let Err(e) = stream.write_all(&response).await {
+                eprintln!("\x1b[31mError {e} occured when trying to write answer to TCP-Stream for Client, aborting\x1b[0m");
+                return;
+            }
             return;
         }
         data.is_complete = true;
@@ -107,22 +127,27 @@ pub async fn handle_connection(mut stream: TcpStream, mountpoints: Vec<MountPoin
     let response = match handled_response {
         Some(val) => match val {
             Outcome::Success(success) => (
-                format!("HTTP/1.1 {}\r\n", success.status.unwrap())
+                format!("HTTP/1.1 {}\r\n", success.status.unwrap_or(Status::Ok))
                     + &success.headers.join("\r\n")
                     + "\r\n\r\n",
                 success.body.get_data(),
             ),
-            Outcome::Failure(error) => failure_handler(error).await,
-            Outcome::Forward(_) => failure_handler(Status::NotFound).await,
+            Outcome::Failure(error) => failure_handler(error),
+            Outcome::Forward(_) => failure_handler(Status::NotFound),
         },
-        None => failure_handler(Status::NotFound).await,
+        None => failure_handler(Status::NotFound),
     };
 
-    stream.write_all(response.0.as_bytes()).await.unwrap();
-    stream.write(&response.1).await.unwrap();
+    let mut resp = response.0.as_bytes().to_vec();
+    resp.extend_from_slice(&response.1);
+    if let Err(e) = stream.write_all(&resp).await {
+        eprintln!("\x1b[31mError {e} occured when trying to write answer to TCP-Stream for Client, aborting\x1b[0m");
+        return;
+    }
+    return;
 }
 
-async fn failure_handler(status: Status) -> (String, Vec<u8>) {
+fn failure_handler(status: Status) -> (String, Vec<u8>) {
     let page_404 = NamedFile::open(PathBuf::from("404.html")).unwrap();
     (
         format!(
@@ -135,7 +160,7 @@ async fn failure_handler(status: Status) -> (String, Vec<u8>) {
     )
 }
 
-async fn len_not_defined(status: Status) -> (String, Vec<u8>) {
+fn len_not_defined(status: Status) -> (String, Vec<u8>) {
     let page_411 = NamedFile::open(PathBuf::from("411.html")).unwrap();
     (
         format!(
diff --git a/core/http/src/handling/request.rs b/core/http/src/handling/request.rs
index b3615f1..7fa10b4 100644
--- a/core/http/src/handling/request.rs
+++ b/core/http/src/handling/request.rs
@@ -64,14 +64,14 @@ impl Request<'_> {
         }
     }
     pub fn get_form_keys(&self, keys: Vec<&str>) -> Result<HashMap<&str, &str>, ParseFormError> {
-        let data = self.uri.split_once("?");
-        if data == None {
+        let data = if let Some(val) = self.uri.split_once("?") {
+            val
+        } else {
             return Err(ParseFormError {
                 error: ParseErrors::NoData,
             });
-        }
+        };
         let data = data
-            .unwrap()
             .1
             .split("&")
             .map(|kvp| kvp.split_once("="))
@@ -79,21 +79,23 @@ impl Request<'_> {
 
         let mut values: HashMap<&str, &str> = HashMap::new();
         for kvp in data {
-            if kvp == None {
+            let kvp = if let Some(kvp) = kvp {
+                kvp
+            } else {
                 continue;
-            }
-            let kvp = kvp.unwrap();
+            };
             values.insert(kvp.0, kvp.1);
         }
         let mut response = HashMap::new();
         for key in keys {
-            let entry = values.get_key_value(key);
-            if entry == None {
+            let entry = if let Some(val) = values.get_key_value(key) {
+                val
+            } else {
                 return Err(ParseFormError {
                     error: ParseErrors::NoData,
                 });
-            }
-            response.insert(*entry.unwrap().0, *entry.unwrap().1);
+            };
+            response.insert(*entry.0, *entry.1);
         }
         Ok(response)
     }
diff --git a/core/http/src/handling/routes.rs b/core/http/src/handling/routes.rs
index 795e760..9ded1ce 100644
--- a/core/http/src/handling/routes.rs
+++ b/core/http/src/handling/routes.rs
@@ -35,7 +35,7 @@ impl Route<'_> {
             format: routeinfo.format,
         }
     }
-    pub fn compare_uri(self, uri: Uri) -> bool {
+    pub fn compare_uri(&self, uri: Uri) -> bool {
         let mut iter_comp_str = uri.split("/");
         for true_str in self.uri.split("/") {
             let comp_str = if let Some(str) = iter_comp_str.next() {
diff --git a/core/http/src/setup.rs b/core/http/src/setup.rs
index a50e7d1..9f57e76 100644
--- a/core/http/src/setup.rs
+++ b/core/http/src/setup.rs
@@ -19,7 +19,22 @@ pub struct Config {
 }
 
 impl<'a> Config {
-    pub fn mount(mut self, mountpoint: Uri<'static>, mut routes: Vec<Route<'static>>) -> Config {
+    fn check_mountpoint_taken(&self, to_insert: Uri) -> bool {
+        if let Some(to_check) = &self.mountpoints {
+            let len = to_check.len();
+            for i in 0..len {
+                if to_check[i].mountpoint == to_insert {
+                    return true; // Found a duplicate &str
+                }
+            }
+        };
+        false
+    }
+    pub fn mount(mut self, mountpoint: Uri<'static>, mut routes: Vec<Route<'static>>) -> Self {
+        if self.check_mountpoint_taken(mountpoint) {
+            eprintln!("\x1b[31mTrying to reassign a mountpoint, mountpoint `{mountpoint}` already taken.\x1b[0m");
+            return self;
+        }
         routes.sort_by(|a, b| a.rank.cmp(&b.rank));
         let mut mount_message = format!("  >> \x1b[35m{}\x1b[0m\n", mountpoint);
         for (index, route) in routes.iter().enumerate() {
@@ -58,10 +73,7 @@ impl<'a> Config {
                     println!("Shutting down...");
                     break;
                 }
-
-                income = self.address.accept() => {
-                    let income = income.unwrap();
-                    let (socket, _) = income;
+                Ok((socket, _)) = self.address.accept() => {
                     let mountpoints = self.mountpoints.clone().unwrap();
                     tokio::spawn(async move { handle_connection(socket, mountpoints).await; });
                 }
@@ -70,7 +82,11 @@ impl<'a> Config {
     }
 }
 pub async fn build(ip: &str) -> Config {
-    let listener = TcpListener::bind(ip).await.unwrap();
+    let listener = if let Ok(listener) = TcpListener::bind(ip).await {
+        listener
+    } else {
+        panic!("\x1b[31mCould't bind Listener to address\x1b[0m");
+    };
     let ip = ip.splitn(2, ":").collect::<Vec<&str>>();
     if ip.len() != 2 {
         panic!("Invalid IP Address");
diff --git a/site/src/main.rs b/site/src/main.rs
index 22897e2..48d109d 100644
--- a/site/src/main.rs
+++ b/site/src/main.rs
@@ -1,4 +1,4 @@
-use std::{path::PathBuf, collections::HashMap};
+use std::{collections::HashMap, path::PathBuf};
 
 use http::handling::{
     file_handlers::NamedFile,
@@ -20,17 +20,19 @@ fn hashmap_to_string(map: &HashMap<&str, &str>) -> String {
     result
 }
 
-
 fn handle_static_hi(request: Request<'_>, _data: Data) -> Outcome<Response, Status, Data> {
     let response = hashmap_to_string(&request.get_form_keys(vec!["asdf", "jkj"]).unwrap());
-    Outcome::Success(Response { headers: vec![
-        format!("Content-Length: {}", response.len()), 
-        format!("Content-Type: text/plain")
-    ], status: Some(Status::Ok), body: Box::new(response) })
+    Outcome::Success(Response {
+        headers: vec![
+            format!("Content-Length: {}", response.len()),
+            format!("Content-Type: text/plain"),
+        ],
+        status: Some(Status::Ok),
+        body: Box::new(response),
+    })
     // Outcome::Forward(data)
 }
 
-
 fn handler(request: Request<'_>, _data: Data) -> Outcome<Response, Status, Data> {
     let response = fileserver(request.uri.strip_prefix("static/").unwrap());
     let response = match response {
@@ -96,15 +98,18 @@ async fn main() {
     };
 
     let static_hi = Route {
-        format: None, 
+        format: None,
         handler: handle_static_hi,
         name: Some("Handle_Static_hi"),
         uri: "static/hi",
         method: Method::Get,
-        rank: 0
+        rank: 0,
     };
 
-    http::build("127.0.0.1:8000").await
+    http::build("127.0.0.1:8000")
+        .await
         .mount("/", vec![fileserver, post_test, static_hi])
-        .launch().await;
+        .mount("/post/", vec![post_test])
+        .launch()
+        .await;
 }
-- 
GitLab