diff --git a/Cargo.lock b/Cargo.lock index fe032f0..0809332 100755 --- a/Cargo.lock +++ b/Cargo.lock @@ -2278,6 +2278,7 @@ dependencies = [ "actix-rt", "actix-service", "actix-web", + "anyhow", "bcrypt", "chrono", "dateparser", diff --git a/Cargo.toml b/Cargo.toml index 781214e..674161f 100755 --- a/Cargo.toml +++ b/Cargo.toml @@ -6,6 +6,7 @@ edition = "2024" # See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html [dependencies] +anyhow = "1" reqwest = { version = "0.13", features = ["json", "blocking"] } tokio = { version = "1", features = ["full"] } rss = { version = "2.0.13" } diff --git a/src/auth/jwt.rs b/src/auth/jwt.rs index f7f1eff..a59d3e1 100755 --- a/src/auth/jwt.rs +++ b/src/auth/jwt.rs @@ -20,7 +20,8 @@ type HmacSha256 = Hmac; fn signing_key() -> HmacSha256 { dotenv().ok(); let secret = env::var("JWT_SECRET").expect("JWT_SECRET must be set"); - HmacSha256::new_from_slice(secret.as_bytes()).unwrap() + // HMAC-SHA256 accepts a key of any length, so this cannot fail. + HmacSha256::new_from_slice(secret.as_bytes()).expect("HMAC accepts a key of any length") } impl JwtToken { @@ -28,7 +29,10 @@ impl JwtToken { let key: HmacSha256 = signing_key(); let mut claims = BTreeMap::new(); claims.insert("user_id", user_id); - claims.sign_with_key(&key).unwrap() + // Signing a simple map of claims with a valid HMAC key cannot fail. + claims + .sign_with_key(&key) + .expect("signing claims with a valid HMAC key cannot fail") } pub fn decode(encoded_token: String) -> Result { @@ -52,7 +56,10 @@ impl JwtToken { #[allow(dead_code)] pub fn decode_from_request(request: HttpRequest) -> Result { match request.headers().get("user-token") { - Some(token) => JwtToken::decode(String::from(token.to_str().unwrap())), + Some(token) => match token.to_str() { + Ok(token_str) => JwtToken::decode(String::from(token_str)), + Err(_) => Err("token header is not valid text"), + }, None => Err("There is no token"), } } diff --git a/src/error.rs b/src/error.rs new file mode 100644 index 0000000..7b63838 --- /dev/null +++ b/src/error.rs @@ -0,0 +1,34 @@ +use actix_web::{HttpResponse, ResponseError}; +use std::fmt; + +/// Wraps any error so it can be returned with `?` from request handlers. +/// Always surfaces as a 500 to the client; the real error is logged. +pub struct AppError(anyhow::Error); + +impl fmt::Debug for AppError { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + fmt::Debug::fmt(&self.0, f) + } +} + +impl fmt::Display for AppError { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + fmt::Display::fmt(&self.0, f) + } +} + +impl ResponseError for AppError { + fn error_response(&self) -> HttpResponse { + log::error!("Unhandled error: {:?}", self.0); + HttpResponse::InternalServerError().finish() + } +} + +impl From for AppError +where + E: Into, +{ + fn from(err: E) -> Self { + AppError(err.into()) + } +} diff --git a/src/json_serialization/articles.rs b/src/json_serialization/articles.rs index 318b004..124a7e0 100755 --- a/src/json_serialization/articles.rs +++ b/src/json_serialization/articles.rs @@ -13,7 +13,12 @@ impl Responder for Articles { type Body = String; fn respond_to(self, _req: &actix_web::HttpRequest) -> actix_web::HttpResponse { - let body = serde_json::to_string(&self).unwrap(); - HttpResponse::with_body(StatusCode::OK, body) + match serde_json::to_string(&self) { + Ok(body) => HttpResponse::with_body(StatusCode::OK, body), + Err(err) => { + log::error!("Failed to serialize response: {}", err); + HttpResponse::with_body(StatusCode::INTERNAL_SERVER_ERROR, String::new()) + } + } } } diff --git a/src/json_serialization/feed_info.rs b/src/json_serialization/feed_info.rs index 8880e86..f18fdcc 100644 --- a/src/json_serialization/feed_info.rs +++ b/src/json_serialization/feed_info.rs @@ -18,7 +18,12 @@ impl Responder for FeedInfoList { type Body = String; fn respond_to(self, _req: &actix_web::HttpRequest) -> actix_web::HttpResponse { - let body = serde_json::to_string(&self).unwrap(); - HttpResponse::with_body(StatusCode::OK, body) + match serde_json::to_string(&self) { + Ok(body) => HttpResponse::with_body(StatusCode::OK, body), + Err(err) => { + log::error!("Failed to serialize response: {}", err); + HttpResponse::with_body(StatusCode::INTERNAL_SERVER_ERROR, String::new()) + } + } } } diff --git a/src/json_serialization/readable.rs b/src/json_serialization/readable.rs index c7a71ae..93be960 100644 --- a/src/json_serialization/readable.rs +++ b/src/json_serialization/readable.rs @@ -11,7 +11,12 @@ impl Responder for Readable { type Body = String; fn respond_to(self, _req: &actix_web::HttpRequest) -> actix_web::HttpResponse { - let body = serde_json::to_string(&self).unwrap(); - HttpResponse::with_body(StatusCode::OK, body) + match serde_json::to_string(&self) { + Ok(body) => HttpResponse::with_body(StatusCode::OK, body), + Err(err) => { + log::error!("Failed to serialize response: {}", err); + HttpResponse::with_body(StatusCode::INTERNAL_SERVER_ERROR, String::new()) + } + } } } diff --git a/src/main.rs b/src/main.rs index 5318d2e..60a6e38 100755 --- a/src/main.rs +++ b/src/main.rs @@ -9,6 +9,7 @@ use futures::future::{ok, Either}; use std::env; mod auth; mod database; +mod error; mod json_serialization; mod models; mod reader; diff --git a/src/models/user/new_user.rs b/src/models/user/new_user.rs index 74fc9b2..0fc1723 100755 --- a/src/models/user/new_user.rs +++ b/src/models/user/new_user.rs @@ -16,14 +16,14 @@ pub struct NewUser { } impl NewUser { - pub fn new(username: String, email: String, password: String) -> NewUser { - let hashed_password: String = hash(password.as_str(), DEFAULT_COST).unwrap(); + pub fn new(username: String, email: String, password: String) -> anyhow::Result { + let hashed_password: String = hash(password.as_str(), DEFAULT_COST)?; let uuid = Uuid::new_v4(); - NewUser { + Ok(NewUser { username, email, password: hashed_password, unique_id: uuid.to_string(), - } + }) } } diff --git a/src/models/user/rss_user.rs b/src/models/user/rss_user.rs index 6e78f0b..7d7e61f 100755 --- a/src/models/user/rss_user.rs +++ b/src/models/user/rss_user.rs @@ -17,7 +17,7 @@ pub struct User { } impl User { - pub fn verify(self, password: String) -> bool { - verify(password.as_str(), &self.password).unwrap() + pub fn verify(self, password: String) -> anyhow::Result { + Ok(verify(password.as_str(), &self.password)?) } } diff --git a/src/reader/add.rs b/src/reader/add.rs index c072071..b90e170 100644 --- a/src/reader/add.rs +++ b/src/reader/add.rs @@ -19,12 +19,12 @@ pub async fn add(new_feed: web::Json) -> HttpResponse { Ok(channel) => { log::info!("valid channel"); if channel.items.is_empty() { - return HttpResponse::ServiceUnavailable().await.unwrap(); + return HttpResponse::ServiceUnavailable().finish(); } } Err(e) => { log::error!("{:?}", e); - return HttpResponse::NotFound().await.unwrap(); + return HttpResponse::NotFound().finish(); } } @@ -35,10 +35,10 @@ pub async fn add(new_feed: web::Json) -> HttpResponse { .execute(&mut connection); match insert_result { - Ok(_) => HttpResponse::Created().await.unwrap(), + Ok(_) => HttpResponse::Created().finish(), Err(e) => { log::error!("{e}"); - HttpResponse::Conflict().await.unwrap() + HttpResponse::Conflict().finish() } } } diff --git a/src/reader/get.rs b/src/reader/get.rs index b5c7995..7383ba5 100755 --- a/src/reader/get.rs +++ b/src/reader/get.rs @@ -1,3 +1,4 @@ +use crate::error::AppError; use crate::json_serialization::user::JsonUser; use crate::models::feed::rss_feed::Feed; use crate::models::feed_item::rss_feed_item::FeedItem; @@ -15,7 +16,7 @@ use diesel::prelude::*; use super::structs::article::Article; -pub async fn get(path: web::Path, req: HttpRequest) -> impl Responder { +pub async fn get(path: web::Path, req: HttpRequest) -> Result { let request = req.clone(); let req_user_id = path.user_id; log::info!("Received user_id: {}", req_user_id); @@ -23,8 +24,7 @@ pub async fn get(path: web::Path, req: HttpRequest) -> impl Responder let mut connection: diesel::PgConnection = establish_connection(); let feeds: Vec = feed::table .filter(user_id.eq(req_user_id)) - .load::(&mut connection) - .unwrap(); + .load::(&mut connection)?; let mut feed_aggregates: Vec = Vec::new(); for feed in feeds { @@ -32,8 +32,7 @@ pub async fn get(path: web::Path, req: HttpRequest) -> impl Responder .filter(feed_id.eq(feed.id)) .filter(read.eq(false)) .order(id.asc()) - .load(&mut connection) - .unwrap(); + .load(&mut connection)?; log::info!( "Load {} feed items for feed: {}", @@ -70,7 +69,7 @@ pub async fn get(path: web::Path, req: HttpRequest) -> impl Responder feeds: feed_aggregates, }; - articles.respond_to(&request) + Ok(articles.respond_to(&request)) } #[cfg(test)] diff --git a/src/reader/list_feeds.rs b/src/reader/list_feeds.rs index 42a5b4d..899873b 100644 --- a/src/reader/list_feeds.rs +++ b/src/reader/list_feeds.rs @@ -3,12 +3,16 @@ use diesel::prelude::*; use crate::{ database::establish_connection, + error::AppError, json_serialization::feed_info::{FeedInfo, FeedInfoList}, json_serialization::user::JsonUser, schema::feed::{self, user_id}, }; -pub async fn list_feeds(path: web::Path, req: HttpRequest) -> impl Responder { +pub async fn list_feeds( + path: web::Path, + req: HttpRequest, +) -> Result { let request = req.clone(); let req_user_id = path.user_id; @@ -16,15 +20,14 @@ pub async fn list_feeds(path: web::Path, req: HttpRequest) -> impl Res let feeds = feed::table .filter(user_id.eq(req_user_id)) .select((feed::id, feed::title, feed::url)) - .load::<(i32, String, String)>(&mut connection) - .unwrap(); + .load::<(i32, String, String)>(&mut connection)?; let feed_list: Vec = feeds .into_iter() .map(|(id, title, url)| FeedInfo { id, title, url }) .collect(); - FeedInfoList { feeds: feed_list }.respond_to(&request) + Ok(FeedInfoList { feeds: feed_list }.respond_to(&request)) } #[cfg(test)] diff --git a/src/reader/mark_read.rs b/src/reader/mark_read.rs index f1d7d1f..aa8f557 100644 --- a/src/reader/mark_read.rs +++ b/src/reader/mark_read.rs @@ -1,3 +1,4 @@ +use crate::error::AppError; use crate::schema::feed_item::{id, read}; use crate::{ database::establish_connection, json_serialization::read_feed_item::ReadItem, @@ -7,27 +8,29 @@ use actix_web::{web, HttpRequest, HttpResponse, Responder}; use diesel::RunQueryDsl; use diesel::{ExpressionMethods, QueryDsl}; -pub async fn mark_read(_req: HttpRequest, path: web::Path) -> impl Responder { +pub async fn mark_read( + _req: HttpRequest, + path: web::Path, +) -> Result { let mut connection = establish_connection(); log::info!("Id: {}", path.id); - let feed_items: Vec = feed_item::table + let mut feed_items: Vec = feed_item::table .filter(id.eq(path.id)) - .load::(&mut connection) - .unwrap(); + .load::(&mut connection)?; if feed_items.len() != 1 { - return HttpResponse::NotFound(); + return Ok(HttpResponse::NotFound().finish()); } - let feed_item: &FeedItem = feed_items.first().unwrap(); + let feed_item: FeedItem = feed_items.remove(0); - let result: Result = diesel::update(feed_item) + let result = diesel::update(&feed_item) .set(read.eq(true)) - .execute(&mut connection); + .execute(&mut connection)?; log::info!("Mark as read: {:?}", result); - HttpResponse::Ok() + Ok(HttpResponse::Ok().finish()) } #[cfg(test)] diff --git a/src/reader/sync.rs b/src/reader/sync.rs index bed19dc..9a8bac8 100644 --- a/src/reader/sync.rs +++ b/src/reader/sync.rs @@ -1,4 +1,5 @@ use super::feeds; +use crate::error::AppError; use crate::json_serialization::user::JsonUser; use crate::models::feed::rss_feed::Feed; use crate::models::feed_item::new_feed_item::NewFeedItem; @@ -60,8 +61,17 @@ fn enclosure_image_html(item: &Item) -> Option { )) } -fn create_feed_item(item: Item, feed: &Feed, connection: &mut PgConnection) { - let item_title = item.title.clone().unwrap(); +fn create_feed_item(item: Item, feed: &Feed, connection: &mut PgConnection) -> anyhow::Result<()> { + // Items without a title or link are malformed/unusable — skip them rather + // than failing the whole sync over one bad entry from an external feed. + let Some(item_title) = item.title.clone() else { + log::warn!("Skipping feed item without a title."); + return Ok(()); + }; + if item.link.is_none() { + log::warn!("Skipping feed item without a link: {}", item_title); + return Ok(()); + } log::info!("Create feed item: {}", item_title); // Items without a pub_date are treated as current (inserted unconditionally) @@ -83,7 +93,7 @@ fn create_feed_item(item: Item, feed: &Feed, connection: &mut PgConnection) { let frag = Html::parse_fragment(base_content); let mut content = "".to_string(); - let selector_img = Selector::parse("img").unwrap(); + let selector_img = Selector::parse("img").expect("\"img\" is a valid CSS selector"); match frag.select(&selector_img).find(image_src_is_resolvable) { Some(image) => { content.push_str(&image.html()); @@ -106,15 +116,14 @@ fn create_feed_item(item: Item, feed: &Feed, connection: &mut PgConnection) { let existing_item: Vec = feed_item::table .filter(feed_id.eq(feed.id)) .filter(title.eq(&item_title)) - .load(connection) - .unwrap(); + .load(connection)?; if existing_item.is_empty() { let new_feed_item = NewFeedItem::new( feed.id, content.clone(), item_title.clone(), - item.link.unwrap(), + item.link.expect("checked above"), Some(time), ); let insert_result = diesel::insert_into(feed_item::table) @@ -125,17 +134,21 @@ fn create_feed_item(item: Item, feed: &Feed, connection: &mut PgConnection) { } else { log::info!("Item {} already exists.", item_title); } + + Ok(()) } -pub async fn sync(_req: HttpRequest, data: web::Json) -> impl Responder { +pub async fn sync( + _req: HttpRequest, + data: web::Json, +) -> Result { let mut connection: diesel::PgConnection = establish_connection(); let req_user_id: i32 = data.user_id; let feeds: Vec = feed::table .filter(user_id.eq(req_user_id)) - .load::(&mut connection) - .unwrap(); + .load::(&mut connection)?; log::info!("Found {} feeds to sync.", feeds.len()); @@ -147,14 +160,16 @@ pub async fn sync(_req: HttpRequest, data: web::Json) -> impl Responde Ok(channel) => { for item in channel.into_items() { log::info!("{:?}", item); - create_feed_item(item, &feed, &mut connection); + if let Err(e) = create_feed_item(item, &feed, &mut connection) { + log::error!("Could not create feed item for {}: {:?}", feed.url, e); + } } } Err(e) => log::error!("Could not get channel {}. Error: {}", feed.url, e), } } - HttpResponse::Ok() + Ok(HttpResponse::Ok()) } #[cfg(test)] @@ -261,7 +276,8 @@ mod tests { format!("age_test_{suffix}"), format!("age_{suffix}@example.test"), "secret".to_string(), - ); + ) + .unwrap(); let user: User = diesel::insert_into(users::table) .values(&new_user) .get_result(&mut connection) @@ -295,8 +311,8 @@ mod tests { fresh_item.set_link(Some(format!("https://example.test/fresh/{suffix}"))); fresh_item.set_content(Some("

fresh

".to_string())); - create_feed_item(old_item, &feed, &mut connection); - create_feed_item(fresh_item, &feed, &mut connection); + create_feed_item(old_item, &feed, &mut connection).unwrap(); + create_feed_item(fresh_item, &feed, &mut connection).unwrap(); let items: Vec = feed_item::table .filter(feed_id.eq(feed.id)) @@ -325,7 +341,8 @@ mod tests { format!("sync_test_{suffix}"), format!("sync_{suffix}@example.test"), "secret".to_string(), - ); + ) + .unwrap(); let user: User = diesel::insert_into(users::table) .values(&new_user) .get_result(&mut connection) @@ -346,8 +363,8 @@ mod tests { item.set_link(Some(format!("https://example.test/article/{suffix}"))); item.set_content(Some("

Hello world

".to_string())); - create_feed_item(item.clone(), &feed, &mut connection); - create_feed_item(item, &feed, &mut connection); + create_feed_item(item.clone(), &feed, &mut connection).unwrap(); + create_feed_item(item, &feed, &mut connection).unwrap(); let items: Vec = feed_item::table .filter(feed_id.eq(feed.id)) diff --git a/src/test_helpers.rs b/src/test_helpers.rs index 92ae087..c3be0b3 100644 --- a/src/test_helpers.rs +++ b/src/test_helpers.rs @@ -28,7 +28,8 @@ pub fn insert_user(connection: &mut PgConnection, password: &str) -> User { format!("test_user_{suffix}"), format!("test_{suffix}@example.test"), password.to_string(), - ); + ) + .expect("failed to hash test user password"); diesel::insert_into(users::table) .values(&new_user) .get_result(connection) diff --git a/src/views/auth/login.rs b/src/views/auth/login.rs index c81c46f..63c9aa2 100755 --- a/src/views/auth/login.rs +++ b/src/views/auth/login.rs @@ -1,5 +1,6 @@ use crate::database::establish_connection; use crate::diesel; +use crate::error::AppError; use crate::json_serialization::login::Login; use crate::models::user::rss_user::User; use crate::schema::users; @@ -7,7 +8,7 @@ use crate::{auth::jwt::JwtToken, schema::users::username}; use actix_web::{web, HttpResponse}; use diesel::prelude::*; -pub async fn login(credentials: web::Json) -> HttpResponse { +pub async fn login(credentials: web::Json) -> Result { let username_cred: String = credentials.username.clone(); let password: String = credentials.password.clone(); @@ -15,32 +16,30 @@ pub async fn login(credentials: web::Json) -> HttpResponse { let users: Vec = users::table .filter(username.eq(username_cred.as_str())) - .load::(&mut connection) - .unwrap(); + .load::(&mut connection)?; if users.is_empty() { - return HttpResponse::NotFound().await.unwrap(); + return Ok(HttpResponse::NotFound().finish()); } else if users.len() > 1 { log::error!( "multiple user have the usernam: {}", credentials.username.clone() ); - return HttpResponse::Conflict().await.unwrap(); + return Ok(HttpResponse::Conflict().finish()); } let user: &User = &users[0]; - match user.clone().verify(password) { + match user.clone().verify(password)? { true => { log::info!("verified password successfully for user {}", user.id); let token: String = JwtToken::encode(user.clone().id); - HttpResponse::Ok() + Ok(HttpResponse::Ok() .insert_header(("token", token)) .insert_header(("user_id", user.id)) - .await - .unwrap() + .finish()) } - false => HttpResponse::Unauthorized().await.unwrap(), + false => Ok(HttpResponse::Unauthorized().finish()), } } diff --git a/src/views/users/create.rs b/src/views/users/create.rs index 9e95116..766cd57 100755 --- a/src/views/users/create.rs +++ b/src/views/users/create.rs @@ -1,27 +1,28 @@ use crate::database::establish_connection; use crate::diesel; +use crate::error::AppError; use crate::json_serialization::new_user::NewUserSchema; use crate::models::user::new_user::NewUser; use crate::schema::users; use actix_web::{web, HttpResponse}; use diesel::prelude::*; -pub async fn create(new_user: web::Json) -> HttpResponse { +pub async fn create(new_user: web::Json) -> Result { let mut connection = establish_connection(); let name: String = new_user.name.clone(); let email: String = new_user.email.clone(); let new_password: String = new_user.password.clone(); - let new_user = NewUser::new(name, email, new_password); + let new_user = NewUser::new(name, email, new_password)?; let insert_result = diesel::insert_into(users::table) .values(&new_user) .execute(&mut connection); - match insert_result { - Ok(_) => HttpResponse::Created().await.unwrap(), - Err(_) => HttpResponse::Conflict().await.unwrap(), - } + Ok(match insert_result { + Ok(_) => HttpResponse::Created().finish(), + Err(_) => HttpResponse::Conflict().finish(), + }) } #[cfg(test)] diff --git a/vue/src/components/AppNav.vue b/vue/src/components/AppNav.vue index dc5c320..27ff530 100644 --- a/vue/src/components/AppNav.vue +++ b/vue/src/components/AppNav.vue @@ -1,11 +1,15 @@