From 758c85a1c9d85600de84b961d246a784511b1916 Mon Sep 17 00:00:00 2001 From: Stephen Paul Weber Date: Sun, 21 Aug 2022 21:40:33 -0500 Subject: [PATCH] Preserve Text Nodes in XML The XML model that was being used was not able to represent elements with text nodes and element children interspersed (such as in "markup like" XML: abc def ghi). This switches from a single content String to a list of child Node. Content is pulled from all children recursively. A list of Element children is also maintained since this list is frequently traversed so it saves runtime checks in all of those loops at the expense of a small amount of memory. Since the children and childNode lists must be kept in sync, they are both made private to avoid a child class trying to mutate one of them without a safe helper. --- .../eu/siacs/conversations/xml/Element.java | 60 +++++++++++++------ .../conversations/xml/LocalizedContent.java | 2 +- .../java/eu/siacs/conversations/xml/Node.java | 5 ++ .../eu/siacs/conversations/xml/TextNode.java | 20 +++++++ .../eu/siacs/conversations/xml/XmlReader.java | 11 +--- .../siacs/conversations/xmpp/forms/Data.java | 9 +-- .../siacs/conversations/xmpp/forms/Field.java | 17 ++---- .../xmpp/jingle/stanzas/Group.java | 2 +- .../xmpp/jingle/stanzas/Propose.java | 2 +- .../xmpp/jingle/stanzas/RtpDescription.java | 24 ++------ .../xmpp/stanzas/MessagePacket.java | 10 ++-- 11 files changed, 87 insertions(+), 75 deletions(-) create mode 100644 src/main/java/eu/siacs/conversations/xml/Node.java create mode 100644 src/main/java/eu/siacs/conversations/xml/TextNode.java diff --git a/src/main/java/eu/siacs/conversations/xml/Element.java b/src/main/java/eu/siacs/conversations/xml/Element.java index 4d53a17b723f3b179bf8446d82829b7ebc0b3348..d70e45ef6093e330bf8d352eb8cd562ba194c102 100644 --- a/src/main/java/eu/siacs/conversations/xml/Element.java +++ b/src/main/java/eu/siacs/conversations/xml/Element.java @@ -3,19 +3,21 @@ package eu.siacs.conversations.xml; import org.jetbrains.annotations.NotNull; import java.util.ArrayList; +import java.util.Collection; import java.util.Hashtable; import java.util.List; +import java.util.stream.Collectors; import eu.siacs.conversations.utils.XmlHelper; import eu.siacs.conversations.xmpp.InvalidJid; import eu.siacs.conversations.xmpp.Jid; import eu.siacs.conversations.xmpp.stanzas.MessagePacket; -public class Element { +public class Element implements Node { private final String name; private Hashtable attributes = new Hashtable<>(); - private String content; - protected List children = new ArrayList<>(); + private List children = new ArrayList<>(); + private List childNodes = new ArrayList<>(); public Element(String name) { this.name = name; @@ -26,30 +28,52 @@ public class Element { this.setAttribute("xmlns", xmlns); } - public Element addChild(Element child) { - this.content = null; - children.add(child); + public Node prependChild(Node child) { + childNodes.add(0, child); + if (child instanceof Element) children.add(0, (Element) child); + return child; + } + + public Node addChild(Node child) { + childNodes.add(child); + if (child instanceof Element) children.add((Element) child); return child; } public Element addChild(String name) { - this.content = null; Element child = new Element(name); + childNodes.add(child); children.add(child); return child; } public Element addChild(String name, String xmlns) { - this.content = null; Element child = new Element(name); child.setAttribute("xmlns", xmlns); + childNodes.add(child); children.add(child); return child; } + public void addChildren(final Collection children) { + if (children == null) return; + + this.childNodes.addAll(children); + for (Node node : children) { + if (node instanceof Element) { + this.children.add((Element) node); + } + } + } + + public void removeChild(Node child) { + this.childNodes.remove(child); + if (child instanceof Element) this.children.remove(child); + } + public Element setContent(String content) { - this.content = content; - this.children.clear(); + clearChildren(); + if (content != null) this.childNodes.add(new TextNode(content)); return this; } @@ -106,17 +130,18 @@ public class Element { return findChild(name, xmlns) != null; } - public List getChildren() { + public final List getChildren() { return this.children; } public Element setChildren(List children) { + this.childNodes = new ArrayList(children); this.children = children; return this; } public final String getContent() { - return content; + return this.childNodes.stream().map(Node::getContent).filter(c -> c != null).collect(Collectors.joining()); } public Element setAttribute(String name, String value) { @@ -170,7 +195,7 @@ public class Element { @NotNull public String toString() { final StringBuilder elementOutput = new StringBuilder(); - if ((content == null) && (children.size() == 0)) { + if (childNodes.size() == 0) { Tag emptyTag = Tag.empty(name); emptyTag.setAtttributes(this.attributes); elementOutput.append(emptyTag.toString()); @@ -178,12 +203,8 @@ public class Element { Tag startTag = Tag.start(name); startTag.setAtttributes(this.attributes); elementOutput.append(startTag); - if (content != null) { - elementOutput.append(XmlHelper.encodeEntities(content)); - } else { - for (Element child : children) { - elementOutput.append(child.toString()); - } + for (Node child : childNodes) { + elementOutput.append(child.toString()); } Tag endTag = Tag.end(name); elementOutput.append(endTag); @@ -197,6 +218,7 @@ public class Element { public void clearChildren() { this.children.clear(); + this.childNodes.clear(); } public void setAttribute(String name, long value) { diff --git a/src/main/java/eu/siacs/conversations/xml/LocalizedContent.java b/src/main/java/eu/siacs/conversations/xml/LocalizedContent.java index fac5099a70f8efed7c8c0b73bbefb6ffbf5d4638..6fa71e9625e95891a9c868f94b13fff82b5c781e 100644 --- a/src/main/java/eu/siacs/conversations/xml/LocalizedContent.java +++ b/src/main/java/eu/siacs/conversations/xml/LocalizedContent.java @@ -23,7 +23,7 @@ public class LocalizedContent { public static LocalizedContent get(final Element element, String name) { final HashMap contents = new HashMap<>(); final String parentLanguage = element.getAttribute("xml:lang"); - for(Element child : element.children) { + for(Element child : element.getChildren()) { if (name.equals(child.getName())) { final String namespace = child.getNamespace(); final String childLanguage = child.getAttribute("xml:lang"); diff --git a/src/main/java/eu/siacs/conversations/xml/Node.java b/src/main/java/eu/siacs/conversations/xml/Node.java new file mode 100644 index 0000000000000000000000000000000000000000..dbe18d252ddc981f9b0d6c3e41d6f97fc211c8b0 --- /dev/null +++ b/src/main/java/eu/siacs/conversations/xml/Node.java @@ -0,0 +1,5 @@ +package eu.siacs.conversations.xml; + +public interface Node { + public String getContent(); +} diff --git a/src/main/java/eu/siacs/conversations/xml/TextNode.java b/src/main/java/eu/siacs/conversations/xml/TextNode.java new file mode 100644 index 0000000000000000000000000000000000000000..edbc1d312a6e282470d56f2ba279cd054b9ee52c --- /dev/null +++ b/src/main/java/eu/siacs/conversations/xml/TextNode.java @@ -0,0 +1,20 @@ +package eu.siacs.conversations.xml; + +import eu.siacs.conversations.utils.XmlHelper; + +public class TextNode implements Node { + protected String content; + + public TextNode(final String content) { + if (content == null) throw new IllegalArgumentException("null TextNode is not allowed"); + this.content = content; + } + + public String getContent() { + return content; + } + + public String toString() { + return XmlHelper.encodeEntities(content); + } +} diff --git a/src/main/java/eu/siacs/conversations/xml/XmlReader.java b/src/main/java/eu/siacs/conversations/xml/XmlReader.java index 240b92b7ae9ae11ab7cbbdfbca51403f8231d356..c775fd34c67804e00f21dbedbd26109dec3f9773 100644 --- a/src/main/java/eu/siacs/conversations/xml/XmlReader.java +++ b/src/main/java/eu/siacs/conversations/xml/XmlReader.java @@ -94,15 +94,10 @@ public class XmlReader implements Closeable { if (nextTag == null) { throw new IOException("interrupted mid tag"); } - if (nextTag.isNo()) { - element.setContent(nextTag.getName()); - nextTag = this.readTag(); - if (nextTag == null) { - throw new IOException("interrupted mid tag"); - } - } while (!nextTag.isEnd(element.getName())) { - if (!nextTag.isNo()) { + if (nextTag.isNo()) { + element.addChild(new TextNode(nextTag.getName())); + } else { Element child = this.readElement(nextTag); element.addChild(child); } diff --git a/src/main/java/eu/siacs/conversations/xmpp/forms/Data.java b/src/main/java/eu/siacs/conversations/xmpp/forms/Data.java index e3bd9eb7481229914bd0fdc144373036420b7a53..16578967586c3f98a95074274b7950c48d6b8388 100644 --- a/src/main/java/eu/siacs/conversations/xmpp/forms/Data.java +++ b/src/main/java/eu/siacs/conversations/xmpp/forms/Data.java @@ -4,8 +4,8 @@ import android.os.Bundle; import java.util.ArrayList; import java.util.Collection; -import java.util.Iterator; import java.util.List; +import java.util.stream.Collectors; import eu.siacs.conversations.xml.Element; import eu.siacs.conversations.xml.Namespace; @@ -77,12 +77,7 @@ public class Data extends Element { } private void removeUnnecessaryChildren() { - for(Iterator iterator = this.children.iterator(); iterator.hasNext();) { - Element element = iterator.next(); - if (!element.getName().equals("field") && !element.getName().equals("title")) { - iterator.remove(); - } - } + setChildren(getChildren().stream().filter(element -> element.getName().equals("field") || element.getName().equals("title")).collect(Collectors.toList())); } public static Data parse(Element element) { diff --git a/src/main/java/eu/siacs/conversations/xmpp/forms/Field.java b/src/main/java/eu/siacs/conversations/xmpp/forms/Field.java index d6ac14707b12dfff17b3937a51546790acf2a9e9..6e97b552c70c2070434cf3bfd170343e62052a5c 100644 --- a/src/main/java/eu/siacs/conversations/xmpp/forms/Field.java +++ b/src/main/java/eu/siacs/conversations/xmpp/forms/Field.java @@ -2,8 +2,8 @@ package eu.siacs.conversations.xmpp.forms; import java.util.ArrayList; import java.util.Collection; -import java.util.Iterator; import java.util.List; +import java.util.stream.Collectors; import eu.siacs.conversations.xml.Element; @@ -23,24 +23,15 @@ public class Field extends Element { } public void setValue(String value) { - this.children.clear(); - this.addChild("value").setContent(value); + setChildren(List.of(new Element("value").setContent(value))); } public void setValues(Collection values) { - this.children.clear(); - for(String value : values) { - this.addChild("value").setContent(value); - } + setChildren(values.stream().map(val -> new Element("value").setContent(val)).collect(Collectors.toList())); } public void removeNonValueChildren() { - for(Iterator iterator = this.children.iterator(); iterator.hasNext();) { - Element element = iterator.next(); - if (!element.getName().equals("value")) { - iterator.remove(); - } - } + setChildren(getChildren().stream().filter(element -> element.getName().equals("value")).collect(Collectors.toList())); } public static Field parse(Element element) { diff --git a/src/main/java/eu/siacs/conversations/xmpp/jingle/stanzas/Group.java b/src/main/java/eu/siacs/conversations/xmpp/jingle/stanzas/Group.java index eb5c32252bf181d5037a02cbe1aaebf7ee8cd4f9..2f7873a860fc2d23dcabc75251578c333fc27b01 100644 --- a/src/main/java/eu/siacs/conversations/xmpp/jingle/stanzas/Group.java +++ b/src/main/java/eu/siacs/conversations/xmpp/jingle/stanzas/Group.java @@ -29,7 +29,7 @@ public class Group extends Element { public List getIdentificationTags() { final ImmutableList.Builder builder = new ImmutableList.Builder<>(); - for (final Element child : this.children) { + for (final Element child : getChildren()) { if ("content".equals(child.getName())) { final String name = child.getAttribute("name"); if (name != null) { diff --git a/src/main/java/eu/siacs/conversations/xmpp/jingle/stanzas/Propose.java b/src/main/java/eu/siacs/conversations/xmpp/jingle/stanzas/Propose.java index da3a93da301dde0d4b0dc19f3f972629d3d7af8e..f72162be8e5896e22184ee41f699c1caafa0e13a 100644 --- a/src/main/java/eu/siacs/conversations/xmpp/jingle/stanzas/Propose.java +++ b/src/main/java/eu/siacs/conversations/xmpp/jingle/stanzas/Propose.java @@ -15,7 +15,7 @@ public class Propose extends Element { public List getDescriptions() { final ImmutableList.Builder builder = new ImmutableList.Builder<>(); - for (final Element child : this.children) { + for (final Element child : getChildren()) { if ("description".equals(child.getName())) { final String namespace = child.getNamespace(); if (FileTransferDescription.NAMESPACES.contains(namespace)) { diff --git a/src/main/java/eu/siacs/conversations/xmpp/jingle/stanzas/RtpDescription.java b/src/main/java/eu/siacs/conversations/xmpp/jingle/stanzas/RtpDescription.java index 650c26bef07af1eb1876641c4c4eb6c95cef3784..b1cb59a34d630230ffbcf0dcfd5b20979b971ac2 100644 --- a/src/main/java/eu/siacs/conversations/xmpp/jingle/stanzas/RtpDescription.java +++ b/src/main/java/eu/siacs/conversations/xmpp/jingle/stanzas/RtpDescription.java @@ -66,7 +66,7 @@ public class RtpDescription extends GenericDescription { public List getSources() { final ImmutableList.Builder builder = new ImmutableList.Builder<>(); - for (final Element child : this.children) { + for (final Element child : getChildren()) { if ("source".equals(child.getName()) && Namespace.JINGLE_RTP_SOURCE_SPECIFIC_MEDIA_ATTRIBUTES.equals(child.getNamespace())) { builder.add(Source.upgrade(child)); } @@ -76,7 +76,7 @@ public class RtpDescription extends GenericDescription { public List getSourceGroups() { final ImmutableList.Builder builder = new ImmutableList.Builder<>(); - for (final Element child : this.children) { + for (final Element child : getChildren()) { if ("ssrc-group".equals(child.getName()) && Namespace.JINGLE_RTP_SOURCE_SPECIFIC_MEDIA_ATTRIBUTES.equals(child.getNamespace())) { builder.add(SourceGroup.upgrade(child)); } @@ -326,16 +326,8 @@ public class RtpDescription extends GenericDescription { return null; } - public void addChildren(final List children) { - if (children != null) { - this.children.addAll(children); - } - } - public void addParameters(List parameters) { - if (parameters != null) { - this.children.addAll(parameters); - } + addChildren(parameters); } } @@ -442,7 +434,7 @@ public class RtpDescription extends GenericDescription { public List getParameters() { ImmutableList.Builder builder = new ImmutableList.Builder<>(); - for (Element child : this.children) { + for (Element child : getChildren()) { if ("parameter".equals(child.getName())) { builder.add(Parameter.upgrade(child)); } @@ -512,7 +504,7 @@ public class RtpDescription extends GenericDescription { public List getSsrcs() { ImmutableList.Builder builder = new ImmutableList.Builder<>(); - for (Element child : this.children) { + for (Element child : getChildren()) { if ("source".equals(child.getName())) { final String ssrc = child.getAttribute("ssrc"); if (ssrc != null) { @@ -610,10 +602,4 @@ public class RtpDescription extends GenericDescription { } return rtpDescription; } - - private void addChildren(List elements) { - if (elements != null) { - this.children.addAll(elements); - } - } } diff --git a/src/main/java/eu/siacs/conversations/xmpp/stanzas/MessagePacket.java b/src/main/java/eu/siacs/conversations/xmpp/stanzas/MessagePacket.java index 86068bf774efe9ed255c1def48812d0286ec8827..bac76adfc7f419c4f5bf10c953d43743772888be 100644 --- a/src/main/java/eu/siacs/conversations/xmpp/stanzas/MessagePacket.java +++ b/src/main/java/eu/siacs/conversations/xmpp/stanzas/MessagePacket.java @@ -22,15 +22,13 @@ public class MessagePacket extends AbstractAcknowledgeableStanza { } public void setBody(String text) { - this.children.remove(findChild("body")); - Element body = new Element("body"); - body.setContent(text); - this.children.add(0, body); + removeChild(findChild("body")); + prependChild(new Element("body").setContent(text)); } public void setAxolotlMessage(Element axolotlMessage) { - this.children.remove(findChild("body")); - this.children.add(0, axolotlMessage); + removeChild(findChild("body")); + prependChild(axolotlMessage); } public void setType(int type) {