From 0ebf01ceb1fa8871c00d3851b010df50f9cf76ce Mon Sep 17 00:00:00 2001
From: alemi <me@alemi.dev>
Date: Sat, 23 Dec 2023 16:54:42 +0100
Subject: [PATCH] fix: properly catch empty contact, small refactor

---
 src/model.rs                 | 14 +++++++++--
 src/notifications/console.rs |  2 +-
 src/routes.rs                | 45 +++++++++++++++++++++++++-----------
 3 files changed, 44 insertions(+), 17 deletions(-)

diff --git a/src/model.rs b/src/model.rs
index 7231881..5a3bd8f 100644
--- a/src/model.rs
+++ b/src/model.rs
@@ -3,18 +3,22 @@ use chrono::{DateTime, Utc};
 
 #[derive(Debug, Clone, Default, Serialize, Deserialize)]
 pub struct GuestBookPage {
-	pub author: Option<String>,
-	pub contact: Option<String>,
+	pub author: String,
 	pub body: String,
 	pub date: DateTime<Utc>,
 	pub avatar: String,
 	pub url: Option<String>,
+	pub contact: Option<String>,
 }
 
 #[derive(Debug, Clone, Default, Serialize, Deserialize)]
 pub struct Insertion {
+	#[serde(deserialize_with = "non_empty_str")]
 	pub author: Option<String>,
+
+	#[serde(deserialize_with = "non_empty_str")]
 	pub contact: Option<String>,
+
 	pub body: String,
 }
 
@@ -40,3 +44,9 @@ pub struct PageOptions {
 	pub limit: Option<usize>,
 }
 
+
+
+
+fn non_empty_str<'de, D: serde::Deserializer<'de>>(d: D) -> Result<Option<String>, D::Error> {
+	Ok(Option::deserialize(d)?.filter(|s: &String| !s.is_empty()))
+}
diff --git a/src/notifications/console.rs b/src/notifications/console.rs
index bc6e6be..60eef30 100644
--- a/src/notifications/console.rs
+++ b/src/notifications/console.rs
@@ -17,6 +17,6 @@ pub struct ConsolePrettyNotifier {}
 #[async_trait::async_trait]
 impl NotificationProcessor<GuestBookPage> for ConsolePrettyNotifier {
 	async fn process(&self, suggestion: &GuestBookPage) {
-		println!("{} -- {} <{}>", suggestion.body, suggestion.author.as_deref().unwrap_or("anon"), suggestion.contact.as_deref().unwrap_or("")); 
+		println!("{} -- {} <{}>", suggestion.body, suggestion.author, suggestion.contact.as_deref().unwrap_or("")); 
 	}
 }
diff --git a/src/routes.rs b/src/routes.rs
index 56a83f8..2aa4c94 100644
--- a/src/routes.rs
+++ b/src/routes.rs
@@ -41,28 +41,25 @@ impl Context {
 }
 
 async fn send_suggestion(unsafe_payload: Insertion, state: SafeContext) -> Result<Redirect, String> {
+	// sanitize all user input! we don't want XSS or html injections!
 	let payload = unsafe_payload.sanitize();
+	// limit author and contact fields to 25 and 50 characters, TODO don't hardcode limits
+	let contact_limited = payload.contact.clone().map(|x| limit_string(&x, 50));
+	let author_limited = payload.author.map(|x| limit_string(&x, 25));
+	// calculate contact hash for libravatar
 	let mut hasher = Md5::new();
-	let id = payload.contact.clone().unwrap_or(Uuid::new_v4().to_string());
-	hasher.update(id.as_bytes());
+	hasher.update(contact_limited.as_deref().unwrap_or(&Uuid::new_v4().to_string()).as_bytes());
 	let avatar = hasher.finalize();
+	// populate guestbook page struct
 	let page = GuestBookPage {
 		avatar: format!("{:x}", avatar),
-		author: payload.author.map(|x| x.chars().take(25).collect()), // TODO don't hardcode char limits!
-		contact: payload.contact.clone().map(|x| x.chars().take(50).collect()),
+		author: author_limited.unwrap_or("anonymous".to_string()),
+		url: url_from_contact(contact_limited.clone()),
+		contact: contact_limited,
 		body: payload.body,
 		date: Utc::now(),
-		url: match payload.contact {
-			None => None,
-			Some(c) => if c.starts_with("http") {
-				Some(c)
-			} else if c.contains('@') {
-				Some(format!("mailto:{}", c))
-			} else {
-				None
-			}
-		},
 	};
+	// lock state, process and archive new page
 	let mut lock = state.write().await;
 	lock.process(&page).await;
 	match lock.storage.archive(page).await {
@@ -84,3 +81,23 @@ async fn get_suggestion(State(state): State<SafeContext>, Query(page): Query<Pag
 		Err(e) => Err(e.to_string()),
 	}
 }
+
+fn url_from_contact(contact: Option<String>) -> Option<String> {
+	match contact {
+		None => None,
+		Some(c) => if c.starts_with("http") {
+			Some(c)
+		} else if c.contains('@') {
+			Some(format!("mailto:{}", c))
+		} else if c.contains('.') {
+			Some(format!("https://{}", c))
+		} else {
+			None
+		}
+	}
+}
+
+fn limit_string(s: &str, l: usize) -> String {
+	// TODO is there a better way? slicing doesn't work when l > s.len
+	s.chars().take(l).collect()
+}