Skip to content

Commit 2d38490

Browse files
adamretteradamretter
authored
Merge pull request #4545 from evolvedbinary/feature/xquery-update-30,binding-conflict-error
[bugfix] - namespace binding conflict errors
2 parents 8a6653e + 5273f9f commit 2d38490

File tree

6 files changed

+207
-30
lines changed

6 files changed

+207
-30
lines changed

‎exist-core/src/main/java/org/exist/dom/memtree/NodeImpl.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,7 @@
4848
import java.util.Properties;
4949

5050

51-
public abstract class NodeImpl<T extends NodeImpl> implements INode<DocumentImpl, T>, NodeValue {
51+
public abstract class NodeImpl<T extends NodeImpl<T>> implements INode<DocumentImpl, T>, NodeValue {
5252

5353
public static final short REFERENCE_NODE = 100;
5454
public static final short NAMESPACE_NODE = 101;

‎exist-core/src/main/java/org/exist/dom/persistent/AttrImpl.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,7 @@
4040

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

43-
public class AttrImpl extends NamedNode implements Attr {
43+
public class AttrImpl extends NamedNode<AttrImpl> implements Attr {
4444

4545
public static final int LENGTH_NS_ID = 2; //sizeof short
4646
public static final int LENGTH_PREFIX_LENGTH = 2; //sizeof short

‎exist-core/src/main/java/org/exist/dom/persistent/ElementImpl.java‎

Lines changed: 41 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@
2626
import org.exist.EXistException;
2727
import org.exist.Namespaces;
2828
import org.exist.dom.NamedNodeMapImpl;
29+
import org.exist.dom.NodeListImpl;
2930
import org.exist.dom.QName;
3031
import org.exist.dom.QName.IllegalQNameException;
3132
import org.exist.indexing.IndexController;
@@ -67,7 +68,7 @@
6768
*
6869
* @author Wolfgang Meier
6970
*/
70-
public class ElementImpl extends NamedNode implements Element {
71+
public class ElementImpl extends NamedNode<ElementImpl> implements Element {
7172

7273
public static final int LENGTH_ELEMENT_CHILD_COUNT = 4; //sizeof int
7374
public static final int LENGTH_ATTRIBUTES_COUNT = 2; //sizeof short
@@ -527,7 +528,7 @@ public Node appendChild(final Node newChild) throws DOMException {
527528
}
528529
}
529530

530-
private void appendAttributes(final Txn transaction, final NodeList attribs) throws DOMException {
531+
private void appendAttributes(final Txn transaction, final NodeListImpl attribs) throws DOMException {
531532
final NodeList duplicateAttrs = findDupAttributes(attribs);
532533
removeAppendAttributes(transaction, duplicateAttrs, attribs);
533534
}
@@ -633,6 +634,16 @@ private void appendChildren(final Txn transaction,
633634
}
634635
}
635636

637+
private QName attrName(Attr attr) {
638+
final String ns = attr.getNamespaceURI();
639+
final String prefix = (Namespaces.XML_NS.equals(ns) ? XMLConstants.XML_NS_PREFIX : attr.getPrefix());
640+
String name = attr.getLocalName();
641+
if(name == null) {
642+
name = attr.getName();
643+
}
644+
return new QName(name, ns, prefix);
645+
}
646+
636647
private Node appendChild(final Txn transaction, final NodeId newNodeId, final NodeImplRef last, final NodePath lastPath, final Node child, final StreamListener listener)
637648
throws DOMException {
638649
if(last == null || last.getNode() == null) {
@@ -725,17 +736,11 @@ private Node appendChild(final Txn transaction, final NodeId newNodeId, final No
725736

726737
case Node.ATTRIBUTE_NODE:
727738
final Attr attr = (Attr) child;
728-
final String ns = attr.getNamespaceURI();
729-
final String prefix = (Namespaces.XML_NS.equals(ns) ? XMLConstants.XML_NS_PREFIX : attr.getPrefix());
730-
String name = attr.getLocalName();
731-
if(name == null) {
732-
name = attr.getName();
733-
}
734-
final QName attrName = new QName(name, ns, prefix);
739+
final QName attrName = attrName(attr);
735740
final AttrImpl attrib = new AttrImpl(getExpression(), attrName, attr.getValue(), broker.getBrokerPool().getSymbols());
736741
attrib.setNodeId(newNodeId);
737742
attrib.setOwnerDocument(owner);
738-
if(ns != null && attrName.compareTo(Namespaces.XML_ID_QNAME) == Constants.EQUAL) {
743+
if(attrName.getNamespaceURI() != null && attrName.compareTo(Namespaces.XML_ID_QNAME) == Constants.EQUAL) {
739744
// an xml:id attribute. Normalize the attribute and set its type to ID
740745
attrib.setValue(StringValue.trimWhitespace(StringValue.collapseWhitespace(attrib.getValue())));
741746
attrib.setType(AttrImpl.ID);
@@ -829,6 +834,10 @@ public NamedNodeMap getAttributes() {
829834
final int childCount = getChildCount();
830835
for(int i = 0; i < childCount; i++) {
831836
final IStoredNode next = iterator.next();
837+
if (next == null) {
838+
LOG.warn("Miscounted getChildCount() index: {} was null of: {}", i, childCount);
839+
continue;
840+
}
832841
if(next.getNodeType() != Node.ATTRIBUTE_NODE) {
833842
break;
834843
}
@@ -1339,10 +1348,17 @@ public void setNamespaceMappings(final Map<String, String> map) {
13391348
}
13401349

13411350
public Iterator<String> getPrefixes() {
1351+
1352+
if (namespaceMappings == null) {
1353+
return Collections.<String>emptySet().iterator();
1354+
}
13421355
return namespaceMappings.keySet().iterator();
13431356
}
13441357

13451358
public String getNamespaceForPrefix(final String prefix) {
1359+
if (namespaceMappings == null) {
1360+
return null;
1361+
}
13461362
return namespaceMappings.get(prefix);
13471363
}
13481364

@@ -2039,4 +2055,19 @@ public boolean accept(final INodeIterator iterator, final NodeVisitor visitor) {
20392055
}
20402056
return true;
20412057
}
2058+
2059+
@Override
2060+
public String lookupNamespaceURI(final String prefix) {
2061+
2062+
for (Node pathNode = this; pathNode != null; pathNode = pathNode.getParentNode()) {
2063+
if (pathNode instanceof ElementImpl) {
2064+
final String namespaceForPrefix = ((ElementImpl)pathNode).getNamespaceForPrefix(prefix);
2065+
if (namespaceForPrefix != null) {
2066+
return namespaceForPrefix;
2067+
}
2068+
}
2069+
}
2070+
2071+
return XMLConstants.NULL_NS_URI;
2072+
}
20422073
}

‎exist-core/src/main/java/org/exist/xquery/ErrorCodes.java‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -134,6 +134,8 @@ public class ErrorCodes {
134134
public static final ErrorCode XQDY0137 = new W3CErrorCode("XQDY0137", "No two keys in a map may have the same key value");
135135
public static final ErrorCode XQDY0138 = new W3CErrorCode("XQDY0138", "Position n does not exist in this array");
136136

137+
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.");
138+
137139
/* XQuery 1.0 and XPath 2.0 Functions and Operators http://www.w3.org/TR/xpath-functions/#error-summary */
138140
public static final ErrorCode FOER0000 = new W3CErrorCode("FOER0000", "Unidentified error.");
139141
public static final ErrorCode FOAR0001 = new W3CErrorCode("FOAR0001", "Division by zero.");

‎exist-core/src/main/java/org/exist/xquery/update/Insert.java‎

Lines changed: 74 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -22,35 +22,31 @@
2222
package org.exist.xquery.update;
2323

2424
import org.exist.EXistException;
25+
import org.exist.Namespaces;
2526
import org.exist.collections.triggers.TriggerException;
27+
import org.exist.dom.NodeListImpl;
28+
import org.exist.dom.QName;
2629
import org.exist.dom.persistent.DocumentImpl;
30+
import org.exist.dom.persistent.ElementImpl;
2731
import org.exist.dom.persistent.NodeImpl;
28-
import org.exist.dom.NodeListImpl;
2932
import org.exist.dom.persistent.StoredNode;
3033
import org.exist.security.Permission;
3134
import org.exist.security.PermissionDeniedException;
3235
import org.exist.storage.NotificationService;
3336
import org.exist.storage.UpdateListener;
3437
import org.exist.storage.txn.Txn;
3538
import org.exist.util.LockException;
36-
import org.exist.xquery.Dependency;
37-
import org.exist.xquery.Expression;
38-
import org.exist.xquery.Profiler;
39-
import org.exist.xquery.XPathException;
40-
import org.exist.xquery.XPathUtil;
41-
import org.exist.xquery.XQueryContext;
39+
import org.exist.xquery.*;
4240
import org.exist.xquery.util.Error;
4341
import org.exist.xquery.util.ExpressionDumper;
4442
import org.exist.xquery.util.Messages;
45-
import org.exist.xquery.value.Item;
46-
import org.exist.xquery.value.NodeValue;
47-
import org.exist.xquery.value.Sequence;
48-
import org.exist.xquery.value.SequenceIterator;
49-
import org.exist.xquery.value.StringValue;
50-
import org.exist.xquery.value.Type;
51-
import org.exist.xquery.value.ValueSequence;
52-
import org.w3c.dom.Attr;
43+
import org.exist.xquery.value.*;
44+
import org.w3c.dom.NamedNodeMap;
45+
import org.w3c.dom.Node;
5346
import org.w3c.dom.NodeList;
47+
48+
import javax.xml.XMLConstants;
49+
5450
/**
5551
* @author wolf
5652
*
@@ -134,20 +130,22 @@ public Sequence eval(Sequence contextSequence, Item contextItem) throws XPathExc
134130

135131
//start a transaction
136132
try (final Txn transaction = getTransaction()) {
137-
final StoredNode[] ql = selectAndLock(transaction, inSeq);
133+
final StoredNode<?>[] ql = selectAndLock(transaction, inSeq);
138134
final NotificationService notifier = context.getBroker().getBrokerPool().getNotificationService();
139135
final NodeList contentList = seq2nodeList(contentSeq);
140-
for (final StoredNode node : ql) {
136+
for (final StoredNode<?> node : ql) {
141137
final DocumentImpl doc = node.getOwnerDocument();
142138
if (!doc.getPermissions().validate(context.getSubject(), Permission.WRITE)) {
143139
throw new PermissionDeniedException("User '" + context.getSubject().getName() + "' does not have permission to write to the document '" + doc.getDocumentURI() + "'!");
144140
}
145141

146142
//update the document
147143
if (mode == INSERT_APPEND) {
144+
validateNonDefaultNamespaces(contentList, node);
148145
node.appendChildren(transaction, contentList, -1);
149146
} else {
150-
final NodeImpl parent = (NodeImpl) getParent(node);
147+
final NodeImpl<?> parent = (NodeImpl<?>) getParent(node);
148+
validateNonDefaultNamespaces(contentList, parent);
151149
switch (mode) {
152150
case INSERT_BEFORE:
153151
parent.insertBefore(transaction, contentList, node);
@@ -180,6 +178,64 @@ public Sequence eval(Sequence contextSequence, Item contextItem) throws XPathExc
180178
return Sequence.EMPTY_SEQUENCE;
181179
}
182180

181+
private QName nodeQName(Node node) {
182+
final String ns = node.getNamespaceURI();
183+
final String prefix = (Namespaces.XML_NS.equals(ns) ? XMLConstants.XML_NS_PREFIX : node.getPrefix());
184+
String name = node.getLocalName();
185+
if(name == null) {
186+
name = node.getNodeName();
187+
}
188+
return new QName(name, ns, prefix);
189+
}
190+
191+
/**
192+
* Generate a namespace attribute as a companion to some other node (element or attribute)
193+
* if the namespace of the other node is explicit
194+
*
195+
* @param parentElement target of the insertion (persistent)
196+
* @param insertNode node to be inserted (in-memory)
197+
* @throws XPathException on an unresolvable namespace conflict
198+
*/
199+
private void validateNonDefaultNamespaceNode(final ElementImpl parentElement, final Node insertNode) throws XPathException {
200+
201+
final QName qName = nodeQName(insertNode);
202+
final String prefix = qName.getPrefix();
203+
if (prefix != null && qName.hasNamespace()) {
204+
final String existingNamespaceURI = parentElement.lookupNamespaceURI(prefix);
205+
if (!XMLConstants.NULL_NS_URI.equals(existingNamespaceURI) && !qName.getNamespaceURI().equals(existingNamespaceURI)) {
206+
throw new XPathException(this, ErrorCodes.XUDY0023,
207+
"The namespace mapping of " + prefix + " -> " +
208+
qName.getNamespaceURI() +
209+
" would conflict with the existing namespace mapping of " +
210+
prefix + " -> " + existingNamespaceURI);
211+
}
212+
}
213+
214+
validateNonDefaultNamespaces(insertNode.getChildNodes(), parentElement);
215+
final NamedNodeMap attributes = insertNode.getAttributes();
216+
for (int i = 0; attributes != null && i < attributes.getLength(); i++) {
217+
validateNonDefaultNamespaceNode(parentElement, attributes.item(i));
218+
}
219+
}
220+
221+
/**
222+
* Validate that a list of nodes (intended for insertion) have no namespace conflicts
223+
*
224+
* @param nodeList nodes to check
225+
* @param parent the position into which the nodes are being inserted
226+
* @throws XPathException if a node has a namespace conflict
227+
*/
228+
private <T extends NodeImpl<T>> void validateNonDefaultNamespaces(final NodeList nodeList, final NodeImpl<T> parent) throws XPathException {
229+
if (parent instanceof ElementImpl) {
230+
final ElementImpl parentAsElement = (ElementImpl) parent;
231+
for (int i = 0; i < nodeList.getLength(); i++) {
232+
final Node node = nodeList.item(i);
233+
234+
validateNonDefaultNamespaceNode(parentAsElement, node);
235+
}
236+
}
237+
}
238+
183239
private NodeList seq2nodeList(Sequence contentSeq) throws XPathException {
184240
final NodeListImpl nl = new NodeListImpl();
185241
for (final SequenceIterator i = contentSeq.iterate(); i.hasNext(); ) {

‎exist-core/src/test/xquery/xquery3/bindingConflict.xqm‎

Lines changed: 88 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,88 @@
1+
(:
2+
: eXist-db Open Source Native XML Database
3+
: Copyright (C) 2001 The eXist-db Authors
4+
:
5+
: [email protected]
6+
: http://www.exist-db.org
7+
:
8+
: This library is free software; you can redistribute it and/or
9+
: modify it under the terms of the GNU Lesser General Public
10+
: License as published by the Free Software Foundation; either
11+
: version 2.1 of the License, or (at your option) any later version.
12+
:
13+
: This library is distributed in the hope that it will be useful,
14+
: but WITHOUT ANY WARRANTY; without even the implied warranty of
15+
: MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
16+
: Lesser General Public License for more details.
17+
:
18+
: You should have received a copy of the GNU Lesser General Public
19+
: License along with this library; if not, write to the Free Software
20+
: Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
21+
:)
22+
xquery version "3.1";
23+
24+
module namespace ut="http://exist-db.org/xquery/update/test";
25+
26+
declare namespace test="http://exist-db.org/xquery/xqsuite";
27+
declare namespace xmldb="http://exist-db.org/xquery/xmldb";
28+
29+
declare namespace myns="http://www.foo.com";
30+
declare namespace myns2="http://www.foo.net";
31+
32+
(: insert node into a ns with a conflicting ns in parent tree :)
33+
declare %test:assertError("XUDY0023")
34+
function ut:insert-child-namespaced-attr-conflicted() {
35+
let $f := xmldb:store('/db', 'xupdate.xml', <root xmlns:myns="http://www.bar.com" attr="1"><!-- foobar --><z blah="wah"/></root>)
36+
let $u := update insert <child myns:baz="qux"/> into doc($f)/root/z
37+
return doc($f)
38+
};
39+
40+
(: insert attr into a ns, but nothing contradictory in the tree - should add ns node :)
41+
declare %test:assertEquals("<z blah=""wah""><child xmlns:myns=""http://www.foo.com"" myns:baz=""qux""/></z>")
42+
function ut:insert-child-namespaced-attr() {
43+
let $f := xmldb:store('/db', 'xupdate.xml', <root attr="1"><!-- foobar --><z blah="wah"/></root>)
44+
let $u := update insert <child myns:baz="qux"/> into doc($f)/root/z
45+
return doc($f)/root/z
46+
};
47+
48+
(: insert attr into a ns, but nothing contradictory in the tree - should add ns node :)
49+
declare %test:assertEquals("<z blah=""wah""><myns:child xmlns:myns=""http://www.foo.com"" baz=""qux""/></z>")
50+
function ut:insert-namespaced-child() {
51+
let $f := xmldb:store('/db', 'xupdate.xml', <root attr="1"><!-- foobar --><z blah="wah"/></root>)
52+
let $u := update insert <myns:child baz="qux"/> into doc($f)/root/z
53+
return doc($f)/root/z
54+
};
55+
56+
(: We "manually" redefined xmlns:myns in <grand> -- what does the code see in <great myns:boz="chux"..>, and should we reject it ? :)
57+
(: Do we need to code up the added namespaces, and check conflicts, thus this would XUDY0023 :)
58+
(: or are we content to ignore manual redefinitions :)
59+
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>")
60+
function ut:insert-namespaced-child-deep() {
61+
let $f := xmldb:store('/db', 'xupdate.xml', <root attr="1"><!-- foobar --><z blah="wah"/></root>)
62+
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
63+
return fn:serialize(doc($f)/root/z)
64+
};
65+
66+
(: insert attr into a ns, but nothing contradictory in the tree - should add ns node :)
67+
declare %test:assertError("XUDY0023")
68+
function ut:insert-namespaced-child-conflicted() {
69+
let $f := xmldb:store('/db', 'xupdate.xml', <root xmlns:myns="http://www.bar.com" attr="1"><!-- foobar --><z blah="wah"/></root>)
70+
let $u := update insert <myns:child baz="qux"/> into doc($f)/root/z
71+
return doc($f)/root/z
72+
};
73+
74+
(: insert attr into a ns with a conflicting ns in parent tree :)
75+
declare %test:assertError("XUDY0023")
76+
function ut:insert-namespaced-attr-conflicted() {
77+
let $f := xmldb:store('/db', 'xupdate.xml', <root xmlns:myns="http://www.bar.com" attr="1"><!-- foobar --><z blah="wah"/></root>)
78+
let $u := update insert attribute myns:baz { "qux" } into doc($f)/root/z
79+
return doc($f)
80+
};
81+
82+
(: insert attr into a ns, but nothing contradictory in the tree - should add ns node :)
83+
declare %test:assertEquals("<z xmlns:myns=""http://www.foo.com"" blah=""wah"" myns:baz=""qux""/>")
84+
function ut:insert-namespaced-attr() {
85+
let $f := xmldb:store('/db', 'xupdate.xml', <root attr="1"><!-- foobar --><z blah="wah"/></root>)
86+
let $u := update insert attribute myns:baz { "qux" } into doc($f)/root/z
87+
return doc($f)/root/z
88+
};

0 commit comments

Comments
 (0)