fix(security): Passing unescaped strings in attribute values

Introduced in #184. Previously, escaped values became unescaped on the serialization step.

Author: Stranger6667

CHANGELOG.md

@@ -10,6 +10,11 @@
 
 - Replace `remove_style_tags` with `keep_style_tags`.
 
+### Fixed
+
+- **SECURITY**: Passing unescaped strings in attribute values introduced in [#184](https://github.com/Stranger6667/css-inline/issues/184).
+  Previously, escaped values became unescaped on the serialization step.
+
 ### Removed
 
 - The `inline_style_tags` configuration option.

bindings/python/CHANGELOG.md

@@ -10,6 +10,11 @@
 
 - Replace `remove_style_tags` with `keep_style_tags`.
 
+### Fixed
+
+- **SECURITY**: Passing unescaped strings in attribute values introduced in [#184](https://github.com/Stranger6667/css-inline/issues/184).
+  Previously, escaped values became unescaped on the serialization step.
+
 ### Removed
 
 - The `inline_style_tags` configuration option.

bindings/wasm/CHANGELOG.md

@@ -10,6 +10,11 @@
 
 - Replace `remove_style_tags` with `keep_style_tags`.
 
+### Fixed
+
+- **SECURITY**: Passing unescaped strings in attribute values introduced in [#184](https://github.com/Stranger6667/css-inline/issues/184).
+  Previously, escaped values became unescaped on the serialization step.
+
 ### Removed
 
 - The `inline_style_tags` configuration option.

css-inline/src/html/serializer.rs

@@ -1,9 +1,11 @@
 use super::{
-    attributes::Attributes,
     document::Document,
     node::{ElementData, NodeData, NodeId},
 };
-use html5ever::{local_name, namespace_url, ns, QualName};
+use html5ever::{
+    local_name,
+    serialize::{serialize, Serialize, SerializeOpts, Serializer, TraversalScope},
+};
 use std::{io, io::Write};
 
 pub(crate) fn serialize_to<W: Write>(
@@ -18,8 +20,7 @@ pub(crate) fn serialize_to<W: Write>(
         keep_style_tags,
         keep_link_tags,
     );
-    let mut serializer = HtmlSerializer::new(writer);
-    sink.serialize(&mut serializer)
+    serialize(writer, &sink, SerializeOpts::default())
 }
 
 /// Intermediary structure for serializing an HTML document.
@@ -69,23 +70,37 @@ impl<'a> Sink<'a> {
             false
         }
     }
-    fn serialize_children<W: Write>(&self, serializer: &mut HtmlSerializer<W>) -> io::Result<()> {
+    fn serialize_children<S: Serializer>(&self, serializer: &mut S) -> io::Result<()> {
         for child in self.document.children(self.node) {
-            self.for_node(child).serialize(serializer)?;
+            self.for_node(child)
+                .serialize(serializer, TraversalScope::IncludeNode)?;
         }
         Ok(())
     }
-    fn serialize<W: Write>(&self, serializer: &mut HtmlSerializer<W>) -> io::Result<()> {
+}
+
+impl<'a> Serialize for Sink<'a> {
+    fn serialize<S>(&self, serializer: &mut S, _: TraversalScope) -> io::Result<()>
+    where
+        S: Serializer,
+    {
         match self.data() {
             NodeData::Element { element, .. } => {
                 if self.should_skip_element(element) {
                     return Ok(());
                 }
-                serializer.start_elem(&element.name, &element.attributes)?;
+                serializer.start_elem(
+                    element.name.clone(),
+                    element
+                        .attributes
+                        .map
+                        .iter()
+                        .map(|(key, value)| (key, &**value)),
+                )?;
 
                 self.serialize_children(serializer)?;
 
-                serializer.end_elem(&element.name)?;
+                serializer.end_elem(element.name.clone())?;
                 Ok(())
             }
             NodeData::Document => self.serialize_children(serializer),
@@ -99,130 +114,6 @@ impl<'a> Sink<'a> {
     }
 }
 
-struct ElemInfo {
-    ignore_children: bool,
-}
-
-struct HtmlSerializer<Wr: Write> {
-    writer: Wr,
-    stack: Vec<ElemInfo>,
-}
-
-impl<Wr: Write> HtmlSerializer<Wr> {
-    fn new(writer: Wr) -> Self {
-        HtmlSerializer {
-            writer,
-            stack: vec![ElemInfo {
-                ignore_children: false,
-            }],
-        }
-    }
-
-    fn parent(&mut self) -> &mut ElemInfo {
-        self.stack.last_mut().expect("Stack is empty")
-    }
-
-    fn start_elem(&mut self, name: &QualName, attrs: &Attributes) -> io::Result<()> {
-        if self.parent().ignore_children {
-            self.stack.push(ElemInfo {
-                ignore_children: true,
-            });
-            return Ok(());
-        }
-
-        self.writer.write_all(b"<")?;
-        self.writer.write_all(name.local.as_bytes())?;
-        for (name, value) in &attrs.map {
-            self.writer.write_all(b" ")?;
-
-            match name.ns {
-                ns!() => (),
-                ns!(xml) => self.writer.write_all(b"xml:")?,
-                ns!(xmlns) => {
-                    if name.local != local_name!("xmlns") {
-                        self.writer.write_all(b"xmlns:")?;
-                    }
-                }
-                ns!(xlink) => self.writer.write_all(b"xlink:")?,
-                _ => {
-                    self.writer.write_all(b"unknown_namespace:")?;
-                }
-            }
-
-            self.writer.write_all(name.local.as_bytes())?;
-            self.writer.write_all(b"=\"")?;
-            self.writer.write_all(value.as_bytes())?;
-            self.writer.write_all(b"\"")?;
-        }
-        self.writer.write_all(b">")?;
-
-        let ignore_children = name.ns == ns!(html)
-            && matches!(
-                name.local,
-                local_name!("area")
-                    | local_name!("base")
-                    | local_name!("basefont")
-                    | local_name!("bgsound")
-                    | local_name!("br")
-                    | local_name!("col")
-                    | local_name!("embed")
-                    | local_name!("frame")
-                    | local_name!("hr")
-                    | local_name!("img")
-                    | local_name!("input")
-                    | local_name!("keygen")
-                    | local_name!("link")
-                    | local_name!("meta")
-                    | local_name!("param")
-                    | local_name!("source")
-                    | local_name!("track")
-                    | local_name!("wbr")
-            );
-
-        self.stack.push(ElemInfo { ignore_children });
-
-        Ok(())
-    }
-
-    fn end_elem(&mut self, name: &QualName) -> io::Result<()> {
-        let info = match self.stack.pop() {
-            Some(info) => info,
-            _ => panic!("no ElemInfo"),
-        };
-        if info.ignore_children {
-            return Ok(());
-        }
-
-        self.writer.write_all(b"</")?;
-        self.writer.write_all(name.local.as_bytes())?;
-        self.writer.write_all(b">")
-    }
-
-    fn write_text(&mut self, text: &str) -> io::Result<()> {
-        self.writer.write_all(text.as_bytes())
-    }
-
-    fn write_comment(&mut self, text: &str) -> io::Result<()> {
-        self.writer.write_all(b"<!--")?;
-        self.writer.write_all(text.as_bytes())?;
-        self.writer.write_all(b"-->")
-    }
-
-    fn write_doctype(&mut self, name: &str) -> io::Result<()> {
-        self.writer.write_all(b"<!DOCTYPE ")?;
-        self.writer.write_all(name.as_bytes())?;
-        self.writer.write_all(b">")
-    }
-
-    fn write_processing_instruction(&mut self, target: &str, data: &str) -> io::Result<()> {
-        self.writer.write_all(b"<?")?;
-        self.writer.write_all(target.as_bytes())?;
-        self.writer.write_all(b" ")?;
-        self.writer.write_all(data.as_bytes())?;
-        self.writer.write_all(b">")
-    }
-}
-
 #[cfg(test)]
 mod tests {
     use super::Document;

css-inline/tests/test_inlining.rs

@@ -245,7 +245,7 @@ fn href_attribute_unchanged() {
 </head>
 <body>
     <h1 style="color:blue;">Big Text</h1>
-    <a href="https://example.org/test?a=b&c=d">Link</a>
+    <a href="https://example.org/test?a=b&amp;c=d">Link</a>
 
 </body></html>"#
     );