From 9c23cc863d7d346e8a5e61e98eb99431cf39896f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?L=C3=A9o=20Gaspard?= Date: Sun, 22 Jan 2023 20:57:08 +0100 Subject: [PATCH] fix panic in risuto-server, leftover after the chrono fix --- risuto-api/src/error.rs | 16 ++++++++++++++++ risuto-server/src/db.rs | 10 +++++----- risuto-server/src/error.rs | 4 ++++ risuto-server/src/handlers.rs | 6 +----- risuto-server/src/query.rs | 35 +++++++++++++++++++---------------- 5 files changed, 45 insertions(+), 26 deletions(-) diff --git a/risuto-api/src/error.rs b/risuto-api/src/error.rs index 0bb6efa..5699143 100644 --- a/risuto-api/src/error.rs +++ b/risuto-api/src/error.rs @@ -31,6 +31,9 @@ pub enum Error { #[error("Invalid time")] InvalidTime(Time), + + #[error("Integer is out of expected range")] + IntegerOutOfRange(i64), } impl Error { @@ -45,6 +48,7 @@ impl Error { Error::NullByteInString(_) => StatusCode::BAD_REQUEST, Error::InvalidName(_) => StatusCode::BAD_REQUEST, Error::InvalidTime(_) => StatusCode::BAD_REQUEST, + Error::IntegerOutOfRange(_) => StatusCode::BAD_REQUEST, } } @@ -87,6 +91,11 @@ impl Error { "type": "invalid-time", "time": t, }), + Error::IntegerOutOfRange(i) => json!({ + "message": "integer is outside of acceptable range", + "type": "integer-out-of-range", + "int": i, + }), }) .expect("serializing conflict") } @@ -135,6 +144,13 @@ impl Error { anyhow!("error is about an invalid time but no time was provided") })?, ), + "integer-out-of-range" => Error::IntegerOutOfRange( + data.get("int").and_then(|s| s.as_i64()).ok_or_else(|| { + anyhow!( + "error is about an integer out of range but no integer was provided" + ) + })?, + ), _ => return Err(anyhow!("error contents has unknown type")), }, ) diff --git a/risuto-server/src/db.rs b/risuto-server/src/db.rs index 127cfaa..ea9c4eb 100644 --- a/risuto-server/src/db.rs +++ b/risuto-server/src/db.rs @@ -445,11 +445,11 @@ pub fn users_interested_by<'conn>( .map(|r| r.map(|u| UserId(u.user_id)).map_err(anyhow::Error::from)) } -async fn with_tmp_tasks_table(conn: &mut sqlx::PgConnection, f: F) -> anyhow::Result +async fn with_tmp_tasks_table(conn: &mut sqlx::PgConnection, f: F) -> Result where F: for<'a> FnOnce( &'a mut sqlx::PgConnection, - ) -> Pin>>>, + ) -> Pin>>>, { sqlx::query("CREATE TEMPORARY TABLE tmp_tasks (id UUID NOT NULL)") .execute(&mut *conn) @@ -570,11 +570,11 @@ pub async fn search_tasks_for_user( conn: &mut sqlx::PgConnection, owner: UserId, query: &Query, -) -> anyhow::Result<(Vec, Vec)> { +) -> Result<(Vec, Vec), Error> { let query::Sql { where_clause, binds, - } = query::to_postgres(&query, 2); + } = query::to_postgres(&query, 2)?; with_tmp_tasks_table(&mut *conn, |conn| { Box::pin(async move { let query = format!( @@ -623,7 +623,7 @@ pub async fn search_tasks_for_user( async fn fetch_tasks_from_tmp_tasks_table( conn: &mut sqlx::PgConnection, -) -> anyhow::Result<(Vec, Vec)> { +) -> Result<(Vec, Vec), Error> { let tasks = sqlx::query_as::<_, DbTask>( " SELECT t.id, t.owner_id, t.date, t.initial_title, t.top_comment_id diff --git a/risuto-server/src/error.rs b/risuto-server/src/error.rs index 83db964..ccb5c08 100644 --- a/risuto-server/src/error.rs +++ b/risuto-server/src/error.rs @@ -25,6 +25,10 @@ impl Error { pub fn invalid_pow() -> Error { Error::Api(ApiError::InvalidPow) } + + pub fn integer_out_of_range(i: i64) -> Error { + Error::Api(ApiError::IntegerOutOfRange(i)) + } } impl axum::response::IntoResponse for Error { diff --git a/risuto-server/src/handlers.rs b/risuto-server/src/handlers.rs index 4c6c2c9..e6122b1 100644 --- a/risuto-server/src/handlers.rs +++ b/risuto-server/src/handlers.rs @@ -98,11 +98,7 @@ pub async fn search_tasks( Json(q): Json, ) -> Result, Vec)>, Error> { q.validate()?; - Ok(Json( - db::search_tasks_for_user(&mut *conn, user, &q) - .await - .with_context(|| format!("fetching task list for {:?}", user))?, - )) + Ok(Json(db::search_tasks_for_user(&mut *conn, user, &q).await?)) } pub async fn submit_action( diff --git a/risuto-server/src/query.rs b/risuto-server/src/query.rs index ebdb100..711fc4f 100644 --- a/risuto-server/src/query.rs +++ b/risuto-server/src/query.rs @@ -1,5 +1,7 @@ use risuto_api::{Query, Time, TimeQuery, Uuid}; +use crate::error::Error; + pub enum Bind { Bool(bool), Uuid(Uuid), @@ -25,19 +27,19 @@ impl Sql { /// Assumes tables vta (v_tasks_archived), vtd(v_tasks_done), vtt (v_tasks_tags), /// vtit (v_tasks_is_tagged), vts (v_tasks_scheduled), vtb (v_tasks_blocked) /// and vtx (v_tasks_text) are available -pub fn to_postgres(q: &Query, first_bind_idx: usize) -> Sql { +pub fn to_postgres(q: &Query, first_bind_idx: usize) -> Result { let mut res = Default::default(); - add_to_postgres(q, first_bind_idx, &mut res); - res + add_to_postgres(q, first_bind_idx, &mut res)?; + Ok(res) } -fn add_to_postgres(q: &Query, first_bind_idx: usize, res: &mut Sql) { +fn add_to_postgres(q: &Query, first_bind_idx: usize, res: &mut Sql) -> Result<(), Error> { match q { Query::Any(queries) => { res.where_clause.push_str("(false"); for q in queries { res.where_clause.push_str(" OR "); - add_to_postgres(q, first_bind_idx, &mut *res); + add_to_postgres(q, first_bind_idx, &mut *res)?; } res.where_clause.push(')'); } @@ -45,13 +47,13 @@ fn add_to_postgres(q: &Query, first_bind_idx: usize, res: &mut Sql) { res.where_clause.push_str("(true"); for q in queries { res.where_clause.push_str(" AND "); - add_to_postgres(q, first_bind_idx, &mut *res); + add_to_postgres(q, first_bind_idx, &mut *res)?; } res.where_clause.push(')'); } Query::Not(q) => { res.where_clause.push_str("NOT "); - add_to_postgres(q, first_bind_idx, &mut *res); + add_to_postgres(q, first_bind_idx, &mut *res)?; } Query::Archived(true) => { res.where_clause.push_str("vta.archived = true"); @@ -86,19 +88,19 @@ fn add_to_postgres(q: &Query, first_bind_idx: usize, res: &mut Sql) { .push_str("(vtit.has_tag = false OR vtit.has_tag IS NULL)"); } Query::ScheduledForBefore(date) => { - let idx = res.add_bind(first_bind_idx, timeq_to_bind(date)); + let idx = res.add_bind(first_bind_idx, timeq_to_bind(date)?); res.where_clause.push_str(&format!("(vts.time <= ${idx})")); } Query::ScheduledForAfter(date) => { - let idx = res.add_bind(first_bind_idx, timeq_to_bind(date)); + let idx = res.add_bind(first_bind_idx, timeq_to_bind(date)?); res.where_clause.push_str(&format!("(vts.time >= ${idx})")); } Query::BlockedUntilAtMost(date) => { - let idx = res.add_bind(first_bind_idx, timeq_to_bind(date)); + let idx = res.add_bind(first_bind_idx, timeq_to_bind(date)?); res.where_clause.push_str(&format!("(vtb.time <= ${idx})")); } Query::BlockedUntilAtLeast(date) => { - let idx = res.add_bind(first_bind_idx, timeq_to_bind(date)); + let idx = res.add_bind(first_bind_idx, timeq_to_bind(date)?); res.where_clause.push_str(&format!("(vtb.time >= ${idx})")); } Query::Phrase(t) => { @@ -107,10 +109,11 @@ fn add_to_postgres(q: &Query, first_bind_idx: usize, res: &mut Sql) { .push_str(&format!("(vtx.text @@ phraseto_tsquery(${idx}))")); } } + Ok(()) } -fn timeq_to_bind(q: &TimeQuery) -> Bind { - Bind::Time(match q { +fn timeq_to_bind(q: &TimeQuery) -> Result { + Ok(Bind::Time(match q { TimeQuery::Absolute(t) => *t, TimeQuery::DayRelative { timezone, @@ -121,10 +124,10 @@ fn timeq_to_bind(q: &TimeQuery) -> Bind { let date = match *day_offset > 0 { true => date .checked_add_days(chrono::naive::Days::new(*day_offset as u64)) - .expect("failed adding days"), + .ok_or_else(|| Error::integer_out_of_range(*day_offset))?, false => date .checked_sub_days(chrono::naive::Days::new((-day_offset) as u64)) - .expect("failed subtracting days"), + .ok_or_else(|| Error::integer_out_of_range(*day_offset))?, }; date.and_hms_opt(0, 0, 0) .expect("naive_date and hms 000 failed") @@ -132,5 +135,5 @@ fn timeq_to_bind(q: &TimeQuery) -> Bind { .unwrap() .with_timezone(&chrono::Utc) } - }) + })) }