Add duplicate attribute tracking for CSP nonce validation

html5ever/src/tokenizer/interface.rs

@@ -40,6 +40,10 @@ pub struct Tag {
     pub name: LocalName,
     pub self_closing: bool,
     pub attrs: Vec<Attribute>,
+    /// Whether duplicate attributes were encountered during tokenization.
+    /// This is used for CSP nonce validation - elements with duplicate
+    /// attributes are not nonceable per the CSP spec.
+    pub had_duplicate_attrs: bool,
 }
 
 impl Tag {

html5ever/src/tokenizer/mod.rs

@@ -133,6 +133,9 @@ pub struct Tokenizer<Sink> {
     /// Current tag is self-closing?
     current_tag_self_closing: Cell<bool>,
 
+    /// Current tag had duplicate attributes?
+    current_tag_had_duplicate_attrs: Cell<bool>,
+
     /// Current tag attributes.
     current_tag_attrs: RefCell<Vec<Attribute>>,
 
@@ -186,6 +189,7 @@ impl<Sink: TokenSink> Tokenizer<Sink> {
             current_tag_kind: Cell::new(StartTag),
             current_tag_name: RefCell::new(StrTendril::new()),
             current_tag_self_closing: Cell::new(false),
+            current_tag_had_duplicate_attrs: Cell::new(false),
             current_tag_attrs: RefCell::new(vec![]),
             current_attr_name: RefCell::new(StrTendril::new()),
             current_attr_value: RefCell::new(StrTendril::new()),
@@ -440,6 +444,7 @@ impl<Sink: TokenSink> Tokenizer<Sink> {
             name,
             self_closing: self.current_tag_self_closing.get(),
             attrs: std::mem::take(&mut self.current_tag_attrs.borrow_mut()),
+            had_duplicate_attrs: self.current_tag_had_duplicate_attrs.get(),
         });
 
         match self.process_token(token) {
@@ -481,6 +486,7 @@ impl<Sink: TokenSink> Tokenizer<Sink> {
     fn discard_tag(&self) {
         self.current_tag_name.borrow_mut().clear();
         self.current_tag_self_closing.set(false);
+        self.current_tag_had_duplicate_attrs.set(false);
         *self.current_tag_attrs.borrow_mut() = vec![];
     }
 
@@ -523,6 +529,7 @@ impl<Sink: TokenSink> Tokenizer<Sink> {
 
         if dup {
             self.emit_error(Borrowed("Duplicate attribute"));
+            self.current_tag_had_duplicate_attrs.set(true);
             self.current_attr_name.borrow_mut().clear();
             self.current_attr_value.borrow_mut().clear();
         } else {
@@ -2217,6 +2224,7 @@ mod test {
             name,
             self_closing: false,
             attrs: vec![],
+            had_duplicate_attrs: false,
         })
     }
 

html5ever/src/tree_builder/mod.rs

@@ -12,6 +12,7 @@
 pub use crate::interface::{create_element, ElemName, ElementFlags, Tracer, TreeSink};
 pub use crate::interface::{AppendNode, AppendText, Attribute, NodeOrText};
 pub use crate::interface::{LimitedQuirks, NoQuirks, Quirks, QuirksMode};
+pub use markup5ever::interface::tree_builder::create_element_with_flags;
 
 use self::types::*;
 
@@ -733,6 +734,7 @@ where
                     name: subject,
                     self_closing: false,
                     attrs: vec![],
+                    had_duplicate_attrs: false,
                 });
             };
 
@@ -828,10 +830,11 @@ where
                 };
                 // FIXME: Is there a way to avoid cloning the attributes twice here (once on their
                 // own, once as part of t.clone() above)?
-                let new_element = create_element(
+                let new_element = create_element_with_flags(
                     &self.sink,
                     QualName::new(None, ns!(html), tag.name.clone()),
                     tag.attrs.clone(),
+                    tag.had_duplicate_attrs,
                 );
                 self.open_elems.borrow_mut()[node_index] = new_element.clone();
                 self.active_formatting.borrow_mut()[node_formatting_index] =
@@ -860,10 +863,11 @@ where
             // 15.
             // FIXME: Is there a way to avoid cloning the attributes twice here (once on their own,
             // once as part of t.clone() above)?
-            let new_element = create_element(
+            let new_element = create_element_with_flags(
                 &self.sink,
                 QualName::new(None, ns!(html), fmt_elem_tag.name.clone()),
                 fmt_elem_tag.attrs.clone(),
+                fmt_elem_tag.had_duplicate_attrs,
             );
             let new_entry = FormatEntry::Element(new_element.clone(), fmt_elem_tag);
 
@@ -1010,6 +1014,7 @@ where
                 ns!(html),
                 tag.name.clone(),
                 tag.attrs.clone(),
+                tag.had_duplicate_attrs,
             );
 
             // Step 9. Replace the entry for entry in the list with an entry for new element.
@@ -1363,6 +1368,7 @@ where
         ns: Namespace,
         name: LocalName,
         attrs: Vec<Attribute>,
+        had_duplicate_attrs: bool,
     ) -> Handle {
         declare_tag_set!(form_associatable =
             "button" "fieldset" "input" "object"
@@ -1372,7 +1378,12 @@ where
 
         // Step 7.
         let qname = QualName::new(None, ns, name);
-        let elem = create_element(&self.sink, qname.clone(), attrs.clone());
+        let elem = create_element_with_flags(
+            &self.sink,
+            qname.clone(),
+            attrs.clone(),
+            had_duplicate_attrs,
+        );
 
         let insertion_point = self.appropriate_place_for_insertion(None);
         let (node1, node2) = match insertion_point {
@@ -1410,15 +1421,27 @@ where
     }
 
     fn insert_element_for(&self, tag: Tag) -> Handle {
-        self.insert_element(PushFlag::Push, ns!(html), tag.name, tag.attrs)
+        self.insert_element(
+            PushFlag::Push,
+            ns!(html),
+            tag.name,
+            tag.attrs,
+            tag.had_duplicate_attrs,
+        )
     }
 
     fn insert_and_pop_element_for(&self, tag: Tag) -> Handle {
-        self.insert_element(PushFlag::NoPush, ns!(html), tag.name, tag.attrs)
+        self.insert_element(
+            PushFlag::NoPush,
+            ns!(html),
+            tag.name,
+            tag.attrs,
+            tag.had_duplicate_attrs,
+        )
     }
 
     fn insert_phantom(&self, name: LocalName) -> Handle {
-        self.insert_element(PushFlag::Push, ns!(html), name, vec![])
+        self.insert_element(PushFlag::Push, ns!(html), name, vec![], false)
     }
 
     /// <https://html.spec.whatwg.org/multipage/parsing.html#insert-an-element-at-the-adjusted-insertion-location>
@@ -1429,8 +1452,13 @@ where
         only_add_to_element_stack: bool,
     ) -> Handle {
         let adjusted_insertion_location = self.appropriate_place_for_insertion(None);
-        let qname = QualName::new(None, ns, tag.name);
-        let elem = create_element(&self.sink, qname.clone(), tag.attrs.clone());
+        let qname = QualName::new(None, ns, tag.name.clone());
+        let elem = create_element_with_flags(
+            &self.sink,
+            qname.clone(),
+            tag.attrs.clone(),
+            tag.had_duplicate_attrs,
+        );
 
         if !only_add_to_element_stack {
             self.insert_at(adjusted_insertion_location, AppendNode(elem.clone()));
@@ -1520,6 +1548,7 @@ where
             ns!(html),
             tag.name.clone(),
             tag.attrs.clone(),
+            tag.had_duplicate_attrs,
         );
         self.active_formatting
             .borrow_mut()
@@ -1655,10 +1684,22 @@ where
         self.adjust_foreign_attributes(&mut tag);
 
         if tag.self_closing {
-            self.insert_element(PushFlag::NoPush, ns, tag.name, tag.attrs);
+            self.insert_element(
+                PushFlag::NoPush,
+                ns,
+                tag.name,
+                tag.attrs,
+                tag.had_duplicate_attrs,
+            );
             ProcessResult::DoneAckSelfClosing
         } else {
-            self.insert_element(PushFlag::Push, ns, tag.name, tag.attrs);
+            self.insert_element(
+                PushFlag::Push,
+                ns,
+                tag.name,
+                tag.attrs,
+                tag.had_duplicate_attrs,
+            );
             ProcessResult::Done
         }
     }
@@ -1823,10 +1864,22 @@ where
         self.adjust_foreign_attributes(&mut tag);
         if tag.self_closing {
             // FIXME(#118): <script /> in SVG
-            self.insert_element(PushFlag::NoPush, current_ns, tag.name, tag.attrs);
+            self.insert_element(
+                PushFlag::NoPush,
+                current_ns,
+                tag.name,
+                tag.attrs,
+                tag.had_duplicate_attrs,
+            );
             ProcessResult::DoneAckSelfClosing
         } else {
-            self.insert_element(PushFlag::Push, current_ns, tag.name, tag.attrs);
+            self.insert_element(
+                PushFlag::Push,
+                current_ns,
+                tag.name,
+                tag.attrs,
+                tag.had_duplicate_attrs,
+            );
             ProcessResult::Done
         }
     }

html5ever/src/tree_builder/rules.rs

@@ -15,10 +15,10 @@ use crate::tokenizer::TagKind::{EndTag, StartTag};
 use crate::tree_builder::tag_sets::*;
 use crate::tree_builder::types::*;
 use crate::tree_builder::{
-    create_element, html_elem, ElemName, NodeOrText::AppendNode, StrTendril, Tag, TreeBuilder,
-    TreeSink,
+    html_elem, ElemName, NodeOrText::AppendNode, StrTendril, Tag, TreeBuilder, TreeSink,
 };
 use crate::QualName;
+use markup5ever::interface::tree_builder::create_element_with_flags;
 use markup5ever::{expanded_name, local_name, ns};
 use std::borrow::Cow::Borrowed;
 
@@ -207,10 +207,11 @@ where
                     },
 
                     Token::Tag(tag @ tag!(<script>)) => {
-                        let elem = create_element(
+                        let elem = create_element_with_flags(
                             &self.sink,
                             QualName::new(None, ns!(html), local_name!("script")),
                             tag.attrs,
+                            tag.had_duplicate_attrs,
                         );
                         if self.is_fragment() {
                             self.sink.mark_script_already_started(&elem);

markup5ever/interface/tree_builder.rs

@@ -62,6 +62,13 @@ pub struct ElementFlags {
     ///
     /// [whatwg integration-point]: https://html.spec.whatwg.org/multipage/#html-integration-point
     pub mathml_annotation_xml_integration_point: bool,
+
+    /// Whether duplicate attributes were encountered during tokenization.
+    /// This is used for CSP nonce validation - elements with duplicate
+    /// attributes are not nonceable per the CSP spec.
+    ///
+    /// See [CSP Level 3 - Is element nonceable](https://www.w3.org/TR/CSP/#is-element-nonceable)
+    pub had_duplicate_attrs: bool,
 }
 
 /// A constructor for an element.
@@ -70,6 +77,22 @@ pub struct ElementFlags {
 ///
 /// Create an element like `<div class="test-class-name"></div>`:
 pub fn create_element<Sink>(sink: &Sink, name: QualName, attrs: Vec<Attribute>) -> Sink::Handle
+where
+    Sink: TreeSink,
+{
+    create_element_with_flags(sink, name, attrs, false)
+}
+
+/// A constructor for an element with duplicate attribute information.
+///
+/// This variant allows passing whether duplicate attributes were encountered
+/// during tokenization, which is needed for CSP nonce validation.
+pub fn create_element_with_flags<Sink>(
+    sink: &Sink,
+    name: QualName,
+    attrs: Vec<Attribute>,
+    had_duplicate_attrs: bool,
+) -> Sink::Handle
 where
     Sink: TreeSink,
 {
@@ -85,6 +108,7 @@ where
         },
         _ => {},
     }
+    flags.had_duplicate_attrs = had_duplicate_attrs;
     sink.create_element(name, attrs, flags)
 }
 

rcdom/custom-html5lib-tokenizer-tests/duplicate-attrs.test

@@ -0,0 +1,66 @@
+{"tests": [
+
+{"description": "Duplicate attribute - simple case",
+"input": "<div id='first' id='second'>",
+"output": [
+    ["StartTag", "div", {"id": "first"}]
+],
+"errors": [
+    {"code": "duplicate-attribute"}
+]},
+
+{"description": "Duplicate attribute - three duplicates",
+"input": "<div id='first' id='second' id='third'>",
+"output": [
+    ["StartTag", "div", {"id": "first"}]
+],
+"errors": [
+    {"code": "duplicate-attribute"},
+    {"code": "duplicate-attribute"}
+]},
+
+{"description": "Duplicate attribute - mixed with other attrs",
+"input": "<div class='foo' id='first' title='bar' id='second'>",
+"output": [
+    ["StartTag", "div", {"class": "foo", "id": "first", "title": "bar"}]
+],
+"errors": [
+    {"code": "duplicate-attribute"}
+]},
+
+{"description": "Duplicate nonce attribute",
+"input": "<script nonce='abc123' nonce='xyz789'>",
+"output": [
+    ["StartTag", "script", {"nonce": "abc123"}]
+],
+"errors": [
+    {"code": "duplicate-attribute"}
+]},
+
+{"description": "No duplicate - different attributes",
+"input": "<div id='test' class='foo'>",
+"output": [
+    ["StartTag", "div", {"id": "test", "class": "foo"}]
+],
+"errors": []},
+
+{"description": "Duplicate attribute - case insensitive (ID vs id)",
+"input": "<div ID='first' id='second'>",
+"output": [
+    ["StartTag", "div", {"id": "first"}]
+],
+"errors": [
+    {"code": "duplicate-attribute"}
+]},
+
+{"description": "Multiple different duplicates",
+"input": "<div id='a' id='b' class='x' class='y'>",
+"output": [
+    ["StartTag", "div", {"id": "a", "class": "x"}]
+],
+"errors": [
+    {"code": "duplicate-attribute"},
+    {"code": "duplicate-attribute"}
+]}
+
+]}