From c71c9c25329a59afd2de647f9251cdcef18bf84e Mon Sep 17 00:00:00 2001 From: Mingun Date: Sat, 15 Aug 2026 19:20:22 +0500 Subject: [PATCH 1/7] Unify implementation of read_to_end across overlapped-lists feature --- src/de/mod.rs | 30 ++++++++++++------------------ 1 file changed, 12 insertions(+), 18 deletions(-) diff --git a/src/de/mod.rs b/src/de/mod.rs index 973e70d0..8a2640b1 100644 --- a/src/de/mod.rs +++ b/src/de/mod.rs @@ -2798,14 +2798,19 @@ where } } + #[cfg(feature = "overlapped-lists")] + fn take_peeked(&mut self) -> Option> { + self.read.pop_front() + } + + #[cfg(not(feature = "overlapped-lists"))] + fn take_peeked(&mut self) -> Option> { + self.peek.take() + } + fn next(&mut self) -> Result, DeError> { // Replay skipped or peeked events - #[cfg(feature = "overlapped-lists")] - if let Some(event) = self.read.pop_front() { - return Ok(event); - } - #[cfg(not(feature = "overlapped-lists"))] - if let Some(e) = self.peek.take() { + if let Some(e) = self.take_peeked() { return Ok(e); } self.reader.next() @@ -2993,11 +2998,10 @@ where /// Drops all events until event with [name](BytesEnd::name()) `name` won't be /// dropped. This method should be called after [`Self::next()`] - #[cfg(feature = "overlapped-lists")] fn read_to_end(&mut self, name: QName) -> Result<(), DeError> { let mut depth = 0; loop { - match self.read.pop_front() { + match self.take_peeked() { Some(DeEvent::Start(e)) if e.name() == name => { depth += 1; } @@ -3037,16 +3041,6 @@ where } Ok(()) } - #[cfg(not(feature = "overlapped-lists"))] - fn read_to_end(&mut self, name: QName) -> Result<(), DeError> { - // First one might be in self.peek - match self.next()? { - DeEvent::Start(e) => self.reader.read_to_end(e.name())?, - DeEvent::End(e) if e.name() == name => return Ok(()), - _ => (), - } - self.reader.read_to_end(name) - } fn skip_next_tree(&mut self) -> Result<(), DeError> { let DeEvent::Start(start) = self.next()? else { From 02e603333cecdce02e7b1ba9c6de7e4c285ddd87 Mon Sep 17 00:00:00 2001 From: Mingun Date: Fri, 14 Aug 2026 00:48:46 +0500 Subject: [PATCH 2/7] Rewrite `deserialize_option` using new helper function Helper will become more complicated in the next commits, so introduce it to keep only one place with such complexness --- src/de/map.rs | 29 ++++---------------- src/de/mod.rs | 76 +++++++++++++++++++++++++++++++++++++++++++-------- 2 files changed, 70 insertions(+), 35 deletions(-) diff --git a/src/de/map.rs b/src/de/map.rs index 324c6130..c4e20861 100644 --- a/src/de/map.rs +++ b/src/de/map.rs @@ -221,21 +221,6 @@ where }) } - /// Determines if subtree started with the specified event should be skipped. - /// - /// Used to map elements with `xsi:nil` attribute set to true to `None` in optional contexts. - /// - /// We need to handle two attributes: - /// - on parent element: `` - /// - on this element: `` - /// - /// We check parent element too because `xsi:nil` affects only nested elements of the - /// tag where it is defined. We can map structure with fields mapped to attributes to - /// the `` element and set to `None` all its optional elements. - fn should_skip_subtree(&self, start: &BytesStart) -> bool { - self.de.reader.reader.has_nil_attr(&self.start) || self.de.reader.reader.has_nil_attr(start) - } - /// Skips whitespaces when they are not preserved #[inline] fn skip_whitespaces(&mut self) -> Result<(), DeError> { @@ -577,15 +562,11 @@ where where V: Visitor<'de>, { - // We cannot use result of `peek()` directly because of borrow checker - let _ = self.map.de.peek()?; - match self.map.de.last_peeked() { - DeEvent::Text(t) if t.is_empty() => visitor.visit_none(), - DeEvent::Start(start) if self.map.should_skip_subtree(start) => { - self.map.de.skip_next_tree()?; - visitor.visit_none() - } - _ => visitor.visit_some(self), + let has_nil = self.map.de.reader.reader.has_nil_attr(&self.map.start); + if self.map.de.deserialize_opt(Some(has_nil))? { + visitor.visit_some(self) + } else { + visitor.visit_none() } } diff --git a/src/de/mod.rs b/src/de/mod.rs index 8a2640b1..2997da86 100644 --- a/src/de/mod.rs +++ b/src/de/mod.rs @@ -3050,6 +3050,67 @@ where self.read_to_end(name) } + /// Determines if `Option` should be deserialized as `Some` or `None`. + /// + /// It handles `xsi:nil` attribute in two places: + /// - on parent element: `` + /// - on checked element: `` + /// + /// According to the [specification], `xsi:nil` controls only ability to (not) have nested + /// elements, but it does not applied to attributes: + /// + /// > 2.7.2 xsi:nil + /// > ------------- + /// > + /// > _XML Schema Definition Language: Structures_ introduces a mechanism for signaling that + /// > an element must be accepted as ·valid· when it has no content despite a content type + /// > which does not require or even necessarily allow empty content. An element can be + /// > ·valid· without content if it has the attribute `xsi:nil` with the value `true`. + /// > An element so labeled must be empty, but can carry attributes if permitted by the + /// > corresponding complex type. + /// + /// Due to that we must deserialize all attributes from the ``. + /// To get an access to them we define Rust struct as follow: + /// + /// ```ignore + /// struct MapTag { + /// #[serde(rename = "@attr")] + /// attr: String, + /// // element + /// opt: Option, + /// } + /// ``` + /// + /// `` should be deserialized as + /// `MapTag { attr: "value", foo: None }`. + /// + /// `` is invalid XML (see the quote from + /// the specification above), but will be deserialized the same. + /// + /// When we at top-level, `parent` is `None` and we handle only the `` case. + /// + /// Returns `true` if `visit_some()` should be called and `false` if `visit_none()`. + /// + /// [specification]: https://www.w3.org/TR/xmlschema11-1/#Instance_Document_Constructions + fn deserialize_opt(&mut self, parent_is_nil: Option) -> Result { + // We cannot use result of `peek()` directly because of borrow checker + let _ = self.peek()?; + Ok(match self.last_peeked() { + DeEvent::Text(t) if t.is_empty() => false, + // If we inside the tree, call visit_some for Eof to get an error from the visitor + // (getting Eof means that XML tag is not closed). On top-level Eof is true None + DeEvent::Eof => parent_is_nil.is_some(), + // if the `xsi:nil` attribute is set to true we got a none value + DeEvent::Start(start) + if parent_is_nil.unwrap_or(false) || self.reader.reader.has_nil_attr(start) => + { + self.skip_next_tree()?; + false + } + _ => true, + }) + } + /// Method for testing Deserializer implementation. Checks that all events was consumed during /// deserialization. Panics if the next event will not be [`DeEvent::Eof`]. #[doc(hidden)] @@ -3364,17 +3425,10 @@ where where V: Visitor<'de>, { - // We cannot use result of `peek()` directly because of borrow checker - let _ = self.peek()?; - match self.last_peeked() { - DeEvent::Text(t) if t.is_empty() => visitor.visit_none(), - DeEvent::Eof => visitor.visit_none(), - // if the `xsi:nil` attribute is set to true we got a none value - DeEvent::Start(start) if self.reader.reader.has_nil_attr(start) => { - self.skip_next_tree()?; - visitor.visit_none() - } - _ => visitor.visit_some(self), + if self.deserialize_opt(None)? { + visitor.visit_some(self) + } else { + visitor.visit_none() } } From 3711cb2aa3b8f9dcafa1b8db84c2ce353a6c5458 Mon Sep 17 00:00:00 2001 From: Mingun Date: Fri, 14 Aug 2026 00:53:35 +0500 Subject: [PATCH 3/7] Get rid of `last_peeked()`, inline `peek()` In next commits `last_peeked()` cannot be used due to starting violating borrow checker rules --- src/de/mod.rs | 40 +++++++++++++++++++++------------------- 1 file changed, 21 insertions(+), 19 deletions(-) diff --git a/src/de/mod.rs b/src/de/mod.rs index 2997da86..74e48878 100644 --- a/src/de/mod.rs +++ b/src/de/mod.rs @@ -2770,6 +2770,7 @@ where return Ok(event); } // SAFETY: `self.read` was filled in the code above. + // NOTE: with msrv=1.95 we may use push_front_mut // NOTE: Can be replaced with `unsafe { std::hint::unreachable_unchecked() }` // if unsafe code will be allowed unreachable!() @@ -2782,22 +2783,6 @@ where } } - #[inline] - fn last_peeked(&self) -> &DeEvent<'de> { - #[cfg(feature = "overlapped-lists")] - { - self.read - .front() - .expect("`Deserializer::peek()` should be called") - } - #[cfg(not(feature = "overlapped-lists"))] - { - self.peek - .as_ref() - .expect("`Deserializer::peek()` should be called") - } - } - #[cfg(feature = "overlapped-lists")] fn take_peeked(&mut self) -> Option> { self.read.pop_front() @@ -3093,9 +3078,26 @@ where /// /// [specification]: https://www.w3.org/TR/xmlschema11-1/#Instance_Document_Constructions fn deserialize_opt(&mut self, parent_is_nil: Option) -> Result { - // We cannot use result of `peek()` directly because of borrow checker - let _ = self.peek()?; - Ok(match self.last_peeked() { + // We cannot use result of `peek()` directly because of borrow checker, so it's inlined here + #[cfg(feature = "overlapped-lists")] + let event = { + if self.read.is_empty() { + self.read.push_front(self.reader.next()?); + } + // SAFETY: `self.read` was filled in the code above. + // NOTE: with msrv=1.95 we may use push_front_mut + self.read + .front() + .expect("`self.read` was filled in the code above") + }; + + #[cfg(not(feature = "overlapped-lists"))] + let event = match &mut self.peek { + Some(event) => event, + empty_peek @ None => empty_peek.insert(self.reader.next()?), + }; + + Ok(match event { DeEvent::Text(t) if t.is_empty() => false, // If we inside the tree, call visit_some for Eof to get an error from the visitor // (getting Eof means that XML tag is not closed). On top-level Eof is true None From ce37aca192914a9af1ddc575f61e9a6663ece48b Mon Sep 17 00:00:00 2001 From: Mingun Date: Sat, 15 Aug 2026 21:33:39 +0500 Subject: [PATCH 4/7] Inline skip_next_tree - now it is used only in one place --- src/de/mod.rs | 13 ++++--------- 1 file changed, 4 insertions(+), 9 deletions(-) diff --git a/src/de/mod.rs b/src/de/mod.rs index 74e48878..66699491 100644 --- a/src/de/mod.rs +++ b/src/de/mod.rs @@ -3027,14 +3027,6 @@ where Ok(()) } - fn skip_next_tree(&mut self) -> Result<(), DeError> { - let DeEvent::Start(start) = self.next()? else { - unreachable!("Only call this if the next event is a start event") - }; - let name = start.name(); - self.read_to_end(name) - } - /// Determines if `Option` should be deserialized as `Some` or `None`. /// /// It handles `xsi:nil` attribute in two places: @@ -3106,7 +3098,10 @@ where DeEvent::Start(start) if parent_is_nil.unwrap_or(false) || self.reader.reader.has_nil_attr(start) => { - self.skip_next_tree()?; + let DeEvent::Start(start) = self.next()? else { + unreachable!("Just checked that the next event is a start event") + }; + self.read_to_end(start.name())?; false } _ => true, From 26b63a6069a47b5a51fa625c574df388a3282afd Mon Sep 17 00:00:00 2001 From: Mingun Date: Tue, 28 Apr 2026 23:37:16 +0500 Subject: [PATCH 5/7] Add regression test for #953 (== err) and new tests for leaking of namespace bindings failures (8): serde-de-xsi (6): as_field::nested_nil::field::nil_on_parent as_field::nested_nil::field::nil_on_self as_field::nested_nil::value::nil_on_parent as_field::nested_nil::value::nil_on_self top_level_option::nested_nil::string top_level_option::nested_nil::unit serde-issues (2): issue953::open_close issue953::self_closed --- tests/serde-de-xsi.rs | 207 ++++++++++++++++++++++++++++++++++++++++++ tests/serde-issues.rs | 53 +++++++++++ 2 files changed, 260 insertions(+) diff --git a/tests/serde-de-xsi.rs b/tests/serde-de-xsi.rs index 31350c94..a2ff454c 100644 --- a/tests/serde-de-xsi.rs +++ b/tests/serde-de-xsi.rs @@ -239,6 +239,43 @@ mod top_level_option { } } } + + /// Namespace binding from nested element must not leak into the parent element + mod nested_nil { + use super::*; + use pretty_assertions::assert_eq; + + #[test] + fn unit() { + assert_eq!( + from_str::>( + "\ + \ + \ + \ + " + ) + .unwrap(), + Some(()) + ); + } + + #[test] + fn string() { + assert!(matches!( + from_str::>( + "\ + \ + \ + text\ + \ + " + ) + .unwrap_err(), + DeError::MixedContent(x) if x == "any" + )); + } + } } mod as_field { @@ -701,4 +738,174 @@ mod as_field { } } } + + /// Namespace binding from nested element must not leak into the parent element + mod nested_nil { + use super::*; + use pretty_assertions::assert_eq; + + mod field { + use super::*; + use pretty_assertions::assert_eq; + + #[derive(Debug, Deserialize, PartialEq)] + struct Xml { + value: Option<()>, + } + + #[test] + fn nil_on_parent() { + assert_eq!( + from_str::( + "\ + \ + \ + \ + " + ) + .unwrap(), + Xml { value: Some(()) } + ); + } + + #[test] + fn nil_on_self() { + assert_eq!( + from_str::( + "\ + \ + \ + \ + \ + \ + " + ) + .unwrap(), + Xml { value: Some(()) } + ); + } + } + + #[test] + fn text() { + #[derive(Debug, Deserialize, PartialEq)] + struct Xml { + #[serde(rename = "$text")] + value: Option<&'static str>, + } + assert_eq!( + from_str::( + "\ + \ + \ + text\ + \ + " + ) + .unwrap(), + Xml { + value: Some("text"), + } + ); + } + + mod value { + use super::*; + use pretty_assertions::assert_eq; + + #[derive(Debug, Deserialize, PartialEq)] + struct Xml { + #[serde(rename = "$value")] + value: Option<()>, + } + + #[test] + fn nil_on_parent() { + assert_eq!( + from_str::( + "\ + \ + \ + \ + " + ) + .unwrap(), + Xml { value: Some(()) } + ); + } + + #[test] + fn nil_on_self() { + assert_eq!( + from_str::( + "\ + \ + \ + \ + \ + \ + " + ) + .unwrap(), + Xml { value: Some(()) } + ); + } + } + + /// Namespace binding from the previous element or its child must not leak into the next element + /// + /// Reaches `Deserializer::read_to_end`, because it is used to skip subtree that was mapped + /// to unit struct. + mod unit { + use super::*; + use pretty_assertions::assert_eq; + + #[derive(Debug, Deserialize, PartialEq)] + struct Xml { + unit: (), + some: Option<()>, + } + + #[test] + fn nil_on_parent() { + assert_eq!( + from_str::( + "\ + \ + \ + \ + \ + " + ) + .unwrap(), + Xml { + unit: (), + some: Some(()), + } + ); + } + + #[test] + fn nil_on_self() { + assert_eq!( + from_str::( + "\ + \ + \ + \ + \ + \ + \ + \ + " + ) + .unwrap(), + Xml { + unit: (), + some: Some(()), + } + ); + } + } + } } diff --git a/tests/serde-issues.rs b/tests/serde-issues.rs index 6e8b2d7b..6320346f 100644 --- a/tests/serde-issues.rs +++ b/tests/serde-issues.rs @@ -931,6 +931,59 @@ fn issue928() { ); } +/// Regression test for https://github.com/tafia/quick-xml/issues/953. +mod issue953 { + use super::*; + use pretty_assertions::assert_eq; + + #[derive(Debug, Deserialize, PartialEq)] + struct Root { + vec: Vec<()>, + opt: Option, + } + + #[derive(Debug, Deserialize, PartialEq)] + struct Value { + field: (), + } + + #[test] + fn open_close() { + let input = r#" + + + + + "#; + let value: Root = from_str(&input).unwrap(); + assert_eq!( + value, + Root { + vec: vec![()], + opt: None, + } + ); + } + + #[test] + fn self_closed() { + let input = r#" + + + + + "#; + let value: Root = from_str(&input).unwrap(); + assert_eq!( + value, + Root { + vec: vec![()], + opt: None, + } + ); + } +} + /// Regression tests for https://github.com/tafia/quick-xml/issues/978. /// /// Deeply nested XML should produce `DeError::TooDeeplyNested` instead of From f48f778b5cfd4d1fd1a8552557375450bb65692d Mon Sep 17 00:00:00 2001 From: Mingun Date: Sat, 15 Aug 2026 00:14:09 +0500 Subject: [PATCH 6/7] Check the nesting level of NamespaceResolver after deserialization - it must be 0 failures (20, +14): serde-de (1): unit::excess_element serde-de-seq (10 - only for "overlapped-lists" feature): variable_name::fixed_size::field_after_list::overlapped_with_nested_list variable_name::fixed_size::field_before_list::overlapped_with_nested_list variable_name::fixed_size::two_lists::choice_and_fixed::overlapped::with_nested_list_fixed_after variable_name::fixed_size::two_lists::choice_and_fixed::overlapped::with_nested_list_fixed_before variable_name::fixed_size::two_lists::fixed_and_choice::overlapped::with_nested_list_fixed_after variable_name::fixed_size::two_lists::fixed_and_choice::overlapped::with_nested_list_fixed_before variable_name::variable_size::field_after_list::overlapped_with_nested_list variable_name::variable_size::field_before_list::overlapped_with_nested_list variable_name::variable_size::two_lists::choice_and_fixed::overlapped::with_nested_list_fixed_after variable_name::variable_size::two_lists::fixed_and_choice::overlapped::with_nested_list_fixed_after serde-de-xsi (9, +3): as_field::nested_nil::field::nil_on_parent as_field::nested_nil::field::nil_on_self as_field::nested_nil::value::nil_on_parent as_field::nested_nil::value::nil_on_self as_field::with_element::nested::true_ top_level_option::nested_nil::string top_level_option::nested_nil::unit top_level_option::with_element::ns0::true_ top_level_option::with_element::xsi::true_ --- tests/serde_helpers/mod.rs | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/tests/serde_helpers/mod.rs b/tests/serde_helpers/mod.rs index 637e732b..647ef818 100644 --- a/tests/serde_helpers/mod.rs +++ b/tests/serde_helpers/mod.rs @@ -1,5 +1,6 @@ //! Utility functions for serde integration tests +use pretty_assertions::assert_eq; use quick_xml::de::Deserializer; use quick_xml::DeError; use serde::Deserialize; @@ -19,6 +20,14 @@ where // If type was deserialized, the whole XML document should be consumed if result.is_ok() { de.check_eof_reached(); + + let resolver = de.get_ref().get_ref().resolver(); + assert_eq!( + resolver.level(), + 0, + "all namespace bindings must be popped, level should be zero: {:#?}", + resolver + ); } result From a82193092e84b018b353a9f9fdbc4f77f5fc3664 Mon Sep 17 00:00:00 2001 From: Mingun Date: Fri, 14 Aug 2026 01:09:06 +0500 Subject: [PATCH 7/7] Handle namespaces only when events actually consumed in Deserializer Co-authored-by: Daniel Alley --- Changelog.md | 12 ++++ src/de/map.rs | 7 ++- src/de/mod.rs | 117 ++++++++++++++++++++++++++----------- src/errors.rs | 7 +++ src/name.rs | 12 ++++ src/reader/ns_reader.rs | 6 +- tests/serde_helpers/mod.rs | 9 ++- 7 files changed, 130 insertions(+), 40 deletions(-) diff --git a/Changelog.md b/Changelog.md index 205e5ecb..26cac81c 100644 --- a/Changelog.md +++ b/Changelog.md @@ -68,6 +68,9 @@ The MSRV has been raised to 1.86. as ` `, ` `, and ` ` respectively, preventing silent data loss from XML attribute-value normalization on round-trip. Likewise `Attribute::from` performs the same transformation. +- [#953]: The serde `Deserializer` now correctly handles namespaces. Previously + the namespace bindings might be applied or removed before the event actually + was consumed which lead to a couple of bugs. - [#989]: `Attributes::new` and `Attributes::html` now return empty iterators when their starting position is past the end of the input instead of panicking. - [#977]: `NamespaceResolver::push` (and hence every `NsReader` `Start`/`Empty` @@ -99,9 +102,17 @@ The MSRV has been raised to 1.86. `decoded_and_normalized_value_with()`, `decode_and_unescape_value()`, and `decode_and_unescape_value_with()`. Use `normalized_value()` and `normalized_value_with()` instead. +- [#1002]: Added `NamespaceResolver::with` that allows temporary applying namespace + bindings from the start tag for the scope of a provided closure F, without making any + persistent change to the resolver. It is useful to check a peeked event which is + not yet consumed in custom implementations of peekable reader. +- [#1002]: Added `Deserializer::resolver` and `Deserializer::resolver_mut` methods + to get a namespace resolver used by this deserializer, because it no longer uses + an `NsReader` internally. [#670]: https://github.com/tafia/quick-xml/issues/670 [#859]: https://github.com/tafia/quick-xml/issues/859 +[#953]: https://github.com/tafia/quick-xml/issues/953 [#963]: https://github.com/tafia/quick-xml/pull/963 [#977]: https://github.com/tafia/quick-xml/issues/977 [#978]: https://github.com/tafia/quick-xml/issues/978 @@ -110,6 +121,7 @@ The MSRV has been raised to 1.86. [#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 +[#1002]: https://github.com/tafia/quick-xml/pull/1002 ## 0.41.0 -- 2026-06-29 diff --git a/src/de/map.rs b/src/de/map.rs index c4e20861..63a14226 100644 --- a/src/de/map.rs +++ b/src/de/map.rs @@ -562,7 +562,12 @@ where where V: Visitor<'de>, { - let has_nil = self.map.de.reader.reader.has_nil_attr(&self.map.start); + // `self.map.start` already was taken from the reader, so its namespace bindings were already processed. + let has_nil = self + .map + .start + .attributes() + .has_nil(&self.map.de.ns_resolver); if self.map.de.deserialize_opt(Some(has_nil))? { visitor.visit_some(self) } else { diff --git a/src/de/mod.rs b/src/de/mod.rs index 66699491..51cc4885 100644 --- a/src/de/mod.rs +++ b/src/de/mod.rs @@ -2114,8 +2114,8 @@ use crate::{ errors::Error, escape::{parse_number, EscapeError}, events::{BytesCData, BytesEnd, BytesRef, BytesStart, BytesText, Event}, - name::QName, - reader::NsReader, + name::{NamespaceResolver, QName}, + reader::{NsReader, Reader}, }; use serde::de::{ self, Deserialize, DeserializeOwned, DeserializeSeed, IntoDeserializer, SeqAccess, Visitor, @@ -2536,6 +2536,8 @@ where { /// An XML reader that streams events into this deserializer reader: XmlReader<'de, R, E>, + /// A buffer to manage namespaces + ns_resolver: NamespaceResolver, /// When deserializing sequences sometimes we have to skip unwanted events. /// That events should be stored and then replayed. This is a replay buffer, @@ -2585,9 +2587,10 @@ where /// /// - [`Deserializer::from_str`] /// - [`Deserializer::from_reader`] - fn new(reader: R, entity_resolver: E) -> Self { + fn new(reader: R, ns_resolver: NamespaceResolver, entity_resolver: E) -> Self { Self { reader: XmlReader::new(reader, entity_resolver), + ns_resolver, #[cfg(feature = "overlapped-lists")] read: VecDeque::new(), @@ -2626,7 +2629,7 @@ where /// # use pretty_assertions::assert_eq; /// use serde::Deserialize; /// use quick_xml::de::Deserializer; - /// use quick_xml::NsReader; + /// use quick_xml::Reader; /// /// #[derive(Deserialize)] /// struct SomeStruct { @@ -2643,7 +2646,7 @@ where /// let err = SomeStruct::deserialize(&mut de); /// assert!(err.is_err()); /// - /// let reader: &NsReader<_> = de.get_ref().get_ref(); + /// let reader: &Reader<_> = de.get_ref().get_ref(); /// /// assert_eq!(reader.error_position(), 28); /// assert_eq!(reader.buffer_position(), 41); @@ -2652,6 +2655,22 @@ where &self.reader.reader } + /// Returns a storage of namespace bindings associated with this deserializer. + #[inline] + pub const fn resolver(&self) -> &NamespaceResolver { + &self.ns_resolver + } + + /// Returns a mutable reference to the storage of namespace bindings + /// associated with this deserializer. + /// + /// Useful for configuring the resolver, e.g. to change the + /// [namespace-binding limit](NamespaceResolver::set_max_namespace_bindings). + #[inline] + pub fn resolver_mut(&mut self) -> &mut NamespaceResolver { + &mut self.ns_resolver + } + /// Set the maximum number of events that could be skipped during deserialization /// of sequences. /// @@ -2793,7 +2812,7 @@ where self.peek.take() } - fn next(&mut self) -> Result, DeError> { + fn next_impl(&mut self) -> Result, DeError> { // Replay skipped or peeked events if let Some(e) = self.take_peeked() { return Ok(e); @@ -2801,6 +2820,20 @@ where self.reader.next() } + fn next(&mut self) -> Result, DeError> { + match self.next_impl() { + Ok(DeEvent::Start(e)) => { + self.ns_resolver.push(&e)?; + Ok(DeEvent::Start(e)) + } + Ok(DeEvent::End(e)) => { + self.ns_resolver.pop(); + Ok(DeEvent::End(e)) + } + e => e, + } + } + fn skip_whitespaces(&mut self) -> Result<(), DeError> { loop { match self.peek()? { @@ -3024,6 +3057,9 @@ where } } } + // read_to_end will consume closing tag. Because nobody can access to its + // content anymore, we directly pop namespace of the opening tag + self.ns_resolver.pop(); Ok(()) } @@ -3096,7 +3132,11 @@ where DeEvent::Eof => parent_is_nil.is_some(), // if the `xsi:nil` attribute is set to true we got a none value DeEvent::Start(start) - if parent_is_nil.unwrap_or(false) || self.reader.reader.has_nil_attr(start) => + // Because we only peek event here, its namespace bindings not yet processed. + // Temporary push them inside `with` to check the presence of `xsi:nil` + if parent_is_nil.unwrap_or(false) || self.ns_resolver.with(start, |resolver| { + start.attributes().has_nil(resolver) + })? => { let DeEvent::Start(start) = self.next()? else { unreachable!("Just checked that the next event is a start event") @@ -3189,15 +3229,25 @@ where /// Note, that config option [`Config::expand_empty_elements`] will be set to `true`. /// /// [`Config::expand_empty_elements`]: crate::reader::Config::expand_empty_elements - pub fn borrowing_with_resolver(mut reader: NsReader<&'de [u8]>, entity_resolver: E) -> Self { + pub fn borrowing_with_resolver(reader: NsReader<&'de [u8]>, entity_resolver: E) -> Self { + let NsReader { + mut reader, + mut ns_resolver, + pending_pop, + } = reader; let config = reader.config_mut(); config.expand_empty_elements = true; + if pending_pop { + ns_resolver.pop(); + } + Self::new( SliceReader { reader, version: XmlVersion::Implicit1_0, }, + ns_resolver, entity_resolver, ) } @@ -3271,7 +3321,7 @@ where /// will borrow instead of copy. If you have `&[u8]` which is known to represent /// UTF-8, you can decode it first before using [`from_str`]. pub fn with_resolver(reader: R, entity_resolver: E) -> Self { - let mut reader = NsReader::from_reader(reader); + let mut reader = Reader::from_reader(reader); let config = reader.config_mut(); config.expand_empty_elements = true; @@ -3281,6 +3331,7 @@ where buf: Vec::new(), version: XmlVersion::Implicit1_0, }, + NamespaceResolver::default(), entity_resolver, ) } @@ -3291,16 +3342,26 @@ where /// Note, that config option [`Config::expand_empty_elements`] will be set to `true`. /// /// [`Config::expand_empty_elements`]: crate::reader::Config::expand_empty_elements - pub fn buffering_with_resolver(mut reader: NsReader, entity_resolver: E) -> Self { + pub fn buffering_with_resolver(reader: NsReader, entity_resolver: E) -> Self { + let NsReader { + mut reader, + mut ns_resolver, + pending_pop, + } = reader; let config = reader.config_mut(); config.expand_empty_elements = true; + if pending_pop { + ns_resolver.pop(); + } + Self::new( IoReader { reader, buf: Vec::new(), version: XmlVersion::Implicit1_0, }, + ns_resolver, entity_resolver, ) } @@ -3525,12 +3586,6 @@ pub trait XmlRead<'i> { /// Return an XML version of the source. fn xml_version(&self) -> XmlVersion; - - /// Checks if the `start` tag has a [`xsi:nil`] attribute. This method ignores - /// any errors in attributes. - /// - /// [`xsi:nil`]: https://www.w3.org/TR/xmlschema-1/#xsi_nil - fn has_nil_attr(&self, start: &BytesStart) -> bool; } /// XML input source that reads from a std::io input stream. @@ -3538,7 +3593,7 @@ pub trait XmlRead<'i> { /// You cannot create it, it is created automatically when you call /// [`Deserializer::from_reader`] pub struct IoReader { - reader: NsReader, + reader: Reader, buf: Vec, version: XmlVersion, } @@ -3551,7 +3606,7 @@ impl IoReader { /// use serde::Deserialize; /// use std::io::Cursor; /// use quick_xml::de::Deserializer; - /// use quick_xml::NsReader; + /// use quick_xml::Reader; /// /// #[derive(Deserialize)] /// struct SomeStruct { @@ -3568,12 +3623,12 @@ impl IoReader { /// let err = SomeStruct::deserialize(&mut de); /// assert!(err.is_err()); /// - /// let reader: &NsReader> = de.get_ref().get_ref(); + /// let reader: &Reader> = de.get_ref().get_ref(); /// /// assert_eq!(reader.error_position(), 28); /// assert_eq!(reader.buffer_position(), 41); /// ``` - pub const fn get_ref(&self) -> &NsReader { + pub const fn get_ref(&self) -> &Reader { &self.reader } } @@ -3604,10 +3659,6 @@ impl<'i, R: BufRead> XmlRead<'i> for IoReader { fn xml_version(&self) -> XmlVersion { self.version } - - fn has_nil_attr(&self, start: &BytesStart) -> bool { - start.attributes().has_nil(self.reader.resolver()) - } } /// XML input source that reads from a slice of bytes and can borrow from it. @@ -3615,7 +3666,7 @@ impl<'i, R: BufRead> XmlRead<'i> for IoReader { /// You cannot create it, it is created automatically when you call /// [`Deserializer::from_str`]. pub struct SliceReader<'de> { - reader: NsReader<&'de [u8]>, + reader: Reader<&'de [u8]>, version: XmlVersion, } @@ -3626,7 +3677,7 @@ impl<'de> SliceReader<'de> { /// # use pretty_assertions::assert_eq; /// use serde::Deserialize; /// use quick_xml::de::Deserializer; - /// use quick_xml::NsReader; + /// use quick_xml::Reader; /// /// #[derive(Deserialize)] /// struct SomeStruct { @@ -3643,12 +3694,12 @@ impl<'de> SliceReader<'de> { /// let err = SomeStruct::deserialize(&mut de); /// assert!(err.is_err()); /// - /// let reader: &NsReader<&[u8]> = de.get_ref().get_ref(); + /// let reader: &Reader<&[u8]> = de.get_ref().get_ref(); /// /// assert_eq!(reader.error_position(), 28); /// assert_eq!(reader.buffer_position(), 41); /// ``` - pub const fn get_ref(&self) -> &NsReader<&'de [u8]> { + pub const fn get_ref(&self) -> &Reader<&'de [u8]> { &self.reader } } @@ -3677,10 +3728,6 @@ impl<'de> XmlRead<'de> for SliceReader<'de> { fn xml_version(&self) -> XmlVersion { self.version } - - fn has_nil_attr(&self, start: &BytesStart) -> bool { - start.attributes().has_nil(self.reader.resolver()) - } } #[cfg(test)] @@ -4263,12 +4310,12 @@ mod tests { "#; let mut reader1 = IoReader { - reader: NsReader::from_reader(s.as_bytes()), + reader: Reader::from_reader(s.as_bytes()), buf: Vec::new(), version: XmlVersion::Implicit1_0, }; let mut reader2 = SliceReader { - reader: NsReader::from_str(s), + reader: Reader::from_str(s), version: XmlVersion::Implicit1_0, }; @@ -4294,7 +4341,7 @@ mod tests { "#; let mut reader = SliceReader { - reader: NsReader::from_str(s), + reader: Reader::from_str(s), version: XmlVersion::Implicit1_0, }; diff --git a/src/errors.rs b/src/errors.rs index abcaf885..0982f448 100644 --- a/src/errors.rs +++ b/src/errors.rs @@ -418,6 +418,13 @@ pub mod serialize { } } + impl From for DeError { + #[inline] + fn from(e: NamespaceError) -> Self { + Self::InvalidXml(e.into()) + } + } + /// Serialization error #[derive(Clone, Debug)] pub enum SeError { diff --git a/src/name.rs b/src/name.rs index e38c106a..e3dd1053 100644 --- a/src/name.rs +++ b/src/name.rs @@ -777,6 +777,18 @@ impl NamespaceResolver { self.set_level(self.nesting_level.saturating_sub(1)); } + /// Runs action as if all namespaces from the specified `start` element were added + /// to the resolver, but without actually changing the resolver state. + pub fn with(&mut self, start: &BytesStart, mut action: F) -> Result + where + F: FnMut(&Self) -> R, + { + self.push(start)?; + let result = action(self); + self.pop(); + Ok(result) + } + /// Sets new number of [`push`] calls that were not followed by [`pop`] calls. /// /// When set to value lesser than current [`level`], behaves as if [`pop`] diff --git a/src/reader/ns_reader.rs b/src/reader/ns_reader.rs index 0d164d89..e7b1c6c7 100644 --- a/src/reader/ns_reader.rs +++ b/src/reader/ns_reader.rs @@ -20,13 +20,13 @@ use crate::reader::{Config, Reader, Span, XmlSource}; #[derive(Debug, Clone)] pub struct NsReader { /// An XML reader - pub(super) reader: Reader, + pub(crate) reader: Reader, /// A buffer to manage namespaces - pub(super) ns_resolver: NamespaceResolver, + pub(crate) ns_resolver: NamespaceResolver, /// We cannot pop data from the namespace stack until returned `Empty` or `End` /// event will be processed by the user, so we only mark that we should that /// in the next [`Self::read_event_impl()`] call. - pending_pop: bool, + pub(crate) pending_pop: bool, } /// Builder methods diff --git a/tests/serde_helpers/mod.rs b/tests/serde_helpers/mod.rs index 647ef818..4132239a 100644 --- a/tests/serde_helpers/mod.rs +++ b/tests/serde_helpers/mod.rs @@ -15,13 +15,20 @@ where // Log XML that we try to deserialize to see it in the failed tests output dbg!(source); let mut de = Deserializer::from_str(source); + assert_eq!( + de.resolver().level(), + 0, + "no user namespace bindings expected just after creation: {:#?}", + de.resolver() + ); + let result = T::deserialize(&mut de); // If type was deserialized, the whole XML document should be consumed if result.is_ok() { de.check_eof_reached(); - let resolver = de.get_ref().get_ref().resolver(); + let resolver = de.resolver(); assert_eq!( resolver.level(), 0,