diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 93ce0367..d942fc58 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -76,9 +76,6 @@ jobs: - name: Check All Features cmd: msrv-all - - name: Check Client - cmd: msrv-client - - name: Check Ruma cmd: msrv-ruma diff --git a/crates/ruma-events/src/room/message/reply.rs b/crates/ruma-events/src/room/message/reply.rs index 0f08e034..e9ec0dcc 100644 --- a/crates/ruma-events/src/room/message/reply.rs +++ b/crates/ruma-events/src/room/message/reply.rs @@ -100,7 +100,7 @@ impl fmt::Display for FormattedOrPlainBody<'_> { if let Some(formatted_body) = self.formatted { #[cfg(feature = "html")] if self.is_reply { - let mut html = Html::parse(&formatted_body.body); + let html = Html::parse(&formatted_body.body); html.sanitize(); write!(f, "{html}") diff --git a/crates/ruma-html/CHANGELOG.md b/crates/ruma-html/CHANGELOG.md index 3a1ea611..454e822f 100644 --- a/crates/ruma-html/CHANGELOG.md +++ b/crates/ruma-html/CHANGELOG.md @@ -4,6 +4,12 @@ Breaking Changes: - `MatrixElement::Div` is now a newtype variant. - `AnchorData`'s `name` field was removed, according to MSC4159. +- html5ever was bumped to a new major version. A breaking change in the parsing + API required us to rewrite the `Html` type. + - `Html::sanitize()` and `Html::sanitize_with()` take a non-mutable reference. + - `NodeRef` and `Children` are now owned types and no longer implement `Copy`. + - `NodeData::Text`'s inner string and the `attrs` field of `ElementData` are + now wrapped in `RefCell`s. Improvements: diff --git a/crates/ruma-html/Cargo.toml b/crates/ruma-html/Cargo.toml index 1883a5f8..ad044eec 100644 --- a/crates/ruma-html/Cargo.toml +++ b/crates/ruma-html/Cargo.toml @@ -18,7 +18,7 @@ matrix = ["dep:ruma-common"] [dependencies] as_variant = { workspace = true } -html5ever = "0.27.0" +html5ever = "0.28.0" phf = { version = "0.11.1", features = ["macros"] } ruma-common = { workspace = true, optional = true } tracing = { workspace = true, features = ["attributes"] } diff --git a/crates/ruma-html/src/helpers.rs b/crates/ruma-html/src/helpers.rs index 0912e479..b56b6987 100644 --- a/crates/ruma-html/src/helpers.rs +++ b/crates/ruma-html/src/helpers.rs @@ -52,7 +52,7 @@ pub fn remove_html_reply_fallback(s: &str) -> String { } fn sanitize_inner(s: &str, config: &SanitizerConfig) -> String { - let mut html = Html::parse(s); + let html = Html::parse(s); html.sanitize_with(config); html.to_string() } diff --git a/crates/ruma-html/src/html.rs b/crates/ruma-html/src/html.rs index f090060c..a0f315d0 100644 --- a/crates/ruma-html/src/html.rs +++ b/crates/ruma-html/src/html.rs @@ -1,4 +1,10 @@ -use std::{collections::BTreeSet, fmt, io, iter::FusedIterator}; +use std::{ + cell::RefCell, + collections::BTreeSet, + fmt, io, + iter::FusedIterator, + rc::{Rc, Weak}, +}; use as_variant::as_variant; use html5ever::{ @@ -6,7 +12,7 @@ use html5ever::{ serialize::{serialize, Serialize, SerializeOpts, Serializer, TraversalScope}, tendril::{StrTendril, TendrilSink}, tree_builder::{NodeOrText, TreeSink}, - Attribute, ParseOpts, QualName, + Attribute, LocalName, ParseOpts, QualName, }; use tracing::debug; @@ -21,7 +27,7 @@ use crate::SanitizerConfig; /// parsed, note that malformed HTML and comments will be stripped from the output. #[derive(Debug)] pub struct Html { - pub(crate) nodes: Vec, + document: NodeRef, } impl Html { @@ -45,179 +51,116 @@ impl Html { /// /// This is equivalent to calling [`Self::sanitize_with()`] with a `config` value of /// `SanitizerConfig::compat().remove_reply_fallback()`. - pub fn sanitize(&mut self) { + pub fn sanitize(&self) { let config = SanitizerConfig::compat().remove_reply_fallback(); self.sanitize_with(&config); } /// Sanitize this HTML according to the given configuration. - pub fn sanitize_with(&mut self, config: &SanitizerConfig) { + pub fn sanitize_with(&self, config: &SanitizerConfig) { config.clean(self); } - /// Construct a new `Node` with the given data and add it to this `Html`. - /// - /// Returns the index of the new node. - pub(crate) fn new_node(&mut self, data: NodeData) -> usize { - self.nodes.push(Node::new(data)); - self.nodes.len() - 1 - } - - /// Append the given node to the given parent in this `Html`. - /// - /// The node is detached from its previous position. - pub(crate) fn append_node(&mut self, parent_id: usize, node_id: usize) { - self.detach(node_id); - - self.nodes[node_id].parent = Some(parent_id); - if let Some(last_child) = self.nodes[parent_id].last_child.take() { - self.nodes[node_id].prev_sibling = Some(last_child); - self.nodes[last_child].next_sibling = Some(node_id); - } else { - self.nodes[parent_id].first_child = Some(node_id); - } - self.nodes[parent_id].last_child = Some(node_id); - } - - /// Insert the given node before the given sibling in this `Html`. - /// - /// The node is detached from its previous position. - pub(crate) fn insert_before(&mut self, sibling_id: usize, node_id: usize) { - self.detach(node_id); - - self.nodes[node_id].parent = self.nodes[sibling_id].parent; - self.nodes[node_id].next_sibling = Some(sibling_id); - if let Some(prev_sibling) = self.nodes[sibling_id].prev_sibling.take() { - self.nodes[node_id].prev_sibling = Some(prev_sibling); - self.nodes[prev_sibling].next_sibling = Some(node_id); - } else if let Some(parent) = self.nodes[sibling_id].parent { - self.nodes[parent].first_child = Some(node_id); - } - self.nodes[sibling_id].prev_sibling = Some(node_id); - } - - /// Detach the given node from this `Html`. - pub(crate) fn detach(&mut self, node_id: usize) { - let (parent, prev_sibling, next_sibling) = { - let node = &mut self.nodes[node_id]; - (node.parent.take(), node.prev_sibling.take(), node.next_sibling.take()) - }; - - if let Some(next_sibling) = next_sibling { - self.nodes[next_sibling].prev_sibling = prev_sibling; - } else if let Some(parent) = parent { - self.nodes[parent].last_child = prev_sibling; - } - - if let Some(prev_sibling) = prev_sibling { - self.nodes[prev_sibling].next_sibling = next_sibling; - } else if let Some(parent) = parent { - self.nodes[parent].first_child = next_sibling; - } - } - - /// Get the ID of the root node of the HTML. - pub(crate) fn root_id(&self) -> usize { - self.nodes[0].first_child.expect("html should always have a root node") - } - /// Get the root node of the HTML. - pub(crate) fn root(&self) -> &Node { - &self.nodes[self.root_id()] + fn root(&self) -> NodeRef { + self.document.first_child().expect("html should always have a root node") } /// Whether the root node of the HTML has children. pub fn has_children(&self) -> bool { - self.root().first_child.is_some() + self.root().has_children() } /// The first child node of the root node of the HTML. /// /// Returns `None` if the root node has no children. - pub fn first_child(&self) -> Option> { - self.root().first_child.map(|id| NodeRef::new(self, id)) + pub fn first_child(&self) -> Option { + self.root().first_child() } /// The last child node of the root node of the HTML . /// /// Returns `None` if the root node has no children. - pub fn last_child(&self) -> Option> { - self.root().last_child.map(|id| NodeRef::new(self, id)) + pub fn last_child(&self) -> Option { + self.root().last_child() } /// Iterate through the children of the root node of the HTML. - pub fn children(&self) -> Children<'_> { + pub fn children(&self) -> Children { Children::new(self.first_child()) } } impl Default for Html { fn default() -> Self { - Self { nodes: vec![Node::new(NodeData::Document)] } + Self { document: NodeRef::new(NodeData::Document) } } } impl TreeSink for Html { - type Handle = usize; + type Handle = NodeRef; type Output = Self; fn finish(self) -> Self::Output { self } - fn parse_error(&mut self, msg: std::borrow::Cow<'static, str>) { + fn parse_error(&self, msg: std::borrow::Cow<'static, str>) { debug!("HTML parse error: {msg}"); } - fn get_document(&mut self) -> Self::Handle { - 0 + fn get_document(&self) -> Self::Handle { + self.document.clone() } fn elem_name<'a>(&'a self, target: &'a Self::Handle) -> html5ever::ExpandedName<'a> { - self.nodes[*target].as_element().expect("not an element").name.expanded() + target.as_element().expect("not an element").name.expanded() } fn create_element( - &mut self, + &self, name: QualName, attrs: Vec, _flags: html5ever::tree_builder::ElementFlags, ) -> Self::Handle { - self.new_node(NodeData::Element(ElementData { name, attrs: attrs.into_iter().collect() })) + NodeRef::new(NodeData::Element(ElementData { + name, + attrs: RefCell::new(attrs.into_iter().collect()), + })) } - fn create_comment(&mut self, _text: StrTendril) -> Self::Handle { - self.new_node(NodeData::Other) + fn create_comment(&self, _text: StrTendril) -> Self::Handle { + NodeRef::new(NodeData::Other) } - fn create_pi(&mut self, _target: StrTendril, _data: StrTendril) -> Self::Handle { - self.new_node(NodeData::Other) + fn create_pi(&self, _target: StrTendril, _data: StrTendril) -> Self::Handle { + NodeRef::new(NodeData::Other) } - fn append(&mut self, parent: &Self::Handle, child: NodeOrText) { + fn append(&self, parent: &Self::Handle, child: NodeOrText) { match child { - NodeOrText::AppendNode(index) => self.append_node(*parent, index), + NodeOrText::AppendNode(node) => parent.append_child(node), NodeOrText::AppendText(text) => { // If the previous sibling is also text, add this text to it. - if let Some(sibling) = - self.nodes[*parent].last_child.and_then(|child| self.nodes[child].as_text_mut()) + if let Some(prev_text) = + parent.last_child().as_ref().and_then(|sibling| sibling.as_text()) { - sibling.push_tendril(&text); + prev_text.borrow_mut().push_tendril(&text); } else { - let index = self.new_node(NodeData::Text(text)); - self.append_node(*parent, index); + let node = NodeRef::new(NodeData::Text(text.into())); + parent.append_child(node); } } } } fn append_based_on_parent_node( - &mut self, + &self, element: &Self::Handle, prev_element: &Self::Handle, child: NodeOrText, ) { - if self.nodes[*element].parent.is_some() { + if element.0.parent.borrow().is_some() { self.append_before_sibling(element, child); } else { self.append(prev_element, child); @@ -225,59 +168,53 @@ impl TreeSink for Html { } fn append_doctype_to_document( - &mut self, + &self, _name: StrTendril, _public_id: StrTendril, _system_id: StrTendril, ) { } - fn get_template_contents(&mut self, target: &Self::Handle) -> Self::Handle { - *target + fn get_template_contents(&self, target: &Self::Handle) -> Self::Handle { + target.clone() } fn same_node(&self, x: &Self::Handle, y: &Self::Handle) -> bool { - x == y + Rc::ptr_eq(&x.0, &y.0) } - fn set_quirks_mode(&mut self, _mode: html5ever::tree_builder::QuirksMode) {} + fn set_quirks_mode(&self, _mode: html5ever::tree_builder::QuirksMode) {} - fn append_before_sibling( - &mut self, - sibling: &Self::Handle, - new_node: NodeOrText, - ) { + fn append_before_sibling(&self, sibling: &Self::Handle, new_node: NodeOrText) { match new_node { - NodeOrText::AppendNode(index) => self.insert_before(*sibling, index), + NodeOrText::AppendNode(node) => node.insert_before_sibling(sibling), NodeOrText::AppendText(text) => { // If the previous sibling is also text, add this text to it. - if let Some(prev_text) = self.nodes[*sibling] - .prev_sibling - .and_then(|prev| self.nodes[prev].as_text_mut()) + if let Some(prev_text) = + sibling.prev_sibling().as_ref().and_then(|prev_sibling| prev_sibling.as_text()) { - prev_text.push_tendril(&text); + prev_text.borrow_mut().push_tendril(&text); } else { - let index = self.new_node(NodeData::Text(text)); - self.insert_before(*sibling, index); + let node = NodeRef::new(NodeData::Text(text.into())); + node.insert_before_sibling(sibling); } } } } - fn add_attrs_if_missing(&mut self, target: &Self::Handle, attrs: Vec) { - let target = self.nodes[*target].as_element_mut().unwrap(); - target.attrs.extend(attrs); + fn add_attrs_if_missing(&self, target: &Self::Handle, attrs: Vec) { + let element = target.as_element().unwrap(); + element.attrs.borrow_mut().extend(attrs); } - fn remove_from_parent(&mut self, target: &Self::Handle) { - self.detach(*target); + fn remove_from_parent(&self, target: &Self::Handle) { + target.detach(); } - fn reparent_children(&mut self, node: &Self::Handle, new_parent: &Self::Handle) { - let mut next_child = self.nodes[*node].first_child; - while let Some(child) = next_child { - next_child = self.nodes[child].next_sibling; - self.append_node(*new_parent, child); + fn reparent_children(&self, node: &Self::Handle, new_parent: &Self::Handle) { + for child in node.0.children.take() { + child.0.parent.take(); + new_parent.append_child(child); } } } @@ -289,13 +226,8 @@ impl Serialize for Html { { match traversal_scope { TraversalScope::IncludeNode => { - let root = self.root(); - - let mut next_child = root.first_child; - while let Some(child) = next_child { - let child = &self.nodes[child]; - child.serialize(self, serializer)?; - next_child = child.next_sibling; + for child in self.children() { + child.serialize(serializer)?; } Ok(()) @@ -324,85 +256,37 @@ impl fmt::Display for Html { /// An HTML node. #[derive(Debug)] #[non_exhaustive] -pub(crate) struct Node { - pub(crate) parent: Option, - pub(crate) prev_sibling: Option, - pub(crate) next_sibling: Option, - pub(crate) first_child: Option, - pub(crate) last_child: Option, - pub(crate) data: NodeData, +struct Node { + parent: RefCell>>, + children: RefCell>, + data: NodeData, } impl Node { - /// Constructs a new `Node` with the given data. + /// Constructs a new `NodeRef` with the given data. fn new(data: NodeData) -> Self { - Self { - parent: None, - prev_sibling: None, - next_sibling: None, - first_child: None, - last_child: None, - data, - } + Self { parent: Default::default(), children: Default::default(), data } } /// Returns the data of this `Node` if it is an Element (aka an HTML tag). - pub(crate) fn as_element(&self) -> Option<&ElementData> { + fn as_element(&self) -> Option<&ElementData> { as_variant!(&self.data, NodeData::Element) } - /// Returns the mutable `ElementData` of this `Node` if it is a `NodeData::Element`. - pub(crate) fn as_element_mut(&mut self) -> Option<&mut ElementData> { - as_variant!(&mut self.data, NodeData::Element) - } - /// Returns the text content of this `Node`, if it is a `NodeData::Text`. - fn as_text(&self) -> Option<&StrTendril> { + fn as_text(&self) -> Option<&RefCell> { as_variant!(&self.data, NodeData::Text) } - /// Returns the mutable text content of this `Node`, if it is a `NodeData::Text`. - fn as_text_mut(&mut self) -> Option<&mut StrTendril> { - as_variant!(&mut self.data, NodeData::Text) + /// Whether this is the root node of the HTML document. + fn is_root(&self) -> bool { + // The root node is the `html` element. + matches!(&self.data, NodeData::Element(element_data) if element_data.name.local.as_bytes() == b"html") } -} -impl Node { - pub(crate) fn serialize(&self, fragment: &Html, serializer: &mut S) -> io::Result<()> - where - S: Serializer, - { - match &self.data { - NodeData::Element(data) => { - serializer.start_elem( - data.name.clone(), - data.attrs.iter().map(|attr| (&attr.name, &*attr.value)), - )?; - - let mut next_child = self.first_child; - while let Some(child) = next_child { - let child = &fragment.nodes[child]; - child.serialize(fragment, serializer)?; - next_child = child.next_sibling; - } - - serializer.end_elem(data.name.clone())?; - - Ok(()) - } - NodeData::Document => { - let mut next_child = self.first_child; - while let Some(child) = next_child { - let child = &fragment.nodes[child]; - child.serialize(fragment, serializer)?; - next_child = child.next_sibling; - } - - Ok(()) - } - NodeData::Text(text) => serializer.write_text(text), - _ => Ok(()), - } + /// The parent of this node, if any. + fn parent(&self) -> Option { + self.parent.borrow().as_ref()?.upgrade().map(NodeRef) } } @@ -414,7 +298,7 @@ pub enum NodeData { Document, /// A text node. - Text(StrTendril), + Text(RefCell), /// An HTML element (aka a tag). Element(ElementData), @@ -431,7 +315,7 @@ pub struct ElementData { pub name: QualName, /// The attributes of the element. - pub attrs: BTreeSet, + pub attrs: RefCell>, } impl ElementData { @@ -440,126 +324,215 @@ impl ElementData { /// [spec]: https://spec.matrix.org/latest/client-server-api/#mroommessage-msgtypes #[cfg(feature = "matrix")] pub fn to_matrix(&self) -> matrix::MatrixElementData { - matrix::MatrixElementData::parse(&self.name, &self.attrs) + matrix::MatrixElementData::parse(&self.name, &self.attrs.borrow()) } } /// A reference to an HTML node. -#[derive(Debug, Clone, Copy)] +#[derive(Debug, Clone)] #[non_exhaustive] -pub struct NodeRef<'a> { - /// The `Html` struct containing the nodes. - pub(crate) html: &'a Html, - /// The referenced node. - pub(crate) node: &'a Node, -} +pub struct NodeRef(Rc); -impl<'a> NodeRef<'a> { - /// Construct a new `NodeRef` for the given HTML and node ID. - fn new(html: &'a Html, id: usize) -> Self { - Self { html, node: &html.nodes[id] } +impl NodeRef { + /// Constructs a new `NodeRef` with the given data. + fn new(data: NodeData) -> Self { + Self(Node::new(data).into()) } - /// Construct a new `NodeRef` from the same HTML as this node with the given node ID. - fn with_id(&self, id: usize) -> Self { - let html = self.html; - Self::new(html, id) + /// Detach this node from the tree, if it has a parent. + pub(crate) fn detach(&self) { + if let Some((parent, index)) = self.parent_and_index() { + parent.0.children.borrow_mut().remove(index); + self.0.parent.take(); + } + } + + /// Append the given child node to this node. + /// + /// The child node is detached from its previous position. + fn append_child(&self, child: NodeRef) { + child.detach(); + + child.0.parent.replace(Some(Rc::downgrade(&self.0))); + self.0.children.borrow_mut().push(child); + } + + /// If this node has a parent, get it and the node's position in the parent's children. + fn parent_and_index(&self) -> Option<(NodeRef, usize)> { + let parent = self.0.parent()?; + let i = parent + .0 + .children + .borrow() + .iter() + .position(|child| Rc::ptr_eq(&child.0, &self.0)) + .expect("child should be in parent's children"); + Some((parent, i)) + } + + /// Insert this node before the given sibling. + /// + /// This node is detached from its previous position. + pub(crate) fn insert_before_sibling(&self, sibling: &NodeRef) { + self.detach(); + + let (parent, index) = sibling.parent_and_index().expect("sibling should have parent"); + + self.0.parent.replace(Some(Rc::downgrade(&parent.0))); + parent.0.children.borrow_mut().insert(index, self.clone()); + } + + /// Constructs a new element `NodeRef` with the same data as this one, but with a different + /// element name and use it to replace this one in the parent. + /// + /// Panics if this node is not in the tree and is not an element node. + pub(crate) fn replace_with_element_name(self, name: LocalName) -> NodeRef { + let mut element_data = self.as_element().unwrap().clone(); + element_data.name.local = name; + + let new_node = NodeRef::new(NodeData::Element(element_data)); + + for child in self.children() { + new_node.append_child(child); + } + + new_node.insert_before_sibling(&self); + self.detach(); + + new_node } /// The data of the node. - pub fn data(&self) -> &'a NodeData { - &self.node.data + pub fn data(&self) -> &NodeData { + &self.0.data } - /// Returns the data of this node if it is a `NodeData::Element`. - pub fn as_element(&self) -> Option<&'a ElementData> { - self.node.as_element() + /// Returns the data of this `Node` if it is an Element (aka an HTML tag). + pub fn as_element(&self) -> Option<&ElementData> { + self.0.as_element() } - /// Returns the text content of this node, if it is a `NodeData::Text`. - pub fn as_text(&self) -> Option<&'a StrTendril> { - self.node.as_text() + /// Returns the text content of this `Node`, if it is a `NodeData::Text`. + pub fn as_text(&self) -> Option<&RefCell> { + self.0.as_text() } /// The parent node of this node. /// /// Returns `None` if the parent is the root node. - pub fn parent(&self) -> Option> { - let parent_id = self.node.parent?; + pub fn parent(&self) -> Option { + let parent = self.0.parent()?; // We don't want users to be able to navigate to the root. - if parent_id == self.html.root_id() { + if parent.0.is_root() { return None; } - Some(self.with_id(parent_id)) + Some(parent) } /// The next sibling node of this node. /// /// Returns `None` if this is the last of its siblings. - pub fn next_sibling(&self) -> Option> { - Some(self.with_id(self.node.next_sibling?)) + pub fn next_sibling(&self) -> Option { + let (parent, index) = self.parent_and_index()?; + let index = index.checked_add(1)?; + let sibling = parent.0.children.borrow().get(index).cloned(); + sibling } /// The previous sibling node of this node. /// /// Returns `None` if this is the first of its siblings. - pub fn prev_sibling(&self) -> Option> { - Some(self.with_id(self.node.prev_sibling?)) + pub fn prev_sibling(&self) -> Option { + let (parent, index) = self.parent_and_index()?; + let index = index.checked_sub(1)?; + let sibling = parent.0.children.borrow().get(index).cloned(); + sibling } /// Whether this node has children. pub fn has_children(&self) -> bool { - self.node.first_child.is_some() + !self.0.children.borrow().is_empty() } /// The first child node of this node. /// /// Returns `None` if this node has no children. - pub fn first_child(&self) -> Option> { - Some(self.with_id(self.node.first_child?)) + pub fn first_child(&self) -> Option { + self.0.children.borrow().first().cloned() } /// The last child node of this node. /// /// Returns `None` if this node has no children. - pub fn last_child(&self) -> Option> { - Some(self.with_id(self.node.last_child?)) + pub fn last_child(&self) -> Option { + self.0.children.borrow().last().cloned() } /// Get an iterator through the children of this node. - pub fn children(&self) -> Children<'a> { + pub fn children(&self) -> Children { Children::new(self.first_child()) } + + pub(crate) fn serialize(&self, serializer: &mut S) -> io::Result<()> + where + S: Serializer, + { + match self.data() { + NodeData::Element(data) => { + serializer.start_elem( + data.name.clone(), + data.attrs.borrow().iter().map(|attr| (&attr.name, &*attr.value)), + )?; + + for child in self.children() { + child.serialize(serializer)?; + } + + serializer.end_elem(data.name.clone())?; + + Ok(()) + } + NodeData::Document => { + for child in self.children() { + child.serialize(serializer)?; + } + + Ok(()) + } + NodeData::Text(text) => serializer.write_text(&text.borrow()), + _ => Ok(()), + } + } } /// An iterator through the children of a node. /// /// Can be constructed with [`Html::children()`] or [`NodeRef::children()`]. -#[derive(Debug, Clone, Copy)] -pub struct Children<'a> { - next: Option>, +#[derive(Debug, Clone)] +pub struct Children { + next: Option, } -impl<'a> Children<'a> { +impl Children { /// Construct a `Children` starting from the given node. - fn new(start_node: Option>) -> Self { + fn new(start_node: Option) -> Self { Self { next: start_node } } } -impl<'a> Iterator for Children<'a> { - type Item = NodeRef<'a>; +impl Iterator for Children { + type Item = NodeRef; fn next(&mut self) -> Option { - let next = self.next?; + let next = self.next.take()?; self.next = next.next_sibling(); Some(next) } } -impl<'a> FusedIterator for Children<'a> {} +impl FusedIterator for Children {} #[cfg(test)] mod tests { diff --git a/crates/ruma-html/src/sanitizer_config/clean.rs b/crates/ruma-html/src/sanitizer_config/clean.rs index 4d0663a3..0b7da0d0 100644 --- a/crates/ruma-html/src/sanitizer_config/clean.rs +++ b/crates/ruma-html/src/sanitizer_config/clean.rs @@ -2,7 +2,7 @@ use html5ever::{tendril::StrTendril, Attribute, LocalName}; use phf::{phf_map, phf_set, Map, Set}; use wildmatch::WildMatch; -use crate::{ElementData, Html, HtmlSanitizerMode, NodeData, SanitizerConfig}; +use crate::{ElementData, Html, HtmlSanitizerMode, NodeData, NodeRef, SanitizerConfig}; /// HTML elements allowed in the Matrix specification. static ALLOWED_ELEMENTS_STRICT: Set<&str> = phf_set! { @@ -104,43 +104,41 @@ impl SanitizerConfig { } /// Clean the given HTML with this sanitizer. - pub(crate) fn clean(&self, html: &mut Html) { - let root = html.root(); - let mut next_child = root.first_child; - - while let Some(child) = next_child { - next_child = html.nodes[child].next_sibling; - self.clean_node(html, child, 0); + pub(crate) fn clean(&self, html: &Html) { + for child in html.children() { + self.clean_node(child, 0); } } - fn clean_node(&self, html: &mut Html, node_id: usize, depth: u32) { - self.apply_replacements(html, node_id); + fn clean_node(&self, node: NodeRef, depth: u32) { + let node = self.apply_replacements(node); - let action = self.node_action(html, node_id, depth); + let action = self.node_action(&node, depth); if action != NodeAction::Remove { - let mut next_child = html.nodes[node_id].first_child; - while let Some(child) = next_child { - next_child = html.nodes[child].next_sibling; - + for child in node.children() { if action == NodeAction::Ignore { - html.insert_before(node_id, child); + child.insert_before_sibling(&node); } - self.clean_node(html, child, depth + 1); + self.clean_node(child, depth + 1); } } if matches!(action, NodeAction::Ignore | NodeAction::Remove) { - html.detach(node_id); - } else if let Some(data) = html.nodes[node_id].as_element_mut() { + node.detach(); + } else if let Some(data) = node.as_element() { self.clean_element_attributes(data); } } - fn apply_replacements(&self, html: &mut Html, node_id: usize) { - if let NodeData::Element(ElementData { name, attrs, .. }) = &mut html.nodes[node_id].data { + /// Apply the attributes and element name replacements to the given node. + /// + /// This might return a different node than the one provided. + fn apply_replacements(&self, node: NodeRef) -> NodeRef { + let mut element_replacement = None; + + if let NodeData::Element(ElementData { name, attrs, .. }) = node.data() { let element_name = name.local.as_ref(); // Replace attributes. @@ -153,6 +151,7 @@ impl SanitizerConfig { .flatten(); if list_replacements.is_some() || mode_replacements.is_some() { + let mut attrs = attrs.borrow_mut(); *attrs = attrs .clone() .into_iter() @@ -174,7 +173,7 @@ impl SanitizerConfig { } // Replace element. - let mut element_replacement = self + element_replacement = self .replace_elements .as_ref() .and_then(|list| list.content.get(element_name)) @@ -191,17 +190,20 @@ impl SanitizerConfig { .flatten() .copied(); } + } - if let Some(element_replacement) = element_replacement { - name.local = LocalName::from(element_replacement); - } + if let Some(element_replacement) = element_replacement { + node.replace_with_element_name(LocalName::from(element_replacement)) + } else { + node } } - fn node_action(&self, html: &Html, node_id: usize, depth: u32) -> NodeAction { - match &html.nodes[node_id].data { + fn node_action(&self, node: &NodeRef, depth: u32) -> NodeAction { + match node.data() { NodeData::Element(ElementData { name, attrs, .. }) => { let element_name = name.local.as_ref(); + let attrs = attrs.borrow(); // Check if element should be removed. if self.remove_elements.as_ref().is_some_and(|set| set.contains(element_name)) { @@ -321,9 +323,10 @@ impl SanitizerConfig { } } - fn clean_element_attributes(&self, data: &mut ElementData) { + fn clean_element_attributes(&self, data: &ElementData) { let ElementData { name, attrs } = data; let element_name = name.local.as_ref(); + let mut attrs = attrs.borrow_mut(); let list_remove_attrs = self.remove_attrs.as_ref().and_then(|map| map.get(element_name)); diff --git a/crates/ruma-html/tests/it/html/navigate.rs b/crates/ruma-html/tests/it/html/navigate.rs index e7c7d83e..ea38ab0e 100644 --- a/crates/ruma-html/tests/it/html/navigate.rs +++ b/crates/ruma-html/tests/it/html/navigate.rs @@ -21,7 +21,7 @@ fn navigate_tree() { let h1_element = h1_node.as_element().unwrap(); assert_eq!(&h1_element.name.local, "h1"); - assert!(h1_element.attrs.is_empty()); + assert!(h1_element.attrs.borrow().is_empty()); assert!(h1_node.parent().is_none()); assert!(h1_node.next_sibling().is_some()); @@ -35,7 +35,7 @@ fn navigate_tree() { // Text of `

` element. let h1_text_node = h1_children.next().unwrap(); let h1_text = h1_text_node.as_text().unwrap(); - assert_eq!(h1_text.as_ref(), "Title"); + assert_eq!(h1_text.borrow().as_ref(), "Title"); assert!(h1_text_node.parent().is_some()); assert!(h1_text_node.next_sibling().is_none()); @@ -54,8 +54,9 @@ fn navigate_tree() { let div_element = div_node.as_element().unwrap(); assert_eq!(&div_element.name.local, "div"); - assert_eq!(div_element.attrs.len(), 1); - let class_attr = div_element.attrs.first().unwrap(); + let attrs = div_element.attrs.borrow(); + assert_eq!(attrs.len(), 1); + let class_attr = attrs.first().unwrap(); assert_eq!(&class_attr.name.local, "class"); assert_eq!(class_attr.value.as_ref(), "text"); @@ -73,7 +74,7 @@ fn navigate_tree() { let p_element = p_node.as_element().unwrap(); assert_eq!(&p_element.name.local, "p"); - assert!(p_element.attrs.is_empty()); + assert!(p_element.attrs.borrow().is_empty()); assert!(p_node.parent().is_some()); assert!(p_node.next_sibling().is_none()); @@ -87,7 +88,7 @@ fn navigate_tree() { // Text of `

` element. let p_text_node = p_children.next().unwrap(); let p_text = p_text_node.as_text().unwrap(); - assert_eq!(p_text.as_ref(), "This is some "); + assert_eq!(p_text.borrow().as_ref(), "This is some "); assert!(p_text_node.parent().is_some()); assert!(p_text_node.next_sibling().is_some()); @@ -104,7 +105,7 @@ fn navigate_tree() { let em_element = em_node.as_element().unwrap(); assert_eq!(&em_element.name.local, "em"); - assert!(em_element.attrs.is_empty()); + assert!(em_element.attrs.borrow().is_empty()); assert!(em_node.parent().is_some()); assert!(em_node.next_sibling().is_none()); @@ -118,7 +119,7 @@ fn navigate_tree() { // Text of `` element. let em_text_node = em_children.next().unwrap(); let em_text = em_text_node.as_text().unwrap(); - assert_eq!(em_text.as_ref(), "text"); + assert_eq!(em_text.borrow().as_ref(), "text"); assert!(em_text_node.parent().is_some()); assert!(em_text_node.next_sibling().is_none()); diff --git a/crates/ruma-html/tests/it/html/sanitize.rs b/crates/ruma-html/tests/it/html/sanitize.rs index ce6dd9a2..39f042a0 100644 --- a/crates/ruma-html/tests/it/html/sanitize.rs +++ b/crates/ruma-html/tests/it/html/sanitize.rs @@ -6,7 +6,7 @@ use ruma_html::{ #[test] fn strict_mode_valid_input() { let config = SanitizerConfig::strict().remove_reply_fallback(); - let mut html = Html::parse( + let html = Html::parse( "\

  • This
  • has
  • no
  • tag
\

This is a paragraph with some color

\ @@ -30,7 +30,7 @@ fn strict_mode_valid_input() { #[test] fn strict_mode_elements_remove() { let config = SanitizerConfig::strict(); - let mut html = Html::parse( + let html = Html::parse( "\ \
\ @@ -66,7 +66,7 @@ fn strict_mode_elements_remove() { #[test] fn strict_mode_elements_reply_remove() { let config = SanitizerConfig::strict().remove_reply_fallback(); - let mut html = Html::parse( + let html = Html::parse( "\ \
\ @@ -94,7 +94,7 @@ fn strict_mode_elements_reply_remove() { #[test] fn remove_only_reply_fallback() { let config = SanitizerConfig::new().remove_reply_fallback(); - let mut html = Html::parse( + let html = Html::parse( "\ \
\ @@ -122,7 +122,7 @@ fn remove_only_reply_fallback() { #[test] fn strict_mode_attrs_remove() { let config = SanitizerConfig::strict(); - let mut html = Html::parse( + let html = Html::parse( "\

Title for important stuff

\

Look at me!

\ @@ -142,7 +142,7 @@ fn strict_mode_attrs_remove() { #[test] fn strict_mode_img_remove_scheme() { let config = SanitizerConfig::strict(); - let mut html = Html::parse( + let html = Html::parse( "\

Look at that picture:

\ \ @@ -156,7 +156,7 @@ fn strict_mode_img_remove_scheme() { #[test] fn strict_mode_link_remove_scheme() { let config = SanitizerConfig::strict(); - let mut html = Html::parse( + let html = Html::parse( "\

Go see my local website

\ ", @@ -174,7 +174,7 @@ fn strict_mode_link_remove_scheme() { #[test] fn compat_mode_link_remove_scheme() { let config = SanitizerConfig::strict(); - let mut html = Html::parse( + let html = Html::parse( "\

Join my room

\

To talk about my cat

\ @@ -190,7 +190,7 @@ fn compat_mode_link_remove_scheme() { ); let config = SanitizerConfig::compat(); - let mut html = Html::parse( + let html = Html::parse( "\

Join my room

\

To talk about my cat

\ @@ -209,7 +209,7 @@ fn compat_mode_link_remove_scheme() { #[test] fn strict_mode_class_remove() { let config = SanitizerConfig::strict(); - let mut html = Html::parse( + let html = Html::parse( "\ 

             type StringList = Vec<String>;
@@ -242,7 +242,7 @@ fn strict_mode_depth_remove() {
         .chain(std::iter::repeat("").take(100))
         .collect();
 
-    let mut html = Html::parse(&deeply_nested_html);
+    let html = Html::parse(&deeply_nested_html);
     html.sanitize_with(&config);
 
     let res = html.to_string();
@@ -253,7 +253,7 @@ fn strict_mode_depth_remove() {
 #[test]
 fn strict_mode_replace_deprecated() {
     let config = SanitizerConfig::strict();
-    let mut html = Html::parse(
+    let html = Html::parse(
         "\
         
Look at you me!
\
         ",
@@ -271,7 +271,7 @@ fn strict_mode_replace_deprecated() {
 #[test]
 fn allow_elements() {
     let config = SanitizerConfig::new().allow_elements(["ul", "li", "p", "img"], ListBehavior::Add);
-    let mut html = Html::parse(
+    let html = Html::parse(
         "\
         
  • This
  • has
  • no
  • tag
\
         
This is a paragraph with some color
\
@@ -296,7 +296,7 @@ fn allow_elements() {
 fn override_elements() {
     let config =
         SanitizerConfig::strict().allow_elements(["ul", "li", "p", "img"], ListBehavior::Override);
-    let mut html = Html::parse(
+    let html = Html::parse(
         "\
         
  • This
  • has
  • no
  • tag
\
         
This is a paragraph with some color
\
@@ -320,7 +320,7 @@ fn override_elements() {
 #[test]
 fn add_elements() {
     let config = SanitizerConfig::strict().allow_elements(["keep-me"], ListBehavior::Add);
-    let mut html = Html::parse(
+    let html = Html::parse(
         "\
         
  • This
  • has
  • no
  • tag
\
         
This is a paragraph with some color
\
@@ -346,7 +346,7 @@ fn add_elements() {
 #[test]
 fn remove_elements() {
     let config = SanitizerConfig::strict().remove_elements(["span", "code"]);
-    let mut html = Html::parse(
+    let html = Html::parse(
         "\
         
  • This
  • has
  • no
  • tag
\
         
This is a paragraph with some color
\
@@ -369,7 +369,7 @@ fn remove_elements() {
 #[test]
 fn ignore_elements() {
     let config = SanitizerConfig::new().ignore_elements(["span", "code"]);
-    let mut html = Html::parse(
+    let html = Html::parse(
         "\
         
  • This
  • has
  • no
  • tag
\
         
This is a paragraph with some color
\
@@ -394,7 +394,7 @@ fn ignore_elements() {
 fn replace_elements() {
     let config = SanitizerConfig::new()
         .replace_elements([NameReplacement { old: "ul", new: "ol" }], ListBehavior::Add);
-    let mut html = Html::parse(
+    let html = Html::parse(
         "\
         
  • This
  • has
  • no
  • tag
\
         
This is a paragraph with some color
\
@@ -419,7 +419,7 @@ fn replace_elements() {
 fn replace_elements_override() {
     let config = SanitizerConfig::strict()
         .replace_elements([NameReplacement { old: "ul", new: "ol" }], ListBehavior::Override);
-    let mut html = Html::parse(
+    let html = Html::parse(
         "\
         
  • This
  • has
  • no
  • tag
\
         
This is a paragraph with some color
\
@@ -446,7 +446,7 @@ fn replace_elements_override() {
 fn replace_elements_add() {
     let config = SanitizerConfig::strict()
         .replace_elements([NameReplacement { old: "ul", new: "ol" }], ListBehavior::Add);
-    let mut html = Html::parse(
+    let html = Html::parse(
         "\
         
  • This
  • has
  • no
  • tag
\
         
This is a paragraph with some color
\
@@ -475,7 +475,7 @@ fn allow_attributes() {
         [PropertiesNames { parent: "img", properties: &["src"] }],
         ListBehavior::Add,
     );
-    let mut html = Html::parse(
+    let html = Html::parse(
         "\
         
  • This
  • has
  • no
  • tag
\
         
This is a paragraph with some color
\
@@ -502,7 +502,7 @@ fn override_attributes() {
         [PropertiesNames { parent: "img", properties: &["src"] }],
         ListBehavior::Override,
     );
-    let mut html = Html::parse(
+    let html = Html::parse(
         "\
         
  • This
  • has
  • no
  • tag
\
         
This is a paragraph with some color
\
@@ -529,7 +529,7 @@ fn add_attributes() {
         [PropertiesNames { parent: "img", properties: &["id"] }],
         ListBehavior::Add,
     );
-    let mut html = Html::parse(
+    let html = Html::parse(
         "\
         
  • This
  • has
  • no
  • tag
\
         
This is a paragraph with some color
\
@@ -554,7 +554,7 @@ fn add_attributes() {
 fn remove_attributes() {
     let config = SanitizerConfig::strict()
         .remove_attributes([PropertiesNames { parent: "span", properties: &["data-mx-color"] }]);
-    let mut html = Html::parse(
+    let html = Html::parse(
         "\
         
  • This
  • has
  • no
  • tag
\
         
This is a paragraph with some color
\
@@ -584,7 +584,7 @@ fn replace_attributes() {
         }],
         ListBehavior::Add,
     );
-    let mut html = Html::parse(
+    let html = Html::parse(
         "\
         
  • This
  • has
  • no
  • tag
\
         
This is a paragraph with some color
\
@@ -614,7 +614,7 @@ fn replace_attributes_override() {
         }],
         ListBehavior::Override,
     );
-    let mut html = Html::parse(
+    let html = Html::parse(
         "\
         
  • This
  • has
  • no
  • tag
\
         
This is a paragraph with some color
\
@@ -644,7 +644,7 @@ fn replace_attributes_add() {
         }],
         ListBehavior::Add,
     );
-    let mut html = Html::parse(
+    let html = Html::parse(
         "\
         
  • This
  • has
  • no
  • tag
\
         
This is a paragraph with some color
\
@@ -674,7 +674,7 @@ fn allow_schemes() {
         }],
         ListBehavior::Add,
     );
-    let mut html = Html::parse(
+    let html = Html::parse(
         "\
         \
         \
@@ -699,7 +699,7 @@ fn override_schemes() {
         }],
         ListBehavior::Override,
     );
-    let mut html = Html::parse(
+    let html = Html::parse(
         "\
         \
         \
@@ -724,7 +724,7 @@ fn add_schemes() {
         }],
         ListBehavior::Add,
     );
-    let mut html = Html::parse(
+    let html = Html::parse(
         "\
         \
         \
@@ -747,7 +747,7 @@ fn deny_schemes() {
         element: "a",
         attr_schemes: &[PropertiesNames { parent: "href", properties: &["http"] }],
     }]);
-    let mut html = Html::parse(
+    let html = Html::parse(
         "\
         Secure link to an image\
         Insecure link to an image\
@@ -770,7 +770,7 @@ fn allow_classes() {
         [PropertiesNames { parent: "img", properties: &["custom-class", "custom-class-*"] }],
         ListBehavior::Add,
     );
-    let mut html = Html::parse(
+    let html = Html::parse(
         "\
         <mx-reply>This is a fake reply</mx-reply>\
         \
@@ -793,7 +793,7 @@ fn override_classes() {
         [PropertiesNames { parent: "code", properties: &["custom-class", "custom-class-*"] }],
         ListBehavior::Override,
     );
-    let mut html = Html::parse(
+    let html = Html::parse(
         "\
         <mx-reply>This is a fake reply</mx-reply>\
         ",
@@ -814,7 +814,7 @@ fn add_classes() {
         [PropertiesNames { parent: "code", properties: &["custom-class", "custom-class-*"] }],
         ListBehavior::Add,
     );
-    let mut html = Html::parse(
+    let html = Html::parse(
         "\
         <mx-reply>This is a fake reply</mx-reply>\
         ",
@@ -833,7 +833,7 @@ fn add_classes() {
 fn remove_classes() {
     let config = SanitizerConfig::strict()
         .remove_classes([PropertiesNames { parent: "code", properties: &["language-rust"] }]);
-    let mut html = Html::parse(
+    let html = Html::parse(
         "\
         <mx-reply>This is a fake reply</mx-reply>\
         ",
diff --git a/crates/ruma-macros/Cargo.toml b/crates/ruma-macros/Cargo.toml
index 27c4d51d..6c62f4a6 100644
--- a/crates/ruma-macros/Cargo.toml
+++ b/crates/ruma-macros/Cargo.toml
@@ -14,7 +14,14 @@ rust-version = { workspace = true }
 [lib]
 proc-macro = true
 
+[features]
+# Make the request and response macros expand internal derives they would
+# usually emit in the `#[derive()]` list directly, such that Rust Analyzer's
+# expand macro helper can render their output. Requires a nightly toolchain.
+__internal_macro_expand = ["syn/visit-mut"]
+
 [dependencies]
+cfg-if = "1.0.0"
 once_cell = "1.13.0"
 proc-macro-crate = "3.1.0"
 proc-macro2 = "1.0.24"
diff --git a/crates/ruma-macros/src/api/request.rs b/crates/ruma-macros/src/api/request.rs
index 733e6801..386f3ecd 100644
--- a/crates/ruma-macros/src/api/request.rs
+++ b/crates/ruma-macros/src/api/request.rs
@@ -1,9 +1,10 @@
+use cfg_if::cfg_if;
 use proc_macro2::TokenStream;
 use quote::{quote, ToTokens};
 use syn::{
     parse::{Parse, ParseStream},
     punctuated::Punctuated,
-    DeriveInput, Field, Generics, Ident, ItemStruct, Token, Type,
+    Field, Generics, Ident, ItemStruct, Token, Type,
 };
 
 use super::{
@@ -26,13 +27,34 @@ pub fn expand_request(attr: RequestAttr, item: ItemStruct) -> TokenStream {
         |DeriveRequestMeta::Error(ty)| quote! { #ty },
     );
 
+    cfg_if! {
+        if #[cfg(feature = "__internal_macro_expand")] {
+            use syn::parse_quote;
+
+            let mut derive_input = item.clone();
+            derive_input.attrs.push(parse_quote! { #[ruma_api(error = #error_ty)] });
+            crate::util::cfg_expand_struct(&mut derive_input);
+
+            let extra_derive = quote! { #ruma_macros::_FakeDeriveRumaApi };
+            let ruma_api_attribute = quote! {};
+            let request_impls =
+                expand_derive_request(derive_input).unwrap_or_else(syn::Error::into_compile_error);
+        } else {
+            let extra_derive = quote! { #ruma_macros::Request };
+            let ruma_api_attribute = quote! { #[ruma_api(error = #error_ty)] };
+            let request_impls = quote! {};
+        }
+    }
+
     quote! {
         #maybe_feature_error
 
-        #[derive(Clone, Debug, #ruma_macros::Request, #ruma_common::serde::_FakeDeriveSerde)]
+        #[derive(Clone, Debug, #ruma_common::serde::_FakeDeriveSerde, #extra_derive)]
         #[cfg_attr(not(feature = "unstable-exhaustive-types"), non_exhaustive)]
-        #[ruma_api(error = #error_ty)]
+        #ruma_api_attribute
         #item
+
+        #request_impls
     }
 }
 
@@ -44,13 +66,9 @@ impl Parse for RequestAttr {
     }
 }
 
-pub fn expand_derive_request(input: DeriveInput) -> syn::Result {
-    let fields = match input.data {
-        syn::Data::Struct(s) => s.fields,
-        _ => panic!("This derive macro only works on structs"),
-    };
-
-    let fields = fields.into_iter().map(RequestField::try_from).collect::>()?;
+pub fn expand_derive_request(input: ItemStruct) -> syn::Result {
+    let fields =
+        input.fields.into_iter().map(RequestField::try_from).collect::>()?;
 
     let mut error_ty = None;
 
diff --git a/crates/ruma-macros/src/api/response.rs b/crates/ruma-macros/src/api/response.rs
index 51da369f..5f28dbaa 100644
--- a/crates/ruma-macros/src/api/response.rs
+++ b/crates/ruma-macros/src/api/response.rs
@@ -1,12 +1,13 @@
 use std::ops::Not;
 
+use cfg_if::cfg_if;
 use proc_macro2::TokenStream;
 use quote::{quote, ToTokens};
 use syn::{
     parse::{Parse, ParseStream},
     punctuated::Punctuated,
     visit::Visit,
-    DeriveInput, Field, Generics, Ident, ItemStruct, Lifetime, Token, Type,
+    Field, Generics, Ident, ItemStruct, Lifetime, Token, Type,
 };
 
 use super::{
@@ -41,13 +42,38 @@ pub fn expand_response(attr: ResponseAttr, item: ItemStruct) -> TokenStream {
         })
         .unwrap_or_else(|| quote! { OK });
 
+    cfg_if! {
+        if #[cfg(feature = "__internal_macro_expand")] {
+            use syn::parse_quote;
+
+            let mut derive_input = item.clone();
+            derive_input.attrs.push(parse_quote! {
+                #[ruma_api(error = #error_ty, status = #status_ident)]
+            });
+            crate::util::cfg_expand_struct(&mut derive_input);
+
+            let extra_derive = quote! { #ruma_macros::_FakeDeriveRumaApi };
+            let ruma_api_attribute = quote! {};
+            let response_impls =
+                expand_derive_response(derive_input).unwrap_or_else(syn::Error::into_compile_error);
+        } else {
+            let extra_derive = quote! { #ruma_macros::Response };
+            let ruma_api_attribute = quote! {
+                #[ruma_api(error = #error_ty, status = #status_ident)]
+            };
+            let response_impls = quote! {};
+        }
+    }
+
     quote! {
         #maybe_feature_error
 
-        #[derive(Clone, Debug, #ruma_macros::Response, #ruma_common::serde::_FakeDeriveSerde)]
+        #[derive(Clone, Debug, #ruma_common::serde::_FakeDeriveSerde, #extra_derive)]
         #[cfg_attr(not(feature = "unstable-exhaustive-types"), non_exhaustive)]
-        #[ruma_api(error = #error_ty, status = #status_ident)]
+        #ruma_api_attribute
         #item
+
+        #response_impls
     }
 }
 
@@ -59,13 +85,9 @@ impl Parse for ResponseAttr {
     }
 }
 
-pub fn expand_derive_response(input: DeriveInput) -> syn::Result {
-    let fields = match input.data {
-        syn::Data::Struct(s) => s.fields,
-        _ => panic!("This derive macro only works on structs"),
-    };
-
-    let fields = fields.into_iter().map(ResponseField::try_from).collect::>()?;
+pub fn expand_derive_response(input: ItemStruct) -> syn::Result {
+    let fields =
+        input.fields.into_iter().map(ResponseField::try_from).collect::>()?;
     let mut manual_body_serde = false;
     let mut error_ty = None;
     let mut status_ident = None;
@@ -90,8 +112,8 @@ pub fn expand_derive_response(input: DeriveInput) -> syn::Result {
         generics: input.generics,
         fields,
         manual_body_serde,
-        error_ty: error_ty.unwrap(),
-        status_ident: status_ident.unwrap(),
+        error_ty: error_ty.expect("missing error_ty attribute"),
+        status_ident: status_ident.expect("missing status_ident attribute"),
     };
 
     response.check()?;
diff --git a/crates/ruma-macros/src/lib.rs b/crates/ruma-macros/src/lib.rs
index 53d1c486..74278dc4 100644
--- a/crates/ruma-macros/src/lib.rs
+++ b/crates/ruma-macros/src/lib.rs
@@ -4,6 +4,7 @@
 //!
 //! See the documentation for the individual macros for usage details.
 
+#![cfg_attr(feature = "__internal_macro_expand", feature(proc_macro_expand))]
 #![warn(missing_docs)]
 #![allow(unreachable_pub)]
 // https://github.com/rust-lang/rust-clippy/issues/9029
@@ -397,14 +398,14 @@ pub fn response(attr: TokenStream, item: TokenStream) -> TokenStream {
 /// Internal helper that the request macro delegates most of its work to.
 #[proc_macro_derive(Request, attributes(ruma_api))]
 pub fn derive_request(input: TokenStream) -> TokenStream {
-    let input = parse_macro_input!(input as DeriveInput);
+    let input = parse_macro_input!(input);
     expand_derive_request(input).unwrap_or_else(syn::Error::into_compile_error).into()
 }
 
 /// Internal helper that the response macro delegates most of its work to.
 #[proc_macro_derive(Response, attributes(ruma_api))]
 pub fn derive_response(input: TokenStream) -> TokenStream {
-    let input = parse_macro_input!(input as DeriveInput);
+    let input = parse_macro_input!(input);
     expand_derive_response(input).unwrap_or_else(syn::Error::into_compile_error).into()
 }
 
diff --git a/crates/ruma-macros/src/util.rs b/crates/ruma-macros/src/util.rs
index 8c3af72a..9dd7e510 100644
--- a/crates/ruma-macros/src/util.rs
+++ b/crates/ruma-macros/src/util.rs
@@ -91,3 +91,114 @@ impl ToTokens for PrivateField<'_> {
         ty.to_tokens(tokens);
     }
 }
+
+#[cfg(feature = "__internal_macro_expand")]
+pub fn cfg_expand_struct(item: &mut syn::ItemStruct) {
+    use std::mem;
+
+    use proc_macro2::TokenTree;
+    use syn::{visit_mut::VisitMut, Fields, LitBool, Meta};
+
+    fn eval_cfg(cfg_expr: TokenStream) -> Option {
+        let cfg_macro_call = quote! { ::core::cfg!(#cfg_expr) };
+        let expanded = match proc_macro::TokenStream::from(cfg_macro_call).expand_expr() {
+            Ok(t) => t,
+            Err(e) => {
+                eprintln!("Failed to expand cfg! {e}");
+                return None;
+            }
+        };
+
+        let lit: LitBool = syn::parse(expanded).expect("cfg! must expand to a boolean literal");
+        Some(lit.value())
+    }
+
+    fn tokentree_not_comma(tree: &TokenTree) -> bool {
+        match tree {
+            TokenTree::Punct(p) => p.as_char() != ',',
+            _ => true,
+        }
+    }
+
+    struct CfgAttrExpand;
+
+    impl VisitMut for CfgAttrExpand {
+        fn visit_attribute_mut(&mut self, attr: &mut syn::Attribute) {
+            if attr.meta.path().is_ident("cfg_attr") {
+                // Ignore invalid cfg attributes
+                let Meta::List(list) = &attr.meta else { return };
+                let mut token_iter = list.tokens.clone().into_iter();
+
+                // Take all the tokens until the first toplevel comma.
+                // That's the cfg-expression part of cfg_attr.
+                let cfg_expr: TokenStream =
+                    token_iter.by_ref().take_while(tokentree_not_comma).collect();
+
+                let Some(cfg_value) = eval_cfg(cfg_expr) else { return };
+                if cfg_value {
+                    // If we had the whole attribute list and could emit more
+                    // than one attribute, we'd split the remaining arguments to
+                    // cfg_attr by commas and turn them into regular attributes
+                    //
+                    // Because we can emit only one, do the first and error if
+                    // there's any more after it.
+                    let attr_tokens: TokenStream =
+                        token_iter.by_ref().take_while(tokentree_not_comma).collect();
+
+                    if attr_tokens.is_empty() {
+                        // no-op cfg_attr??
+                        return;
+                    }
+
+                    attr.meta = syn::parse2(attr_tokens)
+                        .expect("syn must be able to parse cfg-attr arguments as syn::Meta");
+
+                    let rest: TokenStream = token_iter.collect();
+                    assert!(
+                        rest.is_empty(),
+                        "cfg_attr's with multiple arguments after the cfg expression are not \
+                         currently supported by __internal_macro_expand."
+                    );
+                }
+            }
+        }
+    }
+
+    CfgAttrExpand.visit_item_struct_mut(item);
+
+    let Fields::Named(fields) = &mut item.fields else {
+        panic!("only named fields are currently supported by __internal_macro_expand");
+    };
+
+    // Take out all the fields
+    'fields: for mut field in mem::take(&mut fields.named) {
+        // Take out all the attributes
+        for attr in mem::take(&mut field.attrs) {
+            // For non-cfg attrs, put them back
+            if !attr.meta.path().is_ident("cfg") {
+                field.attrs.push(attr);
+                continue;
+            }
+
+            // Also put back / ignore invalid cfg attributes
+            let Meta::List(list) = &attr.meta else {
+                field.attrs.push(attr);
+                continue;
+            };
+            // Also put back / ignore cfg attributes we can't eval
+            let Some(cfg_value) = eval_cfg(list.tokens.clone()) else {
+                field.attrs.push(attr);
+                continue;
+            };
+
+            // Finally, if the cfg is `false`, skip the part where it's put back
+            if !cfg_value {
+                continue 'fields;
+            }
+        }
+
+        // If `continue 'fields` above wasn't hit, we didn't find a cfg that
+        // evals to false, so put the field back
+        fields.named.push(field);
+    }
+}
diff --git a/xtask/src/ci.rs b/xtask/src/ci.rs
index 058c563d..e9275d7d 100644
--- a/xtask/src/ci.rs
+++ b/xtask/src/ci.rs
@@ -28,8 +28,6 @@ pub enum CiCmd {
     Msrv,
     /// Check all crates with all features (msrv)
     MsrvAll,
-    /// Check ruma-client with default features (msrv)
-    MsrvClient,
     /// Check ruma crate with default features (msrv)
     MsrvRuma,
     /// Check ruma-identifiers with `ruma_identifiers_storage="Box"`
@@ -101,7 +99,6 @@ impl CiTask {
         match self.cmd {
             Some(CiCmd::Msrv) => self.msrv()?,
             Some(CiCmd::MsrvAll) => self.msrv_all()?,
-            Some(CiCmd::MsrvClient) => self.msrv_client()?,
             Some(CiCmd::MsrvRuma) => self.msrv_ruma()?,
             Some(CiCmd::MsrvOwnedIdBox) => self.msrv_owned_id_box()?,
             Some(CiCmd::MsrvOwnedIdArc) => self.msrv_owned_id_arc()?,
@@ -139,21 +136,20 @@ impl CiTask {
     /// Check that the crates compile with the MSRV.
     fn msrv(&self) -> Result<()> {
         self.msrv_all()?;
-        self.msrv_client()?;
         self.msrv_ruma()
     }
 
     /// Check all crates with all features with the MSRV, except:
     /// * ruma (would pull in ruma-signatures)
-    /// * ruma-client (tested only with client-api feature due to most / all optional HTTP client
-    ///   deps having less strict MSRV)
+    /// * ruma-macros (it's still pulled as a dependency but don't want to enable its nightly-only
+    ///   internal feature here)
     /// * ruma-signatures (MSRV exception)
     /// * xtask (no real reason to enforce an MSRV for it)
     fn msrv_all(&self) -> Result<()> {
         cmd!(
             "rustup run {MSRV} cargo check --workspace --all-features
                 --exclude ruma
-                --exclude ruma-client
+                --exclude ruma-macros
                 --exclude ruma-signatures
                 --exclude xtask"
         )
@@ -161,13 +157,6 @@ impl CiTask {
         .map_err(Into::into)
     }
 
-    /// Check ruma-client with default features with the MSRV.
-    fn msrv_client(&self) -> Result<()> {
-        cmd!("rustup run {MSRV} cargo check -p ruma-client --features client-api")
-            .run()
-            .map_err(Into::into)
-    }
-
     /// Check ruma crate with default features with the MSRV.
     fn msrv_ruma(&self) -> Result<()> {
         cmd!("rustup run {MSRV} cargo check -p ruma").run().map_err(Into::into)
@@ -185,7 +174,14 @@ impl CiTask {
 
     /// Check all crates with all features with the stable version.
     fn stable_all(&self) -> Result<()> {
-        cmd!("rustup run stable cargo check --workspace --all-features").run().map_err(Into::into)
+        // ruma-macros is pulled in as a dependency, but excluding it on the command line means its
+        // features don't get activated. It has only a single feature, which is nightly-only.
+        cmd!(
+            "rustup run stable cargo check
+                --workspace --all-features --exclude ruma-macros"
+        )
+        .run()
+        .map_err(Into::into)
     }
 
     /// Check ruma-client without default features with the stable version.