Skip to content

Commit

Permalink
[bugfix] - namespace binding conflict errors
Browse files Browse the repository at this point in the history
@see #1482

Report an error (XUDY0023) when an insertion is attempted with a node or attribute name in a namespace that conflicts with one which exists in the document.

@see https://www.w3.org/TR/xquery-update-30/#dt-conflict

Check node/attributes inserted recursively. This change does not look at namespaces introduced in the subtrees being inserted, so there is still potential for a conflict in the final tree.
We will consider rejecting conflicting namespaces introduced in the insertion in a later change.
  • Loading branch information
alanpaxton committed Sep 7, 2022
1 parent f3424d7 commit 8d7bf04
Show file tree
Hide file tree
Showing 7 changed files with 207 additions and 29 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@
import java.util.Properties;


public abstract class NodeImpl<T extends NodeImpl> implements INode<DocumentImpl, T>, NodeValue {
public abstract class NodeImpl<T extends NodeImpl<T>> implements INode<DocumentImpl, T>, NodeValue {

public static final short REFERENCE_NODE = 100;
public static final short NAMESPACE_NODE = 101;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@

import static java.nio.charset.StandardCharsets.UTF_8;

public class AttrImpl extends NamedNode implements Attr {
public class AttrImpl extends NamedNode<AttrImpl> implements Attr {

public static final int LENGTH_NS_ID = 2; //sizeof short
public static final int LENGTH_PREFIX_LENGTH = 2; //sizeof short
Expand Down
51 changes: 41 additions & 10 deletions exist-core/src/main/java/org/exist/dom/persistent/ElementImpl.java
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
import org.exist.EXistException;
import org.exist.Namespaces;
import org.exist.dom.NamedNodeMapImpl;
import org.exist.dom.NodeListImpl;
import org.exist.dom.QName;
import org.exist.dom.QName.IllegalQNameException;
import org.exist.indexing.IndexController;
Expand Down Expand Up @@ -67,7 +68,7 @@
*
* @author Wolfgang Meier
*/
public class ElementImpl extends NamedNode implements Element {
public class ElementImpl extends NamedNode<ElementImpl> implements Element {

public static final int LENGTH_ELEMENT_CHILD_COUNT = 4; //sizeof int
public static final int LENGTH_ATTRIBUTES_COUNT = 2; //sizeof short
Expand Down Expand Up @@ -527,7 +528,7 @@ public Node appendChild(final Node newChild) throws DOMException {
}
}

private void appendAttributes(final Txn transaction, final NodeList attribs) throws DOMException {
private void appendAttributes(final Txn transaction, final NodeListImpl attribs) throws DOMException {
final NodeList duplicateAttrs = findDupAttributes(attribs);
removeAppendAttributes(transaction, duplicateAttrs, attribs);
}
Expand Down Expand Up @@ -633,6 +634,16 @@ private void appendChildren(final Txn transaction,
}
}

private QName attrName(Attr attr) {
final String ns = attr.getNamespaceURI();
final String prefix = (Namespaces.XML_NS.equals(ns) ? XMLConstants.XML_NS_PREFIX : attr.getPrefix());
String name = attr.getLocalName();
if(name == null) {
name = attr.getName();
}
return new QName(name, ns, prefix);
}

private Node appendChild(final Txn transaction, final NodeId newNodeId, final NodeImplRef last, final NodePath lastPath, final Node child, final StreamListener listener)
throws DOMException {
if(last == null || last.getNode() == null) {
Expand Down Expand Up @@ -725,17 +736,11 @@ private Node appendChild(final Txn transaction, final NodeId newNodeId, final No

case Node.ATTRIBUTE_NODE:
final Attr attr = (Attr) child;
final String ns = attr.getNamespaceURI();
final String prefix = (Namespaces.XML_NS.equals(ns) ? XMLConstants.XML_NS_PREFIX : attr.getPrefix());
String name = attr.getLocalName();
if(name == null) {
name = attr.getName();
}
final QName attrName = new QName(name, ns, prefix);
final QName attrName = attrName(attr);
final AttrImpl attrib = new AttrImpl(getExpression(), attrName, attr.getValue(), broker.getBrokerPool().getSymbols());
attrib.setNodeId(newNodeId);
attrib.setOwnerDocument(owner);
if(ns != null && attrName.compareTo(Namespaces.XML_ID_QNAME) == Constants.EQUAL) {
if(attrName.getNamespaceURI() != null && attrName.compareTo(Namespaces.XML_ID_QNAME) == Constants.EQUAL) {
// an xml:id attribute. Normalize the attribute and set its type to ID
attrib.setValue(StringValue.trimWhitespace(StringValue.collapseWhitespace(attrib.getValue())));
attrib.setType(AttrImpl.ID);
Expand Down Expand Up @@ -829,6 +834,10 @@ public NamedNodeMap getAttributes() {
final int childCount = getChildCount();
for(int i = 0; i < childCount; i++) {
final IStoredNode next = iterator.next();
if (next == null) {
LOG.warn("Miscounted getChildCount() index " + i + " was null of " + childCount);
continue;
}
if(next.getNodeType() != Node.ATTRIBUTE_NODE) {
break;
}
Expand Down Expand Up @@ -1339,10 +1348,17 @@ public void setNamespaceMappings(final Map<String, String> map) {
}

public Iterator<String> getPrefixes() {

if (namespaceMappings == null) {
return Collections.EMPTY_SET.iterator();
}
return namespaceMappings.keySet().iterator();
}

public String getNamespaceForPrefix(final String prefix) {
if (namespaceMappings == null) {
return null;
}
return namespaceMappings.get(prefix);
}

Expand Down Expand Up @@ -2039,4 +2055,19 @@ public boolean accept(final INodeIterator iterator, final NodeVisitor visitor) {
}
return true;
}

@Override
public String lookupNamespaceURI(final String prefix) {

for (Node pathNode = this; pathNode != null; pathNode = pathNode.getParentNode()) {
if (pathNode instanceof ElementImpl) {
final String namespaceForPrefix = ((ElementImpl)pathNode).getNamespaceForPrefix(prefix);
if (namespaceForPrefix != null) {
return namespaceForPrefix;
}
}
}

return XMLConstants.NULL_NS_URI;
}
}
2 changes: 2 additions & 0 deletions exist-core/src/main/java/org/exist/xquery/ErrorCodes.java
Original file line number Diff line number Diff line change
Expand Up @@ -134,6 +134,8 @@ public class ErrorCodes {
public static final ErrorCode XQDY0137 = new W3CErrorCode("XQDY0137", "No two keys in a map may have the same key value");
public static final ErrorCode XQDY0138 = new W3CErrorCode("XQDY0138", "Position n does not exist in this array");

public static final ErrorCode XUDY0023 = new W3CErrorCode("XUDY0023", "It is a dynamic error if an insert, replace, or rename expression affects an element node by introducing a new namespace binding that conflicts with one of its existing namespace bindings.");

/* XQuery 1.0 and XPath 2.0 Functions and Operators http://www.w3.org/TR/xpath-functions/#error-summary */
public static final ErrorCode FOER0000 = new W3CErrorCode("FOER0000", "Unidentified error.");
public static final ErrorCode FOAR0001 = new W3CErrorCode("FOAR0001", "Division by zero.");
Expand Down
86 changes: 71 additions & 15 deletions exist-core/src/main/java/org/exist/xquery/update/Insert.java
Original file line number Diff line number Diff line change
Expand Up @@ -22,35 +22,31 @@
package org.exist.xquery.update;

import org.exist.EXistException;
import org.exist.Namespaces;
import org.exist.collections.triggers.TriggerException;
import org.exist.dom.NodeListImpl;
import org.exist.dom.QName;
import org.exist.dom.persistent.DocumentImpl;
import org.exist.dom.persistent.ElementImpl;
import org.exist.dom.persistent.NodeImpl;
import org.exist.dom.NodeListImpl;
import org.exist.dom.persistent.StoredNode;
import org.exist.security.Permission;
import org.exist.security.PermissionDeniedException;
import org.exist.storage.NotificationService;
import org.exist.storage.UpdateListener;
import org.exist.storage.txn.Txn;
import org.exist.util.LockException;
import org.exist.xquery.Dependency;
import org.exist.xquery.Expression;
import org.exist.xquery.Profiler;
import org.exist.xquery.XPathException;
import org.exist.xquery.XPathUtil;
import org.exist.xquery.XQueryContext;
import org.exist.xquery.*;
import org.exist.xquery.util.Error;
import org.exist.xquery.util.ExpressionDumper;
import org.exist.xquery.util.Messages;
import org.exist.xquery.value.Item;
import org.exist.xquery.value.NodeValue;
import org.exist.xquery.value.Sequence;
import org.exist.xquery.value.SequenceIterator;
import org.exist.xquery.value.StringValue;
import org.exist.xquery.value.Type;
import org.exist.xquery.value.ValueSequence;
import org.w3c.dom.Attr;
import org.exist.xquery.value.*;
import org.w3c.dom.NamedNodeMap;
import org.w3c.dom.Node;
import org.w3c.dom.NodeList;

import javax.xml.XMLConstants;

/**
* @author wolf
*
Expand Down Expand Up @@ -145,9 +141,11 @@ public Sequence eval(Sequence contextSequence, Item contextItem) throws XPathExc

//update the document
if (mode == INSERT_APPEND) {
validateNonDefaultNamespaces(contentList, node);
node.appendChildren(transaction, contentList, -1);
} else {
final NodeImpl parent = (NodeImpl) getParent(node);
validateNonDefaultNamespaces(contentList, parent);
switch (mode) {
case INSERT_BEFORE:
parent.insertBefore(transaction, contentList, node);
Expand Down Expand Up @@ -180,6 +178,64 @@ public Sequence eval(Sequence contextSequence, Item contextItem) throws XPathExc
return Sequence.EMPTY_SEQUENCE;
}

private QName nodeQName(Node node) {
final String ns = node.getNamespaceURI();
final String prefix = (Namespaces.XML_NS.equals(ns) ? XMLConstants.XML_NS_PREFIX : node.getPrefix());
String name = node.getLocalName();
if(name == null) {
name = node.getNodeName();
}
return new QName(name, ns, prefix);
}

/**
* Generate a namespace attribute as a companion to some other node (element or attribute)
* if the namespace of the other node is explicit
*
* @param parentElement target of the insertion (persistent)
* @param insertNode node to be inserted (in-memory)
* @throws XPathException on an unresolvable namespace conflict
*/
private void validateNonDefaultNamespaceNode(final ElementImpl parentElement, final Node insertNode) throws XPathException {

final QName qName = nodeQName(insertNode);
final String prefix = qName.getPrefix();
if (prefix != null && qName.hasNamespace()) {
final String existingNamespaceURI = parentElement.lookupNamespaceURI(prefix);
if (!XMLConstants.NULL_NS_URI.equals(existingNamespaceURI) && !qName.getNamespaceURI().equals(existingNamespaceURI)) {
throw new XPathException(this, ErrorCodes.XUDY0023,
"The namespace mapping of " + prefix + " -> " +
qName.getNamespaceURI() +
" would conflict with the existing namespace mapping of " +
prefix + " -> " + existingNamespaceURI);
}
}

validateNonDefaultNamespaces(insertNode.getChildNodes(), parentElement);
final NamedNodeMap attributes = insertNode.getAttributes();
for (int i = 0; attributes != null && i < attributes.getLength(); i++) {
validateNonDefaultNamespaceNode(parentElement, attributes.item(i));
}
}

/**
* Validate that a list of nodes (intended for insertion) have no namespace conflicts
*
* @param nodeList nodes to check
* @param parent the position into which the nodes are being inserted
* @throws XPathException if a node has a namespace conflict
*/
private void validateNonDefaultNamespaces(final NodeList nodeList, final NodeImpl parent) throws XPathException {
if (parent instanceof ElementImpl) {
final ElementImpl parentAsElement = (ElementImpl) parent;
for (int i = 0; i < nodeList.getLength(); i++) {
final Node node = nodeList.item(i);

validateNonDefaultNamespaceNode(parentAsElement, node);
}
}
}

private NodeList seq2nodeList(Sequence contentSeq) throws XPathException {
final NodeListImpl nl = new NodeListImpl();
for (final SequenceIterator i = contentSeq.iterate(); i.hasNext(); ) {
Expand Down
5 changes: 3 additions & 2 deletions exist-core/src/test/java/xquery/xquery3/XQuery3Tests.java
Original file line number Diff line number Diff line change
Expand Up @@ -26,9 +26,10 @@

@RunWith(XSuite.class)
@XSuite.XSuiteFiles({
"src/test/xquery/xquery3",
"src/test/xquery/xquery3/transform",
//"src/test/xquery/xquery3",
//"src/test/xquery/xquery3/transform",
//Reminder - add an individual test like this - "src/test/xquery/xquery3/transform/<test-file>.xqm",
"src/test/xquery/xquery3/bindingConflict.xqm"
})
public class XQuery3Tests {
}
88 changes: 88 additions & 0 deletions exist-core/src/test/xquery/xquery3/bindingConflict.xqm
Original file line number Diff line number Diff line change
@@ -0,0 +1,88 @@
(:
: eXist-db Open Source Native XML Database
: Copyright (C) 2001 The eXist-db Authors
:
: info@exist-db.org
: http://www.exist-db.org
:
: This library is free software; you can redistribute it and/or
: modify it under the terms of the GNU Lesser General Public
: License as published by the Free Software Foundation; either
: version 2.1 of the License, or (at your option) any later version.
:
: This library is distributed in the hope that it will be useful,
: but WITHOUT ANY WARRANTY; without even the implied warranty of
: MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
: Lesser General Public License for more details.
:
: You should have received a copy of the GNU Lesser General Public
: License along with this library; if not, write to the Free Software
: Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
:)
xquery version "3.1";

module namespace ut="http://exist-db.org/xquery/update/test";

declare namespace test="http://exist-db.org/xquery/xqsuite";
declare namespace xmldb="http://exist-db.org/xquery/xmldb";

declare namespace myns="http://www.foo.com";
declare namespace myns2="http://www.foo.net";

(: insert node into a ns with a conflicting ns in parent tree :)
declare %test:assertError("XUDY0023")
function ut:insert-child-namespaced-attr-conflicted() {
let $f := xmldb:store('/db', 'xupdate.xml', <root xmlns:myns="http://www.bar.com" attr="1"><!-- foobar --><z blah="wah"/></root>)
let $u := update insert <child myns:baz="qux"/> into doc($f)/root/z
return doc($f)
};

(: insert attr into a ns, but nothing contradictory in the tree - should add ns node :)
declare %test:assertEquals("<z blah=""wah""><child xmlns:myns=""http://www.foo.com"" myns:baz=""qux""/></z>")
function ut:insert-child-namespaced-attr() {
let $f := xmldb:store('/db', 'xupdate.xml', <root attr="1"><!-- foobar --><z blah="wah"/></root>)
let $u := update insert <child myns:baz="qux"/> into doc($f)/root/z
return doc($f)/root/z
};

(: insert attr into a ns, but nothing contradictory in the tree - should add ns node :)
declare %test:assertEquals("<z blah=""wah""><myns:child xmlns:myns=""http://www.foo.com"" baz=""qux""/></z>")
function ut:insert-namespaced-child() {
let $f := xmldb:store('/db', 'xupdate.xml', <root attr="1"><!-- foobar --><z blah="wah"/></root>)
let $u := update insert <myns:child baz="qux"/> into doc($f)/root/z
return doc($f)/root/z
};

(: We "manually" redefined xmlns:myns in <grand> -- what does the code see in <great myns:boz="chux"..>, and should we reject it ? :)
(: Do we need to code up the added namespaces, and check conflicts, thus this would XUDY0023 :)
(: or are we content to ignore manual redefinitions :)
declare %test:assertEquals("<z blah=""wah""><myns:child xmlns:myns=""http://www.foo.com"" baz=""qux""><grand xmlns:myns=""http://www.fubar.com""><great xmlns:myns2=""http://www.foo.net"" myns:boz=""chux"" myns2:pip=""dickens""/></grand></myns:child></z>")
function ut:insert-namespaced-child-deep() {
let $f := xmldb:store('/db', 'xupdate.xml', <root attr="1"><!-- foobar --><z blah="wah"/></root>)
let $u := update insert <myns:child baz="qux"><grand xmlns:myns="http://www.fubar.com"><great myns:boz="chux" myns2:pip="dickens"/></grand></myns:child> into doc($f)/root/z
return fn:serialize(doc($f)/root/z)
};

(: insert attr into a ns, but nothing contradictory in the tree - should add ns node :)
declare %test:assertError("XUDY0023")
function ut:insert-namespaced-child-conflicted() {
let $f := xmldb:store('/db', 'xupdate.xml', <root xmlns:myns="http://www.bar.com" attr="1"><!-- foobar --><z blah="wah"/></root>)
let $u := update insert <myns:child baz="qux"/> into doc($f)/root/z
return doc($f)/root/z
};

(: insert attr into a ns with a conflicting ns in parent tree :)
declare %test:assertError("XUDY0023")
function ut:insert-namespaced-attr-conflicted() {
let $f := xmldb:store('/db', 'xupdate.xml', <root xmlns:myns="http://www.bar.com" attr="1"><!-- foobar --><z blah="wah"/></root>)
let $u := update insert attribute myns:baz { "qux" } into doc($f)/root/z
return doc($f)
};

(: insert attr into a ns, but nothing contradictory in the tree - should add ns node :)
declare %test:assertEquals("<z xmlns:myns=""http://www.foo.com"" blah=""wah"" myns:baz=""qux""/>")
function ut:insert-namespaced-attr() {
let $f := xmldb:store('/db', 'xupdate.xml', <root attr="1"><!-- foobar --><z blah="wah"/></root>)
let $u := update insert attribute myns:baz { "qux" } into doc($f)/root/z
return doc($f)/root/z
};

0 comments on commit 8d7bf04

Please sign in to comment.