diff --git a/Changelog.md b/Changelog.md index 25c9aab1..97fbbc9b 100644 --- a/Changelog.md +++ b/Changelog.md @@ -25,6 +25,9 @@ - [#1007]: Remove error-prone implementations of `Hash`, `PartialOrd`, and `Ord` from all events except `BytesText` and `BytesCData`, which was introduced in [#1005]. +- [#1017]: Fix panic and wrong output when the indent character is not ASCII. + `Indentation` used a character count as a byte index into its indent buffer, and + `Serializer::indent` truncated its `char` argument with `as u8`. ### Misc Changes @@ -35,6 +38,7 @@ [#1009]: https://github.com/tafia/quick-xml/pull/1009 [#1010]: https://github.com/tafia/quick-xml/pull/1010 [#1015]: https://github.com/tafia/quick-xml/pull/1015 +[#1017]: https://github.com/tafia/quick-xml/pull/1017 ## 0.42.0 -- 2026-08-22 diff --git a/src/se/mod.rs b/src/se/mod.rs index 9db3ef40..72c0f538 100644 --- a/src/se/mod.rs +++ b/src/se/mod.rs @@ -817,7 +817,7 @@ impl<'w, 'r, W: Write> Serializer<'w, 'r, W> { /// ); /// ``` pub fn indent(&mut self, indent_char: char, indent_size: usize) -> &mut Self { - self.ser.indent = Indent::Owned(Indentation::new(indent_char as u8, indent_size)); + self.ser.indent = Indent::Owned(Indentation::with_char(indent_char, indent_size)); self } diff --git a/src/writer.rs b/src/writer.rs index b6a7c32a..9c89edf5 100644 --- a/src/writer.rs +++ b/src/writer.rs @@ -678,13 +678,16 @@ pub(crate) struct Indentation { indent_size: usize, /// Used as a cache for the string used for indentation indents: String, - /// The current amount of indentation + /// The current amount of indentation, in bytes of `indents` current_indent_len: usize, } impl Indentation { pub fn new(indent_char: u8, indent_size: usize) -> Self { - let indent_char = char::from(indent_char); + Self::with_char(char::from(indent_char), indent_size) + } + + pub fn with_char(indent_char: char, indent_size: usize) -> Self { Self { should_line_break: false, indent_char, @@ -694,15 +697,21 @@ impl Indentation { } } + /// Size of one indentation level in bytes + #[inline] + const fn level_len(&self) -> usize { + self.indent_size * self.indent_char.len_utf8() + } + /// Increase indentation by one level pub fn grow(&mut self) { - self.current_indent_len += self.indent_size; + self.current_indent_len += self.level_len(); self.ensure(self.current_indent_len); } /// Decrease indentation by one level. Do nothing, if level already zero pub fn shrink(&mut self) { - self.current_indent_len = self.current_indent_len.saturating_sub(self.indent_size); + self.current_indent_len = self.current_indent_len.saturating_sub(self.level_len()); } /// Returns indent string for current level @@ -711,8 +720,10 @@ impl Indentation { } /// Returns indent with current indent plus additional indent + /// + /// `additional_indent` is counted in characters, not bytes. pub fn additional(&mut self, additional_indent: usize) -> &str { - let new_len = self.current_indent_len + additional_indent; + let new_len = self.current_indent_len + additional_indent * self.indent_char.len_utf8(); self.ensure(new_len); &self.indents[..new_len] } diff --git a/tests/writer-indentation.rs b/tests/writer-indentation.rs index 4e23bbc6..560c79dc 100644 --- a/tests/writer-indentation.rs +++ b/tests/writer-indentation.rs @@ -550,3 +550,170 @@ mod in_attributes_multi { ); } } + +/// `Writer::new_with_indent` converts the indent byte with `char::from`, so bytes +/// above 0x7F select a character that is two bytes long in UTF-8. +mod multi_byte_indent_char { + use super::*; + use pretty_assertions::assert_eq; + + /// NO-BREAK SPACE is `char::from(0xA0)` and occupies two bytes in UTF-8. + const NBSP: char = '\u{a0}'; + + #[test] + fn nested() { + let mut buffer = Vec::new(); + let mut writer = Writer::new_with_indent(&mut buffer, 0xA0, 2); + + let start = BytesStart::new("paired"); + let end = start.to_end().into_owned(); + + writer + .write_event(Event::Start(start.clone())) + .expect("write start 1 tag failed"); + writer + .write_event(Event::Start(start)) + .expect("write start 2 tag failed"); + writer + .write_event(Event::Empty(BytesStart::new("inner"))) + .expect("write inner tag failed"); + writer + .write_event(Event::End(end.clone())) + .expect("write end tag 2 failed"); + writer + .write_event(Event::End(end)) + .expect("write end tag 1 failed"); + + let i = NBSP.to_string().repeat(2); + assert_eq!( + std::str::from_utf8(&buffer).unwrap(), + format!( + "\n{i}\n{i}{i}\n{i}\n", + i = i + ) + ); + } + + #[test] + fn in_attributes() { + let mut buffer = Vec::new(); + let mut writer = Writer::new_with_indent(&mut buffer, 0xA0, 2); + + writer + .create_element("element") + .new_line() + .with_attribute(("first", "1")) + .write_empty() + .expect("write tag failed"); + + assert_eq!( + std::str::from_utf8(&buffer).unwrap(), + format!("", i = NBSP.to_string().repeat(2)) + ); + } + + /// An odd `indent_size` makes the byte length of one level an odd number, + /// which used to land in the middle of a two byte character and panic. + #[test] + fn odd_indent_size() { + let mut buffer = Vec::new(); + let mut writer = Writer::new_with_indent(&mut buffer, 0xA0, 1); + + let start = BytesStart::new("paired"); + writer + .write_event(Event::Start(start.clone())) + .expect("write start tag failed"); + writer + .write_event(Event::Empty(BytesStart::new("inner"))) + .expect("write inner tag failed"); + writer + .write_event(Event::End(start.to_end())) + .expect("write end tag failed"); + + assert_eq!( + std::str::from_utf8(&buffer).unwrap(), + format!("\n{i}\n", i = NBSP) + ); + } + + /// Attribute indent is added on top of the current element indent, so the + /// conversion must apply to the added level only, not to the sum. + #[test] + fn in_attributes_nested() { + let mut buffer = Vec::new(); + let mut writer = Writer::new_with_indent(&mut buffer, 0xA0, 2); + + let outer = BytesStart::new("outer"); + writer + .write_event(Event::Start(outer.clone())) + .expect("write outer tag failed"); + writer + .create_element("element") + .new_line() + .with_attribute(("first", "1")) + .write_empty() + .expect("write tag failed"); + writer + .write_event(Event::End(outer.to_end())) + .expect("write end tag failed"); + + let i = NBSP.to_string(); + assert_eq!( + std::str::from_utf8(&buffer).unwrap(), + format!( + "\n{i2}\n", + i2 = i.repeat(2), + i4 = i.repeat(4) + ) + ); + } + + /// ASCII indents must keep working exactly as before. + #[test] + fn ascii_is_unchanged() { + let mut buffer = Vec::new(); + let mut writer = Writer::new_with_indent(&mut buffer, b' ', 2); + + let start = BytesStart::new("paired"); + writer + .write_event(Event::Start(start.clone())) + .expect("write start tag failed"); + writer + .write_event(Event::Empty(BytesStart::new("inner"))) + .expect("write inner tag failed"); + writer + .write_event(Event::End(start.to_end())) + .expect("write end tag failed"); + + assert_eq!( + std::str::from_utf8(&buffer).unwrap(), + "\n \n" + ); + } + + /// `Serializer::indent` takes a `char`, so it must not be truncated to a byte. + #[cfg(feature = "serialize")] + #[test] + fn serializer_indent_char_is_not_truncated() { + use quick_xml::se::Serializer; + use serde::Serialize; + + #[derive(Serialize)] + struct Response { + message: &'static str, + } + + // IDEOGRAPHIC SPACE: three bytes in UTF-8, `as u8` truncates it to NUL. + let mut output = String::new(); + let mut serializer = Serializer::with_root(&mut output, Some("response")).unwrap(); + serializer.indent('\u{3000}', 2); + Response { message: "Success" } + .serialize(serializer) + .unwrap(); + + assert_eq!( + output, + "\n\u{3000}\u{3000}Success\n" + ); + } +}