Skip to content

Commit 18d76d3

Browse files
adamretteradamretter
committed
[bugfix] Fix DOM compliance of in-memory DOM so that Element#getAttribute(String) and Element#getAttributeNS(String, String) return an empty string (and not null) if the attribute does not exist
Closes eXist-db/exist#5672
1 parent 8a2348e commit 18d76d3

File tree

4 files changed

+133
-52
lines changed

4 files changed

+133
-52
lines changed

‎exist-core/pom.xml‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -726,8 +726,11 @@
726726
<include>src/main/java/org/exist/collections/triggers/CollectionTrigger.java</include>
727727
<include>src/main/java/org/exist/collections/triggers/DocumentTrigger.java</include>
728728
<include>src/main/java/org/exist/config/Configuration.java</include>
729+
<include>src/main/java/org/exist/dom/memtree/ElementImpl.java</include>
729730
<include>src/main/java/org/exist/dom/persistent/DocumentImpl.java</include>
730731
<include>src/main/java/org/exist/dom/persistent/DocumentSet.java</include>
732+
<include>src/main/java/org/exist/dom/persistent/ElementImpl.java</include>
733+
<include>src/test/java/org/exist/dom/persistent/NodeTest.java</include>
731734
<include>src/main/java/org/exist/dom/persistent/SymbolTable.java</include>
732735
<include>src/main/java/org/exist/dom/persistent/VirtualNodeSet.java</include>
733736
<include>src/main/java/org/exist/http/servlets/XSLTServlet.java</include>
@@ -852,8 +855,11 @@
852855
<exclude>src/main/java/org/exist/collections/triggers/CollectionTrigger.java</exclude>
853856
<exclude>src/main/java/org/exist/collections/triggers/DocumentTrigger.java</exclude>
854857
<exclude>src/main/java/org/exist/config/Configuration.java</exclude>
858+
<exclude>src/main/java/org/exist/dom/memtree/ElementImpl.java</exclude>
855859
<exclude>src/main/java/org/exist/dom/persistent/DocumentImpl.java</exclude>
856860
<exclude>src/main/java/org/exist/dom/persistent/DocumentSet.java</exclude>
861+
<exclude>src/main/java/org/exist/dom/persistent/ElementImpl.java</exclude>
862+
<include>src/test/java/org/exist/dom/persistent/NodeTest.java</include>
857863
<exclude>src/main/java/org/exist/dom/persistent/SymbolTable.java</exclude>
858864
<exclude>src/main/java/org/exist/dom/persistent/VirtualNodeSet.java</exclude>
859865
<exclude>src/main/java/org/exist/http/servlets/XSLTServlet.java</exclude>

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

Lines changed: 60 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,28 @@
11
/*
2+
* Elemental
3+
* Copyright (C) 2024, Evolved Binary Ltd
4+
*
5+
* [email protected]
6+
* https://www.evolvedbinary.com | https://www.elemental.xyz
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; version 2.1.
11+
*
12+
* This library is distributed in the hope that it will be useful,
13+
* but WITHOUT ANY WARRANTY; without even the implied warranty of
14+
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
15+
* Lesser General Public License for more details.
16+
*
17+
* You should have received a copy of the GNU Lesser General Public
18+
* License along with this library; if not, write to the Free Software
19+
* Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
20+
*
21+
* NOTE: Parts of this file contain code from 'The eXist-db Authors'.
22+
* The original license header is included below.
23+
*
24+
* =====================================================================
25+
*
226
* eXist-db Open Source Native XML Database
327
* Copyright (C) 2001 The eXist-db Authors
428
*
@@ -102,28 +126,31 @@ public boolean hasAttributes() {
102126
@Override
103127
public String getAttribute(final String name) {
104128
int attr = document.alpha[nodeNumber];
129+
105130
if(-1 < attr) {
106-
while(attr < document.nextAttr && document.attrParent[attr] == nodeNumber) {
131+
while (attr < document.nextAttr && document.attrParent[attr] == nodeNumber) {
107132
final QName attrQName = document.attrName[attr];
108-
if(attrQName.getStringValue().equals(name)) {
133+
if (attrQName.getStringValue().equals(name)) {
109134
return document.attrValue[attr];
110135
}
111136
++attr;
112137
}
113138
}
114-
if(name.startsWith(XMLConstants.XMLNS_ATTRIBUTE + ":")) {
139+
140+
if (name.startsWith(XMLConstants.XMLNS_ATTRIBUTE + ":")) {
115141
int ns = document.alphaLen[nodeNumber];
116-
if(-1 < ns) {
117-
while(ns < document.nextNamespace && document.namespaceParent[ns] == nodeNumber) {
142+
if (-1 < ns) {
143+
while (ns < document.nextNamespace && document.namespaceParent[ns] == nodeNumber) {
118144
final QName nsQName = document.namespaceCode[ns];
119-
if(nsQName.getStringValue().equals(name)) {
145+
if (nsQName.getStringValue().equals(name)) {
120146
return nsQName.getNamespaceURI();
121147
}
122148
++ns;
123149
}
124150
}
125151
}
126-
return null;
152+
153+
return "";
127154
}
128155

129156
@Override
@@ -411,28 +438,31 @@ private NodeList getElementsByTagName(final QName qname) {
411438
@Override
412439
public String getAttributeNS(final String namespaceURI, final String localName) {
413440
int attr = document.alpha[nodeNumber];
414-
if(-1 < attr) {
415-
while(attr < document.nextAttr && document.attrParent[attr] == nodeNumber) {
441+
442+
if (-1 < attr) {
443+
while (attr < document.nextAttr && document.attrParent[attr] == nodeNumber) {
416444
final QName name = document.attrName[attr];
417-
if(name.getLocalPart().equals(localName) && name.getNamespaceURI().equals(namespaceURI)) {
445+
if (name.getLocalPart().equals(localName) && name.getNamespaceURI().equals(namespaceURI)) {
418446
return document.attrValue[attr];
419447
}
420448
++attr;
421449
}
422450
}
423-
if(Namespaces.XMLNS_NS.equals(namespaceURI)) {
451+
452+
if (Namespaces.XMLNS_NS.equals(namespaceURI)) {
424453
int ns = document.alphaLen[nodeNumber];
425-
if(-1 < ns) {
426-
while(ns < document.nextNamespace && document.namespaceParent[ns] == nodeNumber) {
454+
if (-1 < ns) {
455+
while (ns < document.nextNamespace && document.namespaceParent[ns] == nodeNumber) {
427456
final QName nsQName = document.namespaceCode[ns];
428-
if(nsQName.getLocalPart().equals(localName)) {
457+
if (nsQName.getLocalPart().equals(localName)) {
429458
return nsQName.getNamespaceURI();
430459
}
431460
++ns;
432461
}
433462
}
434463
}
435-
return null;
464+
465+
return "";
436466
}
437467

438468
@Override
@@ -556,46 +586,43 @@ public String getBaseURI() {
556586
return "";//UNDERSTAND: is it ok?
557587
}
558588

559-
//TODO please, keep in sync with org.exist.dom.persistent.ElementImpl
589+
// NOTE(AR) please keep in sync with org.exist.dom.persistent.ElementImpl
560590
private XmldbURI calculateBaseURI() {
561591
XmldbURI baseURI = null;
562592

563593
final String nodeBaseURI = getAttributeNS(Namespaces.XML_NS, "base");
564-
if(nodeBaseURI != null) {
594+
if (!nodeBaseURI.isEmpty()) {
565595
baseURI = XmldbURI.create(nodeBaseURI, false);
566-
if(baseURI.isAbsolute()) {
596+
if (baseURI.isAbsolute()) {
567597
return baseURI;
568598
}
569599
}
570600

571601
int parent = -1;
572602
final int test = document.getParentNodeFor(nodeNumber);
573-
if(document.nodeKind[test] != Node.DOCUMENT_NODE) {
603+
if (document.nodeKind[test] != Node.DOCUMENT_NODE) {
574604
parent = test;
575605
}
576606

577-
if(parent != -1) {
578-
if(nodeBaseURI == null) {
607+
if (parent != -1) {
608+
if (nodeBaseURI.isEmpty()) {
579609
baseURI = ((ElementImpl) document.getNode(parent))
580610
.calculateBaseURI();
581611
} else {
582-
XmldbURI parentsBaseURI = ((ElementImpl) document.getNode(parent))
583-
.calculateBaseURI();
584-
585-
if(nodeBaseURI.isEmpty()) {
586-
baseURI = parentsBaseURI;
587-
} else {
612+
final XmldbURI parentsBaseURI = ((ElementImpl) document.getNode(parent)).calculateBaseURI();
613+
if (parentsBaseURI.toString().endsWith("/") || !parentsBaseURI.toString().contains("/")) {
588614
baseURI = parentsBaseURI.append(baseURI);
615+
} else {
616+
// there is a filename, remove it
617+
baseURI = parentsBaseURI.removeLastSegment().append(baseURI);
589618
}
590619
}
591620
} else {
592-
if(nodeBaseURI == null) {
621+
if (nodeBaseURI.isEmpty()) {
593622
return XmldbURI.create(getOwnerDocument().getBaseURI(), false);
594-
} else if(nodeNumber == 1) {
595-
//nothing to do
596-
} else {
623+
} else if (nodeNumber != 1) {
597624
final String docBaseURI = getOwnerDocument().getBaseURI();
598-
if(docBaseURI.endsWith("/")) {
625+
if (docBaseURI.endsWith("/")) {
599626
baseURI = XmldbURI.create(getOwnerDocument().getBaseURI(), false);
600627
baseURI.append(baseURI);
601628
} else {
@@ -605,6 +632,7 @@ private XmldbURI calculateBaseURI() {
605632
}
606633
}
607634
}
635+
608636
return baseURI;
609637
}
610638

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

Lines changed: 40 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,28 @@
11
/*
2+
* Elemental
3+
* Copyright (C) 2024, Evolved Binary Ltd
4+
*
5+
* [email protected]
6+
* https://www.evolvedbinary.com | https://www.elemental.xyz
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; version 2.1.
11+
*
12+
* This library is distributed in the hope that it will be useful,
13+
* but WITHOUT ANY WARRANTY; without even the implied warranty of
14+
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
15+
* Lesser General Public License for more details.
16+
*
17+
* You should have received a copy of the GNU Lesser General Public
18+
* License along with this library; if not, write to the Free Software
19+
* Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
20+
*
21+
* NOTE: Parts of this file contain code from 'The eXist-db Authors'.
22+
* The original license header is included below.
23+
*
24+
* =====================================================================
25+
*
226
* eXist-db Open Source Native XML Database
327
* Copyright (C) 2001 The eXist-db Authors
428
*
@@ -803,8 +827,7 @@ public String getAttribute(final String name) {
803827
@Override
804828
public String getAttributeNS(final String namespaceURI, final String localName) {
805829
final Attr attr = findAttribute(new QName(localName, namespaceURI));
806-
return attr != null ? attr.getValue() : XMLConstants.NULL_NS_URI;
807-
//XXX: if not present must return null
830+
return attr != null ? attr.getValue() : "";
808831
}
809832

810833
@Deprecated //move as soon as getAttributeNS null issue resolved
@@ -1991,41 +2014,37 @@ public String getBaseURI() {
19912014
return ""; //UNDERSTAND: is it ok?
19922015
}
19932016

1994-
//TODO Please, keep in sync with org.exist.dom.memtree.ElementImpl
2017+
// NOTE(AR) please keep in sync with org.exist.dom.memtree.ElementImpl
19952018
private XmldbURI calculateBaseURI() {
19962019
XmldbURI baseURI = null;
19972020

1998-
final String nodeBaseURI = _getAttributeNS(Namespaces.XML_NS, "base");
1999-
if(nodeBaseURI != null) {
2021+
final String nodeBaseURI = getAttributeNS(Namespaces.XML_NS, "base");
2022+
if (!nodeBaseURI.isEmpty()) {
20002023
baseURI = XmldbURI.create(nodeBaseURI, false);
2001-
if(baseURI.isAbsolute()) {
2024+
if (baseURI.isAbsolute()) {
20022025
return baseURI;
20032026
}
20042027
}
20052028

2006-
final IStoredNode parent = getParentStoredNode();
2007-
if(parent != null) {
2008-
if(nodeBaseURI == null) {
2029+
final IStoredNode<?> parent = getParentStoredNode();
2030+
if (parent != null) {
2031+
if (nodeBaseURI.isEmpty()) {
20092032
baseURI = ((ElementImpl) parent).calculateBaseURI();
20102033
} else {
2011-
XmldbURI parentsBaseURI = ((ElementImpl) parent).calculateBaseURI();
2012-
if(nodeBaseURI.isEmpty()) {
2013-
baseURI = parentsBaseURI;
2034+
final XmldbURI parentsBaseURI = ((ElementImpl) parent).calculateBaseURI();
2035+
if (parentsBaseURI.toString().endsWith("/") || !parentsBaseURI.toString().contains("/")) {
2036+
baseURI = parentsBaseURI.append(baseURI);
20142037
} else {
2015-
if(parentsBaseURI.toString().endsWith("/") || !parentsBaseURI.toString().contains("/")){
2016-
baseURI = parentsBaseURI.append(baseURI);
2017-
} else {
2018-
// there is a filename, remove it
2019-
baseURI = parentsBaseURI.removeLastSegment().append(baseURI);
2020-
}
2038+
// there is a filename, remove it
2039+
baseURI = parentsBaseURI.removeLastSegment().append(baseURI);
20212040
}
20222041
}
20232042
} else {
2024-
if(nodeBaseURI == null) {
2043+
if (nodeBaseURI.isEmpty()) {
20252044
return XmldbURI.create(getOwnerDocument().getBaseURI(), false);
20262045
} else {
20272046
final String docBaseURI = getOwnerDocument().getBaseURI();
2028-
if(docBaseURI.endsWith("/")) {
2047+
if (docBaseURI.endsWith("/")) {
20292048
baseURI = XmldbURI.create(getOwnerDocument().getBaseURI(), false);
20302049
baseURI.append(baseURI);
20312050
} else {
@@ -2035,6 +2054,7 @@ private XmldbURI calculateBaseURI() {
20352054
}
20362055
}
20372056
}
2057+
20382058
return baseURI;
20392059
}
20402060

‎exist-core/src/test/java/org/exist/dom/persistent/NodeTest.java‎

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,28 @@
11
/*
2+
* Elemental
3+
* Copyright (C) 2024, Evolved Binary Ltd
4+
*
5+
* [email protected]
6+
* https://www.evolvedbinary.com | https://www.elemental.xyz
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; version 2.1.
11+
*
12+
* This library is distributed in the hope that it will be useful,
13+
* but WITHOUT ANY WARRANTY; without even the implied warranty of
14+
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
15+
* Lesser General Public License for more details.
16+
*
17+
* You should have received a copy of the GNU Lesser General Public
18+
* License along with this library; if not, write to the Free Software
19+
* Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
20+
*
21+
* NOTE: Parts of this file contain code from 'The eXist-db Authors'.
22+
* The original license header is included below.
23+
*
24+
* =====================================================================
25+
*
226
* eXist-db Open Source Native XML Database
327
* Copyright (C) 2001 The eXist-db Authors
428
*
@@ -180,6 +204,9 @@ public void attributeAxis() throws EXistException, LockException, PermissionDeni
180204

181205
assertEquals("1", first.getAttribute("ns:a"));
182206
assertEquals("1", first.getAttributeNS("http://foo.org", "a"));
207+
208+
assertEquals("", first.getAttribute("no-such-attr"));
209+
assertEquals("", first.getAttributeNS("http://foo.org", "no-such-attr"));
183210

184211
Attr attr = first.getAttributeNode("ns:a");
185212
assertNotNull(attr);

0 commit comments

Comments
 (0)