Attribute namespaces are not preserved

[jethrogb]

Attributes can have namespaces, such as in:

<mdui:UIInfo>
  <mdui:DisplayName xml:lang="en">TestShib Test IdP</mdui:DisplayName>
  <mdui:Description xml:lang="en">TestShib IdP. Use this as a source of attributes for your test SP.</mdui:Description>
  <mdui:Logo height="88" width="253">https://www.testshib.org/testshibtwo.jpg</mdui:Logo>
</mdui:UIInfo>

(full XML at https://www.testshib.org/metadata/testshib-providers.xml)

The xml namespace on the lang attribute is not stored:

Element {
    prefix: Some(
        "mdui"
    ),
    namespace: Some(
        "urn:oasis:names:tc:SAML:metadata:ui"
    ),
    namespaces: Some(
        Namespace(
            {
                "": "urn:oasis:names:tc:SAML:2.0:metadata",
                "ds": "http://www.w3.org/2000/09/xmldsig#",
                "mdalg": "urn:oasis:names:tc:SAML:metadata:algsupport",
                "mdui": "urn:oasis:names:tc:SAML:metadata:ui",
                "shibmd": "urn:mace:shibboleth:metadata:1.0",
                "xml": "http://www.w3.org/XML/1998/namespace",
                "xmlns": "http://www.w3.org/2000/xmlns/",
                "xsi": "http://www.w3.org/2001/XMLSchema-instance"
            }
        )
    ),
    name: "DisplayName",
    attributes: {
        "lang": "en"
    },
    children: [],
    text: Some(
        "TestShib Test IdP"
    )
},

[ctrlcctrlv]

@jethrogb if you still care about this issue, I believe I've solved it in #33. It requires a semver-breaking API change. To make the upgrade more comfortable for API consumers I threw the entire weight of modern Rust trait derivation behind my newtype:

xmltree-rs/src/namespace.rs

Lines 9 to 13 in 03bc802

	/// A wrapper around [`xml::namespace::Namespace`] (＝[`XmlNamespace`]).
	#[rustfmt::skip]
	#[derive(AsRef, AsMut, Deref, DerefMut, Debug, Display, Clone, PartialEq, Eq, From, Into)]
	#[display(fmt = "{}", "self.0.0.iter().next().map(|ns| ns.0).expect(\"Can't display empty namespace\")")]
	pub struct Namespace(XmlNamespace);

Going even farther for users of old versions of @eminence's API, I decided to delegate all functions of XmlNamespace:

xmltree-rs/src/namespace.rs

Lines 24 to 39 in 03bc802

	/// See § [Methods from `Deref<Target = XmlNamespace>`](struct.Namespace.html#deref-methods-XmlNamespace).
	impl Namespace {
	pub fn empty() -> Namespace {
	Self(XmlNamespace::empty())
	}
	delegate! {
	to self.0 {
	pub fn is_empty(&self) -> bool;
	pub fn is_essentially_empty(&self) -> bool;
	pub fn contains<P: ?Sized + AsRef<str>>(&self, prefix: &P) -> bool;
	pub fn put<P, U>(&mut self, prefix: P, uri: U) -> bool where P: Into<String>, U: Into<String>;
	pub fn force_put<P, U>(&mut self, prefix: P, uri: U) -> Option<String> where P: Into<String>, U: Into<String>;
	pub fn get<'a, P: ?Sized>(&'a self, prefix: &P) -> Option<&'a str> where P: AsRef<str>;
	}
	}
	}

There is no way within delegate crate to do trait impl delegation, so the last remaining trait, IntoIterator, I did by hand:

xmltree-rs/src/namespace.rs

Lines 41 to 47 in 03bc802

	impl<'a> IntoIterator for &'a Namespace {
	type Item = UriMapping<'a>;
	type IntoIter = NamespaceMappings<'a>;
	fn into_iter(self) -> NamespaceMappings<'a> {
	self.0.into_iter()
	}
	}

I added finally the obvious opposite trait (FromIterator) that @netvl ought to consider upstream—

xmltree-rs/src/namespace.rs

Line 49 in 03bc802

impl FromIterator<(String, String)> for Namespace {

Due to the care taken to make the upgrade seamless I hope @eminence approves.

[adri326]

I'm experiencing the same issue, and dropping namespaces in attributes is a bit of a deal-breaker. Is there an ETA on when the PR will be merged or a known blocker to the merge?

Switching to the PR repository instead of the crates.io version fixed the issue and did not cause any incompatibility with my existing codebase.

[ctrlcctrlv]

@adri326 For me it's not just a bit of a deal breaker so the MFEK project uses my fork until #33 is approved. If it never is, I guess I can maintain it in perpetuity and backport any changes (should any be made) here.

[ctrlcctrlv]

@adri326 If you're interested, I just pushed the two changes I was missing from this master branch and reconciled #19 with my master at MFEK/xmltree.rlib in MFEK/xmltree.rlib@8ca231c and MFEK/xmltree.rlib@447dcd6.

Hopefully the MFEK branches don't need to be a long-term solution for you, but they can be. The tests do pass. The work by @dyst5422 looked fine to me so that's why I merged it.

[thomas725]

Hey there!

I just stumbled upon this issue (that reading an xml file with this library, manipulating the in memory tree and writing it back out to a file causes attribute namespaces to get lost, see also: https://gitlab.com/thomas351/xml-sorter-rs )

I read above that @ctrlcctrlv maintains a fork that fixes that issue, so I've switched my dependency to that repository: https://github.com/MFEK/xmltree.rlib - works fine :)

Would be great if the fix could be merged into a crates.io release.

[eminence]

Thanks for the bump. It sounds like the fix from @ctrlcctrlv in #33 has some good reviews and successful usage in their fork.

I'll try to review these changes and get some stuff merged soon. Thanks for your patience.

[ctrlcctrlv]

My fork works fine for my use case (and the use cases of many others) but the issues raised by @tombailo and others are serious concerns as the whole point of namespaces is being able to have the same XML attr name provided by two different DTD's.

@tombailo made an attempt to patch overtop #33 in #38. We could really use a discussion between all of us on how to move forwards, I hadn't make progress as you'd not been around but the code in #38 looks like it may be workable and fix all issues but is a radical change.