avoid xmlstring builder toString (#103)

* Return CharSequence, do not convert to String. * fix: Avoid using XmlStringBuilder.toString and the generic append(CharSequence).
jitsi · Oct 25, 2023 · 62d03d4 · 62d03d4
1 parent 85a0ea1
commit 62d03d4
Show file tree

Hide file tree

Showing 13 changed files with 101 additions and 30 deletions.
diff --git a/src/main/java/org/jitsi/xmpp/extensions/colibri/ColibriStatsExtension.java b/src/main/java/org/jitsi/xmpp/extensions/colibri/ColibriStatsExtension.java
@@ -679,7 +679,7 @@ public void setValue(Object value)
         }
 
         @Override
-        public String toXML(XmlEnvironment enclosingNamespace)
+        public CharSequence toXML(XmlEnvironment enclosingNamespace)
         {
             String name = getName();
             Object value = getValue();
@@ -694,8 +694,7 @@ public String toXML(XmlEnvironment enclosingNamespace)
                     .halfOpenElement(ELEMENT)
                     .attribute(NAME_ATTR_NAME, name)
                     .attribute(VALUE_ATTR_NAME, value.toString())
-                    .closeEmptyElement()
-                    .toString();
+                    .closeEmptyElement();
             }
         }
     }

diff --git a/src/main/java/org/jitsi/xmpp/extensions/colibri/ColibriStatsIQ.java b/src/main/java/org/jitsi/xmpp/extensions/colibri/ColibriStatsIQ.java
@@ -16,6 +16,7 @@
 package org.jitsi.xmpp.extensions.colibri;
 
 import org.jivesoftware.smack.packet.*;
+import org.jitsi.xmpp.util.*;
 
 /**
  * The stats IQ that can be used to request Colibri stats on demand
@@ -51,7 +52,7 @@ public ColibriStatsIQ()
     protected IQ.IQChildElementXmlStringBuilder getIQChildElementBuilder(IQ.IQChildElementXmlStringBuilder buf)
     {
         buf.rightAngleBracket();
-        buf.append(backEnd.toXML());
+        buf.append(XmlStringBuilderUtil.toStringOpt(backEnd));
         return buf;
     }
 

diff --git a/src/main/java/org/jitsi/xmpp/extensions/inputevt/RemoteControlExtension.java b/src/main/java/org/jitsi/xmpp/extensions/inputevt/RemoteControlExtension.java
@@ -129,7 +129,8 @@ public String getNamespace()
      *
      * @return XML representation of the item
      */
-    public String toXML(XmlEnvironment enclosingNamespace)
+    @Override
+    public CharSequence toXML(XmlEnvironment enclosingNamespace)
     {
         String ret = null;
 
@@ -174,7 +175,7 @@ public String toXML(XmlEnvironment enclosingNamespace)
         else if (event instanceof KeyEvent)
         {
             KeyEvent e = (KeyEvent)event;
-            int keycode = e.getKeyCode();
+            int keycode;
             int key = e.getKeyChar();
 
             if (key != KeyEvent.CHAR_UNDEFINED)

diff --git a/src/main/java/org/jitsi/xmpp/extensions/jibri/StanzaErrorPE.java b/src/main/java/org/jitsi/xmpp/extensions/jibri/StanzaErrorPE.java
@@ -84,8 +84,8 @@ public String getNamespace()
      * {@inheritDoc}
      */
     @Override
-    public String toXML(XmlEnvironment enclosingNamespace)
+    public CharSequence toXML(XmlEnvironment enclosingNamespace)
     {
-        return error.toXML().toString();
+        return error.toXML();
     }
 }
diff --git a/src/main/java/org/jitsi/xmpp/extensions/jingle/JingleIQ.java b/src/main/java/org/jitsi/xmpp/extensions/jingle/JingleIQ.java
@@ -19,6 +19,7 @@
 import java.security.*;
 import java.util.*;
 
+import org.jitsi.xmpp.util.*;
 import org.jivesoftware.smack.packet.*;
 import org.jxmpp.jid.Jid;
 
@@ -155,18 +156,18 @@ protected IQ.IQChildElementXmlStringBuilder getIQChildElementBuilder(IQ.IQChildE
             //content
             for (ContentPacketExtension cpe : contentList)
             {
-                bldr.append(cpe.toXML());
+                XmlStringBuilderUtil.append0(bldr, cpe.toXML());
             }
 
             //reason
             if (reason != null)
-                bldr.append(reason.toXML());
+                XmlStringBuilderUtil.append0(bldr, reason.toXML());
 
             //session-info
             //XXX: this is RTP specific so we should probably handle it in a
             //subclass
             if (sessionInfo != null)
-                bldr.append(sessionInfo.toXML());
+                XmlStringBuilderUtil.append0(bldr, sessionInfo.toXML());
         }
 
         return bldr;

diff --git a/src/main/java/org/jitsi/xmpp/extensions/jingle/ReasonPacketExtension.java b/src/main/java/org/jitsi/xmpp/extensions/jingle/ReasonPacketExtension.java
@@ -15,6 +15,7 @@
  */
 package org.jitsi.xmpp.extensions.jingle;
 
+import org.jitsi.xmpp.util.*;
 import org.jivesoftware.smack.packet.*;
 import org.jivesoftware.smack.util.*;
 
@@ -172,7 +173,8 @@ public String getNamespace()
      *
      * @return the packet extension as XML.
      */
-    public String toXML(XmlEnvironment enclosingNamespace)
+    @Override
+    public CharSequence toXML(XmlEnvironment enclosingNamespace)
     {
         XmlStringBuilder xml = new XmlStringBuilder();
         xml.openElement(getElementName());
@@ -188,11 +190,11 @@ public String toXML(XmlEnvironment enclosingNamespace)
         //add the extra element if it has been specified.
         if (getOtherExtension() != null)
         {
-            xml.append(getOtherExtension().toXML());
+            xml.append(XmlStringBuilderUtil.toStringOpt(getOtherExtension()));
         }
 
         xml.closeElement(getElementName());
 
-        return xml.toString();
+        return xml;
     }
 }
diff --git a/src/main/java/org/jitsi/xmpp/extensions/jingle/SctpMapExtension.java b/src/main/java/org/jitsi/xmpp/extensions/jingle/SctpMapExtension.java
@@ -92,7 +92,7 @@ public String getNamespace()
      * {@inheritDoc}
      */
     @Override
-    public String toXML(XmlEnvironment enclosingNamespace)
+    public CharSequence toXML(XmlEnvironment enclosingNamespace)
     {
         XmlStringBuilder xml = new XmlStringBuilder();
 
@@ -105,7 +105,7 @@ public String toXML(XmlEnvironment enclosingNamespace)
 
         xml.closeEmptyElement();
 
-        return xml.toString();
+        return xml;
     }
 
     public void setPort(int port)

diff --git a/src/main/java/org/jitsi/xmpp/extensions/jingleinfo/RelayPacketExtension.java b/src/main/java/org/jitsi/xmpp/extensions/jingleinfo/RelayPacketExtension.java
@@ -77,7 +77,7 @@ public String getToken()
      * @return XML string representation
      */
     @Override
-    public String toXML(XmlEnvironment enclosingNamespace)
+    public CharSequence toXML(XmlEnvironment enclosingNamespace)
     {
         XmlStringBuilder xml = new XmlStringBuilder();
 
@@ -95,6 +95,6 @@ public String toXML(XmlEnvironment enclosingNamespace)
 
         xml.closeElement(ELEMENT);
 
-        return xml.toString();
+        return xml;
     }
 }
diff --git a/src/main/java/org/jitsi/xmpp/extensions/jitsimeet/AvatarUrl.java b/src/main/java/org/jitsi/xmpp/extensions/jitsimeet/AvatarUrl.java
@@ -87,11 +87,11 @@ public String getNamespace()
      * Returns xml representation of this extension.
      * @return xml representation of this extension.
      */
-    public String toXML(XmlEnvironment enclosingNamespace)
+    @Override
+    public CharSequence toXML(XmlEnvironment enclosingNamespace)
     {
         return new XmlStringBuilder()
-            .element(getElementName(), getAvatarUrl())
-            .toString();
+            .element(getElementName(), getAvatarUrl());
     }
 
     /**

diff --git a/src/main/java/org/jitsi/xmpp/extensions/jitsimeet/Email.java b/src/main/java/org/jitsi/xmpp/extensions/jitsimeet/Email.java
@@ -84,11 +84,11 @@ public String getNamespace()
      * Returns xml representation of this extension.
      * @return xml representation of this extension.
      */
-    public String toXML(XmlEnvironment enclosingNamespace)
+    @Override
+    public CharSequence toXML(XmlEnvironment enclosingNamespace)
     {
         return new XmlStringBuilder()
-            .element(ELEMENT, getAddress())
-            .toString();
+            .element(ELEMENT, getAddress());
     }
 
     /**

diff --git a/src/main/java/org/jitsi/xmpp/extensions/jitsimeet/StatsId.java b/src/main/java/org/jitsi/xmpp/extensions/jitsimeet/StatsId.java
@@ -87,11 +87,11 @@ public String getNamespace()
      * Returns xml representation of this extension.
      * @return xml representation of this extension.
      */
-    public String toXML(XmlEnvironment enclosingNamespace)
+    @Override
+    public CharSequence toXML(XmlEnvironment enclosingNamespace)
     {
         return new XmlStringBuilder()
-            .element(ELEMENT, getStatsId())
-            .toString();
+            .element(ELEMENT, getStatsId());
     }
 
     /**

diff --git a/src/main/java/org/jitsi/xmpp/extensions/vcardavatar/VCardTempXUpdatePresenceExtension.java b/src/main/java/org/jitsi/xmpp/extensions/vcardavatar/VCardTempXUpdatePresenceExtension.java
@@ -50,7 +50,7 @@ public class VCardTempXUpdatePresenceExtension
     /**
      * The whole XML string generated by this presence extension.
      */
-    private String xmlString = null;
+    private CharSequence xmlString = null;
 
     /**
      * Creates a new instance of this presence extension with the avatar image
@@ -146,7 +146,7 @@ private void computeXML()
         }
         xml.closeElement(getElementName());
 
-        this.xmlString = xml.toString();
+        this.xmlString = xml;
     }
 
     /**
@@ -177,7 +177,7 @@ public String getNamespace()
      * @return the packet extension as XML.
      */
     @Override
-    public String toXML(XmlEnvironment enclosingNamespace)
+    public CharSequence toXML(XmlEnvironment enclosingNamespace)
     {
         return this.xmlString;
     }

diff --git a/src/main/kotlin/org/jitsi/xmpp/util/XmlStringBuilderUtil.kt b/src/main/kotlin/org/jitsi/xmpp/util/XmlStringBuilderUtil.kt
@@ -0,0 +1,67 @@
+/*
+ * Copyright @ 2023 - present 8x8, Inc.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.jitsi.xmpp.util
+
+import org.jivesoftware.smack.packet.ExtensionElement
+import org.jivesoftware.smack.packet.IQ
+import org.jivesoftware.smack.packet.XmlEnvironment
+import org.jivesoftware.smack.util.LazyStringBuilder
+import org.jivesoftware.smack.util.XmlStringBuilder
+import java.io.StringWriter
+
+class XmlStringBuilderUtil {
+    companion object {
+        /**
+         *  Avoid calling [XmlStringBuilder.toString] because it can be slow.
+         *  TODO: remove once smack is fixed and updated: https://github.com/igniterealtime/Smack/pull/569
+         */
+        fun CharSequence.toStringOpt(): String = if (this is XmlStringBuilder) {
+            StringWriter().apply {
+                write(this, XmlEnvironment.EMPTY)
+            }.toString()
+        } else {
+            toString()
+        }
+
+        @JvmStatic
+        fun IQ.toStringOpt(): String = this.toXML().toStringOpt()
+
+        @JvmStatic
+        fun ExtensionElement.toStringOpt() = this.toXML().toStringOpt()
+
+        /**
+         *  Avoid using the generic [XmlStringBuilder.append] because it can be slow.
+         *  TODO: remove once smack is fixed and updated: https://github.com/igniterealtime/Smack/pull/569
+         */
+        @JvmStatic
+        fun XmlStringBuilder.append0(cs: CharSequence): XmlStringBuilder {
+            when (cs) {
+                is XmlStringBuilder -> {
+                    append(cs)
+                }
+
+                is LazyStringBuilder -> {
+                    append(cs)
+                }
+
+                else -> {
+                    append(cs)
+                }
+            }
+            return this
+        }
+    }
+}