Carry the formatting a Matrix sender actually sent
formatted_body appeared nowhere, so only content.body was read: code blocks, quotes, lists, links with text and every kind of emphasis arrived flattened to plain text. It travels as its own field rather than replacing body. The plain text is what every other protocol has, what search reads, and what a client without a sanitiser can still show - and only org.matrix.custom.html is taken as HTML, since rendering an unknown format as HTML would be trusting markup with no reason to believe it is HTML at all. Structured is not trusted: this is somebody else's markup and it goes through the same whitelist as everything else on the way to the screen. Outgoing is still plain text. Saying it in HTML means turning what somebody typed into HTML, and there is no markdown parser here to do that honestly - a hand-rolled one would be a worse thing than a plain message, which is valid Matrix and renders correctly everywhere. Filed separately. Co-Authored-By: Claude Opus 5 <[email protected]>
This commit is contained in:
@@ -1824,7 +1824,7 @@ fn store_history_messages(state: &AppState, buffer_id: &str, messages: &[Value],
|
||||
.and_then(|s| chrono::DateTime::parse_from_rfc3339(s).ok())
|
||||
.map(|dt| dt.timestamp())
|
||||
.unwrap_or(0);
|
||||
if let Err(e) = state.store.append_message(buffer_id, msg_id, from, &body, ts, false, false, "chat", reply_to.as_ref(), &reactions, is_own, avatar_url.as_deref(), &embeds, &attachments, None) {
|
||||
if let Err(e) = state.store.append_message(buffer_id, msg_id, from, &body, ts, false, false, "chat", reply_to.as_ref(), &reactions, is_own, avatar_url.as_deref(), &embeds, &attachments, None, None) {
|
||||
tracing::warn!("discord: storing history message: {e}");
|
||||
continue;
|
||||
}
|
||||
@@ -3142,6 +3142,7 @@ mod tests {
|
||||
Message {
|
||||
id: id.into(),
|
||||
buffer_id: "b".into(),
|
||||
html: None,
|
||||
from: "x".into(),
|
||||
body: String::new(),
|
||||
ts: 0,
|
||||
|
||||
+2
-2
@@ -663,9 +663,9 @@ async fn handle_message(
|
||||
(from.clone(), "dm")
|
||||
};
|
||||
if let Some(action_body) = strip_action(&body) {
|
||||
state.runtime.record_message_at(state, account_id, &buffer_name, kind, &from, action_body, true, "chat", None, None, false, None, Vec::new(), Vec::new(), None, sent_at);
|
||||
state.runtime.record_message_at(state, account_id, &buffer_name, kind, &from, action_body, true, "chat", None, None, false, None, Vec::new(), Vec::new(), None, sent_at, None);
|
||||
} else {
|
||||
state.runtime.record_message_at(state, account_id, &buffer_name, kind, &from, &body, false, "chat", None, None, false, None, Vec::new(), Vec::new(), None, sent_at);
|
||||
state.runtime.record_message_at(state, account_id, &buffer_name, kind, &from, &body, false, "chat", None, None, false, None, Vec::new(), Vec::new(), None, sent_at, None);
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -715,6 +715,10 @@ async fn handle_timeline_event(
|
||||
attachments,
|
||||
Some(sender.to_string()),
|
||||
sent_at,
|
||||
// The sender's own formatting, where they sent any. Passed on rather
|
||||
// than rendered here: it is somebody else's markup and the frontend
|
||||
// is where the sanitiser lives.
|
||||
protocol::formatted_body(&content).map(str::to_string),
|
||||
);
|
||||
}
|
||||
|
||||
@@ -1092,6 +1096,7 @@ pub async fn backfill(state: &AppState, account_id: &str, buffer_id: &str, limit
|
||||
if body.is_empty() {
|
||||
continue;
|
||||
}
|
||||
let html = protocol::formatted_body(content).map(str::to_string);
|
||||
let from = protocol::short_sender(event);
|
||||
let sender_mxid = protocol::sender(event);
|
||||
let is_own = sender_mxid == account.user_id;
|
||||
@@ -1102,7 +1107,7 @@ pub async fn backfill(state: &AppState, account_id: &str, buffer_id: &str, limit
|
||||
let avatar = state.runtime.get_matrix_member_avatar(account_id, sender_mxid);
|
||||
if let Err(e) = state.store.append_message(
|
||||
buffer_id, event_id, &from, &body, ts, is_action, false, "message", None, &[], is_own,
|
||||
avatar.as_deref(), &[], &[], Some(sender_mxid),
|
||||
avatar.as_deref(), &[], &[], Some(sender_mxid), html.as_deref(),
|
||||
) {
|
||||
tracing::warn!("matrix: storing history message: {e}");
|
||||
continue;
|
||||
|
||||
@@ -60,6 +60,19 @@ pub fn message_body(content: &Value) -> (String, bool) {
|
||||
/// both real message deletes and reaction removal (Matrix has no
|
||||
/// dedicated "remove reaction" event; un-reacting is redacting the
|
||||
/// `m.reaction` event you sent - see Runtime::take_matrix_reaction_target).
|
||||
/// The sender's formatted version of the body, if there is one.
|
||||
///
|
||||
/// Only `org.matrix.custom.html` counts: the spec allows other formats and a
|
||||
/// client that rendered an unknown one as HTML would be trusting markup it
|
||||
/// has no reason to believe is HTML at all. Still untrusted either way -
|
||||
/// this is somebody else's markup, and the frontend sanitises it.
|
||||
pub fn formatted_body(content: &Value) -> Option<&str> {
|
||||
if content["format"].as_str() != Some("org.matrix.custom.html") {
|
||||
return None;
|
||||
}
|
||||
content["formatted_body"].as_str().filter(|h| !h.is_empty())
|
||||
}
|
||||
|
||||
pub fn redaction_target(event: &Value) -> Option<&str> {
|
||||
event["redacts"].as_str().or_else(|| event["content"]["redacts"].as_str())
|
||||
}
|
||||
@@ -120,3 +133,25 @@ pub fn encrypted_media_file(content: &Value) -> Option<&Value> {
|
||||
}
|
||||
content["file"].is_object().then(|| &content["file"])
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::formatted_body;
|
||||
use serde_json::json;
|
||||
|
||||
/// Only Matrix's own HTML format counts. A client that rendered an
|
||||
/// unknown format as HTML would be trusting markup it has no reason to
|
||||
/// believe is HTML at all.
|
||||
#[test]
|
||||
fn only_the_html_format_is_treated_as_html() {
|
||||
let html = json!({ "format": "org.matrix.custom.html", "formatted_body": "<b>hi</b>", "body": "hi" });
|
||||
assert_eq!(formatted_body(&html), Some("<b>hi</b>"));
|
||||
|
||||
let other = json!({ "format": "org.example.markdown", "formatted_body": "**hi**", "body": "hi" });
|
||||
assert_eq!(formatted_body(&other), None);
|
||||
|
||||
// A plain message, and an empty formatted body, both fall back.
|
||||
assert_eq!(formatted_body(&json!({ "body": "hi" })), None);
|
||||
assert_eq!(formatted_body(&json!({ "format": "org.matrix.custom.html", "formatted_body": "" })), None);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -335,6 +335,14 @@ pub struct Message {
|
||||
/// showing.
|
||||
#[serde(rename = "senderId", skip_serializing_if = "Option::is_none")]
|
||||
pub sender_id: Option<String>,
|
||||
/// The sender's own formatted version of `body`, where the protocol
|
||||
/// carries one (Matrix's `formatted_body`). Restricted HTML, and still
|
||||
/// untrusted - a frontend must put it through the same sanitiser it
|
||||
/// uses for everything else rather than treating it as safe because it
|
||||
/// arrived structured. `body` stays the plain-text fallback, and is
|
||||
/// what search reads.
|
||||
#[serde(skip_serializing_if = "Option::is_none")]
|
||||
pub html: Option<String>,
|
||||
}
|
||||
|
||||
pub fn buffer_id(account_id: &str, name: &str) -> String {
|
||||
|
||||
+4
-2
@@ -1570,7 +1570,7 @@ impl Runtime {
|
||||
) {
|
||||
self.record_message_at(
|
||||
state, account_id, buffer_name, buffer_kind, from, body, is_action, kind, reply_to,
|
||||
msg_id_override, force_highlight, avatar_url, embeds, attachments, sender_id, None,
|
||||
msg_id_override, force_highlight, avatar_url, embeds, attachments, sender_id, None, None,
|
||||
)
|
||||
}
|
||||
|
||||
@@ -1600,6 +1600,7 @@ impl Runtime {
|
||||
attachments: Vec<Attachment>,
|
||||
sender_id: Option<String>,
|
||||
sent_at: Option<i64>,
|
||||
html: Option<String>,
|
||||
) {
|
||||
let buffer = self.ensure_buffer(state, account_id, buffer_name, buffer_kind);
|
||||
let own_nick = self
|
||||
@@ -1634,7 +1635,7 @@ impl Runtime {
|
||||
// passes its real id through here for exactly that reason.
|
||||
let msg_id = msg_id_override.unwrap_or_else(model::next_message_id);
|
||||
|
||||
if let Err(e) = state.store.append_message(&buffer.id, &msg_id, from, body, ts, is_action, is_highlight, kind, reply_to.as_ref(), &[], is_own, avatar_url.as_deref(), &embeds, &attachments, sender_id.as_deref()) {
|
||||
if let Err(e) = state.store.append_message(&buffer.id, &msg_id, from, body, ts, is_action, is_highlight, kind, reply_to.as_ref(), &[], is_own, avatar_url.as_deref(), &embeds, &attachments, sender_id.as_deref(), html.as_deref()) {
|
||||
tracing::warn!("failed to persist message: {e}");
|
||||
}
|
||||
|
||||
@@ -1657,6 +1658,7 @@ impl Runtime {
|
||||
}
|
||||
|
||||
let message = Message {
|
||||
html: html.clone(),
|
||||
id: msg_id,
|
||||
buffer_id: buffer.id.clone(),
|
||||
from: from.to_string(),
|
||||
|
||||
+17
-9
@@ -50,6 +50,10 @@ impl Store {
|
||||
"ALTER TABLE messages ADD COLUMN embeds TEXT NOT NULL DEFAULT '[]'",
|
||||
"ALTER TABLE messages ADD COLUMN attachments TEXT NOT NULL DEFAULT '[]'",
|
||||
"ALTER TABLE messages ADD COLUMN sender_id TEXT",
|
||||
// Matrix sends a formatted body alongside the plain one. Kept
|
||||
// separate rather than replacing body: the plain text is the
|
||||
// fallback every other protocol uses and the one search reads.
|
||||
"ALTER TABLE messages ADD COLUMN html TEXT",
|
||||
] {
|
||||
let _ = conn.execute(stmt, []);
|
||||
}
|
||||
@@ -102,6 +106,7 @@ impl Store {
|
||||
embeds: &[Embed],
|
||||
attachments: &[Attachment],
|
||||
sender_id: Option<&str>,
|
||||
html: Option<&str>,
|
||||
) -> Result<()> {
|
||||
let conn = self.conn.lock().unwrap();
|
||||
// Live messages are always freshly created with no reactions yet
|
||||
@@ -115,8 +120,8 @@ impl Store {
|
||||
// OR IGNORE against the (buffer_id, msg_id) index: recording the
|
||||
// same message twice is a backend replaying history it already
|
||||
// has, and the right answer is to keep the copy already stored.
|
||||
"INSERT OR IGNORE INTO messages (msg_id, buffer_id, from_nick, body, ts, is_action, is_highlight, kind, reply_to_id, reply_to_from, reply_to_body, reactions, is_own, avatar_url, embeds, sender_id, attachments)
|
||||
VALUES (?1, ?2, ?3, ?4, ?5, ?6, ?7, ?8, ?9, ?10, ?11, COALESCE(?12, '[]'), ?13, ?14, COALESCE(?15, '[]'), ?16, COALESCE(?17, '[]'))",
|
||||
"INSERT OR IGNORE INTO messages (msg_id, buffer_id, from_nick, body, ts, is_action, is_highlight, kind, reply_to_id, reply_to_from, reply_to_body, reactions, is_own, avatar_url, embeds, sender_id, attachments, html)
|
||||
VALUES (?1, ?2, ?3, ?4, ?5, ?6, ?7, ?8, ?9, ?10, ?11, COALESCE(?12, '[]'), ?13, ?14, COALESCE(?15, '[]'), ?16, COALESCE(?17, '[]'), ?18)",
|
||||
params![
|
||||
msg_id,
|
||||
buffer_id,
|
||||
@@ -135,6 +140,7 @@ impl Store {
|
||||
embeds_json,
|
||||
sender_id,
|
||||
attachments_json,
|
||||
html,
|
||||
],
|
||||
)?;
|
||||
Ok(())
|
||||
@@ -339,6 +345,7 @@ impl Store {
|
||||
embeds,
|
||||
attachments,
|
||||
sender_id: row.get(15)?,
|
||||
html: row.get(17)?,
|
||||
})
|
||||
}
|
||||
|
||||
@@ -346,7 +353,7 @@ impl Store {
|
||||
let conn = self.conn.lock().unwrap();
|
||||
let limit = if limit > 0 { limit } else { 200 };
|
||||
let mut stmt = conn.prepare(
|
||||
"SELECT msg_id, from_nick, body, ts, is_action, is_highlight, kind, reply_to_id, reply_to_from, reply_to_body, edited, reactions, is_own, avatar_url, embeds, sender_id, attachments
|
||||
"SELECT msg_id, from_nick, body, ts, is_action, is_highlight, kind, reply_to_id, reply_to_from, reply_to_body, edited, reactions, is_own, avatar_url, embeds, sender_id, attachments, html
|
||||
FROM messages
|
||||
WHERE buffer_id = ?1 AND (?2 <= 0 OR ts < ?2)
|
||||
ORDER BY ts DESC LIMIT ?3",
|
||||
@@ -376,7 +383,7 @@ impl Store {
|
||||
let limit = if limit > 0 { limit } else { 100 };
|
||||
let places = std::iter::repeat("?").take(buffer_ids.len()).collect::<Vec<_>>().join(",");
|
||||
let sql = format!(
|
||||
"SELECT msg_id, from_nick, body, ts, is_action, is_highlight, kind, reply_to_id, reply_to_from, reply_to_body, edited, reactions, is_own, avatar_url, embeds, sender_id, attachments, buffer_id
|
||||
"SELECT msg_id, from_nick, body, ts, is_action, is_highlight, kind, reply_to_id, reply_to_from, reply_to_body, edited, reactions, is_own, avatar_url, embeds, sender_id, attachments, html, buffer_id
|
||||
FROM messages
|
||||
WHERE is_highlight = 1 AND is_own = 0 AND buffer_id IN ({places})
|
||||
ORDER BY ts DESC LIMIT ?"
|
||||
@@ -411,7 +418,7 @@ impl Store {
|
||||
let escaped = query.replace('\\', "\\\\").replace('%', "\\%").replace('_', "\\_");
|
||||
let pattern = format!("%{escaped}%");
|
||||
let mut stmt = conn.prepare(
|
||||
"SELECT msg_id, from_nick, body, ts, is_action, is_highlight, kind, reply_to_id, reply_to_from, reply_to_body, edited, reactions, is_own, avatar_url, embeds, sender_id, attachments
|
||||
"SELECT msg_id, from_nick, body, ts, is_action, is_highlight, kind, reply_to_id, reply_to_from, reply_to_body, edited, reactions, is_own, avatar_url, embeds, sender_id, attachments, html
|
||||
FROM messages
|
||||
WHERE buffer_id = ?1 AND body LIKE ?2 ESCAPE '\\'
|
||||
ORDER BY ts DESC LIMIT ?3",
|
||||
@@ -455,7 +462,7 @@ impl Store {
|
||||
pub fn get_message(&self, buffer_id: &str, msg_id: &str) -> Result<Option<Message>> {
|
||||
let conn = self.conn.lock().unwrap();
|
||||
let mut stmt = conn.prepare(
|
||||
"SELECT msg_id, from_nick, body, ts, is_action, is_highlight, kind, reply_to_id, reply_to_from, reply_to_body, edited, reactions, is_own, avatar_url, embeds, sender_id, attachments
|
||||
"SELECT msg_id, from_nick, body, ts, is_action, is_highlight, kind, reply_to_id, reply_to_from, reply_to_body, edited, reactions, is_own, avatar_url, embeds, sender_id, attachments, html
|
||||
FROM messages
|
||||
WHERE buffer_id = ?1 AND msg_id = ?2",
|
||||
)?;
|
||||
@@ -492,6 +499,7 @@ impl Store {
|
||||
embeds,
|
||||
attachments,
|
||||
sender_id: row.get(15)?,
|
||||
html: row.get(17)?,
|
||||
})
|
||||
})?;
|
||||
match rows.next() {
|
||||
@@ -552,13 +560,13 @@ mod tests {
|
||||
}
|
||||
|
||||
fn append(s: &Store, buffer: &str, id: &str) {
|
||||
s.append_message(buffer, id, "someone", "hi", 1, false, false, "chat", None, &[], false, None, &[], &[], None)
|
||||
s.append_message(buffer, id, "someone", "hi", 1, false, false, "chat", None, &[], false, None, &[], &[], None, None)
|
||||
.expect("appending");
|
||||
}
|
||||
|
||||
/// Like `append`, but with text worth searching for.
|
||||
fn append_saying(s: &Store, buffer: &str, id: &str, body: &str) {
|
||||
s.append_message(buffer, id, "someone", body, 1, false, false, "chat", None, &[], false, None, &[], &[], None)
|
||||
s.append_message(buffer, id, "someone", body, 1, false, false, "chat", None, &[], false, None, &[], &[], None, None)
|
||||
.expect("appending");
|
||||
}
|
||||
|
||||
@@ -680,7 +688,7 @@ mod dedupe_tests {
|
||||
}
|
||||
|
||||
fn put(s: &Store, buffer: &str, id: &str, body: &str) {
|
||||
let _ = s.append_message(buffer, id, "nick", body, 1, false, false, "chat", None, &[], false, None, &[], &[], None);
|
||||
let _ = s.append_message(buffer, id, "nick", body, 1, false, false, "chat", None, &[], false, None, &[], &[], None, None);
|
||||
}
|
||||
|
||||
/// A backend replaying history it already has - Sneedchat does this on
|
||||
|
||||
Reference in New Issue
Block a user