diff --git a/crates/matrix-sdk-base/src/event_cache_store/traits.rs b/crates/matrix-sdk-base/src/event_cache_store/traits.rs index a288750f6..eaa1ada19 100644 --- a/crates/matrix-sdk-base/src/event_cache_store/traits.rs +++ b/crates/matrix-sdk-base/src/event_cache_store/traits.rs @@ -61,6 +61,9 @@ pub trait EventCacheStore: AsyncTraitDeps { /// keyed as a file before. The caller is responsible of ensuring that /// the replacement makes sense, according to their use case. /// + /// This should not raise an error when the `from` parameter points to an + /// unknown media, and it should silently continue in this case. + /// /// # Arguments /// /// * `from` - The previous `MediaRequest` of the file. diff --git a/crates/matrix-sdk-base/src/store/send_queue.rs b/crates/matrix-sdk-base/src/store/send_queue.rs index fb8327069..51bc1c7f1 100644 --- a/crates/matrix-sdk-base/src/store/send_queue.rs +++ b/crates/matrix-sdk-base/src/store/send_queue.rs @@ -86,7 +86,7 @@ pub enum QueuedRequestKind { /// /// The bytes must be stored in the media cache, and are identified by the /// cache key. - Upload { + MediaUpload { /// Content type of the media to be uploaded. /// /// Stored as a `String` because `Mime` which we'd really want to use diff --git a/crates/matrix-sdk/src/send_queue.rs b/crates/matrix-sdk/src/send_queue.rs index a4ae6891e..1ec961f1e 100644 --- a/crates/matrix-sdk/src/send_queue.rs +++ b/crates/matrix-sdk/src/send_queue.rs @@ -82,7 +82,7 @@ //! - An initial media event is created and uses this temporary MXC ID, and //! propagated as a local echo for an event. //! - A [`QueuedRequest`] is pushed to upload the file's media -//! ([`QueuedRequestKind::Upload`]). +//! ([`QueuedRequestKind::MediaUpload`]). //! - A [`DependentQueuedRequest`] is pushed to finish the upload //! ([`DependentQueuedRequestKind::FinishUpload`]). //! @@ -106,7 +106,8 @@ //! When there is a thumbnail, things behave similarly, with some tweaks: //! //! - the thumbnail's content is also stored into the cache store immediately, -//! - the thumbnail is sent first as an [`QueuedRequestKind::Upload`] request, +//! - the thumbnail is sent first as an [`QueuedRequestKind::MediaUpload`] +//! request, //! - the file upload is pushed as a dependent request of kind //! [`DependentQueuedRequestKind::UploadFileWithThumbnail`] (this variant //! keeps the file's key used to look it up in the cache store). @@ -117,11 +118,11 @@ //! //! - After the thumbnail has been uploaded, the dependent query will retrieve //! the final MXC ID returned by the homeserver for the thumbnail, and store -//! it into the [`QueuedRequestKind::Upload`]'s `thumbnail_source` field, +//! it into the [`QueuedRequestKind::MediaUpload`]'s `thumbnail_source` field, //! allowing to remember the thumbnail MXC ID when it's time to finish the //! upload later. //! - The dependent request is morphed into another -//! [`QueuedRequestKind::Upload`], for the file itself. +//! [`QueuedRequestKind::MediaUpload`], for the file itself. //! //! The rest of the process is then similar to that of uploading a file without //! a thumbnail. The only difference is that there's a thumbnail source (MXC ID) @@ -559,7 +560,7 @@ impl RoomSendQueue { let txn_id = queued_request.transaction_id.clone(); trace!(txn_id = %txn_id, "received a request to send!"); - let related_txn_id = as_variant!(&queued_request.kind, QueuedRequestKind::Upload { related_to, .. } => related_to.clone()); + let related_txn_id = as_variant!(&queued_request.kind, QueuedRequestKind::MediaUpload { related_to, .. } => related_to.clone()); let Some(room) = room.get() else { if is_dropping.load(Ordering::SeqCst) { @@ -678,7 +679,7 @@ impl RoomSendQueue { Ok(SentRequestKey::Event(res.event_id)) } - QueuedRequestKind::Upload { + QueuedRequestKind::MediaUpload { content_type, cache_key, thumbnail_source, @@ -1055,10 +1056,10 @@ impl QueueStorage { .save_send_queue_request( &self.room_id, upload_thumbnail_txn.clone(), - QueuedRequestKind::Upload { + QueuedRequestKind::MediaUpload { content_type: thumbnail_content_type.to_string(), cache_key: thumbnail_media_request, - thumbnail_source: None, + thumbnail_source: None, // the thumbnail has no thumbnails :) related_to: send_event_txn.clone(), }, ) @@ -1085,7 +1086,7 @@ impl QueueStorage { .save_send_queue_request( &self.room_id, upload_file_txn.clone(), - QueuedRequestKind::Upload { + QueuedRequestKind::MediaUpload { content_type: content_type.to_string(), cache_key: file_media_request, thumbnail_source: None, @@ -1169,7 +1170,7 @@ impl QueueStorage { send_error: queued.error, }, - QueuedRequestKind::Upload { .. } => { + QueuedRequestKind::MediaUpload { .. } => { // Don't return uploaded medias as their own things; the accompanying // event represented as a dependent request should be sufficient. return None; diff --git a/crates/matrix-sdk/src/send_queue/upload.rs b/crates/matrix-sdk/src/send_queue/upload.rs index e31515c9f..2bd6a8c47 100644 --- a/crates/matrix-sdk/src/send_queue/upload.rs +++ b/crates/matrix-sdk/src/send_queue/upload.rs @@ -48,11 +48,17 @@ use crate::{ /// /// This uses a MXC ID that is only locally valid. fn make_local_file_media_request(txn_id: &TransactionId) -> MediaRequest { - // .local is guaranteed to be on the local network. It would be a shame that - // `send-queue.local` resolves to an actual Synapse media server, we don't - // expect this to be likely though. + // This mustn't represent a potentially valid media server, otherwise it'd be + // possible for an attacker to return malicious content under some + // preconditions (e.g. the cache store has been cleared before the upload + // took place). To mitigate against this, we use the .localhost TLD, + // which is guaranteed to be on the local machine. As a result, the only attack + // possible would be coming from the user themselves, which we consider a + // non-threat. MediaRequest { - source: MediaSource::Plain(OwnedMxcUri::from(format!("mxc://send-queue.local/{txn_id}"))), + source: MediaSource::Plain(OwnedMxcUri::from(format!( + "mxc://send-queue.localhost/{txn_id}" + ))), format: MediaFormat::File, } } @@ -66,9 +72,9 @@ fn make_local_thumbnail_media_request( height: UInt, width: UInt, ) -> MediaRequest { - // See comment in [`Self::make_local_file_media_request`]. + // See comment in [`make_local_file_media_request`]. let source = - MediaSource::Plain(OwnedMxcUri::from(format!("mxc://send-queue.local/{}", txn_id))); + MediaSource::Plain(OwnedMxcUri::from(format!("mxc://send-queue.localhost/{}", txn_id))); let format = MediaFormat::Thumbnail(MediaThumbnailSettings { size: MediaThumbnailSize { method: Method::Scale, width, height }, animated: false, @@ -79,7 +85,7 @@ fn make_local_thumbnail_media_request( /// Replace the source by the final ones in all the media types handled by /// [`Room::make_attachment_type()`]. fn update_media_event_after_upload(echo: &mut RoomMessageEventContent, sent: SentMediaInfo) { - // Some variants look eerily similar below, but the `event` and `info` are all + // Some variants look really similar below, but the `event` and `info` are all // different types… match &mut echo.msgtype { MessageType::Audio(event) => { @@ -141,6 +147,7 @@ impl RoomSendQueue { let Some(room) = self.inner.room.get() else { return Err(RoomSendQueueError::RoomDisappeared); }; + if room.state() != RoomState::Joined { return Err(RoomSendQueueError::RoomNotJoined); } @@ -367,7 +374,7 @@ impl QueueStorage { trace!(related_to = %event_txn, "done uploading thumbnail, now queuing a request to send the media file itself"); - let request = QueuedRequestKind::Upload { + let request = QueuedRequestKind::MediaUpload { content_type, cache_key, // The thumbnail for the next upload is the file we just uploaded here. diff --git a/crates/matrix-sdk/tests/integration/send_queue.rs b/crates/matrix-sdk/tests/integration/send_queue.rs index 13ee7bb9c..cf6ec31b0 100644 --- a/crates/matrix-sdk/tests/integration/send_queue.rs +++ b/crates/matrix-sdk/tests/integration/send_queue.rs @@ -2117,7 +2117,7 @@ async fn test_media_uploads() { // Check the data source: it should reference the send queue local storage. let local_source = img_content.source; assert_let!(MediaSource::Plain(mxc) = &local_source); - assert!(mxc.to_string().starts_with("mxc://send-queue.local/"), "{mxc}"); + assert!(mxc.to_string().starts_with("mxc://send-queue.localhost/"), "{mxc}"); // The media is immediately available from the cache. let file_media = client @@ -2140,7 +2140,7 @@ async fn test_media_uploads() { // Check the thumbnail source: it should reference the send queue local storage. let local_thumbnail_source = info.thumbnail_source.unwrap(); assert_let!(MediaSource::Plain(mxc) = &local_thumbnail_source); - assert!(mxc.to_string().starts_with("mxc://send-queue.local/"), "{mxc}"); + assert!(mxc.to_string().starts_with("mxc://send-queue.localhost/"), "{mxc}"); let thumbnail_media = client .media()