From 62d03d4f1b44b6530c795df4aa7ea2b88a882d00 Mon Sep 17 00:00:00 2001 From: bgrozev Date: Wed, 25 Oct 2023 15:01:38 -0700 Subject: [PATCH] avoid xmlstring builder toString (#103) * Return CharSequence, do not convert to String. * fix: Avoid using XmlStringBuilder.toString and the generic append(CharSequence). --- .../colibri/ColibriStatsExtension.java | 5 +- .../extensions/colibri/ColibriStatsIQ.java | 3 +- .../inputevt/RemoteControlExtension.java | 5 +- .../xmpp/extensions/jibri/StanzaErrorPE.java | 4 +- .../xmpp/extensions/jingle/JingleIQ.java | 7 +- .../jingle/ReasonPacketExtension.java | 8 ++- .../extensions/jingle/SctpMapExtension.java | 4 +- .../jingleinfo/RelayPacketExtension.java | 4 +- .../xmpp/extensions/jitsimeet/AvatarUrl.java | 6 +- .../xmpp/extensions/jitsimeet/Email.java | 6 +- .../xmpp/extensions/jitsimeet/StatsId.java | 6 +- .../VCardTempXUpdatePresenceExtension.java | 6 +- .../jitsi/xmpp/util/XmlStringBuilderUtil.kt | 67 +++++++++++++++++++ 13 files changed, 101 insertions(+), 30 deletions(-) create mode 100644 src/main/kotlin/org/jitsi/xmpp/util/XmlStringBuilderUtil.kt diff --git a/src/main/java/org/jitsi/xmpp/extensions/colibri/ColibriStatsExtension.java b/src/main/java/org/jitsi/xmpp/extensions/colibri/ColibriStatsExtension.java index 4f3285b8..cfaefdbb 100644 --- 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 index dd8d8b93..76d17489 100644 --- 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 index cc718c71..0ea682bb 100644 --- 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 index 358e2953..97a5e5c7 100644 --- 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 index 857132ca..04eabcaf 100644 --- 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 index db83ee8d..df67619c 100644 --- 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 index 555edd07..c106090d 100644 --- 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 index e451609e..f2567a7d 100644 --- 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 index 3976834f..d47996e8 100644 --- 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 index 5f1ca08b..1c24b920 100644 --- 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 index 3663a4d3..c66de7ad 100644 --- 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 index e0392fb5..f92b2330 100644 --- 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 new file mode 100644 index 00000000..f9238c23 --- /dev/null +++ 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 + } + } +}