From 0e2694a6c4b9df71adc89df82b9415bf8ce72093 Mon Sep 17 00:00:00 2001 From: Benjamin Lee Date: Thu, 2 May 2024 18:46:58 -0700 Subject: [PATCH] implement contains_url filter for /message The plan is to move all of the per-event checks into the pdu_event_allowed function. TODO: split the `visibility_filter` function into it's own commit. I think this one was inherited from an earlier conduwuit commit. --- src/api/client_server/message.rs | 38 +++++++++++++++----------------- src/utils/filter.rs | 34 ++++++++++++++++++++++++++-- 2 files changed, 50 insertions(+), 22 deletions(-) diff --git a/src/api/client_server/message.rs b/src/api/client_server/message.rs index c2d29ee4..229fb691 100644 --- a/src/api/client_server/message.rs +++ b/src/api/client_server/message.rs @@ -9,14 +9,14 @@ use ruma::{ message::{get_message_events, send_message_event}, }, events::{StateEventType, TimelineEventType}, - uint, UInt, + uint, RoomId, UInt, UserId, }; use crate::{ service::{pdu::PduBuilder, rooms::timeline::PduCount}, services, utils, utils::filter::{load_limit, CompiledRoomEventFilter}, - Ar, Error, Ra, Result, + Ar, Error, PduEvent, Ra, Result, }; /// # `PUT /_matrix/client/r0/rooms/{roomId}/send/{eventType}/{txnId}` @@ -197,15 +197,8 @@ pub(crate) async fn get_message_events_route( .take(load_limit(limit)) .filter_map(Result::ok) .filter(|(_, pdu)| { - services() - .rooms - .state_accessor - .user_can_see_event( - sender_user, - &body.room_id, - &pdu.event_id, - ) - .unwrap_or(false) + filter.pdu_event_allowed(pdu) + && visibility_filter(pdu, sender_user, &body.room_id) }) .take_while(|&(k, _)| Some(k) != to) .take(limit) @@ -254,15 +247,8 @@ pub(crate) async fn get_message_events_route( .take(load_limit(limit)) .filter_map(Result::ok) .filter(|(_, pdu)| { - services() - .rooms - .state_accessor - .user_can_see_event( - sender_user, - &body.room_id, - &pdu.event_id, - ) - .unwrap_or(false) + filter.pdu_event_allowed(pdu) + && visibility_filter(pdu, sender_user, &body.room_id) }) .take_while(|&(k, _)| Some(k) != to) .take(limit) @@ -331,3 +317,15 @@ pub(crate) async fn get_message_events_route( Ok(Ra(resp)) } + +fn visibility_filter( + pdu: &PduEvent, + user_id: &UserId, + room_id: &RoomId, +) -> bool { + services() + .rooms + .state_accessor + .user_can_see_event(user_id, room_id, &pdu.event_id) + .unwrap_or(false) +} diff --git a/src/utils/filter.rs b/src/utils/filter.rs index d4be2b16..d4de0575 100644 --- a/src/utils/filter.rs +++ b/src/utils/filter.rs @@ -14,9 +14,12 @@ use std::{collections::HashSet, hash::Hash}; -use ruma::{api::client::filter::RoomEventFilter, RoomId}; +use ruma::{ + api::client::filter::{RoomEventFilter, UrlFilter}, + RoomId, +}; -use crate::Error; +use crate::{Error, PduEvent}; // 'DoS' is not a type #[allow(clippy::doc_markdown)] @@ -82,6 +85,7 @@ impl<'a, T: ?Sized + Hash + PartialEq + Eq> AllowDenyList<'a, T> { pub(crate) struct CompiledRoomEventFilter<'a> { rooms: AllowDenyList<'a, RoomId>, + url_filter: Option, } impl<'a> TryFrom<&'a RoomEventFilter> for CompiledRoomEventFilter<'a> { @@ -95,6 +99,7 @@ impl<'a> TryFrom<&'a RoomEventFilter> for CompiledRoomEventFilter<'a> { source.rooms.as_deref(), &source.not_rooms, ), + url_filter: source.url_filter, }) } } @@ -110,4 +115,29 @@ impl CompiledRoomEventFilter<'_> { pub(crate) fn room_allowed(&self, room_id: &RoomId) -> bool { self.rooms.allowed(room_id) } + + /// Returns `true` if a PDU event is allowed by the filter. + /// + /// This tests against the `url_filter` field. + /// + /// This does *not* check whether the event's room is allowed. It is + /// expected that callers have already filtered out rejected rooms using + /// [`CompiledRoomEventFilter::room_allowed`] and + /// [`CompiledRoomFilter::rooms`]. + pub(crate) fn pdu_event_allowed(&self, pdu: &PduEvent) -> bool { + self.allowed_by_url_filter(pdu) + } + + fn allowed_by_url_filter(&self, pdu: &PduEvent) -> bool { + let Some(filter) = self.url_filter else { + return true; + }; + // TODO: is this unwrap okay? + let content: serde_json::Value = + serde_json::from_str(pdu.content.get()).unwrap(); + match filter { + UrlFilter::EventsWithoutUrl => !content["url"].is_string(), + UrlFilter::EventsWithUrl => content["url"].is_string(), + } + } }