Skip to content

Commit 20c6718

Browse files
committed
Feat: validate input and guide against sql injection
1 parent fec5bfa commit 20c6718

File tree

8 files changed

+133
-43
lines changed

8 files changed

+133
-43
lines changed

Cargo.lock

Lines changed: 1 addition & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

Cargo.toml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ tracing-log = "0.1"
2727
secrecy = { version = "0.8", features = ["serde"] }
2828
tracing-actix-web = "0.5"
2929
sqlx = { version = "0.5.7", default-features = false, features = ["offline", "runtime-actix-rustls", "macros", "postgres", "uuid", "chrono", "migrate"] }
30+
unicode-segmentation = "1"
3031

3132
[dev-dependencies]
3233
reqwest = "0.11.18"

spec.yaml

Lines changed: 44 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -9,34 +9,50 @@ name: robust-rust
99
region: fra
1010
services:
1111
- name: robust-rust
12-
# Relative to the repository root
13-
dockerfile_path: Dockerfile
14-
source_dir: .
15-
github:
16-
# Depending on when you created the repository,
17-
# the default branch on GitHub might have been named `master`
18-
branch: main
19-
# Deploy a new version on every commit to `main`!
20-
# Continuous Deployment, here we come!
21-
deploy_on_push: true
22-
# !!! Fill in with your details
23-
# e.g. LukeMathWalker/zero-to-production
24-
repo: utilitycoder/robust-rust
25-
# Active probe used by DigitalOcean's to ensure our application is healthy
26-
health_check:
27-
# The path to our health check endpoint!
28-
# It turned out to be useful in the end!
29-
http_path: /health_check
30-
# The port the application will be listening on for incoming requests
31-
# It should match what we specified in our configuration/production.yaml file!
32-
http_port: 8000
33-
# For production workloads we'd go for at least two!
34-
# But let's try to keep the bill under control for now...
35-
instance_count: 1
36-
instance_size_slug: basic-xxs
37-
# All incoming requests should be routed to our app
38-
routes:
39-
- path: /
12+
# Relative to the repository root
13+
dockerfile_path: Dockerfile
14+
source_dir: .
15+
github:
16+
# Depending on when you created the repository,
17+
# the default branch on GitHub might have been named `master`
18+
branch: main
19+
# Deploy a new version on every commit to `main`!
20+
# Continuous Deployment, here we come!
21+
deploy_on_push: true
22+
# !!! Fill in with your details
23+
# e.g. LukeMathWalker/zero-to-production
24+
repo: utilitycoder/robust-rust
25+
# Active probe used by DigitalOcean's to ensure our application is healthy
26+
health_check:
27+
# The path to our health check endpoint!
28+
# It turned out to be useful in the end!
29+
http_path: /health_check
30+
# The port the application will be listening on for incoming requests
31+
# It should match what we specified in our configuration/production.yaml file!
32+
http_port: 8000
33+
# For production workloads we'd go for at least two!
34+
# But let's try to keep the bill under control for now...
35+
instance_count: 1
36+
instance_size_slug: basic-xxs
37+
# All incoming requests should be routed to our app
38+
routes:
39+
- path: /
40+
envs:
41+
- key: APP_DATABASE__USERNAME
42+
scope: RUN_TIME
43+
value: ${newsletter.USERNAME}
44+
- key: APP_DATABASE__PASSWORD
45+
scope: RUN_TIME
46+
value: ${newsletter.PASSWORD}
47+
- key: APP_DATABASE__HOST
48+
scope: RUN_TIME
49+
value: ${newsletter.HOSTNAME}
50+
- key: APP_DATABASE__PORT
51+
scope: RUN_TIME
52+
value: ${newsletter.PORT}
53+
- key: APP_DATABASE__DATABASE_NAME
54+
scope: RUN_TIME
55+
value: ${newsletter.DATABASE}
4056
databases:
4157
# PG = Postgres
4258
- engine: PG

src/configuration.rs

Lines changed: 15 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
use secrecy::{ExposeSecret, Secret};
22
use serde_aux::field_attributes::deserialize_number_from_string;
3-
use sqlx::postgres::PgConnectOptions;
3+
use sqlx::postgres::{PgConnectOptions, PgSslMode};
4+
use sqlx::ConnectOptions;
5+
use tracing_log::log;
46

57
#[derive(serde::Deserialize)]
68
pub struct Settings {
@@ -23,24 +25,28 @@ pub struct DatabaseSettings {
2325
pub port: u16,
2426
pub host: String,
2527
pub database_name: String,
28+
pub require_ssl: bool,
2629
}
2730

2831
impl DatabaseSettings {
29-
pub fn with_db(&self) -> PgConnectOptions {
32+
pub fn without_db(&self) -> PgConnectOptions {
33+
let ssl_mode = if self.require_ssl {
34+
PgSslMode::Require
35+
} else {
36+
PgSslMode::Prefer
37+
};
3038
PgConnectOptions::new()
3139
.host(&self.host)
3240
.username(&self.username)
3341
.password(self.password.expose_secret())
3442
.port(self.port)
35-
.database(&self.database_name)
43+
.ssl_mode(ssl_mode)
3644
}
3745

38-
pub fn without_db(&self) -> PgConnectOptions {
39-
PgConnectOptions::new()
40-
.host(&self.host)
41-
.username(&self.username)
42-
.password(self.password.expose_secret())
43-
.port(self.port)
46+
pub fn with_db(&self) -> PgConnectOptions {
47+
let mut options = self.without_db().database(&self.database_name);
48+
options.log_statements(log::LevelFilter::Trace);
49+
options
4450
}
4551
}
4652

src/domain.rs

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
pub struct NewSubscriber {
2+
pub email: String,
3+
pub name: SubscriberName,
4+
}
5+
6+
pub struct SubscriberName(String);
7+
8+
impl SubscriberName {
9+
pub fn inner_ref(&self) -> &str {
10+
&self.0
11+
}
12+
13+
pub fn parse(s: String) -> SubscriberName {
14+
let is_empty_or_whitespace = s.trim().is_empty();
15+
let is_too_long = s.len() > 256;
16+
let forbidden_characters = ['/', '(', ')', '"', '<', '>', '\\', '{', '}'];
17+
let contains_forbidden_characters = s.chars().any(|char| forbidden_characters.contains(&char));
18+
if is_empty_or_whitespace || is_too_long || contains_forbidden_characters {
19+
panic!(
20+
"`{}` is not a valid subscriber name. Subscriber name cannot be empty, more than 256 characters long, or contain the following characters: {:?}",
21+
s,
22+
forbidden_characters,
23+
)
24+
} else {
25+
Self(s)
26+
}
27+
}
28+
}

src/lib.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
pub mod configuration;
2+
pub mod domain;
23
pub mod routes;
34
pub mod startup;
45
pub mod telemetry;

src/routes/subscriptions.rs

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ use actix_web::{web, HttpResponse};
22
use chrono::Utc;
33
use sqlx::types::Uuid;
44
use sqlx::PgPool;
5-
// use uuid::Uuid;
5+
use crate::domain::{NewSubscriber, SubscriberName};
66

77
#[allow(dead_code)]
88
#[derive(serde::Deserialize)]
@@ -21,7 +21,11 @@ pub struct FormData {
2121
)]
2222

2323
pub async fn subscribe(form: web::Form<FormData>, pool: web::Data<PgPool>) -> HttpResponse {
24-
match insert_subscriber(&form, &pool).await {
24+
let new_subscriber = NewSubscriber {
25+
email: form.0.email,
26+
name: SubscriberName::parse(form.0.name),
27+
};
28+
match insert_subscriber(&new_subscriber, &pool).await {
2529
Ok(_) => HttpResponse::Ok().finish(),
2630
Err(e) => {
2731
tracing::error!("Failed to execute query: {:?}", e);
@@ -32,18 +36,18 @@ pub async fn subscribe(form: web::Form<FormData>, pool: web::Data<PgPool>) -> Ht
3236

3337
#[tracing::instrument(
3438
name = "Saving a new subscriber details in the database",
35-
skip(form, pool)
39+
skip(new_subscriber, pool)
3640
)]
37-
async fn insert_subscriber(form: &FormData, pool: &PgPool) -> Result<(), sqlx::Error> {
41+
async fn insert_subscriber(new_subscriber: &NewSubscriber, pool: &PgPool) -> Result<(), sqlx::Error> {
3842
let subscribe_id = Uuid::new_v4();
3943
sqlx::query!(
4044
r#"
4145
INSERT INTO subscriptions (id, email, name, subscribed_at)
4246
VALUES ($1, $2, $3, $4)
4347
"#,
4448
subscribe_id,
45-
form.email,
46-
form.name,
49+
new_subscriber.email,
50+
new_subscriber.name.inner_ref(),
4751
Utc::now()
4852
)
4953
.execute(pool)

tests/health_check.rs

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -147,3 +147,36 @@ async fn subscribe_returns_a_400_when_data_is_missing() {
147147
);
148148
}
149149
}
150+
151+
#[tokio::test]
152+
async fn subscribe_returns_a_400_when_fields_are_present_but_empty() {
153+
// Arrange
154+
let app_details = spawn_app().await;
155+
let client = reqwest::Client::new();
156+
let test_cases = vec![
157+
("name=&email=ursula_le_guin%40gmail.com", "empty name"),
158+
("name=le%20guin&email=", "empty email"),
159+
(
160+
"name=le%20guin&email=definitely_not_an_email",
161+
"invalid email",
162+
),
163+
];
164+
for (invalid_body, error_message) in test_cases {
165+
// Act
166+
let response = client
167+
.post(&format!("{}/subscriptions", &app_details.address))
168+
.header("Content-Type", "application/x-www-form-urlencoded")
169+
.body(invalid_body)
170+
.send()
171+
.await
172+
.expect("Failed to execute request.");
173+
// Assert
174+
assert_eq!(
175+
200,
176+
response.status().as_u16(),
177+
// Additional customised error message on test failure
178+
"The API did not return a 200 OK when the payload was {}.",
179+
error_message
180+
);
181+
}
182+
}

0 commit comments

Comments
 (0)