From 9dd3b0590a1366aa135f00a87bd67351ab841afb Mon Sep 17 00:00:00 2001
From: Luca Palmieri <lpalmieri@truelayer.com>
Date: Fri, 11 Dec 2020 15:50:09 +0000
Subject: [PATCH] Return 400 on bad names.

---
 Cargo.lock                  | 10 ++++++++
 Cargo.toml                  |  1 +
 src/domain.rs               | 47 ++++++++++++++++++++++++++++++++++++-
 src/routes/subscriptions.rs |  4 +++-
 4 files changed, 60 insertions(+), 2 deletions(-)

diff --git a/Cargo.lock b/Cargo.lock
index b947c80..1e66c55 100644
--- a/Cargo.lock
+++ b/Cargo.lock
@@ -503,6 +503,15 @@ dependencies = [
  "winapi 0.3.9",
 ]
 
+[[package]]
+name = "claim"
+version = "0.4.0"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "68ad37958d55b29a7088909368968d2fe876a24c203f8441195130f3b15194b9"
+dependencies = [
+ "autocfg",
+]
+
 [[package]]
 name = "config"
 version = "0.10.1"
@@ -2888,6 +2897,7 @@ dependencies = [
  "actix-rt",
  "actix-web",
  "chrono",
+ "claim",
  "config",
  "lazy_static",
  "reqwest",
diff --git a/Cargo.toml b/Cargo.toml
index 044e166..6fbf417 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -33,3 +33,4 @@ unicode-segmentation = "1.7.1"
 [dev-dependencies]
 reqwest = { version = "0.10.7", features = ["json"] }
 lazy_static = "1.4.0"
+claim = "0.4.0"
diff --git a/src/domain.rs b/src/domain.rs
index 5961b46..cabb0b2 100644
--- a/src/domain.rs
+++ b/src/domain.rs
@@ -5,6 +5,7 @@ pub struct NewSubscriber {
     pub name: SubscriberName,
 }
 
+#[derive(Debug)]
 pub struct SubscriberName(String);
 
 impl SubscriberName {
@@ -36,7 +37,7 @@ impl SubscriberName {
             > 0;
 
         if is_empty_or_whitespace || is_too_long || contains_forbidden_characters {
-            panic!(format!("{} is not a valid subscriber name.", s))
+            Err(format!("{} is not a valid subscriber name.", s))
         } else {
             Ok(Self(s))
         }
@@ -48,3 +49,47 @@ impl AsRef<str> for SubscriberName {
         &self.0
     }
 }
+
+#[cfg(test)]
+mod tests {
+    use crate::domain::SubscriberName;
+    use claim::{assert_err, assert_ok};
+
+    #[test]
+    fn a_256_grapheme_long_name_is_valid() {
+        let name = "a̐".repeat(256);
+        assert_ok!(SubscriberName::parse(name));
+    }
+
+    #[test]
+    fn a_name_longer_than_256_graphemes_is_rejected() {
+        let name = "a".repeat(257);
+        assert_err!(SubscriberName::parse(name));
+    }
+
+    #[test]
+    fn whitespace_only_names_are_rejected() {
+        let name = " ".to_string();
+        assert_err!(SubscriberName::parse(name));
+    }
+
+    #[test]
+    fn empty_string_is_rejected() {
+        let name = "".to_string();
+        assert_err!(SubscriberName::parse(name));
+    }
+
+    #[test]
+    fn names_containing_an_invalid_characters_are_rejected() {
+        for name in vec!['/', '(', ')', '"', '<', '>', '\\', '{', '}'] {
+            let name = name.to_string();
+            assert_err!(SubscriberName::parse(name));
+        }
+    }
+
+    #[test]
+    fn a_valid_name_is_parsed_successfully() {
+        let name = "Ursula Le Guin".to_string();
+        assert_ok!(SubscriberName::parse(name));
+    }
+}
diff --git a/src/routes/subscriptions.rs b/src/routes/subscriptions.rs
index 6e8715c..7e55663 100644
--- a/src/routes/subscriptions.rs
+++ b/src/routes/subscriptions.rs
@@ -22,9 +22,11 @@ pub async fn subscribe(
     form: web::Form<FormData>,
     pool: web::Data<PgPool>,
 ) -> Result<HttpResponse, HttpResponse> {
+    let name =
+        SubscriberName::parse(form.0.name).map_err(|_| HttpResponse::BadRequest().finish())?;
     let new_subscriber = NewSubscriber {
         email: form.0.email,
-        name: SubscriberName::parse(form.0.name).expect("Name validation failed."),
+        name,
     };
     insert_subscriber(&pool, &new_subscriber)
         .await