diff --git a/Cargo.lock b/Cargo.lock index 26bfd56..6958f23 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -50,7 +50,7 @@ dependencies = [ "mime", "percent-encoding", "pin-project-lite", - "rand", + "rand 0.8.4", "sha-1 0.10.0", "smallvec", "zstd", @@ -194,7 +194,7 @@ version = "0.7.6" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "fcb51a0695d8f838b1ee009b3fbf66bda078cd64590202a864a8f3e8c4315c47" dependencies = [ - "getrandom", + "getrandom 0.2.3", "once_cell", "version_check", ] @@ -349,6 +349,15 @@ dependencies = [ "winapi", ] +[[package]] +name = "claim" +version = "0.5.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "f81099d6bb72e1df6d50bb2347224b666a670912bb7f06dbe867a4a070ab3ce8" +dependencies = [ + "autocfg", +] + [[package]] name = "config" version = "0.11.0" @@ -565,6 +574,25 @@ dependencies = [ "cfg-if", ] +[[package]] +name = "env_logger" +version = "0.7.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "44533bbbb3bb3c1fa17d9f2e4e38bbbaf8396ba82193c4cb1b6445d711445d36" +dependencies = [ + "log", + "regex", +] + +[[package]] +name = "fake" +version = "2.3.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "6479fa2c7e83ddf8be7d435421e093b072ca891b99a49bc84eba098f4044f818" +dependencies = [ + "rand 0.7.3", +] + [[package]] name = "firestorm" version = "0.4.6" @@ -687,6 +715,17 @@ dependencies = [ "winapi", ] +[[package]] +name = "getrandom" +version = "0.1.16" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "8fc3cb4d91f53b50155bdcfd23f6a4c39ae1969c2ae85982b135750cccaf5fce" +dependencies = [ + "cfg-if", + "libc", + "wasi 0.9.0+wasi-snapshot-preview1", +] + [[package]] name = "getrandom" version = "0.2.3" @@ -695,7 +734,7 @@ checksum = "7fcd999463524c52659517fe2cea98493cfe485d10565e7b0fb07dbba7ad2753" dependencies = [ "cfg-if", "libc", - "wasi", + "wasi 0.10.2+wasi-snapshot-preview1", ] [[package]] @@ -994,8 +1033,12 @@ version = "0.1.0" dependencies = [ "actix-web", "chrono", + "claim", "config", + "fake", "once_cell", + "quickcheck", + "quickcheck_macros", "reqwest", "secrecy", "serde", @@ -1007,7 +1050,9 @@ dependencies = [ "tracing-bunyan-formatter", "tracing-log", "tracing-subscriber", + "unicode-segmentation", "uuid", + "validator", ] [[package]] @@ -1327,6 +1372,29 @@ dependencies = [ "unicode-xid", ] +[[package]] +name = "quickcheck" +version = "0.9.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "a44883e74aa97ad63db83c4bf8ca490f02b2fc02f92575e720c8551e843c945f" +dependencies = [ + "env_logger", + "log", + "rand 0.7.3", + "rand_core 0.5.1", +] + +[[package]] +name = "quickcheck_macros" +version = "0.9.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "608c156fd8e97febc07dc9c2e2c80bf74cfc6ef26893eae3daf8bc2bc94a4b7f" +dependencies = [ + "proc-macro2", + "quote", + "syn", +] + [[package]] name = "quote" version = "1.0.10" @@ -1336,6 +1404,19 @@ dependencies = [ "proc-macro2", ] +[[package]] +name = "rand" +version = "0.7.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "6a6b1679d49b24bbfe0c803429aa1874472f50d9b363131f0e89fc356b544d03" +dependencies = [ + "getrandom 0.1.16", + "libc", + "rand_chacha 0.2.2", + "rand_core 0.5.1", + "rand_hc 0.2.0", +] + [[package]] name = "rand" version = "0.8.4" @@ -1343,9 +1424,19 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "2e7573632e6454cf6b99d7aac4ccca54be06da05aca2ef7423d22d27d4d4bcd8" dependencies = [ "libc", - "rand_chacha", - "rand_core", - "rand_hc", + "rand_chacha 0.3.1", + "rand_core 0.6.3", + "rand_hc 0.3.1", +] + +[[package]] +name = "rand_chacha" +version = "0.2.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "f4c8ed856279c9737206bf725bf36935d8666ead7aa69b52be55af369d193402" +dependencies = [ + "ppv-lite86", + "rand_core 0.5.1", ] [[package]] @@ -1355,7 +1446,16 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "e6c10a63a0fa32252be49d21e7709d4d4baf8d231c2dbce1eaa8141b9b127d88" dependencies = [ "ppv-lite86", - "rand_core", + "rand_core 0.6.3", +] + +[[package]] +name = "rand_core" +version = "0.5.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "90bde5296fc891b0cef12a6d03ddccc162ce7b2aff54160af9338f8d40df6d19" +dependencies = [ + "getrandom 0.1.16", ] [[package]] @@ -1364,7 +1464,16 @@ version = "0.6.3" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "d34f1408f55294453790c48b2f1ebbb1c5b4b7563eb1f418bcfcfdbb06ebb4e7" dependencies = [ - "getrandom", + "getrandom 0.2.3", +] + +[[package]] +name = "rand_hc" +version = "0.2.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "ca3129af7b92a17112d59ad498c6f81eaf463253766b90396d39ea7a39d6613c" +dependencies = [ + "rand_core 0.5.1", ] [[package]] @@ -1373,7 +1482,7 @@ version = "0.3.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "d51e9f596de227fda2ea6c84607f5558e196eeaf43c986b724ba4fb8fdf497e7" dependencies = [ - "rand_core", + "rand_core 0.6.3", ] [[package]] @@ -1391,7 +1500,7 @@ version = "0.4.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "528532f3d801c87aec9def2add9ca802fe569e44a544afe633765267840abe64" dependencies = [ - "getrandom", + "getrandom 0.2.3", "redox_syscall", ] @@ -1809,7 +1918,7 @@ dependencies = [ "once_cell", "parking_lot", "percent-encoding", - "rand", + "rand 0.8.4", "rustls", "serde", "serde_json", @@ -1961,7 +2070,7 @@ checksum = "dac1c663cfc93810f88aed9b8941d48cabf856a1b111c29a40439018d870eb22" dependencies = [ "cfg-if", "libc", - "rand", + "rand 0.8.4", "redox_syscall", "remove_dir_all", "winapi", @@ -2328,7 +2437,33 @@ version = "0.8.2" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "bc5cf98d8186244414c848017f0e2676b3fcb46807f6668a97dfe67359a3c4b7" dependencies = [ - "getrandom", + "getrandom 0.2.3", +] + +[[package]] +name = "validator" +version = "0.14.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "6d0f08911ab0fee2c5009580f04615fa868898ee57de10692a45da0c3bcc3e5e" +dependencies = [ + "idna", + "lazy_static", + "regex", + "serde", + "serde_derive", + "serde_json", + "url", + "validator_types", +] + +[[package]] +name = "validator_types" +version = "0.14.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "ded9d97e1d42327632f5f3bae6403c04886e2de3036261ef42deebd931a6a291" +dependencies = [ + "proc-macro2", + "syn", ] [[package]] @@ -2353,6 +2488,12 @@ dependencies = [ "try-lock", ] +[[package]] +name = "wasi" +version = "0.9.0+wasi-snapshot-preview1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "cccddf32554fecc6acb585f82a32a72e28b48f8c4c1883ddfeeeaa96f7d8e519" + [[package]] name = "wasi" version = "0.10.2+wasi-snapshot-preview1" diff --git a/Cargo.toml b/Cargo.toml index 96336e7..2358902 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -26,7 +26,13 @@ tracing-log = "0.1.1" serde-aux = "3" tracing-actix-web = "0.5.0-beta.7" secrecy = { version = "0.8", features = ["serde"] } +unicode-segmentation = "1" +validator = "0.14" [dev-dependencies] reqwest = { version = "0.11", features = ["json"] } -once_cell = "1.7.2" \ No newline at end of file +once_cell = "1.7.2" +claim = "0.5" +fake = "~2.3" +quickcheck = "0.9.2" +quickcheck_macros = "0.9.1" \ No newline at end of file diff --git a/src/domain/mod.rs b/src/domain/mod.rs new file mode 100644 index 0000000..f032f08 --- /dev/null +++ b/src/domain/mod.rs @@ -0,0 +1,7 @@ +mod subscriber_name; +mod subscriber_email; +mod new_subscriber; + +pub use subscriber_name::SubscriberName; +pub use new_subscriber::NewSubscriber; +pub use subscriber_email::SubscriberEmail; \ No newline at end of file diff --git a/src/domain/new_subscriber.rs b/src/domain/new_subscriber.rs new file mode 100644 index 0000000..f1b6c1f --- /dev/null +++ b/src/domain/new_subscriber.rs @@ -0,0 +1,7 @@ +use crate::domain::SubscriberName; +use crate::domain::SubscriberEmail; + +pub struct NewSubscriber { + pub email: SubscriberEmail, + pub name: SubscriberName, +} \ No newline at end of file diff --git a/src/domain/subscriber_email.rs b/src/domain/subscriber_email.rs new file mode 100644 index 0000000..6e1a0a1 --- /dev/null +++ b/src/domain/subscriber_email.rs @@ -0,0 +1,61 @@ +use validator::validate_email; + +#[derive(Debug)] +pub struct SubscriberEmail(String); + +impl SubscriberEmail { + pub fn parse(s: String) -> Result { + if validate_email(&s) { + Ok(Self(s)) + } else { + Err(format!("{} is not a valid email address", s)) + } + } +} + +impl AsRef for SubscriberEmail { + fn as_ref(&self) -> &str { + &self.0 + } +} + +#[cfg(test)] +mod tests { + use super::SubscriberEmail; + use claim::assert_err; + use fake::faker::internet::en::SafeEmail; + use fake::Fake; + + #[test] + fn empty_string_is_rejected() { + let email = "".to_string(); + assert_err!(SubscriberEmail::parse(email)); + } + + #[test] + fn email_missing_at_symbol_is_rejected() { + let email = "ursuladomain.com".to_string(); + assert_err!(SubscriberEmail::parse(email)); + } + + #[test] + fn email_missing_subject_is_rejected() { + let email = "@domain.com".to_string(); + assert_err!(SubscriberEmail::parse(email)); + } + + #[derive(Debug, Clone)] + struct ValidEmailFixture(pub String); + + impl quickcheck::Arbitrary for ValidEmailFixture { + fn arbitrary(g: &mut G) -> Self { + let email = SafeEmail().fake_with_rng(g); + Self(email) + } + } + + #[quickcheck_macros::quickcheck] + fn valid_emails_are_parsed_successfully(valid_email: ValidEmailFixture) -> bool { + SubscriberEmail::parse(valid_email.0).is_ok() + } +} \ No newline at end of file diff --git a/src/domain/subscriber_name.rs b/src/domain/subscriber_name.rs new file mode 100644 index 0000000..0a0d7eb --- /dev/null +++ b/src/domain/subscriber_name.rs @@ -0,0 +1,64 @@ +use unicode_segmentation::UnicodeSegmentation; + +#[derive(Debug)] +pub struct SubscriberName(String); + +impl SubscriberName { + pub fn parse(s: String) -> Result { + let is_empty_or_whitespace = s.trim().is_empty(); // Remove trailing whitespaces and check if containsd any characters + let is_too_long = s.graphemes(true).count() > 256; + let forbidden_characters = ['/', '(', ')', '"', '<', '>', '\\', '{', '}']; + let contains_forbidden_characters = s.chars().any(|g| forbidden_characters.contains(&g)); // Iterate to check if name contains any of the forbidden characters + + // Return `false` if any conditions are violated + if is_empty_or_whitespace || is_too_long || contains_forbidden_characters { + Err(format!("{} is not a valid subscriber name", s)) + } else { + Ok(Self(s)) + } + } +} + +impl AsRef for SubscriberName { + fn as_ref(&self) -> &str { + &self.0 + } +} + +#[cfg(test)] +mod tests { + use crate::domain::SubscriberName; + use claim::{assert_err, assert_ok}; + + #[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_invalid_characters_are_rejected() { + for name in &['/', '(', ')', '"', '<', '>', '\\', '{', '}'] { + 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)); + } +} \ No newline at end of file diff --git a/src/lib.rs b/src/lib.rs index acfce7b..c959ba3 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -2,4 +2,5 @@ pub mod configuration; pub mod routes; pub mod startup; -pub mod telemetry; \ No newline at end of file +pub mod telemetry; +pub mod domain; \ No newline at end of file diff --git a/src/routes/subscriptions.rs b/src/routes/subscriptions.rs index e6ae3f1..631a541 100644 --- a/src/routes/subscriptions.rs +++ b/src/routes/subscriptions.rs @@ -2,7 +2,8 @@ use actix_web::{web, HttpResponse}; use sqlx::PgPool; use chrono::Utc; use uuid::Uuid; -// use tracing_futures::Instrument; + +use crate::domain::{NewSubscriber, SubscriberName, SubscriberEmail}; #[derive(serde::Deserialize)] pub struct FormData { @@ -10,6 +11,16 @@ pub struct FormData { name: String } +impl TryFrom for NewSubscriber { + type Error = String; + + fn try_from(value: FormData) -> Result { + let name = SubscriberName::parse(value.name)?; + let email = SubscriberEmail::parse(value.email)?; + Ok(Self { email, name }) + } +} + #[allow(clippy::async_yields_async)] #[tracing::instrument( name = "Adding a new subscriber", @@ -19,9 +30,12 @@ pub struct FormData { subscriber_name = %form.name ) )] - pub async fn subscribe(form: web::Form, pool: web::Data,) -> HttpResponse { - match insert_subscriber(&pool, &form).await { + let new_subscriber = match form.0.try_into() { + Ok(form) => form, + Err(_) => return HttpResponse::BadRequest().finish(), + }; + match insert_subscriber(&pool, &new_subscriber).await { Ok(_) => HttpResponse::Ok().finish(), Err(_) => HttpResponse::InternalServerError().finish(), } @@ -29,17 +43,17 @@ pub async fn subscribe(form: web::Form, pool: web::Data,) -> H #[tracing::instrument( name = "Saving new subscriber details in the database", - skip(form, pool) + skip(new_subscriber, pool) )] -pub async fn insert_subscriber(pool: &PgPool, form: &FormData) -> Result<(), sqlx::Error> { +pub async fn insert_subscriber(pool: &PgPool, new_subscriber: &NewSubscriber) -> Result<(), sqlx::Error> { sqlx::query!( r#" INSERT INTO subscriptions (id, email, name, subscribed_at) VALUES ($1, $2, $3, $4) "#, Uuid::new_v4(), - form.email, - form.name, + new_subscriber.email.as_ref(), + new_subscriber.name.as_ref(), Utc::now() ) .execute(pool) diff --git a/tests/health_check.rs b/tests/health_check.rs index faf2ec5..0fc8427 100644 --- a/tests/health_check.rs +++ b/tests/health_check.rs @@ -144,4 +144,33 @@ async fn subscribe_returns_400_for_missing_form_data() { "The API did not fail with 400 Bad Request when the payload was {}.", error_message ); } +} + +#[tokio::test] +async fn subscribe_returns_a_400_when_fields_are_present_but_empty() { + // Arrange + let app = spawn_app().await; + let client = reqwest::Client::new(); + let test_cases = vec![ + ("name=&email=ursula_le_guin%40gmail.com", "empty name"), + ("name=Ursula&email=", "empty email"), + ("name=Ursula&email=definitely-not-an-email", "invalid email"), + ]; + + for (body, description) in test_cases { + // Act + let response = client + .post(&format!("{}/subscriptions", &app.address)) + .header("Content-Type", "application/x-www-form-urlencoded") + .body(body) + .send() + .await + .expect("Failed to execute request."); + + // Assert + assert_eq!(400, + response.status().as_u16(), + "The API did not return a 400 Bad Request when the payload was {}.", description + ); + } } \ No newline at end of file