diff options
| author | Owen Jacobson <owen@grimoire.ca> | 2025-07-01 01:42:38 -0400 |
|---|---|---|
| committer | Owen Jacobson <owen@grimoire.ca> | 2025-07-03 21:47:41 -0400 |
| commit | b4db819ef8daa583a165aed01eb3d70d98e37fc8 (patch) | |
| tree | 81f18139d11f6f197f90958a7a28a83aab6c14cf /src | |
| parent | b3ce81945621e9026e687b590e7aa541008575ac (diff) | |
Prevent sending messages to deleted channels.
I've opted to make it clear in the error message which scenario - deleted vs. non-existant - a channel falls into. This isn't particularly consistent with the rest of the API, so we might need to review this decision later, but it's at least relatively harmless if it's mistaken. (Formally, they're both 404s, so clients that go by error code won't notice.)
Diffstat (limited to 'src')
| -rw-r--r-- | src/channel/app.rs | 8 | ||||
| -rw-r--r-- | src/channel/handlers/send/mod.rs | 4 | ||||
| -rw-r--r-- | src/channel/handlers/send/test.rs | 2 | ||||
| -rw-r--r-- | src/channel/history.rs | 9 | ||||
| -rw-r--r-- | src/message/app.rs | 16 | ||||
| -rw-r--r-- | src/message/repo.rs | 7 |
6 files changed, 31 insertions, 15 deletions
diff --git a/src/channel/app.rs b/src/channel/app.rs index dc9e584..e3b169c 100644 --- a/src/channel/app.rs +++ b/src/channel/app.rs @@ -48,14 +48,14 @@ impl<'a> Channels<'a> { // This function is careless with respect to time, and gets you the channel as // it exists in the specific moment when you call it. pub async fn get(&self, channel: &Id) -> Result<Channel, Error> { - let not_found = || Error::NotFound(channel.clone()); - let deleted = || Error::Deleted(channel.clone()); + let to_not_found = || Error::NotFound(channel.clone()); + let to_deleted = || Error::Deleted(channel.clone()); let mut tx = self.db.begin().await?; - let channel = tx.channels().by_id(channel).await.not_found(not_found)?; + let channel = tx.channels().by_id(channel).await.not_found(to_not_found)?; tx.commit().await?; - channel.as_snapshot().ok_or_else(deleted) + channel.as_snapshot().ok_or_else(to_deleted) } pub async fn delete(&self, channel: &Id, deleted_at: &DateTime) -> Result<(), DeleteError> { diff --git a/src/channel/handlers/send/mod.rs b/src/channel/handlers/send/mod.rs index aa241e2..bde39e5 100644 --- a/src/channel/handlers/send/mod.rs +++ b/src/channel/handlers/send/mod.rs @@ -54,7 +54,9 @@ impl IntoResponse for Error { fn into_response(self) -> response::Response { let Self(error) = self; match error { - SendError::ChannelNotFound(_) => NotFound(error).into_response(), + SendError::ChannelNotFound(_) | SendError::ChannelDeleted(_) => { + NotFound(error).into_response() + } SendError::Name(_) | SendError::Database(_) => Internal::from(error).into_response(), } } diff --git a/src/channel/handlers/send/test.rs b/src/channel/handlers/send/test.rs index d77e07d..70d45eb 100644 --- a/src/channel/handlers/send/test.rs +++ b/src/channel/handlers/send/test.rs @@ -125,6 +125,6 @@ async fn deleted_channel() { assert!(matches!( error, - SendError::ChannelNotFound(error_channel) if channel.id == error_channel + SendError::ChannelDeleted(error_channel) if channel.id == error_channel )); } diff --git a/src/channel/history.rs b/src/channel/history.rs index 7f18e45..85da5a5 100644 --- a/src/channel/history.rs +++ b/src/channel/history.rs @@ -27,6 +27,15 @@ impl History { self.channel.clone() } + pub fn as_of<S>(&self, sequence: S) -> Option<Channel> + where + S: Into<Sequence>, + { + self.events() + .filter(Sequence::up_to(sequence.into())) + .collect() + } + // Snapshot of this channel as of all events recorded in this history. pub fn as_snapshot(&self) -> Option<Channel> { self.events().collect() diff --git a/src/message/app.rs b/src/message/app.rs index 3c74628..9792c8f 100644 --- a/src/message/app.rs +++ b/src/message/app.rs @@ -29,13 +29,17 @@ impl<'a> Messages<'a> { sent_at: &DateTime, body: &Body, ) -> Result<Message, SendError> { + let to_not_found = || SendError::ChannelNotFound(channel.clone()); + let to_deleted = || SendError::ChannelDeleted(channel.clone()); + let mut tx = self.db.begin().await?; - let channel = tx - .channels() - .by_id(channel) - .await - .not_found(|| SendError::ChannelNotFound(channel.clone()))?; + let channel = tx.channels().by_id(channel).await.not_found(to_not_found)?; + + // Ordering: don't bother allocating a sequence number before we know the channel might + // exist. let sent = tx.sequence().next(sent_at).await?; + let channel = channel.as_of(sent).ok_or_else(to_deleted)?; + let message = tx.messages().create(&channel, sender, &sent, body).await?; tx.commit().await?; @@ -126,6 +130,8 @@ impl<'a> Messages<'a> { pub enum SendError { #[error("channel {0} not found")] ChannelNotFound(channel::Id), + #[error("channel {0} deleted")] + ChannelDeleted(channel::Id), #[error(transparent)] Database(#[from] sqlx::Error), #[error(transparent)] diff --git a/src/message/repo.rs b/src/message/repo.rs index 9a4f72f..e753134 100644 --- a/src/message/repo.rs +++ b/src/message/repo.rs @@ -2,7 +2,7 @@ use sqlx::{SqliteConnection, Transaction, sqlite::Sqlite}; use super::{Body, History, Id, snapshot::Message}; use crate::{ - channel, + channel::{self, Channel}, clock::DateTime, event::{Instant, Sequence}, user::{self, User}, @@ -23,13 +23,12 @@ pub struct Messages<'t>(&'t mut SqliteConnection); impl Messages<'_> { pub async fn create( &mut self, - channel: &channel::History, + channel: &Channel, sender: &User, sent: &Instant, body: &Body, ) -> Result<History, sqlx::Error> { let id = Id::generate(); - let channel_id = channel.id(); let message = sqlx::query!( r#" @@ -45,7 +44,7 @@ impl Messages<'_> { body as "body: Body" "#, id, - channel_id, + channel.id, sender.id, sent.at, sent.sequence, |
