Skip to content

Commit a097a28

Browse files
line-oline-o
committed
[fix] in memory DOM implementation getAttribute nonnull
fixes #5672 Previously ElementImpl.getAttribute and ElementImpl.getAttributeNodeNS of in memory nodes violated the API contract of org.w3c.dom.element. It was returning null if an attribute was not set instead of an empty String. Fixing this lead to a host of changes throughout the project. The if-conditions are simplified and even flipped in a few locations were this greatly improved readability of thd code. The import of StringUtils was dropped were possible.
1 parent f76bf26 commit a097a28

File tree

44 files changed

+496
-523
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

44 files changed

+496
-523
lines changed

‎exist-core/src/main/java/org/exist/collections/CollectionConfiguration.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -163,7 +163,7 @@ protected void read(final DBBroker broker, final Document doc, final boolean che
163163
case VALIDATION_ELEMENT -> {
164164
final Element elem = (Element) node;
165165
final String mode = elem.getAttribute(VALIDATION_MODE_ATTR);
166-
if (mode == null) {
166+
if (mode.isEmpty()) {
167167
LOG.debug("Unable to determine validation mode in {}", srcCollectionURI);
168168
validationMode = XMLReaderObjectFactory.VALIDATION_SETTING.UNKNOWN;
169169
} else {

‎exist-core/src/main/java/org/exist/config/ConfigurationImpl.java‎

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -342,17 +342,17 @@ public Long getPropertyLong(final String name, final Long defaultValue, final bo
342342

343343
public Integer getPropertyMegabytes(String name, Integer defaultValue) {
344344
String cacheMem = element.getAttribute(name);
345-
if (cacheMem != null) {
346-
if (cacheMem.endsWith("M") || cacheMem.endsWith("m")) {
347-
cacheMem = cacheMem.substring(0, cacheMem.length() - 1);
348-
}
349-
final Integer result = Integer.valueOf(cacheMem);
350-
if (result < 0) {
351-
return defaultValue;
352-
}
353-
return result;
345+
if (cacheMem.isEmpty()) {
346+
return defaultValue;
347+
}
348+
if (cacheMem.endsWith("M") || cacheMem.endsWith("m")) {
349+
cacheMem = cacheMem.substring(0, cacheMem.length() - 1);
354350
}
355-
return defaultValue;
351+
final int result = Integer.parseInt(cacheMem);
352+
if (result < 0) {
353+
return defaultValue;
354+
}
355+
return result;
356356
}
357357

358358
public String getConfigFilePath() {

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

Lines changed: 54 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@
3737
import org.w3c.dom.*;
3838

3939
import javax.xml.XMLConstants;
40+
import javax.annotation.Nonnull;
4041
import java.util.HashMap;
4142
import java.util.HashSet;
4243
import java.util.Map;
@@ -77,6 +78,7 @@ public Node getFirstChild() {
7778
}
7879

7980
@Override
81+
@Nonnull
8082
public NodeList getChildNodes() {
8183
final NodeListImpl nl = new NodeListImpl(1); // nil elements are rare, so we use 1 here
8284
int nextNode = document.getFirstChildFor(nodeNumber);
@@ -100,6 +102,7 @@ public boolean hasAttributes() {
100102
}
101103

102104
@Override
105+
@Nonnull
103106
public String getAttribute(final String name) {
104107
int attr = document.alpha[nodeNumber];
105108
if(-1 < attr) {
@@ -123,7 +126,7 @@ public String getAttribute(final String name) {
123126
}
124127
}
125128
}
126-
return null;
129+
return "";
127130
}
128131

129132
@Override
@@ -349,6 +352,7 @@ public void selectDescendants(final boolean includeSelf, final NodeTest test, fi
349352
}
350353

351354
@Override
355+
@Nonnull
352356
public NodeList getElementsByTagName(final String name) {
353357
if(name != null && name.equals(QName.WILDCARD)) {
354358
return getElementsByTagName(new QName.WildcardLocalPartQName(XMLConstants.DEFAULT_NS_PREFIX));
@@ -368,6 +372,7 @@ public NodeList getElementsByTagName(final String name) {
368372
}
369373

370374
@Override
375+
@Nonnull
371376
public NodeList getElementsByTagNameNS(final String namespaceURI, final String localName) {
372377
final boolean wildcardNS = namespaceURI != null && namespaceURI.equals(QName.WILDCARD);
373378
final boolean wildcardLocalPart = localName != null && localName.equals(QName.WILDCARD);
@@ -409,6 +414,7 @@ private NodeList getElementsByTagName(final QName qname) {
409414
}
410415

411416
@Override
417+
@Nonnull
412418
public String getAttributeNS(final String namespaceURI, final String localName) {
413419
int attr = document.alpha[nodeNumber];
414420
if(-1 < attr) {
@@ -432,7 +438,7 @@ public String getAttributeNS(final String namespaceURI, final String localName)
432438
}
433439
}
434440
}
435-
return null;
441+
return "";
436442
}
437443

438444
@Override
@@ -473,12 +479,56 @@ public Attr setAttributeNodeNS(final Attr newAttr) throws DOMException {
473479

474480
@Override
475481
public boolean hasAttribute(final String name) {
476-
return getAttribute(name) != null;
482+
int attr = document.alpha[nodeNumber];
483+
if(-1 < attr) {
484+
while(attr < document.nextAttr && document.attrParent[attr] == nodeNumber) {
485+
final QName attrQName = document.attrName[attr];
486+
if(attrQName.getStringValue().equals(name)) {
487+
return true;
488+
}
489+
++attr;
490+
}
491+
}
492+
if(name.startsWith(XMLConstants.XMLNS_ATTRIBUTE + ":")) {
493+
int ns = document.alphaLen[nodeNumber];
494+
if(-1 < ns) {
495+
while(ns < document.nextNamespace && document.namespaceParent[ns] == nodeNumber) {
496+
final QName nsQName = document.namespaceCode[ns];
497+
if(nsQName.getStringValue().equals(name)) {
498+
return true;
499+
}
500+
++ns;
501+
}
502+
}
503+
}
504+
return false;
477505
}
478506

479507
@Override
480508
public boolean hasAttributeNS(final String namespaceURI, final String localName) {
481-
return getAttributeNS(namespaceURI, localName) != null;
509+
int attr = document.alpha[nodeNumber];
510+
if(-1 < attr) {
511+
while(attr < document.nextAttr && document.attrParent[attr] == nodeNumber) {
512+
final QName name = document.attrName[attr];
513+
if(name.getLocalPart().equals(localName) && name.getNamespaceURI().equals(namespaceURI)) {
514+
return true;
515+
}
516+
++attr;
517+
}
518+
}
519+
if(Namespaces.XMLNS_NS.equals(namespaceURI)) {
520+
int ns = document.alphaLen[nodeNumber];
521+
if(-1 < ns) {
522+
while(ns < document.nextNamespace && document.namespaceParent[ns] == nodeNumber) {
523+
final QName nsQName = document.namespaceCode[ns];
524+
if(nsQName.getLocalPart().equals(localName)) {
525+
return true;
526+
}
527+
++ns;
528+
}
529+
}
530+
}
531+
return false;
482532
}
483533

484534
/**

‎exist-core/src/main/java/org/exist/http/Descriptor.java‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -144,8 +144,9 @@ private Descriptor() {
144144
final Path logFile = Paths.get("request-replay-log.txt");
145145
bufWriteReplayLog = Files.newBufferedWriter(logFile);
146146
final String attr = doc.getDocumentElement().getAttribute("filtered");
147-
if (attr != null)
147+
if (!attr.isEmpty()) {
148148
requestsFiltered = "true".equals(attr);
149+
}
149150
}
150151

151152
//load <allow-source> settings
@@ -206,7 +207,7 @@ private void configureAllowSourceXQuery(Element allowsourcexqueries) {
206207
String path = elem.getAttribute("path"); //@path
207208

208209
//must be a path to allow source for
209-
if (path == null) {
210+
if (path.isEmpty()) {
210211
LOG.warn("Error element 'xquery' requires an attribute 'path'");
211212
return;
212213
}
@@ -242,15 +243,15 @@ private void configureMaps(Element maps) {
242243
String view = elem.getAttribute("view"); //@view
243244

244245
//must be a path or a pattern to map from
245-
if (path == null /*&& pattern == null*/) {
246+
if (path.isEmpty() /*&& pattern == null*/) {
246247
LOG.warn("Error element 'map' requires an attribute 'path' or an attribute 'pattern'");
247248
return;
248249
}
249250
path = path.replaceAll("\\$\\{WEBAPP_HOME\\}",
250251
SingleInstanceConfiguration.getWebappHome().orElse(Paths.get(".")).toAbsolutePath().toString().replace('\\', '/'));
251252

252253
//must be a view to map to
253-
if (view == null) {
254+
if (view.isEmpty()) {
254255
LOG.warn("Error element 'map' requires an attribute 'view'");
255256
return;
256257
}

‎exist-core/src/main/java/org/exist/http/RESTServer.java‎

Lines changed: 10 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -774,7 +774,7 @@ public void doPost(final DBBroker broker, final Txn transaction, final HttpServl
774774
if (Query.xmlKey().equals(root.getLocalName())) {
775775
// process <query>xpathQuery</query>
776776
String option = root.getAttribute(Start.xmlKey());
777-
if (option != null) {
777+
if (option.isEmpty()) {
778778
try {
779779
start = Integer.parseInt(option);
780780
} catch (final NumberFormatException e) {
@@ -783,7 +783,7 @@ public void doPost(final DBBroker broker, final Txn transaction, final HttpServl
783783
}
784784

785785
option = root.getAttribute(Max.xmlKey());
786-
if (option != null) {
786+
if (option.isEmpty()) {
787787
try {
788788
howmany = Integer.parseInt(option);
789789
} catch (final NumberFormatException e) {
@@ -792,42 +792,41 @@ public void doPost(final DBBroker broker, final Txn transaction, final HttpServl
792792
}
793793

794794
option = root.getAttribute(Enclose.xmlKey());
795-
if (option != null) {
795+
if (!option.isEmpty()) {
796796
if ("no".equals(option)) {
797797
enclose = false;
798798
}
799799
} else {
800800
option = root.getAttribute(Wrap.xmlKey());
801-
if (option != null) {
801+
if (!option.isEmpty()) {
802802
if ("no".equals(option)) {
803803
enclose = false;
804804
}
805805
}
806806
}
807807

808808
option = root.getAttribute(Method.xmlKey());
809-
if ((option != null) && (!option.isEmpty())) {
809+
if (!option.isEmpty()) {
810810
outputProperties.setProperty(SERIALIZATION_METHOD_PROPERTY, option);
811811
}
812812

813813
option = root.getAttribute(Typed.xmlKey());
814-
if (option != null) {
814+
if (!option.isEmpty()) {
815815
if ("yes".equals(option)) {
816816
typed = true;
817817
}
818818
}
819819

820820
option = root.getAttribute(Mime.xmlKey());
821-
if ((option != null) && (!option.isEmpty())) {
821+
if (!option.isEmpty()) {
822822
mimeType = option;
823823
}
824824

825-
if ((option = root.getAttribute(Cache.xmlKey())) != null) {
825+
if (!(option = root.getAttribute(Cache.xmlKey())).isEmpty()) {
826826
cache = "yes".equals(option);
827827
}
828828

829-
if ((option = root.getAttribute(Session.xmlKey())) != null
830-
&& !option.isEmpty()) {
829+
if (!(option = root.getAttribute(Session.xmlKey())).isEmpty()) {
831830
outputProperties.setProperty(
832831
Serializer.PROPERTY_SESSION_ID, option);
833832
}
@@ -866,7 +865,7 @@ public void doPost(final DBBroker broker, final Txn transaction, final HttpServl
866865
final String value = property.getAttribute("value");
867866
LOG.debug("{} = {}", key, value);
868867

869-
if (key != null && value != null) {
868+
if (!key.isEmpty() && !value.isEmpty()) {
870869
outputProperties.setProperty(key, value);
871870
}
872871
}

‎exist-core/src/main/java/org/exist/http/urlrewrite/ModuleCall.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,7 @@ public class ModuleCall extends URLRewrite {
4444
public ModuleCall(final Element config, final XQueryContext context, final String uri) throws ServletException {
4545
super(config, uri);
4646
String funcName = config.getAttribute("function");
47-
if (funcName == null || funcName.isEmpty()) {
47+
if (funcName.isEmpty()) {
4848
throw new ServletException("<exist:call> requires an attribute 'function'.");
4949
}
5050
int arity = 0;

‎exist-core/src/main/java/org/exist/http/urlrewrite/PathForward.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -38,11 +38,11 @@ public PathForward(final ServletConfig filterConfig, final Element config, final
3838
this.filterConfig = filterConfig;
3939
final String url = config.getAttribute("url");
4040
servletName = config.getAttribute("servlet");
41-
if (servletName != null && servletName.isEmpty()) {
41+
if (servletName.isEmpty()) {
4242
servletName = null;
4343
}
4444
if (servletName == null) {
45-
if (url == null || url.isEmpty()) {
45+
if (url.isEmpty()) {
4646
throw new ServletException("<exist:forward> needs either an attribute 'url' or 'servlet'.");
4747
}
4848
setTarget(URLRewrite.normalizePath(url));

‎exist-core/src/main/java/org/exist/http/urlrewrite/RewriteConfig.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -206,7 +206,7 @@ private void parse(final Document doc) throws ServletException {
206206
if (child.getNodeType() == Node.ELEMENT_NODE && Namespaces.EXIST_NS.equals(ns)) {
207207
final Element elem = (Element) child;
208208
final String pattern = elem.getAttribute(PATTERN_ATTRIBUTE);
209-
if (pattern == null) {
209+
if (pattern.isEmpty()) {
210210
throw new ServletException("Action in controller-config.xml has no pattern: " + elem.toString());
211211
}
212212
final URLRewrite urw = parseAction(urlRewrite.getConfig(), pattern, elem);
@@ -232,7 +232,7 @@ private URLRewrite parseAction(final ServletConfig config, final String pattern,
232232
* as an attribute on the ControllerForward object.
233233
*/
234234
final String serverName = action.getAttribute(SERVER_NAME_ATTRIBUTE);
235-
if (serverName != null && !serverName.isEmpty()) {
235+
if (!serverName.isEmpty()) {
236236
cf.setServerName(serverName);
237237
}
238238
rewrite = cf;

‎exist-core/src/main/java/org/exist/http/urlrewrite/URLRewrite.java‎

Lines changed: 23 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -56,30 +56,35 @@ public abstract class URLRewrite {
5656

5757
protected URLRewrite(final Element config, final String uri) {
5858
this.uri = uri;
59-
if (config != null && config.hasAttribute("absolute"))
59+
if (config == null) {
60+
return;
61+
}
62+
if (config.hasAttribute("absolute")) {
6063
absolute = "yes".equals(config.getAttribute("absolute"));
61-
if (config != null && config.hasAttribute("method")) {
64+
}
65+
if (config.hasAttribute("method")) {
6266
method = config.getAttribute("method").toUpperCase();
6367
}
6468
// Check for add-parameter elements etc.
65-
if (config != null && config.hasChildNodes()) {
66-
Node node = config.getFirstChild();
67-
while (node != null) {
68-
final String ns = node.getNamespaceURI();
69-
if (node.getNodeType() == Node.ELEMENT_NODE && Namespaces.EXIST_NS.equals(ns)) {
70-
final Element elem = (Element) node;
71-
if ("add-parameter".equals(elem.getLocalName())) {
72-
addParameter(elem.getAttribute("name"), elem.getAttribute("value"));
73-
} else if ("set-attribute".equals(elem.getLocalName())) {
74-
setAttribute(elem.getAttribute("name"), elem.getAttribute("value"));
75-
} else if ("clear-attribute".equals(elem.getLocalName())) {
76-
unsetAttribute(elem.getAttribute("name"));
77-
} else if ("set-header".equals(elem.getLocalName())) {
78-
setHeader(elem.getAttribute("name"), elem.getAttribute("value"));
79-
}
69+
if (!config.hasChildNodes()) {
70+
return;
71+
}
72+
Node node = config.getFirstChild();
73+
while (node != null) {
74+
final String ns = node.getNamespaceURI();
75+
if (node.getNodeType() == Node.ELEMENT_NODE && Namespaces.EXIST_NS.equals(ns)) {
76+
final Element elem = (Element) node;
77+
if ("add-parameter".equals(elem.getLocalName())) {
78+
addParameter(elem.getAttribute("name"), elem.getAttribute("value"));
79+
} else if ("set-attribute".equals(elem.getLocalName())) {
80+
setAttribute(elem.getAttribute("name"), elem.getAttribute("value"));
81+
} else if ("clear-attribute".equals(elem.getLocalName())) {
82+
unsetAttribute(elem.getAttribute("name"));
83+
} else if ("set-header".equals(elem.getLocalName())) {
84+
setHeader(elem.getAttribute("name"), elem.getAttribute("value"));
8085
}
81-
node = node.getNextSibling();
8286
}
87+
node = node.getNextSibling();
8388
}
8489
}
8590

‎exist-core/src/main/java/org/exist/indexing/AbstractIndex.java‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -49,8 +49,9 @@ public void configure(final BrokerPool pool, final Path dataDir, final Element c
4949
throws DatabaseConfigurationException {
5050
this.pool = pool;
5151
this.dataDir = dataDir;
52-
if (config != null && config.hasAttribute("id"))
53-
{name = config.getAttribute("id");}
52+
if (config != null && config.hasAttribute("id")) {
53+
name = config.getAttribute("id");
54+
}
5455
}
5556

5657
@Override

0 commit comments

Comments
 (0)