diff --git a/Changelog.md b/Changelog.md index 964162e6..82fcc2d2 100644 --- a/Changelog.md +++ b/Changelog.md @@ -76,13 +76,15 @@ The MSRV has been raised to 1.86. `u16` depth counter. Previously the unguarded `nesting_level += 1` panicked under `overflow-checks` builds and silently wrapped in release, corrupting namespace-scope bookkeeping on deeply nested untrusted input. -- [#980]: `NamespaceResolver` now caps the total number of in-scope namespace - bindings (default 128, configurable via `set_max_namespace_bindings`), - replacing the previous per-element `max_declarations_per_element` limit. - [#978]: The serde `Deserializer` now enforces a configurable recursion-depth limit (default 128, matching `serde_json`). Deeply nested XML returns `DeError::TooDeeplyNested` instead of overflowing the native call stack. Use `Deserializer::recursion_limit()` to adjust. +- [#980]: `NamespaceResolver` now caps the total number of in-scope namespace + bindings (default 128, configurable via `set_max_namespace_bindings`), + replacing the previous per-element `max_declarations_per_element` limit. +- [#984]: `Reader` no longer emits an empty `Text` event when `trim_text_end` + removes whitespace-only text immediately before markup or a reference. - [#990]: `\r` in text content is now escaped as ` ` by the serde serializer, `BytesText::new()`, `escape()`, `partial_escape()`, and `minimal_escape()`, preventing silent conversion to `\n` from XML end-of-line normalization on @@ -107,6 +109,7 @@ The MSRV has been raised to 1.86. [#859]: https://github.com/tafia/quick-xml/issues/859 [#978]: https://github.com/tafia/quick-xml/issues/978 [#983]: https://github.com/tafia/quick-xml/issues/983 +[#984]: https://github.com/tafia/quick-xml/issues/984 [#989]: https://github.com/tafia/quick-xml/issues/989 [#990]: https://github.com/tafia/quick-xml/issues/990 [#1000]: https://github.com/tafia/quick-xml/pull/1000 diff --git a/src/reader/buffered_reader.rs b/src/reader/buffered_reader.rs index 571f8e63..29ebd0fb 100644 --- a/src/reader/buffered_reader.rs +++ b/src/reader/buffered_reader.rs @@ -54,6 +54,7 @@ macro_rules! impl_buffered_source { &mut self, buf: &'b mut Vec, position: &mut u64, + trim_text_end: bool, ) -> ReadTextResult<'b, &'b mut Vec> { let mut read = 0; let start = buf.len(); @@ -83,6 +84,9 @@ macro_rules! impl_buffered_source { read += i as u64; *position += read; + if trim_text_end && buf[start..].iter().all(|&b| is_whitespace(b)) { + return ReadTextResult::Markup(buf); + } return match std::str::from_utf8(&buf[start..]) { Ok(s) => ReadTextResult::UpToMarkup(s), Err(e) => ReadTextResult::Err(e.into()), @@ -95,6 +99,9 @@ macro_rules! impl_buffered_source { read += i as u64; *position += read; + if trim_text_end && buf[start..].iter().all(|&b| is_whitespace(b)) { + return ReadTextResult::Ref(buf); + } return match std::str::from_utf8(&buf[start..]) { Ok(s) => ReadTextResult::UpToRef(s), Err(e) => ReadTextResult::Err(e.into()), diff --git a/src/reader/mod.rs b/src/reader/mod.rs index 928c3b7c..f0f3db65 100644 --- a/src/reader/mod.rs +++ b/src/reader/mod.rs @@ -325,7 +325,11 @@ macro_rules! read_event_impl { $reader.skip_whitespace(&mut $self.state.offset) $(.$await)? ?; } - match $reader.read_text($buf, &mut $self.state.offset) $(.$await)? { + match $reader.read_text( + $buf, + &mut $self.state.offset, + $self.state.config.trim_text_end, + ) $(.$await)? { ReadTextResult::Markup(buf) => { $self.state.state = ParseState::InsideMarkup; // Pass `buf` to the next next iteration of parsing loop @@ -340,15 +344,10 @@ macro_rules! read_event_impl { } ReadTextResult::UpToMarkup(bytes) => { $self.state.state = ParseState::InsideMarkup; - // FIXME: Can produce an empty event if: - // - event contains only spaces - // - trim_text_start = false - // - trim_text_end = true Ok(Event::Text($self.state.emit_text(bytes)?)) } ReadTextResult::UpToRef(bytes) => { $self.state.state = ParseState::InsideRef; - // Return Text event with `bytes` content or Eof if bytes is empty Ok(Event::Text($self.state.emit_text(bytes)?)) } ReadTextResult::UpToEof(bytes) => { @@ -493,8 +492,8 @@ macro_rules! read_to_end { // it is important that this position indicates beginning of the End event. // If between last event and the End event would be only spaces, then we // take position before the spaces, but spaces would be skipped without - // generating event if `trim_text_start` is set to `true`. To prevent that - // we temporary disable start text trimming. + // generating an event if text trimming is enabled. To prevent that we + // temporarily disable text trimming. // // We also cannot take position after getting End event, because if // `trim_markup_names_in_closing_tags` is set to `true` (which is the default), @@ -502,8 +501,10 @@ macro_rules! read_to_end { // the source and cannot correct the position after the End event. // So, we in any case should tweak parser configuration. let config = $self.config_mut(); - let trim = config.trim_text_start; + let trim_start = config.trim_text_start; + let trim_end = config.trim_text_end; config.trim_text_start = false; + config.trim_text_end = false; let start = $self.buffer_position(); let mut depth = 0; @@ -512,20 +513,26 @@ macro_rules! read_to_end { let end = $self.buffer_position(); match $self.$read_event($buf) $(.$await)? { Err(e) => { - $self.config_mut().trim_text_start = trim; + let config = $self.config_mut(); + config.trim_text_start = trim_start; + config.trim_text_end = trim_end; return Err(e); } Ok(Event::Start(e)) if e.name() == $end => depth += 1, Ok(Event::End(e)) if e.name() == $end => { if depth == 0 { - $self.config_mut().trim_text_start = trim; + let config = $self.config_mut(); + config.trim_text_start = trim_start; + config.trim_text_end = trim_end; break start..end; } depth -= 1; } Ok(Event::Eof) => { - $self.config_mut().trim_text_start = trim; + let config = $self.config_mut(); + config.trim_text_start = trim_start; + config.trim_text_end = trim_end; return Err(Error::missed_end($end)); } _ => (), @@ -998,16 +1005,19 @@ impl Reader { /// Result of an attempt to read XML textual data from the source. #[derive(Debug)] enum ReadTextResult<'r, B> { - /// Start of markup (`<` character) was found in the first byte. `<` was consumed. - /// Contains buffer that should be returned back to the next iteration cycle - /// to satisfy borrow checker requirements. + /// The reader is positioned at `<` (start of markup). `<` was not consumed. + /// Returned either when `<` was the first byte, or when `trim_text_end` is + /// enabled and only whitespace preceded it (that whitespace was consumed and + /// discarded). Contains buffer that should be returned back to the next + /// iteration cycle to satisfy borrow checker requirements. Markup(B), - /// Start of reference (`&` character) was found in the first byte. - /// `&` was not consumed. - /// Contains buffer that should be returned back to the next iteration cycle - /// to satisfy borrow checker requirements. + /// The reader is positioned at `&` (start of a reference). `&` was not consumed. + /// Returned either when `&` was the first byte, or when `trim_text_end` is + /// enabled and only whitespace preceded it (that whitespace was consumed and + /// discarded). Contains buffer that should be returned back to the next + /// iteration cycle to satisfy borrow checker requirements. Ref(B), - /// Contains text block up to start of markup (`<` character). `<` was consumed. + /// Contains text block up to start of markup (`<` character). `<` was not consumed. UpToMarkup(&'r str), /// Contains text block up to start of reference (`&` character). /// `&` was not consumed. @@ -1069,9 +1079,16 @@ trait XmlSource<'r, B> { /// - `buf`: Buffer that could be filled from an input (`Self`) and /// from which [events] could borrow their data /// - `position`: Will be increased by amount of bytes consumed + /// - `trim_text_end`: When true, a whitespace-only text block is suppressed + /// (returns [`ReadTextResult::Markup`] or [`ReadTextResult::Ref`] instead) /// /// [events]: crate::events::Event - fn read_text(&mut self, buf: B, position: &mut u64) -> ReadTextResult<'r, B>; + fn read_text( + &mut self, + buf: B, + position: &mut u64, + trim_text_end: bool, + ) -> ReadTextResult<'r, B>; /// Read input until end of general reference (the `;`) is found, start of /// another general reference (the `&`) is found or end of input is reached. @@ -1646,7 +1663,7 @@ mod test { let mut input = b"".as_ref(); // ^= 1 - match $source(&mut input).read_text(buf, &mut position) $(.$await)? { + match $source(&mut input).read_text(buf, &mut position, false) $(.$await)? { ReadTextResult::UpToEof(bytes) => assert_eq!(bytes, ""), x => panic!("Expected `UpToEof(_)`, but got `{:?}`", x), } @@ -1660,7 +1677,7 @@ mod test { let mut input = b"<".as_ref(); // ^= 1 - match $source(&mut input).read_text(buf, &mut position) $(.$await)? { + match $source(&mut input).read_text(buf, &mut position, false) $(.$await)? { ReadTextResult::Markup(b) => assert_eq!(b, $buf), x => panic!("Expected `Markup(_)`, but got `{:?}`", x), } @@ -1674,7 +1691,7 @@ mod test { let mut input = b"&".as_ref(); // ^= 1 - match $source(&mut input).read_text(buf, &mut position) $(.$await)? { + match $source(&mut input).read_text(buf, &mut position, false) $(.$await)? { ReadTextResult::Ref(b) => assert_eq!(b, $buf), x => panic!("Expected `Ref(_)`, but got `{:?}`", x), } @@ -1688,7 +1705,7 @@ mod test { let mut input = b"a<".as_ref(); // ^= 2 - match $source(&mut input).read_text(buf, &mut position) $(.$await)? { + match $source(&mut input).read_text(buf, &mut position, false) $(.$await)? { ReadTextResult::UpToMarkup(bytes) => assert_eq!(bytes, "a"), x => panic!("Expected `UpToMarkup(_)`, but got `{:?}`", x), } @@ -1702,7 +1719,7 @@ mod test { let mut input = b"a&".as_ref(); // ^= 2 - match $source(&mut input).read_text(buf, &mut position) $(.$await)? { + match $source(&mut input).read_text(buf, &mut position, false) $(.$await)? { ReadTextResult::UpToRef(bytes) => assert_eq!(bytes, "a"), x => panic!("Expected `UpToRef(_)`, but got `{:?}`", x), } @@ -1716,7 +1733,7 @@ mod test { let mut input = b"a".as_ref(); // ^= 2 - match $source(&mut input).read_text(buf, &mut position) $(.$await)? { + match $source(&mut input).read_text(buf, &mut position, false) $(.$await)? { ReadTextResult::UpToEof(bytes) => assert_eq!(bytes, "a"), x => panic!("Expected `UpToEof(_)`, but got `{:?}`", x), } @@ -2060,7 +2077,7 @@ mod test { /// Ensures, that no empty `Text` events are generated mod $read_event { - use crate::events::{BytesCData, BytesDecl, BytesEnd, BytesPI, BytesStart, BytesText, Event}; + use crate::events::{BytesCData, BytesDecl, BytesEnd, BytesPI, BytesRef, BytesStart, BytesText, Event}; use crate::reader::Reader; use pretty_assertions::assert_eq; @@ -2150,6 +2167,55 @@ mod test { ); } + #[$test] + $($async)? fn trim_text_end_skips_empty_text_before_markup() { + let mut reader = Reader::from_str(" "); + reader.config_mut().trim_text_end = true; + + assert_eq!( + reader.$read_event($buf) $(.$await)? .unwrap(), + Event::Empty(BytesStart::new("tag")) + ); + assert_eq!( + reader.$read_event($buf) $(.$await)? .unwrap(), + Event::Eof + ); + } + + #[$test] + $($async)? fn trim_text_end_skips_empty_text_before_reference() { + let mut reader = Reader::from_str(" &"); + reader.config_mut().trim_text_end = true; + + assert_eq!( + reader.$read_event($buf) $(.$await)? .unwrap(), + Event::GeneralRef(BytesRef::new("amp")) + ); + assert_eq!( + reader.$read_event($buf) $(.$await)? .unwrap(), + Event::Eof + ); + } + + #[$test] + $($async)? fn trim_text_end_preserves_non_empty_text() { + let mut reader = Reader::from_str(" text "); + reader.config_mut().trim_text_end = true; + + assert_eq!( + reader.$read_event($buf) $(.$await)? .unwrap(), + Event::Text(BytesText::new(" text")) + ); + assert_eq!( + reader.$read_event($buf) $(.$await)? .unwrap(), + Event::Empty(BytesStart::new("tag")) + ); + assert_eq!( + reader.$read_event($buf) $(.$await)? .unwrap(), + Event::Eof + ); + } + #[$test] $($async)? fn empty() { let mut reader = Reader::from_str(""); diff --git a/src/reader/slice_reader.rs b/src/reader/slice_reader.rs index 6f5c2ede..161ba502 100644 --- a/src/reader/slice_reader.rs +++ b/src/reader/slice_reader.rs @@ -268,7 +268,12 @@ impl<'a> XmlSource<'a, ()> for &'a [u8] { } #[inline] - fn read_text(&mut self, _buf: (), position: &mut u64) -> ReadTextResult<'a, ()> { + fn read_text( + &mut self, + _buf: (), + position: &mut u64, + trim_text_end: bool, + ) -> ReadTextResult<'a, ()> { // Search for start of markup or an entity or character reference match memchr::memchr2(b'<', b'&', self) { Some(0) if self[0] == b'<' => ReadTextResult::Markup(()), @@ -280,6 +285,9 @@ impl<'a> XmlSource<'a, ()> for &'a [u8] { let (bytes, rest) = self.split_at(i); *self = rest; *position += i as u64; + if trim_text_end && bytes.iter().all(|&b| is_whitespace(b)) { + return ReadTextResult::Markup(()); + } match std::str::from_utf8(bytes) { Ok(s) => ReadTextResult::UpToMarkup(s), Err(e) => ReadTextResult::Err(e.into()), @@ -289,6 +297,9 @@ impl<'a> XmlSource<'a, ()> for &'a [u8] { let (bytes, rest) = self.split_at(i); *self = rest; *position += i as u64; + if trim_text_end && bytes.iter().all(|&b| is_whitespace(b)) { + return ReadTextResult::Ref(()); + } match std::str::from_utf8(bytes) { Ok(s) => ReadTextResult::UpToRef(s), Err(e) => ReadTextResult::Err(e.into()), diff --git a/tests/reader-config.rs b/tests/reader-config.rs index e03ca1b2..8206a470 100644 --- a/tests/reader-config.rs +++ b/tests/reader-config.rs @@ -563,6 +563,12 @@ mod trim_markup_names_in_closing_tags { } } +// NOTE: These tests currently do NOT apply XML end-of-line normalization (XML 1.0 Section 2.11). +// Per the spec, `\r\n` must be normalized to `\n` before any other processing, which +// means every `\r\n` in this constant should become `\n` in the parsed output. That +// normalization applies universally: text content, element/attribute whitespace, comments, +// PIs, CDATA, and DOCTYPE. All assertions below that expect `\r\n` in their output are +// therefore incorrect with respect to a spec-compliant parser. const XML: &str = " \t\r\n\ \t\r\n\ \t\r\n\ @@ -819,6 +825,30 @@ mod trim_text_end { use super::*; use pretty_assertions::assert_eq; + /// Whitespace-only text before `<` should be suppressed (issue #984) + #[test] + fn skips_whitespace_only_text_before_markup() { + let mut reader = Reader::from_str(" "); + reader.config_mut().trim_text_end = true; + + assert_eq!( + reader.read_event().unwrap(), + Event::Empty(BytesStart::new("a")) + ); + assert_eq!(reader.read_event().unwrap(), Event::Eof); + + let mut reader = Reader::from_reader(" ".as_bytes()); + reader.config_mut().trim_text_end = true; + let mut buffer = Vec::new(); + + assert_eq!( + reader.read_event_into(&mut buffer).unwrap(), + Event::Empty(BytesStart::new("a")) + ); + buffer.clear(); + assert_eq!(reader.read_event_into(&mut buffer).unwrap(), Event::Eof); + } + #[test] fn false_() { let mut reader = Reader::from_str(XML); @@ -895,9 +925,7 @@ mod trim_text_end { assert_eq!(reader.read_event().unwrap(), Event::Eof); } - // TODO: Enable test after rewriting parser #[test] - #[ignore = "currently it is hard to fix incorrect behavior, but this will much easy after parser rewrite"] fn true_() { let mut reader = Reader::from_str(XML); reader.config_mut().trim_text_end = true;