Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

element in incorrect namespace after adding as a child #1332

Closed
ccutrer opened this issue Aug 13, 2015 · 9 comments
Closed

element in incorrect namespace after adding as a child #1332

ccutrer opened this issue Aug 13, 2015 · 9 comments

Comments

@ccutrer
Copy link
Contributor

ccutrer commented Aug 13, 2015

require 'nokogiri'

doc = Nokogiri::XML('<xml xmlns:a="abc"></xml>')
doc2 = Nokogiri::XML('<more xmlns="abc" />')
doc.root << doc2.root

puts doc

I would expect the output to be either

a) it used the outer namespace definition, and moved the element into it

<?xml version="1.0"?>
<xml xmlns:a="abc">
  <a:more/>
</xml>

or

b) it preserves the namespace that doesn't match on prefix, returning

<?xml version="1.0"?>
<xml xmlns:a="abc">
  <more xmlns="abc"/>
</xml>

but instead the output is

<?xml version="1.0"?>
<xml xmlns:a="abc">
  <more/>
</xml>

and more's namespace is no longer "abc". This seems to be a problem with the relink_namespace function, that it removes duplicate namespaces, even if the prefixes don't match (and even if they did, you'd need to make sure the prefix wasn't redeclared between the higher definition and the lower definition)

@ccutrer
Copy link
Contributor Author

ccutrer commented Aug 13, 2015

This seems mildly related to #1200, but cloning doc2's root does not fix it.

@flavorjones
Copy link
Member

Hi @ccutrer,

Thanks for asking this question. The issue here is that you're copying a node from one document to another, and libxml2 doesn't consider those namespaces "equal" in its implementation. None of the XML specs have anything to say about copying nodes between documents, and so hopefully you'll agree that we're in "undefined behavior" territory.

I'm happy to take a look at making this work in a less-unexpected way.

Can you please help me understand what your use case is for this? I'm curious.

@ccutrer
Copy link
Contributor Author

ccutrer commented Aug 31, 2015

My actual use case is an XMLSec encrypted element inside a document. xmlsec decrypts it and hands me back a node, which when I put back into the outer document gets changed by nokogiri, thus breaking an xmlsec signature of the unencrypted document (this a SAML assertion).

@ccutrer
Copy link
Contributor Author

ccutrer commented Aug 31, 2015

I should note that the JRuby version of Nokogiri doesn't have this problem, because it doesn't even attempt to prune seemingly equivalent namespaces.

@afn
Copy link

afn commented Jan 21, 2016

Are there any plans to fix this issue? Or are there any known workarounds?

@sgringwe
Copy link

@afn i have been using a fork by @ccutrer

gem 'nokogiri', '1.6.6.2.20150813143452', require: false, github: 'codekitchen/nokogiri', ref: 'd47e53f885'

@afn
Copy link

afn commented Jan 22, 2016

Cool! @flavorjones I've created a PR for the bugfix, #1416.

@flavorjones
Copy link
Member

Please note that the PR addressing this bug is targetted for 1.6.8.

@flavorjones
Copy link
Member

Please note that the PR addressing this bug is #1333

flavorjones added a commit that referenced this issue May 30, 2016
to prepare for more/better test coverage

related to #1332
flavorjones added a commit that referenced this issue May 30, 2016
and use same pattern in the existing test

related to #1332
flavorjones added a commit that referenced this issue May 30, 2016
and adding tests for reparented nodes' namespaces

related to #1332
flavorjones added a commit that referenced this issue May 30, 2016
that wasn't in either @ccutrer's or @bradleybeddoes's PRs

related to #1332
flavorjones added a commit that referenced this issue May 30, 2016
and adding test for matching parent's default ns

related to #1332
flavorjones added a commit that referenced this issue May 30, 2016
flavorjones added a commit that referenced this issue May 30, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants