diff --git a/rcdom/tests/xml-tokenizer.rs b/rcdom/tests/xml-tokenizer.rs index 150dc6ed..af1e1797 100644 --- a/rcdom/tests/xml-tokenizer.rs +++ b/rcdom/tests/xml-tokenizer.rs @@ -283,10 +283,26 @@ fn json_to_tokens(js: &Value, exact_errors: bool) -> Vec { sink.get_tokens() } -fn mk_xml_test(name: String, input: String, expect: Value, opts: XmlTokenizerOpts) -> Test { +/// Descriptions of xml5lib tests that we do not run. +/// +/// The XML5 draft's "PI after state" throws away a '?' that turns out not to be +/// the start of a '?>', instead of treating it as ordinary data. XML 1.0 §2.6 +/// says a processing instruction runs up to the first '?>', so such a '?' +/// belongs in the data, and that is what libxml2 and other parsers produce. +/// This test bakes in the draft's behaviour, so it disagrees with us now that +/// we keep the character. See https://github.com/servo/html5ever/issues/774 +const IGNORED_TESTS: &[&str] = &["PI tag with char in PiAfter state"]; + +fn mk_xml_test( + name: String, + skip: bool, + input: String, + expect: Value, + opts: XmlTokenizerOpts, +) -> Test { Test { name, - skip: false, + skip, test: Box::new(move || { // Split up the input at different points to test incremental tokenization. let insplits = splits(&input, 3); @@ -308,7 +324,9 @@ fn mk_xml_test(name: String, input: String, expect: Value, opts: XmlTokenizerOpt fn mk_xml_tests(tests: &mut Vec, filename: &str, js: &Value) { let input: &str = &js.find("input").get_str(); let expect = js.find("output"); - let desc = format!("tok: {}: {}", filename, js.find("description").get_str()); + let description = js.find("description").get_str(); + let skip = IGNORED_TESTS.contains(&&*description); + let desc = format!("tok: {filename}: {description}"); // Some tests want to start in a state other than Data. let state_overrides = vec![None]; @@ -326,6 +344,7 @@ fn mk_xml_tests(tests: &mut Vec, filename: &str, js: &Value) { tests.push(mk_xml_test( newdesc, + skip, String::from(input), expect.clone(), XmlTokenizerOpts { diff --git a/xml5ever/src/tokenizer/mod.rs b/xml5ever/src/tokenizer/mod.rs index 0d87e637..5f9d14c5 100644 --- a/xml5ever/src/tokenizer/mod.rs +++ b/xml5ever/src/tokenizer/mod.rs @@ -742,8 +742,11 @@ impl XmlTokenizer { XmlState::PiAfter => loop { match get_char!(self, input) { '>' => go!(self: emit_pi Data), - '?' => go!(self: to XmlState::PiAfter), - cl => go!(self: push_pi_data cl), + // The '?' that got us here is not part of a '?>', so it is + // ordinary data. The one we just read may still close the + // processing instruction, so stay in this state. + '?' => go!(self: push_pi_data '?'; to XmlState::PiAfter), + cl => go!(self: push_pi_data '?'; push_pi_data cl; to XmlState::PiData), } }, //§ markup-declaration-state @@ -1196,7 +1199,9 @@ impl XmlTokenizer { go!(self: error_eof; to XmlState::Data) }, XmlState::Pi => go!(self: error_eof; to XmlState::BogusComment), - XmlState::PiTargetAfter | XmlState::PiAfter => go!(self: reconsume XmlState::PiData), + XmlState::PiTargetAfter => go!(self: reconsume XmlState::PiData), + // The '?' we consumed to get here never became a '?>', so keep it. + XmlState::PiAfter => go!(self: push_pi_data '?'; reconsume XmlState::PiData), XmlState::MarkupDecl => go!(self: error_eof; to XmlState::BogusComment), XmlState::TagName | XmlState::TagAttrNameBefore @@ -1319,9 +1324,93 @@ impl XmlTokenizer { #[cfg(test)] mod test { - use super::process_qname; - use crate::tendril::SliceExt; + use super::{process_qname, ProcessResult, Token, TokenSink, XmlTokenizer}; + use crate::tendril::{SliceExt, StrTendril}; use crate::{LocalName, Prefix}; + use markup5ever::buffer_queue::BufferQueue; + use std::cell::RefCell; + + struct PiCollector { + pis: RefCell>, + } + + impl TokenSink for PiCollector { + type Handle = (); + + fn process_token(&self, token: Token) -> ProcessResult<()> { + if let Token::ProcessingInstruction(pi) = token { + self.pis + .borrow_mut() + .push((pi.target.to_string(), pi.data.to_string())); + } + ProcessResult::Continue + } + } + + fn tokenize_pis(input: &str) -> Vec<(String, String)> { + let sink = PiCollector { + pis: 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.pis.into_inner() + } + + #[test] + fn pi_data_keeps_question_marks() { + assert_eq!( + tokenize_pis(r#""#), + vec![( + "xml-stylesheet".to_owned(), + r#"href="style.xsl?v=2""#.to_owned() + )] + ); + + assert_eq!( + tokenize_pis(""), + vec![("target".to_owned(), "a?b?c".to_owned())] + ); + + // A run of question marks only ends the instruction when one of them + // is followed by '>'. + assert_eq!( + tokenize_pis(""), + vec![("target".to_owned(), "a??".to_owned())] + ); + } + + #[test] + fn pi_only_ends_on_question_mark_gt() { + // The '>' here is data, because the character before it is not a '?'. + assert_eq!( + tokenize_pis("c?>"), + vec![("target".to_owned(), "a?b>c".to_owned())] + ); + } + + #[test] + fn pi_without_data_is_empty() { + assert_eq!( + tokenize_pis(""), + vec![("target".to_owned(), String::new())] + ); + + assert_eq!( + tokenize_pis(""), + vec![("target".to_owned(), String::new())] + ); + } + + #[test] + fn unterminated_pi_keeps_trailing_question_mark() { + assert_eq!( + tokenize_pis("