xml5ever: compare prefix and local name when looking for duplicate attributes

xml5ever/src/tokenizer/mod.rs

 } 
 fn finish_attribute(&self) { 
     if self.current_attr_name.borrow().is_empty() { 
         return; 
     } 
     let name = LocalName::from(&**self.current_attr_name.borrow()); 
     self.current_attr_name.borrow_mut().clear(); 
     // Check for a duplicate attribute. 
     // FIXME: the spec says we should error as soon as the name is finished. 
     let dup = { 
         self.current_tag_attrs 
             .borrow() 
             .iter() 
             .any(|a| a.name.local == name) 
     }; 
     if dup { 
         self.emit_error(Borrowed("Duplicate attribute")); 
         self.current_tag_had_duplicate_attributes.set(true); 
         self.current_attr_value.borrow_mut().clear(); 
     } else { 
         self.current_tag_attrs.borrow_mut().push(Attribute { 
             // The tree builder will adjust the namespace if necessary. 
             // This only happens in foreign elements. 
             name: QualName::new(None, ns!(), name), 
             value: mem::take(&mut self.current_attr_value.borrow_mut()), 
         }); 
     } 
 } 
@@ -1278,27 +1278,26 @@ impl<Sink: TokenSink> XmlTokenizer<Sink> {
             return;
         }
 
-        // Check for a duplicate attribute.
+        let qname = process_qname(replace(
+            &mut self.current_attr_name.borrow_mut(),
+            StrTendril::new(),
+        ));
+
+        // Check for a duplicate attribute. Two attributes are the same only if
+        // both their prefix and their local name match, so xml:lang and lang
+        // are distinct names and may sit on the same element.
         // FIXME: the spec says we should error as soon as the name is finished.
         // FIXME: linear time search, do we care?
-        let dup = {
-            let current_attr_name = self.current_attr_name.borrow();
-            let name = &current_attr_name[..];
-            self.current_tag_attrs
-                .borrow()
-                .iter()
-                .any(|a| &*a.name.local == name)
-        };
+        let dup = self
+            .current_tag_attrs
+            .borrow()
+            .iter()
+            .any(|a| a.name.prefix == qname.prefix && a.name.local == qname.local);
 
         if dup {
             self.emit_error(Borrowed("Duplicate attribute"));
-            self.current_attr_name.borrow_mut().clear();
             self.current_attr_value.borrow_mut().clear();
         } else {
-            let qname = process_qname(replace(
-                &mut self.current_attr_name.borrow_mut(),
-                StrTendril::new(),
-            ));
             let attr = Attribute {
                 name: qname.clone(),
                 value: replace(&mut self.current_attr_value.borrow_mut(), StrTendril::new()),
@@ -1347,6 +1346,21 @@ mod test {
         }
     }
 
+    struct ErrorCollector {
+        errors: RefCell<Vec<String>>,
+    }
+
+    impl TokenSink for ErrorCollector {
+        type Handle = ();
+
+        fn process_token(&self, token: Token) -> ProcessResult<()> {
+            if let Token::ParseError(error) = token {
+                self.errors.borrow_mut().push(error.to_string());
+            }
+            ProcessResult::Continue
+        }
+    }
+
     fn tokenize_pis(input: &str) -> Vec<(String, String)> {
         let sink = PiCollector {
             pis: RefCell::new(Vec::new()),
@@ -1359,6 +1373,18 @@ mod test {
         tokenizer.sink.pis.into_inner()
     }
 
+    fn tokenize_errors(input: &str) -> Vec<String> {
+        let sink = ErrorCollector {
+            errors: RefCell::new(Vec::new()),
+        };
+        let queue = BufferQueue::default();
+        queue.push_back(StrTendril::from(input));
+        let tokenizer = XmlTokenizer::new(sink, Default::default());
+        let _ = tokenizer.feed(&queue);
+        tokenizer.end();
+        tokenizer.sink.errors.into_inner()
+    }
+
     #[test]
     fn pi_data_keeps_question_marks() {
         assert_eq!(
@@ -1412,6 +1438,38 @@ mod test {
         );
     }
 
+    #[test]
+    fn qualified_and_unqualified_names_are_distinct() {
+        // The xml prefix is bound to http://www.w3.org/XML/1998/namespace by
+        // definition, so xml:lang and lang have different expanded names and
+        // both orderings are fine. This is the second of the two legal cases
+        // in https://www.w3.org/TR/REC-xml-names/#uniqAttrs
+        assert!(tokenize_errors(r#"<root xml:lang="en" lang="en"/>"#).is_empty());
+        assert!(tokenize_errors(r#"<root lang="en" xml:lang="en"/>"#).is_empty());
+    }
+
+    #[test]
+    fn different_prefixes_are_left_to_the_tree_builder() {
+        // Whether these two are duplicates depends on what a and b are bound
+        // to, and the tokenizer has no bindings, so it says nothing either way.
+        // XmlTreeBuilder::bind_attr_qname resolves the prefixes and compares
+        // expanded names, which is where a real duplicate gets caught.
+        assert!(tokenize_errors(r#"<root a:name="1" b:name="2"/>"#).is_empty());
+    }
+
+    #[test]
+    fn real_duplicates_are_still_reported() {
+        assert_eq!(
+            tokenize_errors(r#"<root lang="en" lang="fr"/>"#),
+            vec!["Duplicate attribute".to_owned()]
+        );
+
+        assert_eq!(
+            tokenize_errors(r#"<root xml:lang="en" xml:lang="fr"/>"#),
+            vec!["Duplicate attribute".to_owned()]
+        );
+    }
+
     #[test]
     fn simple_namespace() {
         let qname = process_qname("prefix:local".to_tendril());