Fix #597: Pop namespace scope after NsReader::read_to_end

Changelog.md

@@ -16,12 +16,19 @@
 
 ### New Features
 
+- [#598]: Add method `NamespaceResolver::set_level` which may be helpful in som circumstances.
+
 ### Bug Fixes
 
+- [#597]: Fix incorrect processing of namespace scopes in `NsReader::read_to_end`
+  `NsReader::read_to_end_into`, `NsReader::read_to_end_into_async` and `NsReader::read_text`.
+  The scope started by a start element was not ended after that call.
 - [#936]: Fix incorrect result of `.read_text()` when it is called after reading `Text` or `GeneralRef` event.
 
 ### Misc Changes
 
+[#597]: https://github.com/tafia/quick-xml/issues/597
+[#598]: https://github.com/tafia/quick-xml/pull/598
 [#936]: https://github.com/tafia/quick-xml/pull/936
 
 

src/name.rs

@@ -5,7 +5,7 @@
 
 use crate::events::attributes::Attribute;
 use crate::events::{BytesStart, Event};
-use crate::utils::write_byte_string;
+use crate::utils::{write_byte_string, Bytes};
 use memchr::memchr;
 use std::fmt::{self, Debug, Formatter};
 use std::iter::FusedIterator;
@@ -480,7 +480,7 @@ impl NamespaceBinding {
 /// prefixes into namespaces.
 ///
 /// Holds all internal logic to push/pop namespaces with their levels.
-#[derive(Debug, Clone)]
+#[derive(Clone)]
 pub struct NamespaceResolver {
     /// Buffer that contains names of namespace prefixes (the part between `xmlns:`
     /// and an `=`) and namespace values.
@@ -492,6 +492,16 @@ pub struct NamespaceResolver {
     nesting_level: u16,
 }
 
+impl Debug for NamespaceResolver {
+    fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result {
+        f.debug_struct("NamespaceResolver")
+            .field("buffer", &Bytes(&self.buffer))
+            .field("bindings", &self.bindings)
+            .field("nesting_level", &self.nesting_level)
+            .finish()
+    }
+}
+
 /// That constant define the one of [reserved namespaces] for the xml standard.
 ///
 /// The prefix `xml` is by definition bound to the namespace name
@@ -674,11 +684,54 @@ impl NamespaceResolver {
     /// last call to [`Self::push()`] and [`Self::add()`].
     ///
     /// [namespace bindings]: https://www.w3.org/TR/xml-names11/#dt-NSDecl
+    #[inline]
     pub fn pop(&mut self) {
-        self.nesting_level = self.nesting_level.saturating_sub(1);
-        let current_level = self.nesting_level;
+        self.set_level(self.nesting_level.saturating_sub(1));
+    }
+
+    /// 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`]
+    /// will be called until the level reaches the corresponding value.
+    ///
+    /// When set to value bigger than current [`level`] just increases internal
+    /// counter. You may need to call [`pop`] more times that required before.
+    ///
+    /// # Example
+    ///
+    /// ```
+    /// # use pretty_assertions::assert_eq;
+    /// # use quick_xml::events::BytesStart;
+    /// # use quick_xml::name::{Namespace, NamespaceResolver, PrefixDeclaration, QName, ResolveResult};
+    /// #
+    /// let mut resolver = NamespaceResolver::default();
+    ///
+    /// assert_eq!(resolver.level(), 0);
+    ///
+    /// resolver.push(&BytesStart::new("tag"));
+    /// assert_eq!(resolver.level(), 1);
+    ///
+    /// resolver.set_level(10);
+    /// assert_eq!(resolver.level(), 10);
+    ///
+    /// resolver.pop();
+    /// assert_eq!(resolver.level(), 9);
+    ///
+    /// resolver.set_level(0);
+    /// assert_eq!(resolver.level(), 0);
+    ///
+    /// // pop from empty resolver does nothing
+    /// resolver.pop();
+    /// assert_eq!(resolver.level(), 0);
+    /// ```
+    ///
+    /// [`push`]: Self::push
+    /// [`pop`]: Self::pop
+    /// [`level`]: Self::level
+    pub fn set_level(&mut self, level: u16) {
+        self.nesting_level = level;
         // from the back (most deeply nested scope), look for the first scope that is still valid
-        match self.bindings.iter().rposition(|n| n.level <= current_level) {
+        match self.bindings.iter().rposition(|n| n.level <= level) {
             // none of the namespaces are valid, remove all of them
             None => {
                 self.buffer.clear();

src/reader/async_tokio.rs

@@ -337,7 +337,11 @@ impl<R: AsyncBufRead + Unpin> NsReader<R> {
     ) -> Result<Span> {
         // According to the https://www.w3.org/TR/xml11/#dt-etag, end name should
         // match literally the start name. See `Config::check_end_names` documentation
-        self.reader.read_to_end_into_async(end, buf).await
+        let result = self.reader.read_to_end_into_async(end, buf).await?;
+        // read_to_end_into_async 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(result)
     }
 
     /// An asynchronous version of [`read_resolved_event_into()`]. Reads the next

src/reader/ns_reader.rs

@@ -23,7 +23,7 @@ pub struct NsReader<R> {
     /// An XML reader
     pub(super) reader: Reader<R>,
     /// A buffer to manage namespaces
-    ns_resolver: NamespaceResolver,
+    pub(super) 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.
@@ -604,7 +604,11 @@ impl<R: BufRead> NsReader<R> {
     pub fn read_to_end_into(&mut self, end: QName, buf: &mut Vec<u8>) -> Result<Span> {
         // According to the https://www.w3.org/TR/xml11/#dt-etag, end name should
         // match literally the start name. See `Config::check_end_names` documentation
-        self.reader.read_to_end_into(end, buf)
+        let result = self.reader.read_to_end_into(end, buf)?;
+        // read_to_end_into 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(result)
     }
 }
 
@@ -840,7 +844,11 @@ impl<'i> NsReader<&'i [u8]> {
     pub fn read_to_end(&mut self, end: QName) -> Result<Span> {
         // According to the https://www.w3.org/TR/xml11/#dt-etag, end name should
         // match literally the start name. See `Config::check_end_names` documentation
-        self.reader.read_to_end(end)
+        let result = self.reader.read_to_end(end)?;
+        // 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(result)
     }
 
     /// Reads content between start and end tags, including any markup. This
@@ -910,7 +918,13 @@ impl<'i> NsReader<&'i [u8]> {
     /// [`decoder()`]: Reader::decoder()
     #[inline]
     pub fn read_text(&mut self, end: QName) -> Result<Cow<'i, str>> {
-        self.reader.read_text(end)
+        // According to the https://www.w3.org/TR/xml11/#dt-etag, end name should
+        // match literally the start name. See `Self::check_end_names` documentation
+        let result = self.reader.read_text(end)?;
+        // read_text 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(result)
     }
 }
 

src/reader/state.rs

@@ -1,3 +1,5 @@
+use std::fmt::Debug;
+
 #[cfg(feature = "encoding")]
 use encoding_rs::UTF_8;
 
@@ -8,12 +10,12 @@ use crate::parser::{Parser, PiParser};
 #[cfg(feature = "encoding")]
 use crate::reader::EncodingRef;
 use crate::reader::{BangType, Config, DtdParser, ParseState};
-use crate::utils::{is_whitespace, name_len};
+use crate::utils::{is_whitespace, name_len, Bytes};
 
 /// A struct that holds a current reader state and a parser configuration.
 /// It is independent on a way of reading data: the reader feed data into it and
 /// get back produced [`Event`]s.
-#[derive(Clone, Debug)]
+#[derive(Clone)]
 pub(super) struct ReaderState {
     /// Number of bytes read from the source of data since the reader was created
     pub offset: u64,
@@ -373,3 +375,21 @@ impl Default for ReaderState {
         }
     }
 }
+
+impl Debug for ReaderState {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        let mut d = f.debug_struct("ReaderState");
+
+        d.field("offset", &self.offset);
+        d.field("last_error_offset", &self.last_error_offset);
+        d.field("state", &self.state);
+        d.field("config", &self.config);
+        d.field("opened_buffer", &Bytes(&self.opened_buffer));
+        d.field("opened_starts", &self.opened_starts);
+
+        #[cfg(feature = "encoding")]
+        d.field("encoding", &self.encoding);
+
+        d.finish()
+    }
+}

tests/issues.rs

@@ -8,8 +8,8 @@ use std::sync::mpsc;
 
 use quick_xml::errors::{Error, IllFormedError, SyntaxError};
 use quick_xml::events::{BytesDecl, BytesEnd, BytesStart, BytesText, Event};
-use quick_xml::name::QName;
-use quick_xml::reader::Reader;
+use quick_xml::name::{Namespace, QName, ResolveResult};
+use quick_xml::reader::{NsReader, Reader};
 use quick_xml::utils::Bytes;
 
 use pretty_assertions::assert_eq;
@@ -190,6 +190,39 @@ fn issue590() {
     }
 }
 
+#[test]
+fn issue597() {
+    const S: &'static str = r#"
+    <?xml version="1.0" encoding="UTF-8"?>
+    <oval_definitions xmlns="http://oval.mitre.org/XMLSchema/oval-definitions-5">
+        <tests>
+            <xmlfilecontent_test xmlns="http://oval.mitre.org/XMLSchema/oval-definitions-5#independent">
+            </xmlfilecontent_test>
+            <xmlfilecontent_test xmlns="http://oval.mitre.org/XMLSchema/oval-definitions-5#independent">
+            </xmlfilecontent_test>
+        </tests>
+        <objects/>
+    </oval_definitions>"#;
+
+    let mut reader = NsReader::from_str(S);
+    let objects_ns = loop {
+        let (ns, ev) = reader.read_resolved_event().unwrap();
+        match ev {
+            Event::Start(v) if v.local_name().as_ref() == b"xmlfilecontent_test" => {
+                reader.read_to_end(v.name()).unwrap();
+            }
+            Event::Empty(v) if v.local_name().as_ref() == b"objects" => break ns,
+            _ => (),
+        }
+    };
+    assert_eq!(
+        objects_ns,
+        ResolveResult::Bound(Namespace(
+            b"http://oval.mitre.org/XMLSchema/oval-definitions-5"
+        ))
+    );
+}
+
 /// Regression test for https://github.com/tafia/quick-xml/issues/604
 mod issue604 {
     use super::*;