Skip to content

Commit

Permalink
fix: check invalid timestamp values
Browse files Browse the repository at this point in the history
  • Loading branch information
flosse committed Sep 19, 2023
1 parent 74ea69f commit dd3c17a
Show file tree
Hide file tree
Showing 24 changed files with 359 additions and 281 deletions.
19 changes: 12 additions & 7 deletions ofdb-boundary/src/conv.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,15 +14,17 @@ impl From<e::time::Timestamp> for UnixTimeSeconds {
}
}

impl From<UnixTimeSeconds> for e::time::Timestamp {
fn from(from: UnixTimeSeconds) -> Self {
Self::from_secs(from.0)
impl TryFrom<UnixTimeSeconds> for e::time::Timestamp {
type Error = e::time::OutOfRangeError;
fn try_from(from: UnixTimeSeconds) -> Result<Self, Self::Error> {
Self::try_from_secs(from.0)
}
}

impl From<UnixTimeMillis> for e::time::Timestamp {
fn from(from: UnixTimeMillis) -> Self {
Self::from_millis(from.0)
impl TryFrom<UnixTimeMillis> for e::time::Timestamp {
type Error = e::time::OutOfRangeError;
fn try_from(from: UnixTimeMillis) -> Result<Self, Self::Error> {
Self::try_from_millis(from.0)
}
}

Expand Down Expand Up @@ -467,14 +469,17 @@ impl From<e::activity::Activity> for Activity {
pub enum ActivityConversionError {
#[error(transparent)]
Email(#[from] e::email::EmailAddressParseError),
#[error(transparent)]
Time(#[from] e::time::OutOfRangeError),
}

impl TryFrom<Activity> for e::activity::Activity {
type Error = ActivityConversionError;
fn try_from(from: Activity) -> Result<Self, Self::Error> {
let Activity { at, by } = from;
let by = by.map(|email| email.parse()).transpose()?;
Ok(Self { at: at.into(), by })
let at = at.try_into()?;
Ok(Self { at, by })
}
}

Expand Down
93 changes: 63 additions & 30 deletions ofdb-core/src/usecases/store_event.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
use std::str::FromStr;

use crate::{
repositories::Error as RepoError,
usecases::{
Expand All @@ -9,31 +11,65 @@ use crate::{
validate::{AutoCorrect, Validate},
},
};
use std::str::FromStr;

#[rustfmt::skip]
#[derive(Default, Debug, Clone)]
#[derive(Debug, Clone)]
pub struct NewEvent {
pub title : String,
pub description : Option<String>,
pub start : i64,
pub end : Option<i64>,
pub lat : Option<f64>,
pub lng : Option<f64>,
pub street : Option<String>,
pub zip : Option<String>,
pub city : Option<String>,
pub country : Option<String>,
pub state : Option<String>,
pub email : Option<EmailAddress>,
pub telephone : Option<String>,
pub homepage : Option<String>,
pub tags : Option<Vec<String>>,
pub created_by : Option<EmailAddress>,
pub registration : Option<String>,
pub organizer : Option<String>,
pub image_url : Option<String>,
pub image_link_url: Option<String>,
pub title : String,
pub description : Option<String>,
pub start : Timestamp,
pub end : Option<Timestamp>,
pub lat : Option<f64>,
pub lng : Option<f64>,
pub street : Option<String>,
pub zip : Option<String>,
pub city : Option<String>,
pub country : Option<String>,
pub state : Option<String>,
pub email : Option<EmailAddress>,
pub telephone : Option<String>,
pub homepage : Option<String>,
pub tags : Option<Vec<String>>,
pub created_by : Option<EmailAddress>,
pub registration : Option<String>,
pub organizer : Option<String>,
pub image_url : Option<String>,
pub image_link_url : Option<String>,
}

// TODO:
// Avoid using this outside of tests.
impl Default for NewEvent {
fn default() -> Self {
Self::new(String::new(), Timestamp::now())
}
}

impl NewEvent {
pub fn new(title: String, start: Timestamp) -> Self {
Self {
title,
start,
description: None,
end: None,
lat: None,
lng: None,
street: None,
zip: None,
city: None,
country: None,
state: None,
email: None,
telephone: None,
homepage: None,
tags: None,
created_by: None,
registration: None,
organizer: None,
image_url: None,
image_link_url: None,
}
}
}

pub enum NewEventMode<'a> {
Expand Down Expand Up @@ -263,9 +299,6 @@ where
None => None,
};

let start = Timestamp::from_secs(start);
let end = end.map(Timestamp::from_secs);

let homepage = homepage
.and_then(|ref url| parse_url_param(url).transpose())
.transpose()?;
Expand Down Expand Up @@ -338,7 +371,7 @@ mod tests {

#[test]
fn create_new_valid_event() {
let now = Timestamp::now().as_secs();
let now = Timestamp::now();
#[rustfmt::skip]
let x = NewEvent {
title : "foo".into(),
Expand Down Expand Up @@ -369,7 +402,7 @@ mod tests {
assert_eq!(mock_db.tags.borrow().len(), 2);
let x = &mock_db.events.borrow()[0];
assert_eq!(x.title, "foo");
assert_eq!(x.start.as_secs(), now);
assert_eq!(x.start, now);
assert!(x.location.is_none());
assert_eq!(x.description.as_ref().unwrap(), "bar");
assert!(x.id.is_valid());
Expand All @@ -390,7 +423,7 @@ mod tests {
let x = NewEvent {
title : "foo".into(),
description : Some("bar".into()),
start : Timestamp::now().as_secs(),
start : Timestamp::now(),
end : None,
lat : None,
lng : None,
Expand Down Expand Up @@ -419,7 +452,7 @@ mod tests {
let x = NewEvent {
title : "foo".into(),
description : Some("bar".into()),
start : Timestamp::now().as_secs(),
start : Timestamp::now(),
end : None,
lat : None,
lng : None,
Expand Down Expand Up @@ -465,7 +498,7 @@ mod tests {
let x = NewEvent {
title : "foo".into(),
description : Some("bar".into()),
start : Timestamp::now().as_secs(),
start : Timestamp::now(),
end : None,
lat : None,
lng : None,
Expand Down
2 changes: 1 addition & 1 deletion ofdb-core/src/usecases/tests/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -886,7 +886,7 @@ fn receive_event_with_creators_email() {
id: "x".into(),
title: "t".into(),
description: None,
start: Timestamp::from_secs(0),
start: Timestamp::try_from_secs(0).unwrap(),
end: None,
contact: None,
location: None,
Expand Down
6 changes: 3 additions & 3 deletions ofdb-core/src/util/validate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -202,7 +202,7 @@ mod tests {
id: "x".into(),
title: "foo".into(),
description: None,
start: Timestamp::from_secs(0),
start: Timestamp::try_from_secs(0).unwrap(),
end: None,
location: None,
contact: None,
Expand Down Expand Up @@ -339,8 +339,8 @@ mod tests {
id: "x".into(),
title: "foo".into(),
description: None,
start: Timestamp::from_secs(100),
end: Some(Timestamp::from_secs(99)),
start: Timestamp::try_from_secs(100).unwrap(),
end: Some(Timestamp::try_from_secs(99).unwrap()),
location: None,
contact: None,
tags: vec![],
Expand Down
6 changes: 3 additions & 3 deletions ofdb-db-sqlite/src/repo_impl/event.rs
Original file line number Diff line number Diff line change
Expand Up @@ -288,16 +288,16 @@ fn get_events_chronologically(conn: &mut SqliteConnection, ids: &[&str]) -> Resu
let event = Event {
id: uid.into(),
title,
start: Timestamp::from_secs(start),
end: end.map(Timestamp::from_secs),
start: Timestamp::try_from_secs(start).unwrap(),
end: end.map(Timestamp::try_from_secs).transpose().unwrap(),
description,
location,
contact,
homepage: homepage.and_then(load_url),
tags,
created_by,
registration,
archived: archived.map(Timestamp::from_secs),
archived: archived.map(Timestamp::try_from_secs).transpose().unwrap(),
image_url: image_url.and_then(load_url),
image_link_url: image_link_url.and_then(load_url),
};
Expand Down
6 changes: 3 additions & 3 deletions ofdb-db-sqlite/src/repo_impl/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -174,7 +174,7 @@ fn load_place(
license,
revision: Revision::from(rev as u64),
created: Activity {
at: Timestamp::from_millis(created_at),
at: Timestamp::try_from_millis(created_at).unwrap(),
by: created_by.map(EmailAddress::new_unchecked),
},
title,
Expand Down Expand Up @@ -290,7 +290,7 @@ fn load_place_with_status_review(
license,
revision: Revision::from(rev as u64),
created: Activity {
at: Timestamp::from_millis(created_at),
at: Timestamp::try_from_millis(created_at).unwrap(),
by: created_by.map(EmailAddress::new_unchecked),
},
title,
Expand All @@ -305,7 +305,7 @@ fn load_place_with_status_review(

let activity_log = ActivityLog {
activity: Activity {
at: Timestamp::from_millis(review_created_at),
at: Timestamp::try_from_millis(review_created_at).unwrap(),
by: review_created_by.map(EmailAddress::new_unchecked),
},
context: review_context,
Expand Down
22 changes: 12 additions & 10 deletions ofdb-db-sqlite/src/repo_impl/place.rs
Original file line number Diff line number Diff line change
Expand Up @@ -635,17 +635,19 @@ fn get_place_history(
.map_err(from_diesel_err)?;
let mut review_logs = Vec::with_capacity(rows.len());
for row in rows {
let revision = Revision::from(row.rev as u64);
let at = Timestamp::try_from_millis(row.created_at).unwrap();
let by = row.created_by_email.map(EmailAddress::new_unchecked);
let activity = ActivityLog {
activity: Activity { at, by },
context: row.context,
comment: row.comment,
};
let status = ReviewStatus::try_from(row.status).unwrap();
let review_log = ReviewStatusLog {
revision: Revision::from(row.rev as u64),
activity: ActivityLog {
activity: Activity {
at: Timestamp::from_millis(row.created_at),
by: row.created_by_email.map(EmailAddress::new_unchecked),
},
context: row.context,
comment: row.comment,
},
status: ReviewStatus::try_from(row.status).unwrap(),
revision,
activity,
status,
};
review_logs.push(review_log);
}
Expand Down
2 changes: 1 addition & 1 deletion ofdb-db-sqlite/src/repo_impl/reminder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ fn find_last_sent_reminder(
.map_err(from_diesel_err)?
.into_iter()
.map(|r| r.sent_at)
.map(Timestamp::from_millis);
.map(|millis| Timestamp::try_from_millis(millis).unwrap());
let first = iter.next();
debug_assert!(iter.next().is_none());
Ok(first)
Expand Down
26 changes: 16 additions & 10 deletions ofdb-db-sqlite/src/util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -165,15 +165,15 @@ pub(crate) fn event_from_event_entity_and_tags(e: EventEntity, tag_rels: &[Event
id: uid.into(),
title,
description,
start: Timestamp::from_secs(start),
end: end.map(Timestamp::from_secs),
start: Timestamp::try_from_secs(start).unwrap(),
end: end.map(Timestamp::try_from_secs).transpose().unwrap(),
location,
contact,
homepage: homepage.and_then(load_url),
tags,
created_by,
registration,
archived: archived.map(Timestamp::from_secs),
archived: archived.map(Timestamp::try_from_secs).transpose().unwrap(),
image_url: image_url.and_then(load_url),
image_link_url: image_link_url.and_then(load_url),
}
Expand Down Expand Up @@ -245,8 +245,11 @@ impl From<PlaceRatingComment> for e::Comment {
Self {
id: id.into(),
rating_id: rating_id.into(),
created_at: Timestamp::from_millis(created_at),
archived_at: archived_at.map(Timestamp::from_millis),
created_at: Timestamp::try_from_millis(created_at).unwrap(),
archived_at: archived_at
.map(Timestamp::try_from_millis)
.transpose()
.unwrap(),
text,
}
}
Expand All @@ -268,8 +271,11 @@ impl From<PlaceRating> for e::Rating {
Self {
id: id.into(),
place_id: place_id.into(),
created_at: Timestamp::from_millis(created_at),
archived_at: archived_at.map(Timestamp::from_millis),
created_at: Timestamp::try_from_millis(created_at).unwrap(),
archived_at: archived_at
.map(Timestamp::try_from_millis)
.transpose()
.unwrap(),
title,
value: (value as i8).into(),
context: rating_context_from_str(&context).unwrap(),
Expand Down Expand Up @@ -310,7 +316,7 @@ impl From<UserTokenEntity> for e::UserToken {
email: e::EmailAddress::new_unchecked(from.user_email),
nonce: from.nonce.parse::<Nonce>().unwrap_or_default(),
},
expires_at: Timestamp::from_millis(from.expires_at),
expires_at: Timestamp::try_from_millis(from.expires_at).unwrap(),
}
}
}
Expand All @@ -320,7 +326,7 @@ impl From<ReviewTokenEntity> for e::ReviewToken {
let place_revision = e::Revision::from(from.place_revision as u64);
let place_id = e::Id::from(from.place_id);
let nonce = from.nonce.parse::<Nonce>().unwrap_or_default();
let expires_at = Timestamp::from_millis(from.expires_at);
let expires_at = Timestamp::try_from_millis(from.expires_at).unwrap();
let review_nonce = e::ReviewNonce {
place_id,
place_revision,
Expand Down Expand Up @@ -456,7 +462,7 @@ impl From<PendingClearanceForPlace> for e::PendingClearanceForPlace {
let last_cleared_revision = last_cleared_revision.map(|rev| e::Revision::from(rev as u64));
Self {
place_id: place_id.into(),
created_at: e::Timestamp::from_millis(created_at),
created_at: e::Timestamp::try_from_millis(created_at).unwrap(),
last_cleared_revision,
}
}
Expand Down
Loading

0 comments on commit dd3c17a

Please sign in to comment.