From 69c807bdc1dda35d68f2314e67238bd024db932e Mon Sep 17 00:00:00 2001 From: Xiretza Date: Thu, 3 Nov 2022 13:16:01 +0100 Subject: [PATCH] events: Escape plain bodies in replies Replies generate an HTML body even if the reply itself only consists of plain text. In order to convert the plain text to HTML, it has to be escaped, which did not happen previously. --- crates/ruma-common/CHANGELOG.md | 5 ++ .../src/events/room/message/reply.rs | 47 +++++++++-------- .../ruma-common/tests/events/room_message.rs | 52 ++++++++++++++++++- 3 files changed, 81 insertions(+), 23 deletions(-) diff --git a/crates/ruma-common/CHANGELOG.md b/crates/ruma-common/CHANGELOG.md index ef6df0eb..de8fcbb8 100644 --- a/crates/ruma-common/CHANGELOG.md +++ b/crates/ruma-common/CHANGELOG.md @@ -1,5 +1,10 @@ # [unreleased] +Bug fixes: + +* HTML-relevant characters (`<`, `>`, etc) in plaintext replies are now escaped + during creation of the rich reply + Breaking changes: * Remove deprecated `EventType` enum diff --git a/crates/ruma-common/src/events/room/message/reply.rs b/crates/ruma-common/src/events/room/message/reply.rs index 2ae771db..e9d6c2ab 100644 --- a/crates/ruma-common/src/events/room/message/reply.rs +++ b/crates/ruma-common/src/events/room/message/reply.rs @@ -48,6 +48,29 @@ fn get_message_quote_fallbacks(original_message: &OriginalRoomMessageEvent) -> ( } } +/// Converts a plaintext body to HTML, escaping any characters that would cause problems. +fn escape_html_entities(body: &str) -> String { + let mut escaped_body = String::with_capacity(body.len()); + for c in body.chars() { + // Escape reserved HTML entities and new lines. + // + let s = match c { + '&' => Some("&"), + '<' => Some("<"), + '>' => Some(">"), + '"' => Some("""), + '\n' => Some("
"), + _ => None, + }; + if let Some(s) = s { + escaped_body.push_str(s); + } else { + escaped_body.push(c); + } + } + escaped_body +} + fn formatted_or_plain_body( formatted: Option<&FormattedBody>, body: &str, @@ -64,25 +87,7 @@ fn formatted_or_plain_body( #[cfg(not(feature = "unstable-sanitize"))] formatted_body.body.clone() } else { - let mut escaped_body = String::with_capacity(body.len()); - for c in body.chars() { - // Escape reserved HTML entities and new lines. - // - let s = match c { - '&' => Some("&"), - '<' => Some("<"), - '>' => Some(">"), - '"' => Some("""), - '\n' => Some("
"), - _ => None, - }; - if let Some(s) = s { - escaped_body.push_str(s); - } else { - escaped_body.push(c); - } - } - escaped_body + escape_html_entities(body) } } @@ -97,7 +102,7 @@ fn formatted_or_plain_body( /// [HTML tags and attributes]: https://spec.matrix.org/v1.4/client-server-api/#mroommessage-msgtypes /// [rich reply fallbacks]: https://spec.matrix.org/v1.4/client-server-api/#fallbacks-for-rich-replies pub fn plain_and_formatted_reply_body( - body: impl fmt::Display, + body: &str, formatted: Option, original_message: &OriginalRoomMessageEvent, ) -> (String, String) { @@ -106,7 +111,7 @@ pub fn plain_and_formatted_reply_body( let plain = format!("{quoted}\n{body}"); let html = match formatted { Some(formatted) => format!("{quoted_html}{formatted}"), - None => format!("{quoted_html}{body}"), + None => format!("{quoted_html}{}", escape_html_entities(body)), }; (plain, html) diff --git a/crates/ruma-common/tests/events/room_message.rs b/crates/ruma-common/tests/events/room_message.rs index 48c8f1a2..4d26001a 100644 --- a/crates/ruma-common/tests/events/room_message.rs +++ b/crates/ruma-common/tests/events/room_message.rs @@ -8,8 +8,9 @@ use ruma_common::{ key::verification::VerificationMethod, room::{ message::{ - AudioMessageEventContent, KeyVerificationRequestEventContent, MessageType, - OriginalRoomMessageEvent, RoomMessageEventContent, TextMessageEventContent, + AudioMessageEventContent, ForwardThread, KeyVerificationRequestEventContent, + MessageType, OriginalRoomMessageEvent, RoomMessageEventContent, + TextMessageEventContent, }, MediaSource, }, @@ -434,6 +435,53 @@ fn content_deserialization_failure() { assert_matches!(from_json_value::(json_data), Err(_)); } +#[test] +fn escape_tags_in_plain_reply_body() { + let first_message = OriginalRoomMessageEvent { + content: RoomMessageEventContent::text_plain("Usage: cp "), + event_id: event_id!("$143273582443PhrSn:example.org").to_owned(), + origin_server_ts: MilliSecondsSinceUnixEpoch(uint!(10_000)), + room_id: room_id!("!testroomid:example.org").to_owned(), + sender: user_id!("@user:example.org").to_owned(), + unsigned: MessageLikeUnsigned::default(), + }; + let second_message = RoomMessageEventContent::text_plain("Usage: rm ") + .make_reply_to(&first_message, ForwardThread::Yes); + + let body = assert_matches!( + first_message.content.msgtype, + MessageType::Text(TextMessageEventContent { body, formatted: None, .. }) => body + ); + assert_eq!(body, "Usage: cp "); + + let (body, formatted) = assert_matches!( + second_message.msgtype, + MessageType::Text(TextMessageEventContent { body, formatted, .. }) => (body, formatted) + ); + assert_eq!( + body, + "\ + > <@user:example.org> Usage: cp \n\ + Usage: rm \ + " + ); + let formatted = formatted.unwrap(); + assert_eq!( + formatted.body, + "\ + \ +
\ + In reply to \ + @user:example.org\ +
\ + Usage: cp <source> <destination>\ +
\ + \ + Usage: rm <path>\ + " + ); +} + #[test] #[cfg(feature = "unstable-sanitize")] fn reply_sanitize() {