From 92f253cc74a436388b15369e370bd96c3e19713a Mon Sep 17 00:00:00 2001 From: Florian Schmaus Date: Wed, 28 Sep 2022 13:52:58 +0200 Subject: [PATCH 1/8] [core] Replace AbstractXMPPConnection.inOrderListeners Using AsyncButOrdered for the receive listeners means that a listener may not have been yet run once invokeStanzaCollectorsAndNotifyRecvListeners() returnes. This can lead to deadlocks as reported by Boris Grozev [1]. Fixes SMACK-927. 1: https://discourse.igniterealtime.org/t/thread-stuck-in-multiuserchat-enter-forever/92090 --- .../org/jivesoftware/smack/AbstractXMPPConnection.java | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/smack-core/src/main/java/org/jivesoftware/smack/AbstractXMPPConnection.java b/smack-core/src/main/java/org/jivesoftware/smack/AbstractXMPPConnection.java index 5a3ae04ac..0e710ee03 100644 --- a/smack-core/src/main/java/org/jivesoftware/smack/AbstractXMPPConnection.java +++ b/smack-core/src/main/java/org/jivesoftware/smack/AbstractXMPPConnection.java @@ -33,6 +33,7 @@ import java.util.concurrent.CopyOnWriteArraySet; import java.util.concurrent.Executor; import java.util.concurrent.ExecutorService; import java.util.concurrent.Executors; +import java.util.concurrent.Semaphore; import java.util.concurrent.ThreadFactory; import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicInteger; @@ -348,8 +349,6 @@ public abstract class AbstractXMPPConnection implements XMPPConnection { protected static final AsyncButOrdered ASYNC_BUT_ORDERED = new AsyncButOrdered<>(); - protected final AsyncButOrdered inOrderListeners = new AsyncButOrdered<>(); - /** * The used host to establish the connection to */ @@ -1613,8 +1612,9 @@ public abstract class AbstractXMPPConnection implements XMPPConnection { listenersToNotify.clear(); extractMatchingListeners(packet, recvListeners, listenersToNotify); + final Semaphore listenerSemaphore = new Semaphore(1 - listenersToNotify.size()); for (StanzaListener stanzaListener : listenersToNotify) { - inOrderListeners.performAsyncButOrdered(stanzaListener, () -> { + asyncGoLimited(() -> { try { stanzaListener.processStanza(packet); } @@ -1623,9 +1623,12 @@ public abstract class AbstractXMPPConnection implements XMPPConnection { } catch (Exception e) { LOGGER.log(Level.SEVERE, "Exception in packet listener", e); + } finally { + listenerSemaphore.release(); } }); } + listenerSemaphore.acquireUninterruptibly(); // Notify the receive listeners interested in the packet listenersToNotify.clear(); From dee34b5efc10cfe8c9b2ebba3ae3a33c29ac8ba5 Mon Sep 17 00:00:00 2001 From: Florian Schmaus Date: Sun, 12 Feb 2023 16:06:53 +0100 Subject: [PATCH 2/8] [core] Add InternetAddress.fromIgnoringZoneId(String) --- .../smack/util/InternetAddress.java | 55 +++++++++++++++---- .../jivesoftware/smack/util/ParserUtils.java | 10 +++- .../smack/util/InternetAddressTest.java | 35 ++++++++++++ 3 files changed, 89 insertions(+), 11 deletions(-) create mode 100644 smack-core/src/test/java/org/jivesoftware/smack/util/InternetAddressTest.java diff --git a/smack-core/src/main/java/org/jivesoftware/smack/util/InternetAddress.java b/smack-core/src/main/java/org/jivesoftware/smack/util/InternetAddress.java index 0fca8182f..3b94d8ad3 100644 --- a/smack-core/src/main/java/org/jivesoftware/smack/util/InternetAddress.java +++ b/smack-core/src/main/java/org/jivesoftware/smack/util/InternetAddress.java @@ -1,6 +1,6 @@ /** * - * Copyright 2019 Florian Schmaus + * Copyright 2019-2023 Florian Schmaus * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -65,12 +65,32 @@ public abstract class InternetAddress implements CharSequence { return originalString.subSequence(start, end); } + public String getRaw() { + return originalString; + } + + public static InternetAddress fromIgnoringZoneId(String address) { + return from(address, true); + } + public static InternetAddress from(String address) { + return from(address, false); + } + + private static InternetAddress from(String address, boolean ignoreZoneId) { + String raw = address; + if (ignoreZoneId) { + int percentPosition = address.indexOf('%'); + if (percentPosition > 1) { + address = address.substring(0, percentPosition); + } + } + final InternetAddress internetAddress; if (InetAddressUtil.isIpV4Address(address)) { - internetAddress = new InternetAddress.Ipv4(address); + internetAddress = new InternetAddress.Ipv4(address, raw); } else if (InetAddressUtil.isIpV6Address(address)) { - internetAddress = new InternetAddress.Ipv6(address); + internetAddress = new InternetAddress.Ipv6(address, raw); } else if (address.contains(".")) { InternetAddress domainNameInternetAddress; try { @@ -99,9 +119,11 @@ public abstract class InternetAddress implements CharSequence { private static class InetAddressInternetAddress extends InternetAddress { private final InetAddress inetAddress; + private final String raw; - protected InetAddressInternetAddress(String originalString, InetAddress inetAddress) { + protected InetAddressInternetAddress(String originalString, String raw, InetAddress inetAddress) { super(originalString); + this.raw = raw; this.inetAddress = inetAddress; } @@ -109,18 +131,27 @@ public abstract class InternetAddress implements CharSequence { public InetAddress asInetAddress() { return inetAddress; } + + @Override + public final String getRaw() { + return raw; + } } public static final class Ipv4 extends InetAddressInternetAddress { private final Inet4Address inet4Address; - private Ipv4(String originalString) { - this(originalString, InetAddressUtil.ipv4From(originalString)); + private Ipv4(String originalString, String raw) { + this(originalString, raw, InetAddressUtil.ipv4From(originalString)); } private Ipv4(String originalString, Inet4Address inet4Address) { - super(originalString, inet4Address); + this(originalString, originalString, inet4Address); + } + + private Ipv4(String originalString, String raw, Inet4Address inet4Address) { + super(originalString, raw, inet4Address); this.inet4Address = inet4Address; } @@ -133,12 +164,16 @@ public abstract class InternetAddress implements CharSequence { private Inet6Address inet6Address; - private Ipv6(String originalString) { - this(originalString, InetAddressUtil.ipv6From(originalString)); + private Ipv6(String originalString, String raw) { + this(originalString, raw, InetAddressUtil.ipv6From(originalString)); } private Ipv6(String originalString, Inet6Address inet6Address) { - super(originalString, inet6Address); + this(originalString, originalString, inet6Address); + } + + private Ipv6(String originalString, String raw, Inet6Address inet6Address) { + super(originalString, raw, inet6Address); this.inet6Address = inet6Address; } diff --git a/smack-core/src/main/java/org/jivesoftware/smack/util/ParserUtils.java b/smack-core/src/main/java/org/jivesoftware/smack/util/ParserUtils.java index be526aaba..ce21db988 100644 --- a/smack-core/src/main/java/org/jivesoftware/smack/util/ParserUtils.java +++ b/smack-core/src/main/java/org/jivesoftware/smack/util/ParserUtils.java @@ -1,6 +1,6 @@ /** * - * Copyright © 2014-2019 Florian Schmaus + * Copyright © 2014-2023 Florian Schmaus * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -387,4 +387,12 @@ public class ParserUtils { public static QName getQName(XmlPullParser parser) { return parser.getQName(); } + + public static InternetAddress getInternetAddressIngoringZoneIdAttribute(XmlPullParser parser, String attribute) { + String inetAddressString = parser.getAttributeValue(attribute); + if (inetAddressString == null) { + return null; + } + return InternetAddress.fromIgnoringZoneId(inetAddressString); + } } diff --git a/smack-core/src/test/java/org/jivesoftware/smack/util/InternetAddressTest.java b/smack-core/src/test/java/org/jivesoftware/smack/util/InternetAddressTest.java new file mode 100644 index 000000000..b9cde1c2b --- /dev/null +++ b/smack-core/src/test/java/org/jivesoftware/smack/util/InternetAddressTest.java @@ -0,0 +1,35 @@ +/** + * + * Copyright 2023 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.smack.util; + +import static org.junit.jupiter.api.Assertions.assertEquals; + +import org.junit.jupiter.api.Test; + +public class InternetAddressTest { + + @Test + public void testFromIgnoringZoneId() { + assertInternetAddressEqualsIgnoringZoneId("fe80::641a:cdff:febd:d665", "fe80::641a:cdff:febd:d665%dummy0"); + } + + private static void assertInternetAddressEqualsIgnoringZoneId(String expected, String input) { + InternetAddress internetAddress = InternetAddress.fromIgnoringZoneId(input); + assertEquals(expected, internetAddress.toString()); + assertEquals(input, internetAddress.getRaw()); + } +} From 6da9773bbfccf13935b642cb0541a499fc4f71c3 Mon Sep 17 00:00:00 2001 From: Boris Grozev Date: Mon, 30 Jan 2023 11:19:58 -0600 Subject: [PATCH 3/8] Add null check for mucUser. XEP-0045 requires "unavailable" presence to contain extended presence, but Smack shouldn't throw an exception if it doesn't. --- .../main/java/org/jivesoftware/smackx/muc/MultiUserChat.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/smack-extensions/src/main/java/org/jivesoftware/smackx/muc/MultiUserChat.java b/smack-extensions/src/main/java/org/jivesoftware/smackx/muc/MultiUserChat.java index 2c6316854..632c8e0cf 100644 --- a/smack-extensions/src/main/java/org/jivesoftware/smackx/muc/MultiUserChat.java +++ b/smack-extensions/src/main/java/org/jivesoftware/smackx/muc/MultiUserChat.java @@ -276,7 +276,7 @@ public class MultiUserChat { } } - Destroy destroy = mucUser.getDestroy(); + Destroy destroy = mucUser == null ? null : mucUser.getDestroy(); // The room has been destroyed. if (destroy != null) { EntityBareJid alternateMucJid = destroy.getJid(); From f00e585f3d7456c9c62745943636348be4d3ecba Mon Sep 17 00:00:00 2001 From: Florian Schmaus Date: Fri, 10 Mar 2023 09:16:52 +0100 Subject: [PATCH 4/8] [github ci]: Bump to Ubuntu 22.04 --- .github/workflows/ci.yml | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index ddaf29404..ca6991367 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -9,18 +9,20 @@ jobs: build: name: Build Smack - runs-on: ubuntu-20.04 + runs-on: ubuntu-22.04 strategy: matrix: - java: [ 1.8, 11 ] + java: [ 8, 11 ] steps: - name: Checkout - uses: actions/checkout@v2 + uses: actions/checkout@v3 + - name: Set up JDK ${{ matrix.java }} - uses: actions/setup-java@v1 + uses: actions/setup-java@v3 with: java-version: ${{ matrix.java }} + distribution: temurin # Caches - name: Cache Maven From d565354dea74bdd9d6cb4a73752fba0f25fc8833 Mon Sep 17 00:00:00 2001 From: Florian Schmaus Date: Sun, 12 Feb 2023 16:07:13 +0100 Subject: [PATCH 5/8] [jingle] Ignore IP Zone ID is transport candidates Fixes SMACK-929. --- .../jingle_s5b/provider/JingleS5BTransportProvider.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/smack-extensions/src/main/java/org/jivesoftware/smackx/jingle/transports/jingle_s5b/provider/JingleS5BTransportProvider.java b/smack-extensions/src/main/java/org/jivesoftware/smackx/jingle/transports/jingle_s5b/provider/JingleS5BTransportProvider.java index c88651661..638860e5e 100644 --- a/smack-extensions/src/main/java/org/jivesoftware/smackx/jingle/transports/jingle_s5b/provider/JingleS5BTransportProvider.java +++ b/smack-extensions/src/main/java/org/jivesoftware/smackx/jingle/transports/jingle_s5b/provider/JingleS5BTransportProvider.java @@ -28,6 +28,7 @@ import static org.jivesoftware.smackx.jingle.transports.jingle_s5b.elements.Jing import java.io.IOException; import org.jivesoftware.smack.packet.XmlEnvironment; +import org.jivesoftware.smack.util.ParserUtils; import org.jivesoftware.smack.xml.XmlPullParser; import org.jivesoftware.smack.xml.XmlPullParserException; @@ -69,7 +70,7 @@ public class JingleS5BTransportProvider extends JingleContentTransportProvider Date: Mon, 27 Feb 2023 06:46:41 +0800 Subject: [PATCH 6/8] Rename ELEMENT 'candidate-activated' to 'activated' per XEP-0260 Fixes SMACK-930. --- .../transports/jingle_s5b/elements/JingleS5BTransportInfo.java | 3 ++- .../jingle/transports/jingle_s5b/JingleS5BTransportTest.java | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/smack-extensions/src/main/java/org/jivesoftware/smackx/jingle/transports/jingle_s5b/elements/JingleS5BTransportInfo.java b/smack-extensions/src/main/java/org/jivesoftware/smackx/jingle/transports/jingle_s5b/elements/JingleS5BTransportInfo.java index 92733a5c0..694e56e87 100644 --- a/smack-extensions/src/main/java/org/jivesoftware/smackx/jingle/transports/jingle_s5b/elements/JingleS5BTransportInfo.java +++ b/smack-extensions/src/main/java/org/jivesoftware/smackx/jingle/transports/jingle_s5b/elements/JingleS5BTransportInfo.java @@ -23,6 +23,7 @@ import org.jivesoftware.smackx.jingle.element.JingleContentTransportInfo; /** * Class representing possible SOCKS5 TransportInfo elements. + * @see XEP-0260: Jingle SOCKS5 Bytestreams Transport Method 1.0.3 (2018-05-15) */ public abstract class JingleS5BTransportInfo implements JingleContentTransportInfo { @@ -71,7 +72,7 @@ public abstract class JingleS5BTransportInfo implements JingleContentTransportIn } public static final class CandidateActivated extends JingleS5BCandidateTransportInfo { - public static final String ELEMENT = "candidate-activated"; + public static final String ELEMENT = "activated"; public CandidateActivated(String candidateId) { super(candidateId); diff --git a/smack-extensions/src/test/java/org/jivesoftware/smackx/jingle/transports/jingle_s5b/JingleS5BTransportTest.java b/smack-extensions/src/test/java/org/jivesoftware/smackx/jingle/transports/jingle_s5b/JingleS5BTransportTest.java index 8d5a908e4..deea4d21e 100644 --- a/smack-extensions/src/test/java/org/jivesoftware/smackx/jingle/transports/jingle_s5b/JingleS5BTransportTest.java +++ b/smack-extensions/src/test/java/org/jivesoftware/smackx/jingle/transports/jingle_s5b/JingleS5BTransportTest.java @@ -164,7 +164,7 @@ public class JingleS5BTransportTest extends SmackTestSuite { String candidateActivated = "" + - "" + + "" + ""; JingleS5BTransport candidateActivatedTransport = new JingleS5BTransportProvider() .parse(TestUtils.getParser(candidateActivated)); From 3ba1aba2e310f767e19b1a065f8f571bda7f808f Mon Sep 17 00:00:00 2001 From: Florian Schmaus Date: Fri, 17 Mar 2023 17:52:33 +0100 Subject: [PATCH 7/8] [github ci]: Disable Java 8 (potentially temprorary) We currently run into https://github.com/android-actions/setup-android/issues/378 --- .github/workflows/ci.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index ca6991367..cf69f7140 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -12,7 +12,7 @@ jobs: runs-on: ubuntu-22.04 strategy: matrix: - java: [ 8, 11 ] + java: [ 11 ] steps: - name: Checkout From bf3a27df9a51c49a3e1c458f2801dd160bb14b28 Mon Sep 17 00:00:00 2001 From: Florian Schmaus Date: Wed, 8 Mar 2023 17:40:29 +0100 Subject: [PATCH 8/8] [core] Fix ErrorIQ not showing potential original IQ child Also delete StanzaIdTest since the test was fundamentally weak and flawed. It does not work anyway, since TestIQ has a fixed stanza ID. Fixes SMACK-931. --- .../smack/packet/AbstractIqBuilder.java | 2 +- .../smack/packet/EmptyResultIQ.java | 4 +- .../jivesoftware/smack/packet/ErrorIQ.java | 88 +++++++++++++++---- .../org/jivesoftware/smack/packet/IQ.java | 54 +++--------- .../org/jivesoftware/smack/packet/IqView.java | 24 ++++- .../smack/util/PacketParserUtils.java | 7 +- .../org/jivesoftware/smack/StanzaIdTest.java | 43 --------- .../org/jivesoftware/smack/packet/IqTest.java | 38 ++++++++ .../org/jivesoftware/smack/packet/TestIQ.java | 6 +- .../bytestreams/ibb/IBBPacketUtils.java | 9 +- .../socks5/Socks5ByteStreamManagerTest.java | 8 +- .../socks5/Socks5ClientForInitiatorTest.java | 7 +- 12 files changed, 164 insertions(+), 126 deletions(-) delete mode 100644 smack-core/src/test/java/org/jivesoftware/smack/StanzaIdTest.java create mode 100644 smack-core/src/test/java/org/jivesoftware/smack/packet/IqTest.java diff --git a/smack-core/src/main/java/org/jivesoftware/smack/packet/AbstractIqBuilder.java b/smack-core/src/main/java/org/jivesoftware/smack/packet/AbstractIqBuilder.java index 5eac12435..f2c080690 100644 --- a/smack-core/src/main/java/org/jivesoftware/smack/packet/AbstractIqBuilder.java +++ b/smack-core/src/main/java/org/jivesoftware/smack/packet/AbstractIqBuilder.java @@ -49,7 +49,7 @@ public abstract class AbstractIqBuilder> extend } protected static IqData createResponse(IqView request, IQ.ResponseType responseType) { - if (!(request.getType() == IQ.Type.get || request.getType() == IQ.Type.set)) { + if (!request.isRequestIQ()) { throw new IllegalArgumentException("IQ request must be of type 'set' or 'get'. Original IQ: " + request); } diff --git a/smack-core/src/main/java/org/jivesoftware/smack/packet/EmptyResultIQ.java b/smack-core/src/main/java/org/jivesoftware/smack/packet/EmptyResultIQ.java index 248f0ebaa..a4d977ac3 100644 --- a/smack-core/src/main/java/org/jivesoftware/smack/packet/EmptyResultIQ.java +++ b/smack-core/src/main/java/org/jivesoftware/smack/packet/EmptyResultIQ.java @@ -1,6 +1,6 @@ /** * - * Copyright © 2014-2019 Florian Schmaus + * Copyright © 2014-2023 Florian Schmaus * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -24,7 +24,7 @@ public class EmptyResultIQ extends IQ { // TODO: Deprecate when stanza builder and parsing logic is ready. public EmptyResultIQ() { - super(null, null); + super((String) null, null); setType(IQ.Type.result); } diff --git a/smack-core/src/main/java/org/jivesoftware/smack/packet/ErrorIQ.java b/smack-core/src/main/java/org/jivesoftware/smack/packet/ErrorIQ.java index 23bf999b0..5b8dd7733 100644 --- a/smack-core/src/main/java/org/jivesoftware/smack/packet/ErrorIQ.java +++ b/smack-core/src/main/java/org/jivesoftware/smack/packet/ErrorIQ.java @@ -1,6 +1,6 @@ /** * - * Copyright © 2014 Florian Schmaus + * Copyright © 2014-2023 Florian Schmaus * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -16,24 +16,82 @@ */ package org.jivesoftware.smack.packet; -import org.jivesoftware.smack.util.Objects; +import java.util.Objects; -public class ErrorIQ extends SimpleIQ { +import javax.xml.namespace.QName; + +/** + * An XMPP error IQ. + *

+ * According to RFC 6120 § 8.3.1 "4. An error stanza MUST contain an <error/> child element.", so this class can + * only be constructed if a stanza error is provided. + */ +public final class ErrorIQ extends IQ { public static final String ELEMENT = StanzaError.ERROR; - /** - * Constructs a new error IQ. - *

- * According to RFC 6120 § 8.3.1 "4. An error stanza MUST contain an <error/> child element.", so the xmppError argument is mandatory. - *

- * @param stanzaError the stanzaError (required). - */ - public ErrorIQ(StanzaError stanzaError) { - super(ELEMENT, null); - Objects.requireNonNull(stanzaError, "stanzaError must not be null"); - setType(IQ.Type.error); - setError(stanzaError); + private final IQ request; + + private ErrorIQ(Builder builder, QName childElementQName) { + super(builder, childElementQName); + Objects.requireNonNull(builder.getError(), "Must provide an stanza error when building error IQs"); + this.request = builder.request; } + public static ErrorIQ createErrorResponse(final IQ request, final StanzaError error) { + Builder builder = new Builder(error, request); + builder.setError(error); + return builder.build(); + } + + @Override + protected IQChildElementXmlStringBuilder getIQChildElementBuilder(IQChildElementXmlStringBuilder xml) { + if (request == null) { + return null; + } + + return request.getIQChildElementBuilder(xml); + } + + public static Builder builder(StanzaError error) { + return new Builder(error, IqData.EMPTY.ofType(IQ.Type.error)); + } + + public static Builder builder(StanzaError error, IqData iqData) { + return new Builder(error, iqData); + } + + public static final class Builder extends IqBuilder { + + private IQ request; + + Builder(StanzaError error, IqData iqData) { + super(iqData); + if (iqData.getType() != IQ.Type.error) { + throw new IllegalArgumentException("Error IQs must be of type 'error'"); + } + Objects.requireNonNull(error, "Must provide an stanza error when building error IQs"); + setError(error); + } + + Builder(StanzaError error, IQ request) { + this(error, AbstractIqBuilder.createErrorResponse(request)); + this.request = request; + } + + @Override + public Builder getThis() { + return this; + } + + @Override + public ErrorIQ build() { + QName childElementQname = null; + if (request != null) { + childElementQname = request.getChildElementQName(); + } + return new ErrorIQ(this, childElementQname); + } + + } } 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 6af350a66..7a5fb376f 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 @@ -68,16 +68,22 @@ public abstract class IQ extends Stanza implements IqView { } protected IQ(AbstractIqBuilder iqBuilder, String childElementName, String childElementNamespace) { + this(iqBuilder, childElementName != null ? new QName(childElementNamespace, childElementName) : null); + } + + protected IQ(AbstractIqBuilder iqBuilder, QName childElementQName) { super(iqBuilder); type = iqBuilder.type; - this.childElementName = childElementName; - this.childElementNamespace = childElementNamespace; - if (childElementName == null) { - childElementQName = null; + if (childElementQName != null) { + this.childElementQName = childElementQName; + this.childElementName = childElementQName.getLocalPart(); + this.childElementNamespace = childElementQName.getNamespaceURI(); } else { - childElementQName = new QName(childElementNamespace, childElementName); + this.childElementQName = null; + this.childElementName = null; + this.childElementNamespace = null; } } @@ -100,32 +106,6 @@ public abstract class IQ extends Stanza implements IqView { this.type = Objects.requireNonNull(type, "type must not be null"); } - /** - * Return true if this IQ is a request IQ, i.e. an IQ of type {@link Type#get} or {@link Type#set}. - * - * @return true if IQ type is 'get' or 'set', false otherwise. - * @since 4.1 - */ - public boolean isRequestIQ() { - switch (type) { - case get: - case set: - return true; - default: - return false; - } - } - - /** - * Return true if this IQ is a request, i.e. an IQ of type {@link Type#result} or {@link Type#error}. - * - * @return true if IQ type is 'result' or 'error', false otherwise. - * @since 4.4 - */ - public boolean isResponseIQ() { - return !isRequestIQ(); - } - public final QName getChildElementQName() { return childElementQName; } @@ -194,7 +174,6 @@ public abstract class IQ extends Stanza implements IqView { if (type == Type.error) { // Add the error sub-packet, if there is one. appendErrorIfExists(xml); - return; } if (childElementName == null) { return; @@ -305,16 +284,7 @@ public abstract class IQ extends Stanza implements IqView { * @return a new {@link Type#error IQ.Type.error} IQ based on the originating IQ. */ public static ErrorIQ createErrorResponse(final IQ request, final StanzaError error) { - if (!request.isRequestIQ()) { - throw new IllegalArgumentException( - "IQ must be of type 'set' or 'get'. Original IQ: " + request.toXML()); - } - final ErrorIQ result = new ErrorIQ(error); - result.setStanzaId(request.getStanzaId()); - result.setFrom(request.getTo()); - result.setTo(request.getFrom()); - - return result; + return ErrorIQ.createErrorResponse(request, error); } /** diff --git a/smack-core/src/main/java/org/jivesoftware/smack/packet/IqView.java b/smack-core/src/main/java/org/jivesoftware/smack/packet/IqView.java index a888712eb..2fbb6b709 100644 --- a/smack-core/src/main/java/org/jivesoftware/smack/packet/IqView.java +++ b/smack-core/src/main/java/org/jivesoftware/smack/packet/IqView.java @@ -1,6 +1,6 @@ /** * - * Copyright 2019 Florian Schmaus + * Copyright 2019-2023 Florian Schmaus * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -16,6 +16,8 @@ */ package org.jivesoftware.smack.packet; +import org.jivesoftware.smack.packet.IQ.Type; + public interface IqView extends StanzaView { /** @@ -25,4 +27,24 @@ public interface IqView extends StanzaView { */ IQ.Type getType(); + /** + * Return true if this IQ is a request IQ, i.e. an IQ of type {@link Type#get} or {@link Type#set}. + * + * @return true if IQ type is 'get' or 'set', false otherwise. + * @since 4.1 + */ + default boolean isRequestIQ() { + IQ.Type type = getType(); + return type == IQ.Type.get || type == IQ.Type.set; + } + + /** + * Return true if this IQ is a request, i.e. an IQ of type {@link Type#result} or {@link Type#error}. + * + * @return true if IQ type is 'result' or 'error', false otherwise. + * @since 4.4 + */ + default boolean isResponseIQ() { + return !isRequestIQ(); + } } diff --git a/smack-core/src/main/java/org/jivesoftware/smack/util/PacketParserUtils.java b/smack-core/src/main/java/org/jivesoftware/smack/util/PacketParserUtils.java index 82cace29d..4a52261ef 100644 --- a/smack-core/src/main/java/org/jivesoftware/smack/util/PacketParserUtils.java +++ b/smack-core/src/main/java/org/jivesoftware/smack/util/PacketParserUtils.java @@ -1,6 +1,6 @@ /** * - * Copyright 2003-2007 Jive Software, 2019 Florian Schmaus. + * Copyright 2003-2007 Jive Software, 2019-2023 Florian Schmaus. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -579,8 +579,9 @@ public class PacketParserUtils { switch (iqData.getType()) { case error: // If an IQ packet wasn't created above, create an empty error IQ packet. - iqPacket = new ErrorIQ(error); - break; + iqPacket = ErrorIQ.builder(error, iqData).build(); + // The following return is simply to avoid setting iqData again below. + return iqPacket; case result: iqPacket = new EmptyResultIQ(); break; diff --git a/smack-core/src/test/java/org/jivesoftware/smack/StanzaIdTest.java b/smack-core/src/test/java/org/jivesoftware/smack/StanzaIdTest.java deleted file mode 100644 index 8a73f1a96..000000000 --- a/smack-core/src/test/java/org/jivesoftware/smack/StanzaIdTest.java +++ /dev/null @@ -1,43 +0,0 @@ -/** - * - * Copyright © 2014 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.smack; - -import static org.junit.Assert.assertFalse; -import static org.junit.Assert.assertTrue; - -import org.jivesoftware.smack.packet.IQ; -import org.jivesoftware.smack.packet.TestIQ; -import org.jivesoftware.smack.util.StringUtils; - -import org.junit.Test; - -public class StanzaIdTest { - - @Test - public void testIqId() { - IQ iq1 = new TestIQ(); - String iq1Id = iq1.getStanzaId(); - assertTrue(StringUtils.isNotEmpty(iq1Id)); - - IQ iq2 = new TestIQ(); - String iq2Id = iq2.getStanzaId(); - assertTrue(StringUtils.isNotEmpty(iq2Id)); - - assertFalse(iq1Id.equals(iq2Id)); - } - -} diff --git a/smack-core/src/test/java/org/jivesoftware/smack/packet/IqTest.java b/smack-core/src/test/java/org/jivesoftware/smack/packet/IqTest.java new file mode 100644 index 000000000..b8f192bd7 --- /dev/null +++ b/smack-core/src/test/java/org/jivesoftware/smack/packet/IqTest.java @@ -0,0 +1,38 @@ +/** + * + * Copyright © 2023 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.smack.packet; + +import static org.jivesoftware.smack.test.util.XmlAssertUtil.assertXmlSimilar; + +import org.junit.jupiter.api.Test; + +public class IqTest { + + @Test + public void testIqErrorWithChildElement() { + IQ request = new TestIQ(); + StanzaError error = StanzaError.getBuilder().setCondition(StanzaError.Condition.bad_request).build(); + ErrorIQ errorIq = IQ.createErrorResponse(request, error); + + String expected = "" + + "" + + "" + + ""; + assertXmlSimilar(expected, errorIq.toXML()); + } + +} diff --git a/smack-core/src/test/java/org/jivesoftware/smack/packet/TestIQ.java b/smack-core/src/test/java/org/jivesoftware/smack/packet/TestIQ.java index c222035e1..ec80fb7ea 100644 --- a/smack-core/src/test/java/org/jivesoftware/smack/packet/TestIQ.java +++ b/smack-core/src/test/java/org/jivesoftware/smack/packet/TestIQ.java @@ -1,6 +1,6 @@ /** * - * Copyright © 2014-2019 Florian Schmaus + * Copyright © 2014-2023 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,11 +21,11 @@ import org.jivesoftware.smack.SmackConfiguration; public class TestIQ extends SimpleIQ { public TestIQ() { - this(SmackConfiguration.SMACK_URL_STRING, "test-iq"); + this("test-iq", SmackConfiguration.SMACK_URL_STRING); } public TestIQ(String element, String namespace) { - super(element, namespace); + super(StanzaBuilder.buildIqData("42"), element, namespace); } @Override diff --git a/smack-extensions/src/test/java/org/jivesoftware/smackx/bytestreams/ibb/IBBPacketUtils.java b/smack-extensions/src/test/java/org/jivesoftware/smackx/bytestreams/ibb/IBBPacketUtils.java index 9aa69919c..f8ab45a4f 100644 --- a/smack-extensions/src/test/java/org/jivesoftware/smackx/bytestreams/ibb/IBBPacketUtils.java +++ b/smack-extensions/src/test/java/org/jivesoftware/smackx/bytestreams/ibb/IBBPacketUtils.java @@ -40,11 +40,10 @@ public class IBBPacketUtils { */ public static IQ createErrorIQ(Jid from, Jid to, StanzaError.Condition condition) { StanzaError xmppError = StanzaError.getBuilder(condition).build(); - IQ errorIQ = new ErrorIQ(xmppError); - errorIQ.setType(IQ.Type.error); - errorIQ.setFrom(from); - errorIQ.setTo(to); - return errorIQ; + return ErrorIQ.builder(xmppError) + .from(from) + .to(to) + .build(); } /** diff --git a/smack-extensions/src/test/java/org/jivesoftware/smackx/bytestreams/socks5/Socks5ByteStreamManagerTest.java b/smack-extensions/src/test/java/org/jivesoftware/smackx/bytestreams/socks5/Socks5ByteStreamManagerTest.java index c2b748686..cfa59197a 100644 --- a/smack-extensions/src/test/java/org/jivesoftware/smackx/bytestreams/socks5/Socks5ByteStreamManagerTest.java +++ b/smack-extensions/src/test/java/org/jivesoftware/smackx/bytestreams/socks5/Socks5ByteStreamManagerTest.java @@ -38,12 +38,11 @@ import org.jivesoftware.smack.SmackException.FeatureNotSupportedException; import org.jivesoftware.smack.XMPPConnection; import org.jivesoftware.smack.XMPPException; import org.jivesoftware.smack.XMPPException.XMPPErrorException; -import org.jivesoftware.smack.packet.ErrorIQ; import org.jivesoftware.smack.packet.IQ; import org.jivesoftware.smack.packet.StanzaError; import org.jivesoftware.smack.test.util.NetworkUtil; import org.jivesoftware.smack.util.ExceptionUtil; - +import org.jivesoftware.smackx.bytestreams.ibb.IBBPacketUtils; import org.jivesoftware.smackx.bytestreams.socks5.packet.Bytestream; import org.jivesoftware.smackx.bytestreams.socks5.packet.Bytestream.StreamHost; import org.jivesoftware.smackx.disco.ServiceDiscoveryManager; @@ -415,10 +414,7 @@ public class Socks5ByteStreamManagerTest { Verification.requestTypeGET); // build error packet to reject SOCKS5 Bytestream - StanzaError stanzaError = StanzaError.getBuilder(StanzaError.Condition.not_acceptable).build(); - IQ rejectPacket = new ErrorIQ(stanzaError); - rejectPacket.setFrom(targetJID); - rejectPacket.setTo(initiatorJID); + IQ rejectPacket = IBBPacketUtils.createErrorIQ(targetJID, initiatorJID, StanzaError.Condition.not_acceptable); // return error packet as response to the bytestream initiation protocol.addResponse(rejectPacket, Verification.correspondingSenderReceiver, diff --git a/smack-extensions/src/test/java/org/jivesoftware/smackx/bytestreams/socks5/Socks5ClientForInitiatorTest.java b/smack-extensions/src/test/java/org/jivesoftware/smackx/bytestreams/socks5/Socks5ClientForInitiatorTest.java index 91ec22b68..3ed68d212 100644 --- a/smack-extensions/src/test/java/org/jivesoftware/smackx/bytestreams/socks5/Socks5ClientForInitiatorTest.java +++ b/smack-extensions/src/test/java/org/jivesoftware/smackx/bytestreams/socks5/Socks5ClientForInitiatorTest.java @@ -30,11 +30,10 @@ import org.jivesoftware.smack.SmackException; import org.jivesoftware.smack.XMPPConnection; import org.jivesoftware.smack.XMPPException.XMPPErrorException; import org.jivesoftware.smack.packet.EmptyResultIQ; -import org.jivesoftware.smack.packet.ErrorIQ; import org.jivesoftware.smack.packet.IQ; import org.jivesoftware.smack.packet.StanzaError; import org.jivesoftware.smack.util.CloseableUtil; - +import org.jivesoftware.smackx.bytestreams.ibb.IBBPacketUtils; import org.jivesoftware.smackx.bytestreams.socks5.packet.Bytestream; import org.jivesoftware.smackx.bytestreams.socks5.packet.Bytestream.StreamHost; @@ -184,9 +183,7 @@ public class Socks5ClientForInitiatorTest { XMPPConnection connection = ConnectionUtils.createMockedConnection(protocol, initiatorJID); // build error response as reply to the stream activation - IQ error = new ErrorIQ(StanzaError.getBuilder(StanzaError.Condition.internal_server_error).build()); - error.setFrom(proxyJID); - error.setTo(initiatorJID); + IQ error = IBBPacketUtils.createErrorIQ(proxyJID, initiatorJID, StanzaError.Condition.internal_server_error); protocol.addResponse(error, Verification.correspondingSenderReceiver, Verification.requestTypeSET);