From 0f9a9dd98a312d7e9b8d11838d87d0b8114e2ef6 Mon Sep 17 00:00:00 2001 From: JerryNee Date: Sun, 26 Jul 2026 16:14:01 -0500 Subject: [PATCH] Fix empty text events after trailing trim Return the reader buffer directly when trim_text_end removes a whitespace-only run before markup or a reference, allowing parsing to continue without emitting an empty Text event. Preserve read_to_end span boundaries while trimming is enabled. Closes #984 --- Changelog.md | 3 + src/reader/buffered_reader.rs | 17 +++++- src/reader/mod.rs | 108 +++++++++++++++++++++++++++------- src/reader/slice_reader.rs | 19 +++++- tests/reader-config.rs | 2 - 5 files changed, 120 insertions(+), 29 deletions(-) diff --git a/Changelog.md b/Changelog.md index 04ff1a3e7..2ed588b28 100644 --- a/Changelog.md +++ b/Changelog.md @@ -18,6 +18,8 @@ ### Bug Fixes +- [#984]: Readers no longer emit empty `Text` events when `trim_text_end` + removes whitespace-only text before markup or a general reference. - [#977]: `NamespaceResolver::push` (and hence every `NsReader` `Start`/`Empty` event) now returns the new `NamespaceError::TooDeeplyNested` when a document nests elements deeper than `u16::MAX`, instead of overflowing the internal @@ -29,6 +31,7 @@ - [#983]: Adopted an AI use and contribution policy for new upstream contributions. +[#984]: https://github.com/tafia/quick-xml/issues/984 [#977]: https://github.com/tafia/quick-xml/issues/977 [#983]: https://github.com/tafia/quick-xml/issues/983 diff --git a/src/reader/buffered_reader.rs b/src/reader/buffered_reader.rs index 4317d640a..2a0f67029 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,7 +84,13 @@ macro_rules! impl_buffered_source { read += i as u64; *position += read; - return ReadTextResult::UpToMarkup(&buf[start..]); + return if trim_text_end + && buf[start..].iter().all(|&b| is_whitespace(b)) + { + ReadTextResult::Markup(buf) + } else { + ReadTextResult::UpToMarkup(&buf[start..]) + }; } Some(i) => { buf.extend_from_slice(&available[..i]); @@ -92,7 +99,13 @@ macro_rules! impl_buffered_source { read += i as u64; *position += read; - return ReadTextResult::UpToRef(&buf[start..]); + return if trim_text_end + && buf[start..].iter().all(|&b| is_whitespace(b)) + { + ReadTextResult::Ref(buf) + } else { + ReadTextResult::UpToRef(&buf[start..]) + }; } None => { buf.extend_from_slice(available); diff --git a/src/reader/mod.rs b/src/reader/mod.rs index c359c5890..00fa4faf7 100644 --- a/src/reader/mod.rs +++ b/src/reader/mod.rs @@ -326,7 +326,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 @@ -341,15 +345,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) => { @@ -494,8 +493,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 event if either text trimming option is set to `true`. To + // prevent that we temporarily disable both text trimming options. // // 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), @@ -503,8 +502,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; @@ -513,20 +514,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, $self.decoder())); } _ => (), @@ -997,11 +1004,11 @@ 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. + /// No text should be emitted before the start of markup (`<` character). /// 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. + /// No text should be emitted before the start of reference (`&` character). /// `&` was not consumed. /// Contains buffer that should be returned back to the next iteration cycle /// to satisfy borrow checker requirements. @@ -1068,9 +1075,17 @@ 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`: Whether trailing whitespace should be trimmed. When + /// enabled, whitespace-only text before markup or a reference is skipped + /// so that `buf` can be reused by the next parser iteration /// /// [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. @@ -1645,7 +1660,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(bytes), Bytes(b"")), x => panic!("Expected `UpToEof(_)`, but got `{:?}`", x), } @@ -1659,7 +1674,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), } @@ -1673,7 +1688,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), } @@ -1687,7 +1702,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(bytes), Bytes(b"a")), x => panic!("Expected `UpToMarkup(_)`, but got `{:?}`", x), } @@ -1701,7 +1716,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(bytes), Bytes(b"a")), x => panic!("Expected `UpToRef(_)`, but got `{:?}`", x), } @@ -1715,7 +1730,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(bytes), Bytes(b"a")), x => panic!("Expected `UpToEof(_)`, but got `{:?}`", x), } @@ -2059,7 +2074,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; @@ -2169,6 +2184,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 cdata() { let mut reader = Reader::from_str(""); diff --git a/src/reader/slice_reader.rs b/src/reader/slice_reader.rs index ffc8708e2..b15a2d4fc 100644 --- a/src/reader/slice_reader.rs +++ b/src/reader/slice_reader.rs @@ -264,7 +264,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(()), @@ -275,13 +280,21 @@ impl<'a> XmlSource<'a, ()> for &'a [u8] { let (bytes, rest) = self.split_at(i); *self = rest; *position += i as u64; - ReadTextResult::UpToMarkup(bytes) + if trim_text_end && bytes.iter().all(|&b| is_whitespace(b)) { + ReadTextResult::Markup(()) + } else { + ReadTextResult::UpToMarkup(bytes) + } } Some(i) => { let (bytes, rest) = self.split_at(i); *self = rest; *position += i as u64; - ReadTextResult::UpToRef(bytes) + if trim_text_end && bytes.iter().all(|&b| is_whitespace(b)) { + ReadTextResult::Ref(()) + } else { + ReadTextResult::UpToRef(bytes) + } } None => { let bytes = &self[..]; diff --git a/tests/reader-config.rs b/tests/reader-config.rs index 09f820a38..b636116b1 100644 --- a/tests/reader-config.rs +++ b/tests/reader-config.rs @@ -895,9 +895,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;