From 585bcb4dc8d6a2164945ff3dd9b55cad9472f872 Mon Sep 17 00:00:00 2001 From: Florian Schmaus Date: Tue, 19 Oct 2021 11:16:35 +0200 Subject: [PATCH 1/5] [jingle] Add empty element optimization for --- .../jivesoftware/smackx/jingle/element/JingleContent.java | 5 +++++ .../org/jivesoftware/smackx/jingle/JingleContentTest.java | 3 +-- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/smack-extensions/src/main/java/org/jivesoftware/smackx/jingle/element/JingleContent.java b/smack-extensions/src/main/java/org/jivesoftware/smackx/jingle/element/JingleContent.java index 171231a9b..4a73db0a0 100644 --- a/smack-extensions/src/main/java/org/jivesoftware/smackx/jingle/element/JingleContent.java +++ b/smack-extensions/src/main/java/org/jivesoftware/smackx/jingle/element/JingleContent.java @@ -145,6 +145,11 @@ public final class JingleContent implements FullyQualifiedElement { xml.optAttribute(DISPOSITION_ATTRIBUTE_NAME, disposition); xml.attribute(NAME_ATTRIBUTE_NAME, name); xml.optAttribute(SENDERS_ATTRIBUTE_NAME, senders); + + if (description == null && transport == null) { + return xml.closeEmptyElement(); + } + xml.rightAngleBracket(); xml.optAppend(description); diff --git a/smack-extensions/src/test/java/org/jivesoftware/smackx/jingle/JingleContentTest.java b/smack-extensions/src/test/java/org/jivesoftware/smackx/jingle/JingleContentTest.java index 45904af4e..946ae95e8 100644 --- a/smack-extensions/src/test/java/org/jivesoftware/smackx/jingle/JingleContentTest.java +++ b/smack-extensions/src/test/java/org/jivesoftware/smackx/jingle/JingleContentTest.java @@ -75,8 +75,7 @@ public class JingleContentTest extends SmackTestSuite { assertEquals(content1.toXML().toString(), builder.build().toXML().toString()); String xml = - "" + - ""; + ""; assertEquals(xml, content1.toXML().toString()); } } From b243a40e26ebb1e0f33c80c91372dc1124a3fa0f Mon Sep 17 00:00:00 2001 From: Florian Schmaus Date: Tue, 19 Oct 2021 11:29:23 +0200 Subject: [PATCH 2/5] [core] Pass down the XML environment in IQChildElementXmlStringBuilder This allows to avoid redundant XML namespaces within IQs, like for example here: Fixes SMACK-917 Reported-by: Jonathan Lennox --- .../org/jivesoftware/smack/packet/IQ.java | 19 +++++++++---------- .../smack/util/XmlStringBuilder.java | 12 +++++++----- .../ibb/packet/DataPacketExtension.java | 2 +- .../workgroup/packet/RoomInvitation.java | 2 +- .../smackx/workgroup/packet/RoomTransfer.java | 2 +- 5 files changed, 19 insertions(+), 18 deletions(-) diff --git a/smack-core/src/main/java/org/jivesoftware/smack/packet/IQ.java b/smack-core/src/main/java/org/jivesoftware/smack/packet/IQ.java index 0f412b602..6af350a66 100644 --- a/smack-core/src/main/java/org/jivesoftware/smack/packet/IQ.java +++ b/smack-core/src/main/java/org/jivesoftware/smack/packet/IQ.java @@ -202,7 +202,7 @@ public abstract class IQ extends Stanza implements IqView { // Add the query section if there is one. IQChildElementXmlStringBuilder iqChildElement = getIQChildElementBuilder( - new IQChildElementXmlStringBuilder(this)); + new IQChildElementXmlStringBuilder(getChildElementName(), getChildElementNamespace(), null, xml.getXmlEnvironment())); // TOOD: Document the cases where iqChildElement is null but childElementName not. And if there are none, change // the logic. if (iqChildElement == null) { @@ -399,17 +399,16 @@ public abstract class IQ extends Stanza implements IqView { private boolean isEmptyElement; - private IQChildElementXmlStringBuilder(IQ iq) { - this(iq.getChildElementName(), iq.getChildElementNamespace()); + public IQChildElementXmlStringBuilder(ExtensionElement extensionElement, + XmlEnvironment enclosingXmlEnvironment) { + this(extensionElement.getElementName(), extensionElement.getNamespace(), extensionElement.getLanguage(), + enclosingXmlEnvironment); } - public IQChildElementXmlStringBuilder(ExtensionElement pe) { - this(pe.getElementName(), pe.getNamespace()); - } - - private IQChildElementXmlStringBuilder(String element, String namespace) { - prelude(element, namespace); - this.element = element; + private IQChildElementXmlStringBuilder(String elementName, String xmlNs, String xmlLang, + XmlEnvironment enclosingXmlEnvironment) { + super(elementName, xmlNs, xmlLang, enclosingXmlEnvironment); + this.element = elementName; } public void setEmptyElement() { diff --git a/smack-core/src/main/java/org/jivesoftware/smack/util/XmlStringBuilder.java b/smack-core/src/main/java/org/jivesoftware/smack/util/XmlStringBuilder.java index a494307e7..5a1fae11f 100644 --- a/smack-core/src/main/java/org/jivesoftware/smack/util/XmlStringBuilder.java +++ b/smack-core/src/main/java/org/jivesoftware/smack/util/XmlStringBuilder.java @@ -1,6 +1,6 @@ /** * - * Copyright 2014-2020 Florian Schmaus + * Copyright 2014-2021 Florian Schmaus * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -53,11 +53,13 @@ public class XmlStringBuilder implements Appendable, CharSequence, Element { } public XmlStringBuilder(FullyQualifiedElement element, XmlEnvironment enclosingXmlEnvironment) { - sb = new LazyStringBuilder(); - halfOpenElement(element); + this(element.getElementName(), element.getNamespace(), element.getLanguage(), enclosingXmlEnvironment); + } + + public XmlStringBuilder(String elementName, String xmlNs, String xmlLang, XmlEnvironment enclosingXmlEnvironment) { + sb = new LazyStringBuilder(); + halfOpenElement(elementName); - String xmlNs = element.getNamespace(); - String xmlLang = element.getLanguage(); if (enclosingXmlEnvironment == null) { xmlnsAttribute(xmlNs); xmllangAttribute(xmlLang); diff --git a/smack-extensions/src/main/java/org/jivesoftware/smackx/bytestreams/ibb/packet/DataPacketExtension.java b/smack-extensions/src/main/java/org/jivesoftware/smackx/bytestreams/ibb/packet/DataPacketExtension.java index 532ac9a11..008c941c6 100644 --- a/smack-extensions/src/main/java/org/jivesoftware/smackx/bytestreams/ibb/packet/DataPacketExtension.java +++ b/smack-extensions/src/main/java/org/jivesoftware/smackx/bytestreams/ibb/packet/DataPacketExtension.java @@ -153,7 +153,7 @@ public class DataPacketExtension implements ExtensionElement { @Override public XmlStringBuilder toXML(org.jivesoftware.smack.packet.XmlEnvironment enclosingNamespace) { - XmlStringBuilder xml = getIQChildElementBuilder(new IQChildElementXmlStringBuilder(this)); + XmlStringBuilder xml = getIQChildElementBuilder(new IQChildElementXmlStringBuilder(this, enclosingNamespace)); xml.closeElement(this); return xml; } diff --git a/smack-legacy/src/main/java/org/jivesoftware/smackx/workgroup/packet/RoomInvitation.java b/smack-legacy/src/main/java/org/jivesoftware/smackx/workgroup/packet/RoomInvitation.java index d2e56b29d..c7577e727 100644 --- a/smack-legacy/src/main/java/org/jivesoftware/smackx/workgroup/packet/RoomInvitation.java +++ b/smack-legacy/src/main/java/org/jivesoftware/smackx/workgroup/packet/RoomInvitation.java @@ -114,7 +114,7 @@ public class RoomInvitation implements ExtensionElement { @Override public XmlStringBuilder toXML(org.jivesoftware.smack.packet.XmlEnvironment enclosingNamespace) { - XmlStringBuilder xml = getIQChildElementBuilder(new IQChildElementXmlStringBuilder(this)); + XmlStringBuilder xml = getIQChildElementBuilder(new IQChildElementXmlStringBuilder(this, enclosingNamespace)); xml.closeElement(this); return xml; } diff --git a/smack-legacy/src/main/java/org/jivesoftware/smackx/workgroup/packet/RoomTransfer.java b/smack-legacy/src/main/java/org/jivesoftware/smackx/workgroup/packet/RoomTransfer.java index 0e8012aae..09b09f657 100644 --- a/smack-legacy/src/main/java/org/jivesoftware/smackx/workgroup/packet/RoomTransfer.java +++ b/smack-legacy/src/main/java/org/jivesoftware/smackx/workgroup/packet/RoomTransfer.java @@ -109,7 +109,7 @@ public class RoomTransfer implements ExtensionElement { @Override public XmlStringBuilder toXML(org.jivesoftware.smack.packet.XmlEnvironment enclosingNamespace) { - XmlStringBuilder xml = getIQChildElementBuilder(new IQChildElementXmlStringBuilder(this)); + XmlStringBuilder xml = getIQChildElementBuilder(new IQChildElementXmlStringBuilder(this, enclosingNamespace)); xml.closeElement(this); return xml; } From a3840659aa626d921ab05beabd627a36a283e3d8 Mon Sep 17 00:00:00 2001 From: Florian Schmaus Date: Tue, 19 Oct 2021 11:31:08 +0200 Subject: [PATCH 3/5] [jingle] Mimic Manager.connection() in JingleTransportManager Eventually JingleTransportManager should be a subclass of Manager (or be replaced by Manager), as JingleTransportManager holds a strong reference to the XMPPConnection. This could cause memory leaks. But for now, we mimic the Manager API in JingleTransportManger to make a future transition to Manager easier. --- .../smackx/jingle/transports/JingleTransportManager.java | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/smack-extensions/src/main/java/org/jivesoftware/smackx/jingle/transports/JingleTransportManager.java b/smack-extensions/src/main/java/org/jivesoftware/smackx/jingle/transports/JingleTransportManager.java index 5b348c5d3..8c91c8976 100644 --- a/smack-extensions/src/main/java/org/jivesoftware/smackx/jingle/transports/JingleTransportManager.java +++ b/smack-extensions/src/main/java/org/jivesoftware/smackx/jingle/transports/JingleTransportManager.java @@ -36,6 +36,10 @@ public abstract class JingleTransportManager i } public XMPPConnection getConnection() { + return connection(); + } + + public XMPPConnection connection() { return connection; } From 453ca6aeb06d97ffaa9e9937e17877ccc83958df Mon Sep 17 00:00:00 2001 From: Florian Schmaus Date: Tue, 19 Oct 2021 11:32:51 +0200 Subject: [PATCH 4/5] [jingle] Make Jingle.Builder extend IqBuilder This makes Jingle.Builder to follow the new IqBuilder pattern, allowing to construct Jingle IQs with a given stanza ID (mostly useful for unit tests). --- .../smackx/jingle/JingleUtil.java | 16 +++--- .../smackx/jingle/element/Jingle.java | 53 ++++++++++++++++--- .../jingle/provider/JingleProvider.java | 11 ++-- .../jingle_s5b/JingleS5BTransportManager.java | 8 +-- .../smackx/jingle/JingleTest.java | 6 +-- 5 files changed, 67 insertions(+), 27 deletions(-) diff --git a/smack-extensions/src/main/java/org/jivesoftware/smackx/jingle/JingleUtil.java b/smack-extensions/src/main/java/org/jivesoftware/smackx/jingle/JingleUtil.java index 1ac93c448..1cac1f095 100644 --- a/smack-extensions/src/main/java/org/jivesoftware/smackx/jingle/JingleUtil.java +++ b/smack-extensions/src/main/java/org/jivesoftware/smackx/jingle/JingleUtil.java @@ -51,7 +51,7 @@ public class JingleUtil { JingleContentDescription description, JingleContentTransport transport) { - Jingle.Builder jb = Jingle.getBuilder(); + Jingle.Builder jb = Jingle.builder(connection); jb.setAction(JingleAction.session_initiate) .setSessionId(sessionId) .setInitiator(connection.getUser()); @@ -116,7 +116,7 @@ public class JingleUtil { JingleContentDescription description, JingleContentTransport transport) { - Jingle.Builder jb = Jingle.getBuilder(); + Jingle.Builder jb = Jingle.builder(connection); jb.setResponder(connection.getUser()) .setAction(JingleAction.session_accept) .setSessionId(sessionId); @@ -151,7 +151,7 @@ public class JingleUtil { } public Jingle createSessionTerminate(FullJid recipient, String sessionId, JingleReason reason) { - Jingle.Builder jb = Jingle.getBuilder(); + Jingle.Builder jb = Jingle.builder(connection); jb.setAction(JingleAction.session_terminate) .setSessionId(sessionId) .setReason(reason); @@ -230,7 +230,7 @@ public class JingleUtil { public Jingle createSessionTerminateContentCancel(FullJid recipient, String sessionId, JingleContent.Creator contentCreator, String contentName) { - Jingle.Builder jb = Jingle.getBuilder(); + Jingle.Builder jb = Jingle.builder(connection); jb.setAction(JingleAction.session_terminate) .setSessionId(sessionId); @@ -312,7 +312,7 @@ public class JingleUtil { } public Jingle createSessionPing(FullJid recipient, String sessionId) { - Jingle.Builder jb = Jingle.getBuilder(); + Jingle.Builder jb = Jingle.builder(connection); jb.setSessionId(sessionId) .setAction(JingleAction.session_info); @@ -341,7 +341,7 @@ public class JingleUtil { public Jingle createTransportReplace(FullJid recipient, FullJid initiator, String sessionId, JingleContent.Creator contentCreator, String contentName, JingleContentTransport transport) { - Jingle.Builder jb = Jingle.getBuilder(); + Jingle.Builder jb = Jingle.builder(connection); jb.setInitiator(initiator) .setSessionId(sessionId) .setAction(JingleAction.transport_replace); @@ -368,7 +368,7 @@ public class JingleUtil { public Jingle createTransportAccept(FullJid recipient, FullJid initiator, String sessionId, JingleContent.Creator contentCreator, String contentName, JingleContentTransport transport) { - Jingle.Builder jb = Jingle.getBuilder(); + Jingle.Builder jb = Jingle.builder(connection); jb.setAction(JingleAction.transport_accept) .setInitiator(initiator) .setSessionId(sessionId); @@ -395,7 +395,7 @@ public class JingleUtil { public Jingle createTransportReject(FullJid recipient, FullJid initiator, String sessionId, JingleContent.Creator contentCreator, String contentName, JingleContentTransport transport) { - Jingle.Builder jb = Jingle.getBuilder(); + Jingle.Builder jb = Jingle.builder(connection); jb.setAction(JingleAction.transport_reject) .setInitiator(initiator) .setSessionId(sessionId); diff --git a/smack-extensions/src/main/java/org/jivesoftware/smackx/jingle/element/Jingle.java b/smack-extensions/src/main/java/org/jivesoftware/smackx/jingle/element/Jingle.java index 40acf187a..7598d1894 100644 --- a/smack-extensions/src/main/java/org/jivesoftware/smackx/jingle/element/Jingle.java +++ b/smack-extensions/src/main/java/org/jivesoftware/smackx/jingle/element/Jingle.java @@ -1,6 +1,6 @@ /** * - * Copyright 2003-2007 Jive Software, 2014-2017 Florian Schmaus + * Copyright 2003-2007 Jive Software, 2014-2021 Florian Schmaus * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -21,7 +21,11 @@ import java.util.ArrayList; import java.util.Collections; import java.util.List; +import org.jivesoftware.smack.XMPPConnection; import org.jivesoftware.smack.packet.IQ; +import org.jivesoftware.smack.packet.IqBuilder; +import org.jivesoftware.smack.packet.IqData; +import org.jivesoftware.smack.packet.id.StandardStanzaIdSource; import org.jivesoftware.smack.util.Objects; import org.jivesoftware.smack.util.StringUtils; @@ -65,9 +69,9 @@ public final class Jingle extends IQ { private final List contents; - private Jingle(String sessionId, JingleAction action, FullJid initiator, FullJid responder, JingleReason reason, + private Jingle(Builder builder, String sessionId, JingleAction action, FullJid initiator, FullJid responder, JingleReason reason, List contents) { - super(ELEMENT, NAMESPACE); + super(builder, ELEMENT, NAMESPACE); this.sessionId = StringUtils.requireNotNullNorEmpty(sessionId, "Jingle session ID must not be null"); this.action = Objects.requireNonNull(action, "Jingle action must not be null"); this.initiator = initiator; @@ -169,11 +173,31 @@ public final class Jingle extends IQ { return xml; } + /** + * Deprecated, do not use. + * + * @return a builder. + * @deprecated use {@link #builder(XMPPConnection)} instead. + */ + @Deprecated + // TODO: Remove in Smack 4.6. public static Builder getBuilder() { - return new Builder(); + return builder(StandardStanzaIdSource.DEFAULT.getNewStanzaId()); } - public static final class Builder { + public static Builder builder(XMPPConnection connection) { + return new Builder(connection); + } + + public static Builder builder(IqData iqData) { + return new Builder(iqData); + } + + public static Builder builder(String stanzaId) { + return new Builder(stanzaId); + } + + public static final class Builder extends IqBuilder { private String sid; private JingleAction action; @@ -186,7 +210,16 @@ public final class Jingle extends IQ { private List contents; - private Builder() { + Builder(IqData iqCommon) { + super(iqCommon); + } + + Builder(XMPPConnection connection) { + super(connection); + } + + Builder(String stanzaId) { + super(stanzaId); } public Builder setSessionId(String sessionId) { @@ -228,8 +261,14 @@ public final class Jingle extends IQ { return this; } + @Override public Jingle build() { - return new Jingle(sid, action, initiator, responder, reason, contents); + return new Jingle(this, sid, action, initiator, responder, reason, contents); + } + + @Override + public Builder getThis() { + return this; } } } diff --git a/smack-extensions/src/main/java/org/jivesoftware/smackx/jingle/provider/JingleProvider.java b/smack-extensions/src/main/java/org/jivesoftware/smackx/jingle/provider/JingleProvider.java index 8ff1e269b..f6c352a75 100644 --- a/smack-extensions/src/main/java/org/jivesoftware/smackx/jingle/provider/JingleProvider.java +++ b/smack-extensions/src/main/java/org/jivesoftware/smackx/jingle/provider/JingleProvider.java @@ -1,6 +1,6 @@ /** * - * Copyright 2017-2019 Florian Schmaus + * Copyright 2017-2021 Florian Schmaus * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -19,11 +19,12 @@ package org.jivesoftware.smackx.jingle.provider; import java.io.IOException; import java.util.logging.Logger; +import org.jivesoftware.smack.packet.IqData; import org.jivesoftware.smack.packet.StandardExtensionElement; import org.jivesoftware.smack.packet.XmlEnvironment; import org.jivesoftware.smack.parsing.SmackParsingException; import org.jivesoftware.smack.parsing.StandardExtensionElementProvider; -import org.jivesoftware.smack.provider.IQProvider; +import org.jivesoftware.smack.provider.IqProvider; import org.jivesoftware.smack.util.ParserUtils; import org.jivesoftware.smack.xml.XmlPullParser; import org.jivesoftware.smack.xml.XmlPullParserException; @@ -40,13 +41,13 @@ import org.jivesoftware.smackx.jingle.element.UnknownJingleContentTransport; import org.jxmpp.jid.FullJid; -public class JingleProvider extends IQProvider { +public class JingleProvider extends IqProvider { private static final Logger LOGGER = Logger.getLogger(JingleProvider.class.getName()); @Override - public Jingle parse(XmlPullParser parser, int initialDepth, XmlEnvironment xmlEnvironment) throws XmlPullParserException, IOException, SmackParsingException { - Jingle.Builder builder = Jingle.getBuilder(); + public Jingle parse(XmlPullParser parser, int initialDepth, IqData iqData, XmlEnvironment xmlEnvironment) throws XmlPullParserException, IOException, SmackParsingException { + Jingle.Builder builder = Jingle.builder(iqData); String actionString = parser.getAttributeValue("", Jingle.ACTION_ATTRIBUTE_NAME); if (actionString != null) { diff --git a/smack-extensions/src/main/java/org/jivesoftware/smackx/jingle/transports/jingle_s5b/JingleS5BTransportManager.java b/smack-extensions/src/main/java/org/jivesoftware/smackx/jingle/transports/jingle_s5b/JingleS5BTransportManager.java index 60b1e9f88..cae0f8e11 100644 --- a/smack-extensions/src/main/java/org/jivesoftware/smackx/jingle/transports/jingle_s5b/JingleS5BTransportManager.java +++ b/smack-extensions/src/main/java/org/jivesoftware/smackx/jingle/transports/jingle_s5b/JingleS5BTransportManager.java @@ -148,7 +148,7 @@ public final class JingleS5BTransportManager extends JingleTransportManager { builder.build(); }); @@ -48,7 +48,7 @@ public class JingleTest extends SmackTestSuite { public void onlySessionIdBuilderTest() { String sessionId = "testSessionId"; - Jingle.Builder builder = Jingle.getBuilder(); + Jingle.Builder builder = Jingle.builder("id"); builder.setSessionId(sessionId); assertThrows(IllegalArgumentException.class, () -> { builder.build(); @@ -59,7 +59,7 @@ public class JingleTest extends SmackTestSuite { public void parserTest() throws XmppStringprepException { String sessionId = "testSessionId"; - Jingle.Builder builder = Jingle.getBuilder(); + Jingle.Builder builder = Jingle.builder("id"); builder.setSessionId(sessionId); builder.setAction(JingleAction.session_initiate); From 6f67553fcf4d8340f15e47f8a6bf711efcf98971 Mon Sep 17 00:00:00 2001 From: Florian Schmaus Date: Tue, 19 Oct 2021 11:34:07 +0200 Subject: [PATCH 5/5] [jingle] Add unit test to check that there are no redundant namespaces Related to SMACK-917. --- .../smackx/jingle/element/JingleTest.java | 48 +++++++++++++++++++ 1 file changed, 48 insertions(+) create mode 100644 smack-extensions/src/test/java/org/jivesoftware/smackx/jingle/element/JingleTest.java diff --git a/smack-extensions/src/test/java/org/jivesoftware/smackx/jingle/element/JingleTest.java b/smack-extensions/src/test/java/org/jivesoftware/smackx/jingle/element/JingleTest.java new file mode 100644 index 000000000..ae198617f --- /dev/null +++ b/smack-extensions/src/test/java/org/jivesoftware/smackx/jingle/element/JingleTest.java @@ -0,0 +1,48 @@ +/** + * + * Copyright 2021 Florian Schmaus + * + * 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.jivesoftware.smackx.jingle.element; + +import static org.junit.jupiter.api.Assertions.assertEquals; + +import org.jivesoftware.smack.packet.StreamOpen; + +import org.junit.jupiter.api.Test; + +public class JingleTest { + + @Test + public void noRedundantNamespaceTest() { + Jingle.Builder jingleBuilder = Jingle.builder("test-id"); + jingleBuilder.setSessionId("MySession"); + jingleBuilder.setAction(JingleAction.content_accept); + + JingleContent.Builder jingleContentBuilder = JingleContent.getBuilder(); + jingleContentBuilder.setName("Hello world"); + jingleContentBuilder.setCreator(JingleContent.Creator.initiator); + + jingleBuilder.addJingleContent(jingleContentBuilder.build()); + Jingle iq = jingleBuilder.build(); + + String actualXml = iq.toXML(StreamOpen.CLIENT_NAMESPACE).toString(); + String expectedXml + = "" + + "" + + "" + + ""; + assertEquals(expectedXml, actualXml); + } +}