Make `NamespaceResolver` public #893

Mingun · 2025-08-10T10:06:55Z

This introduces small breaking change -- Attributes::has_nil now accepts &NamespaceResolver instead of Reader<R> and does not need to be generic anymore (which is good).

Other changes that may look as breaking in the diff not breaking changes, because they was made on initially private types.

…Iter` name

Mingun · 2025-08-10T10:12:42Z

@faassen want to do a review?

codecov-commenter · 2025-08-10T10:18:26Z

⚠️ Please install the to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

❌ Patch coverage is 67.69231% with 42 lines in your changes missing coverage. Please review.
✅ Project coverage is 58.17%. Comparing base (38b44d4) to head (b12ef9f).
⚠️ Report is 84 commits behind head on master.

Files with missing lines	Patch %	Lines
src/name.rs	67.24%	38 Missing ⚠️
src/reader/ns_reader.rs	63.63%	4 Missing ⚠️
❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #893      +/-   ##
==========================================
+ Coverage   55.52%   58.17%   +2.64%     
==========================================
  Files          42       42              
  Lines       15511    15231     -280     
==========================================
+ Hits         8613     8860     +247     
+ Misses       6898     6371     -527

Flag	Coverage Δ
unittests	`58.17% <67.69%> (+2.64%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

faassen

I think the main blocker for my use cases is a way to push my own namespace bindings without having to construct a BytesStart.

I wanted to mention this use case in addition:

In my codebase I have both OwnedPrefixDeclaration and OwnedNamespace. Their primary use case is to be able to create and store this kind of stuff in configuration for the various operations I need to do over them. So, I need to be able to create them independently from events. Perhaps I overlooked a way to do owned versions of PrefixDeclaration and Namespace themselves.

I'll also note that my OwnedPrefixDeclaration stores the whole xmlns:foo bit, rather than just the prefix like PrefixDeclaration does, because otherwise I need to do allocation each time I want to turn it into a QName, which I want to do when I want to construct an namespace declaration XML attribute from it. Note that this doing this without allocation is a use case not currently supported by quick_xml's PrefixDeclaration either, afaik.

faassen · 2025-08-11T09:30:06Z

src/events/attributes.rs

        self.any(|attr| {
            if let Ok(attr) = attr {
-                match reader.resolve_attribute(attr.key) {
+                match resolver.resolve(attr.key, true) {


I think it would be nice if the resolver gained resolve_element and resolve_attribute as convenience APIs too, mirroring what's on BytesStart.

Ok, will add

faassen · 2025-08-11T09:42:27Z

src/name.rs

    ///
-    /// [namespace binding]: https://www.w3.org/TR/xml-names11/#dt-NSDecl
+    /// [namespace bindings]: https://www.w3.org/TR/xml-names11/#dt-NSDecl
    pub fn push(&mut self, start: &BytesStart) -> Result<(), NamespaceError> {


In c14n I have a use case where I want to filter through the actually utilized namespace declarations and selectively add them on a namespace resolver I manage (and also use the one for the NsReader).

So I'm looking for an API where I can add PrefixDeclaration directly.

I expected a new for NamespaceResolver resolver but there is only a Default. Should we add a new too that calls default? In addition to that I'd like to be able to pass some "top-level" bindings in when I construct the resolver, but I think doing a push immediately after construction should be sufficient to do this, as its nesting level is independent from that of the document anyway as far as I can tell.

So you would like to have this lines extracted into the new add(PrefixDeclaration, Namespace) method?

quick-xml/src/name.rs

Lines 547 to 591 in 73c66f8

match k.as_namespace_binding() {

Some(PrefixDeclaration::Default) => {

let start = self.buffer.len();

self.buffer.extend_from_slice(&v);

self.bindings.push(NamespaceBinding {

start,

prefix_len: 0,

value_len: v.len(),

level,

});

}

Some(PrefixDeclaration::Named(b"xml")) => {

if Namespace(&v) != RESERVED_NAMESPACE_XML.1 {

// error, `xml` prefix explicitly set to different value

return Err(NamespaceError::InvalidXmlPrefixBind(v.to_vec()));

}

// don't add another NamespaceEntry for the `xml` namespace prefix

}

Some(PrefixDeclaration::Named(b"xmlns")) => {

// error, `xmlns` prefix explicitly set

return Err(NamespaceError::InvalidXmlnsPrefixBind(v.to_vec()));

}

Some(PrefixDeclaration::Named(prefix)) => {

let ns = Namespace(&v);

if ns == RESERVED_NAMESPACE_XML.1 {

// error, non-`xml` prefix set to xml uri

return Err(NamespaceError::InvalidPrefixForXml(prefix.to_vec()));

} else if ns == RESERVED_NAMESPACE_XMLNS.1 {

// error, non-`xmlns` prefix set to xmlns uri

return Err(NamespaceError::InvalidPrefixForXmlns(prefix.to_vec()));

}

let start = self.buffer.len();

self.buffer.extend_from_slice(prefix);

self.buffer.extend_from_slice(&v);

self.bindings.push(NamespaceBinding {

start,

prefix_len: prefix.len(),

value_len: v.len(),

level,

});

}

None => {}

}

I doesn't add new because there is already default, but calling that requires that Default trait was in scope. So maybe the new method has the right to life. Actually, I would like to see it constant (which is impossible for trait method implementations yet).

faassen · 2025-08-11T09:50:25Z

src/name.rs

+    /// [`Empty`]: Event::Empty
+    /// [`Start`]: Event::Start
+    /// [`End`]: Event::End
+    pub fn resolve_event<'i>(&self, event: Event<'i>) -> (ResolveResult<'_>, Event<'i>) {


One operation I use is this:

is_declared (given a prefix declaration and namespace, return a boolean if it's declared. this also needs to understand undeclarations, I think quick_xml supports namespaces 1.1 so I think anything could be undeclared?)

I can build this myself but I'm mentioning it in case we want to add this.

Probably .bindings().any() is enough for this?

I think quick_xml supports namespaces 1.1 so I think anything could be undeclared?

Yes, we support 1.1, but I also want to support 1.0 rules, because when XML declaration defined 1.0 document, we 1.1 parser should apply 1.0 rules.

xmlns and xml prefixes cannot be undeclared, it is an error to do that (that is follow from forbidding of they redeclaration with another namespace)

faassen · 2025-08-11T09:57:43Z

src/name.rs

+        use Event::*;
+
+        match event {
+            Empty(e) => (self.resolve_prefix(e.name().prefix(), true), Empty(e)),


It's a minor comment, but initially I responded with, but I need resolve_prefix in the public API! But now I realize it's already exposed indirectly through resolve. Perhaps this code can use resolve rather than resolve_prefix, or we could even merge the two into one - always use the "attribute" bool rather than introduce a use_default concept where the boolean needs to be flipped. (even though the use_default concept is fine by itself).

I used the attribute parameter, because that is what using by the NsReader methods, and I want to be consistent here. But if make resolve_prefix public, we can return to the use_default conception

faassen · 2025-08-11T09:59:19Z

src/reader/ns_reader.rs

    #[inline]
    pub fn resolve_element<'n>(&self, name: QName<'n>) -> (ResolveResult<'_>, LocalName<'n>) {
-        self.ns_resolver.resolve(name, true)
+        self.ns_resolver.resolve(name, false)


The flipping of true to false in this PR in quite a few places is basically because we're not using a single resolve_prefix API with a consistent meaning for the boolean.

…NamespaceResolver`

…f defined bindings on each level

…ethods

Mingun added 3 commits August 10, 2025 02:50

Rename NamespaceEntry to NamespaceBinding

f408ca4

Rename PrefixIter to NamespaceBindingsIter, deprecate the `Prefix…

df814e5

…Iter` name

Implement FusedIterator for NamespaceBindingsIter

6258ecd

Mingun added namespaces Issues related to namespaces support enhancement labels Aug 10, 2025

Mingun force-pushed the public-ns-resolver branch from c1bf733 to 73c66f8 Compare August 10, 2025 19:14

faassen reviewed Aug 11, 2025

View reviewed changes

Mingun added 8 commits August 17, 2025 14:03

Make code of NamespaceBindingsIter slightly more readable

f58b754

Better debug implementation for PrefixDeclaration

4c5fb95

Optimize resolve_prefix

c7e740d

Make NamespaceResolver public and better document it

5ea16e1

Add convenient resolve_element and resolve_attribute methods to `…

cf4f19d

…NamespaceResolver`

Add NamespaceResolver::resolve_event

9f1a557

Add ability to get current nesting level from the resolver and list o…

58bb2b6

…f defined bindings on each level

Extract NamespaceResolver::add from NamespaceResolver::push

54fc5b3

Mingun force-pushed the public-ns-resolver branch from 73c66f8 to 63e192b Compare August 17, 2025 12:34

Use and recommend NamespaceResolver methods instead of NsReader m…

b12ef9f

…ethods

Mingun force-pushed the public-ns-resolver branch from 63e192b to b12ef9f Compare August 19, 2025 16:20

Mingun merged commit b6b9ead into tafia:master Aug 19, 2025
7 checks passed

Mingun deleted the public-ns-resolver branch August 19, 2025 16:37

	match k.as_namespace_binding() {
	Some(PrefixDeclaration::Default) => {
	let start = self.buffer.len();
	self.buffer.extend_from_slice(&v);
	self.bindings.push(NamespaceBinding {
	start,
	prefix_len: 0,
	value_len: v.len(),
	level,
	});
	}
	Some(PrefixDeclaration::Named(b"xml")) => {
	if Namespace(&v) != RESERVED_NAMESPACE_XML.1 {
	// error, `xml` prefix explicitly set to different value
	return Err(NamespaceError::InvalidXmlPrefixBind(v.to_vec()));
	}
	// don't add another NamespaceEntry for the `xml` namespace prefix
	}
	Some(PrefixDeclaration::Named(b"xmlns")) => {
	// error, `xmlns` prefix explicitly set
	return Err(NamespaceError::InvalidXmlnsPrefixBind(v.to_vec()));
	}
	Some(PrefixDeclaration::Named(prefix)) => {
	let ns = Namespace(&v);

	if ns == RESERVED_NAMESPACE_XML.1 {
	// error, non-`xml` prefix set to xml uri
	return Err(NamespaceError::InvalidPrefixForXml(prefix.to_vec()));
	} else if ns == RESERVED_NAMESPACE_XMLNS.1 {
	// error, non-`xmlns` prefix set to xmlns uri
	return Err(NamespaceError::InvalidPrefixForXmlns(prefix.to_vec()));
	}

	let start = self.buffer.len();
	self.buffer.extend_from_slice(prefix);
	self.buffer.extend_from_slice(&v);
	self.bindings.push(NamespaceBinding {
	start,
	prefix_len: prefix.len(),
	value_len: v.len(),
	level,
	});
	}
	None => {}
	}

Make NamespaceResolver public #893

Make NamespaceResolver public #893

Conversation

Mingun commented Aug 10, 2025

Uh oh!

Mingun commented Aug 10, 2025

Uh oh!

codecov-commenter commented Aug 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

faassen left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Mingun Aug 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Make `NamespaceResolver` public #893

Make `NamespaceResolver` public #893

codecov-commenter commented Aug 10, 2025 •

edited

Loading

Mingun Aug 11, 2025 •

edited

Loading