Skip to content

Commit

Permalink
* Fix the NAMESPACE_ERR issues (e.g, Issue sparklemotion#801)
Browse files Browse the repository at this point in the history 
* Untangle handling of namespaces in JRuby
* Secure XmlNode and NokogiriNamespaceCache against internal node object changes
  • Loading branch information
@mbklein
mbklein committed Dec 19, 2012
1 parent 0c19cb8 commit 6acedf5
Show file tree
Hide file tree
Showing 5 changed files with 90 additions and 28 deletions.
8 changes: 4 additions & 4 deletions ext/java/nokogiri/XmlDocument.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -179,11 +179,11 @@ private void resolveNamespaceIfNecessary(ThreadContext context, Node node, Strin
if (node == null) return;
String nodePrefix = node.getPrefix();
if (nodePrefix == null) { // default namespace
node.getOwnerDocument().renameNode(node, default_href, node.getNodeName());
NokogiriHelpers.renameNode(node, default_href, node.getNodeName());
} else {
XmlNamespace xmlNamespace = nsCache.get(nodePrefix);
String href = rubyStringToString(xmlNamespace.href(context));
node.getOwnerDocument().renameNode(node, href, node.getNodeName());
NokogiriHelpers.renameNode(node, href, node.getNodeName());
}
resolveNamespaceIfNecessary(context, node.getNextSibling(), default_href);
NodeList children = node.getChildNodes();
Expand Down Expand Up @@ -358,15 +358,15 @@ private void removeNamespceRecursively(ThreadContext context, XmlNode xmlNode) {
Node node = xmlNode.node;
if (node.getNodeType() == Node.ELEMENT_NODE) {
node.setPrefix(null);
node.getOwnerDocument().renameNode(node, null, node.getLocalName());
NokogiriHelpers.renameNode(node, null, node.getLocalName());
NamedNodeMap attrs = node.getAttributes();
for (int i=0; i<attrs.getLength(); i++) {
Attr attr = (Attr) attrs.item(i);
if (isNamespace(attr.getNodeName())) {
((org.w3c.dom.Element)node).removeAttributeNode(attr);
} else {
attr.setPrefix(null);
attr.getOwnerDocument().renameNode(attr, null, attr.getLocalName());
NokogiriHelpers.renameNode(attr, null, attr.getLocalName());
}
}
}
Expand Down
68 changes: 48 additions & 20 deletions ext/java/nokogiri/XmlNode.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -327,12 +327,15 @@ protected void init(ThreadContext context, IRubyObject[] args) {

Element element = null;
String node_name = rubyStringToString(name);
try {
element = document.createElementNS(null, node_name);
} catch (org.w3c.dom.DOMException e) {
// issue#683 NAMESPACE_ERR is thrown from RDF::RDFXML::Writer.new
// retry without namespace
String prefix = NokogiriHelpers.getPrefix(node_name);
if (prefix == null) {
element = document.createElement(node_name);
} else {
String namespace_uri = null;
if (document.getDocumentElement() != null) {
namespace_uri = document.getDocumentElement().lookupNamespaceURI(prefix);
}
element = document.createElementNS(namespace_uri, node_name);
}
setNode(context, element);
}
Expand Down Expand Up @@ -450,33 +453,45 @@ public void post_add_child(ThreadContext context, XmlNode current, XmlNode child
public void relink_namespace(ThreadContext context) {
if (node instanceof Element) {
Element e = (Element) node;
String prefix = e.getPrefix();
String currentNS = e.getNamespaceURI();
if (prefix == null && currentNS == null) {
prefix = NokogiriHelpers.getPrefix(e.getNodeName());
} else if (currentNS != null) {
prefix = e.lookupPrefix(currentNS);
}
e.getOwnerDocument().setStrictErrorChecking(false);
e.getOwnerDocument().renameNode(e, e.lookupNamespaceURI(e.getPrefix()), e.getNodeName());
String nsURI = e.lookupNamespaceURI(prefix);
this.node = NokogiriHelpers.renameNode(e, nsURI, e.getNodeName());

if (e.hasAttributes()) {
NamedNodeMap attrs = e.getAttributes();

for (int i = 0; i < attrs.getLength(); i++) {
Attr attr = (Attr) attrs.item(i);
String nsUri = "";
String prefix = attr.getPrefix();
String attrPrefix = attr.getPrefix();
if (attrPrefix == null) {
attrPrefix = NokogiriHelpers.getPrefix(attr.getNodeName());
}
String nodeName = attr.getNodeName();
if ("xml".equals(prefix)) {
nsUri = "http://www.w3.org/XML/1998/namespace";
} else if ("xmlns".equals(prefix) || nodeName.equals("xmlns")) {
} else if ("xmlns".equals(attrPrefix) || nodeName.equals("xmlns")) {
nsUri = "http://www.w3.org/2000/xmlns/";
} else {
nsUri = attr.getNamespaceURI();
nsUri = attr.lookupNamespaceURI(attrPrefix);
}
if (!(nsUri == null || "".equals(nsUri))) {
XmlNamespace.createFromAttr(context.getRuntime(), attr);
}
e.getOwnerDocument().renameNode(attr, nsUri, nodeName);
NokogiriHelpers.renameNode(attr, nsUri, nodeName);
}
}

if (e.hasChildNodes()) {
((XmlNodeSet) children(context)).relink_namespace(context);
if (this.node.hasChildNodes()) {
XmlNodeSet nodeSet = (XmlNodeSet)(children(context));
nodeSet.relink_namespace(context);
}
}
}
Expand Down Expand Up @@ -535,7 +550,7 @@ public void updateNodeNamespaceIfNecessary(ThreadContext context, XmlNamespace n
&& oldPrefix.equals(rubyStringToString(ns.prefix(context))));

if(update) {
this.node.getOwnerDocument().renameNode(this.node, uri, this.node.getNodeName());
this.node = NokogiriHelpers.renameNode(this.node, uri, this.node.getNodeName());
}
}

Expand Down Expand Up @@ -575,7 +590,7 @@ public IRubyObject add_namespace_definition(ThreadContext context,
else namespaceOwner = node.getParentNode();
XmlNamespace ns = XmlNamespace.createFromPrefixAndHref(namespaceOwner, prefix, href);
if (node != namespaceOwner) {
node.getOwnerDocument().renameNode(node, ns.getHref(), ns.getPrefix() + node.getLocalName());
this.node = NokogiriHelpers.renameNode(node, ns.getHref(), ns.getPrefix() + node.getLocalName());
}

updateNodeNamespaceIfNecessary(context, ns);
Expand Down Expand Up @@ -1170,7 +1185,7 @@ public IRubyObject node_name(ThreadContext context) {
@JRubyMethod(name = {"node_name=", "name="})
public IRubyObject node_name_set(ThreadContext context, IRubyObject nodeName) {
String newName = rubyStringToString(nodeName);
getOwnerDocument().renameNode(node, null, newName);
this.node = NokogiriHelpers.renameNode(node, null, newName);
setName(nodeName);
return this;
}
Expand All @@ -1188,6 +1203,8 @@ public IRubyObject set(ThreadContext context, IRubyObject rbkey, IRubyObject rbv
String uri = null;
if (prefix.equals("xml")) {
uri = "http://www.w3.org/XML/1998/namespace";
} else if (prefix.equals("xmlns")) {
uri = "http://www.w3.org/2000/xmlns/";
} else {
uri = findNamespaceHref(context, prefix);
}
Expand All @@ -1212,7 +1229,11 @@ private String findNamespaceHref(ThreadContext context, String prefix) {
return namespace.getHref();
}
}
currentNode = (XmlNode) currentNode.parent(context);
if (currentNode.parent(context).isNil()) {
break;
} else {
currentNode = (XmlNode) currentNode.parent(context);
}
}
return null;
}
Expand Down Expand Up @@ -1259,7 +1280,7 @@ public IRubyObject set_namespace(ThreadContext context, IRubyObject namespace) {
String prefix = n.getPrefix();
String href = n.getNamespaceURI();
((XmlDocument)doc).getNamespaceCache().remove(prefix == null ? "" : prefix, href);
n.getOwnerDocument().renameNode(n, null, n.getNodeName());
this.node = NokogiriHelpers.renameNode(n, null, NokogiriHelpers.getLocalPart(n.getNodeName()));
}
} else {
XmlNamespace ns = (XmlNamespace) namespace;
Expand All @@ -1268,7 +1289,14 @@ public IRubyObject set_namespace(ThreadContext context, IRubyObject namespace) {

// Assigning node = ...renameNode() or not seems to make no
// difference. Why not? -pmahoney
node = node.getOwnerDocument().renameNode(node, href, NokogiriHelpers.newQName(prefix, node));

// It actually makes a great deal of difference. renameNode()
// will operate in place if it can, but sometimes it can't.
// The node you passed in *might* come back as you expect, but
// it might not. It's much safer to throw away the original
// and keep the return value. -mbklein
String new_name = NokogiriHelpers.newQName(prefix, node);
this.node = NokogiriHelpers.renameNode(node, href, new_name);
}

return this;
Expand Down Expand Up @@ -1465,7 +1493,7 @@ private void addNamespaceURIIfNeeded(Node child) {
XmlElement fragmentContext = ((XmlDocumentFragment)this).getFragmentContext();
String namespace_uri = fragmentContext.node.getNamespaceURI();
if (namespace_uri != null && namespace_uri.length() > 0) {
node.getOwnerDocument().renameNode(child, namespace_uri, child.getNodeName());
NokogiriHelpers.renameNode(child, namespace_uri, child.getNodeName());
}
}
}
Expand Down Expand Up @@ -1522,7 +1550,7 @@ protected void adoptAsReplacement(ThreadContext context,
try {
parentNode.replaceChild(otherNode, thisNode);
if (otherNode.getNodeType() != Node.TEXT_NODE) {
otherNode.getOwnerDocument().renameNode(otherNode, thisNode.getNamespaceURI(), otherNode.getNodeName());
NokogiriHelpers.renameNode(otherNode, thisNode.getNamespaceURI(), otherNode.getNodeName());
}
} catch (Exception e) {
String prefix = "could not replace child: ";
Expand Down
19 changes: 17 additions & 2 deletions ext/java/nokogiri/internals/NokogiriHelpers.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -72,9 +72,11 @@
import org.jruby.runtime.builtin.IRubyObject;
import org.jruby.util.ByteList;
import org.w3c.dom.Attr;
import org.w3c.dom.Document;
import org.w3c.dom.NamedNodeMap;
import org.w3c.dom.Node;
import org.w3c.dom.NodeList;
import org.w3c.dom.DOMException;

/**
* A class for various utility methods.
Expand Down Expand Up @@ -635,10 +637,11 @@ public static String canonicalizeWhitespce(String s) {
}

public static String newQName(String newPrefix, Node node) {
String tagName = getLocalPart(node.getNodeName());
if(newPrefix == null) {
return node.getLocalName();
return tagName;
} else {
return newPrefix + ":" + node.getLocalName();
return newPrefix + ":" + tagName;
}
}

Expand Down Expand Up @@ -815,4 +818,16 @@ public static boolean shouldEncode(Node text) {
public static boolean shouldDecode(Node text) {
return !shouldEncode(text);
}

public static Node renameNode(Node n, String namespaceURI, String qualifiedName) throws DOMException {
Document doc = n.getOwnerDocument();
XmlDocument xmlDoc = (XmlDocument)getCachedNode(doc);
NokogiriNamespaceCache nsCache = xmlDoc.getNamespaceCache();
int oldHash = n.hashCode();
Node result = doc.renameNode(n, namespaceURI, qualifiedName);
if (result != n) {
nsCache.replaceNode(n, result);
}
return result;
}
}
19 changes: 18 additions & 1 deletion ext/java/nokogiri/internals/NokogiriNamespaceCache.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ public List<XmlNamespace> get(Node node) {
List<XmlNamespace> namespaces = new ArrayList<XmlNamespace>();
for (int i=0; i < keys.size(); i++) {
CacheEntry entry = cache.get(i);
if (entry.ownerNode == node) {
if (entry.isOwner(node)) {
namespaces.add(entry.namespace);
}
}
Expand Down Expand Up @@ -151,6 +151,15 @@ public void clear() {
defaultNamespace = null;
}

public void replaceNode(Node oldNode, Node newNode) {
for (int i=0; i < keys.size(); i++) {
CacheEntry entry = cache.get(i);
if (entry.isOwner(oldNode)) {
entry.replaceOwner(newNode);
}
}
}

private class CacheEntry {
private XmlNamespace namespace;
private Node ownerNode;
Expand All @@ -159,5 +168,13 @@ private class CacheEntry {
this.namespace = namespace;
this.ownerNode = ownerNode;
}

public Boolean isOwner(Node n) {
return this.ownerNode.isSameNode(n);
}

public void replaceOwner(Node newNode) {
this.ownerNode = newNode;
}
}
}
4 changes: 3 additions & 1 deletion lib/nokogiri/xml/document.rb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -87,14 +87,16 @@ def create_element name, *args, &block
if key =~ NCNAME_RE
ns_name = key.split(":", 2)[1]
elm.add_namespace_definition ns_name, v
next
end
elm[k.to_s] = v.to_s
}
else
elm.content = arg
end
end
if ns = elm.namespace_definitions.find { |n| n.prefix.nil? or n.prefix == '' }
elm.namespace = ns
end
elm
end

Expand Down

0 comments on commit 6acedf5

Please sign in to comment.