From 47f76952e3da3901b43f68c1b39a8e89726ad727 Mon Sep 17 00:00:00 2001 From: Florian Schmaus Date: Fri, 22 Feb 2019 10:57:48 +0100 Subject: [PATCH 01/20] Smack 4.3.3-SNAPSHOT --- version.gradle | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/version.gradle b/version.gradle index 3fc19bb3e..ffd608d0c 100644 --- a/version.gradle +++ b/version.gradle @@ -1,7 +1,7 @@ allprojects { ext { - shortVersion = '4.3.2' - isSnapshot = false + shortVersion = '4.3.3' + isSnapshot = true jxmppVersion = '[0.6, 0.7)' miniDnsVersion = '[0.3, 0.4)' smackMinAndroidSdk = 9 From 27749b21376a34870986635df5c4619aec240e0b Mon Sep 17 00:00:00 2001 From: Florian Schmaus Date: Sat, 23 Feb 2019 23:27:06 +0100 Subject: [PATCH 02/20] Remove unused filter in StableUniqueStanzaIdManager --- .../jivesoftware/smackx/sid/StableUniqueStanzaIdManager.java | 3 --- 1 file changed, 3 deletions(-) diff --git a/smack-experimental/src/main/java/org/jivesoftware/smackx/sid/StableUniqueStanzaIdManager.java b/smack-experimental/src/main/java/org/jivesoftware/smackx/sid/StableUniqueStanzaIdManager.java index 677a38910..85c08c437 100644 --- a/smack-experimental/src/main/java/org/jivesoftware/smackx/sid/StableUniqueStanzaIdManager.java +++ b/smack-experimental/src/main/java/org/jivesoftware/smackx/sid/StableUniqueStanzaIdManager.java @@ -27,7 +27,6 @@ import org.jivesoftware.smack.XMPPConnectionRegistry; import org.jivesoftware.smack.filter.AndFilter; import org.jivesoftware.smack.filter.MessageTypeFilter; import org.jivesoftware.smack.filter.NotFilter; -import org.jivesoftware.smack.filter.StanzaExtensionFilter; import org.jivesoftware.smack.filter.StanzaFilter; import org.jivesoftware.smack.filter.ToTypeFilter; import org.jivesoftware.smack.packet.Message; @@ -46,8 +45,6 @@ public final class StableUniqueStanzaIdManager extends Manager { MessageTypeFilter.NORMAL_OR_CHAT_OR_HEADLINE, ToTypeFilter.ENTITY_FULL_OR_BARE_JID); - private static final StanzaFilter ORIGIN_ID_FILTER = new StanzaExtensionFilter(OriginIdElement.ELEMENT, NAMESPACE); - // Listener for outgoing stanzas that adds origin-ids to outgoing stanzas. private final StanzaListener stanzaListener = new StanzaListener() { @Override From 4da4558b29857a02cdc32f09befd84295186d8ef Mon Sep 17 00:00:00 2001 From: Florian Schmaus Date: Sat, 23 Feb 2019 23:39:58 +0100 Subject: [PATCH 03/20] Make origin-id interceptor static and rename it --- .../smackx/sid/StableUniqueStanzaIdManager.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/smack-experimental/src/main/java/org/jivesoftware/smackx/sid/StableUniqueStanzaIdManager.java b/smack-experimental/src/main/java/org/jivesoftware/smackx/sid/StableUniqueStanzaIdManager.java index 85c08c437..ec6523dff 100644 --- a/smack-experimental/src/main/java/org/jivesoftware/smackx/sid/StableUniqueStanzaIdManager.java +++ b/smack-experimental/src/main/java/org/jivesoftware/smackx/sid/StableUniqueStanzaIdManager.java @@ -46,7 +46,7 @@ public final class StableUniqueStanzaIdManager extends Manager { ToTypeFilter.ENTITY_FULL_OR_BARE_JID); // Listener for outgoing stanzas that adds origin-ids to outgoing stanzas. - private final StanzaListener stanzaListener = new StanzaListener() { + private static final StanzaListener ADD_ORIGIN_ID_INTERCEPTOR = new StanzaListener() { @Override public void processStanza(Stanza stanza) { OriginIdElement.addOriginId((Message) stanza); @@ -92,7 +92,7 @@ public final class StableUniqueStanzaIdManager extends Manager { public synchronized void enable() { ServiceDiscoveryManager.getInstanceFor(connection()).addFeature(NAMESPACE); StanzaFilter filter = new AndFilter(OUTGOING_FILTER, new NotFilter(OUTGOING_FILTER)); - connection().addStanzaInterceptor(stanzaListener, filter); + connection().addStanzaInterceptor(ADD_ORIGIN_ID_INTERCEPTOR, filter); } /** @@ -100,7 +100,7 @@ public final class StableUniqueStanzaIdManager extends Manager { */ public synchronized void disable() { ServiceDiscoveryManager.getInstanceFor(connection()).removeFeature(NAMESPACE); - connection().removeStanzaInterceptor(stanzaListener); + connection().removeStanzaInterceptor(ADD_ORIGIN_ID_INTERCEPTOR); } /** From 5a2109e73fd346baa5aba2ff64e351b8a98e30d4 Mon Sep 17 00:00:00 2001 From: Florian Schmaus Date: Sat, 23 Feb 2019 23:40:27 +0100 Subject: [PATCH 04/20] Move cast in extra line in ADD_ORIGIN_ID_INTERCEPTOR --- .../jivesoftware/smackx/sid/StableUniqueStanzaIdManager.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/smack-experimental/src/main/java/org/jivesoftware/smackx/sid/StableUniqueStanzaIdManager.java b/smack-experimental/src/main/java/org/jivesoftware/smackx/sid/StableUniqueStanzaIdManager.java index ec6523dff..99ec03f5d 100644 --- a/smack-experimental/src/main/java/org/jivesoftware/smackx/sid/StableUniqueStanzaIdManager.java +++ b/smack-experimental/src/main/java/org/jivesoftware/smackx/sid/StableUniqueStanzaIdManager.java @@ -49,7 +49,8 @@ public final class StableUniqueStanzaIdManager extends Manager { private static final StanzaListener ADD_ORIGIN_ID_INTERCEPTOR = new StanzaListener() { @Override public void processStanza(Stanza stanza) { - OriginIdElement.addOriginId((Message) stanza); + Message message = (Message) stanza; + OriginIdElement.addOriginId(message); } }; From 5f7cfd04bdccb140383124235f7fea424dd566d6 Mon Sep 17 00:00:00 2001 From: Florian Schmaus Date: Sat, 23 Feb 2019 23:54:42 +0100 Subject: [PATCH 05/20] Add further unit test to StableUniqueStanzaIdTest --- .../smackx/sid/StableUniqueStanzaIdTest.java | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/smack-experimental/src/test/java/org/jivesoftware/smackx/sid/StableUniqueStanzaIdTest.java b/smack-experimental/src/test/java/org/jivesoftware/smackx/sid/StableUniqueStanzaIdTest.java index f6486ee43..95e42e962 100644 --- a/smack-experimental/src/test/java/org/jivesoftware/smackx/sid/StableUniqueStanzaIdTest.java +++ b/smack-experimental/src/test/java/org/jivesoftware/smackx/sid/StableUniqueStanzaIdTest.java @@ -25,6 +25,7 @@ import static org.custommonkey.xmlunit.XMLAssert.assertXMLEqual; import org.jivesoftware.smack.packet.Message; import org.jivesoftware.smack.test.util.SmackTestSuite; import org.jivesoftware.smack.test.util.TestUtils; +import org.jivesoftware.smack.util.PacketParserUtils; import org.jivesoftware.smackx.sid.element.OriginIdElement; import org.jivesoftware.smackx.sid.element.StanzaIdElement; import org.jivesoftware.smackx.sid.provider.OriginIdProvider; @@ -81,4 +82,17 @@ public class StableUniqueStanzaIdTest extends SmackTestSuite { assertTrue(StanzaIdElement.hasStanzaId(message)); assertEquals(stanzaId, StanzaIdElement.getStanzaId(message)); } + + @Test + public void testMultipleUssidExtensions() throws Exception { + String message = "" + + "Test message" + + "" + + "" + + "" + + ""; + Message messageStanza = PacketParserUtils.parseStanza(message); + + assertTrue(StanzaIdElement.hasStanzaId(messageStanza)); + } } From 3bdc1d30b11686ca50bd537663d9ea1b77bba9a1 Mon Sep 17 00:00:00 2001 From: Florian Schmaus Date: Sat, 23 Feb 2019 23:57:31 +0100 Subject: [PATCH 06/20] Correctly name provider INSTANCE fields and make them 'final' where possible. --- .../jivesoftware/smackx/sid/provider/OriginIdProvider.java | 6 +++++- .../jivesoftware/smackx/sid/provider/StanzaIdProvider.java | 6 +++++- .../jivesoftware/smackx/sid/StableUniqueStanzaIdTest.java | 4 ++-- 3 files changed, 12 insertions(+), 4 deletions(-) diff --git a/smack-experimental/src/main/java/org/jivesoftware/smackx/sid/provider/OriginIdProvider.java b/smack-experimental/src/main/java/org/jivesoftware/smackx/sid/provider/OriginIdProvider.java index 19ef6edd7..a84bf6266 100644 --- a/smack-experimental/src/main/java/org/jivesoftware/smackx/sid/provider/OriginIdProvider.java +++ b/smack-experimental/src/main/java/org/jivesoftware/smackx/sid/provider/OriginIdProvider.java @@ -23,7 +23,11 @@ import org.xmlpull.v1.XmlPullParser; public class OriginIdProvider extends ExtensionElementProvider { - public static final OriginIdProvider TEST_INSTANCE = new OriginIdProvider(); + public static final OriginIdProvider INSTANCE = new OriginIdProvider(); + + // TODO: Remove in Smack 4.4. + @Deprecated + public static final OriginIdProvider TEST_INSTANCE = INSTANCE; @Override public OriginIdElement parse(XmlPullParser parser, int initialDepth) throws Exception { diff --git a/smack-experimental/src/main/java/org/jivesoftware/smackx/sid/provider/StanzaIdProvider.java b/smack-experimental/src/main/java/org/jivesoftware/smackx/sid/provider/StanzaIdProvider.java index 2a3fcad28..a37aaf10f 100644 --- a/smack-experimental/src/main/java/org/jivesoftware/smackx/sid/provider/StanzaIdProvider.java +++ b/smack-experimental/src/main/java/org/jivesoftware/smackx/sid/provider/StanzaIdProvider.java @@ -23,7 +23,11 @@ import org.xmlpull.v1.XmlPullParser; public class StanzaIdProvider extends ExtensionElementProvider { - public static StanzaIdProvider TEST_INSTANCE = new StanzaIdProvider(); + public static final StanzaIdProvider INSTANCE = new StanzaIdProvider(); + + // TODO: Remove in Smack 4.4. + @Deprecated + public static final StanzaIdProvider TEST_INSTANCE = INSTANCE; @Override public StanzaIdElement parse(XmlPullParser parser, int initialDepth) throws Exception { diff --git a/smack-experimental/src/test/java/org/jivesoftware/smackx/sid/StableUniqueStanzaIdTest.java b/smack-experimental/src/test/java/org/jivesoftware/smackx/sid/StableUniqueStanzaIdTest.java index 95e42e962..9b12fc0c3 100644 --- a/smack-experimental/src/test/java/org/jivesoftware/smackx/sid/StableUniqueStanzaIdTest.java +++ b/smack-experimental/src/test/java/org/jivesoftware/smackx/sid/StableUniqueStanzaIdTest.java @@ -43,7 +43,7 @@ public class StableUniqueStanzaIdTest extends SmackTestSuite { assertEquals("alice@wonderland.lit", element.getBy()); assertXMLEqual(xml, element.toXML(null).toString()); - StanzaIdElement parsed = StanzaIdProvider.TEST_INSTANCE.parse(TestUtils.getParser(xml)); + StanzaIdElement parsed = StanzaIdProvider.INSTANCE.parse(TestUtils.getParser(xml)); assertEquals(element.getId(), parsed.getId()); assertEquals(element.getBy(), parsed.getBy()); } @@ -55,7 +55,7 @@ public class StableUniqueStanzaIdTest extends SmackTestSuite { assertEquals("de305d54-75b4-431b-adb2-eb6b9e546013", element.getId()); assertXMLEqual(xml, element.toXML(null).toString()); - OriginIdElement parsed = OriginIdProvider.TEST_INSTANCE.parse(TestUtils.getParser(xml)); + OriginIdElement parsed = OriginIdProvider.INSTANCE.parse(TestUtils.getParser(xml)); assertEquals(element.getId(), parsed.getId()); } From 456d645e275d3439c3a8369dbbc9238600b84167 Mon Sep 17 00:00:00 2001 From: Florian Schmaus Date: Sun, 24 Feb 2019 22:09:47 +0100 Subject: [PATCH 07/20] Use different version specifier for jxmpp and MiniDNS Fixes SMACK-858. --- version.gradle | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/version.gradle b/version.gradle index ffd608d0c..a478e9a52 100644 --- a/version.gradle +++ b/version.gradle @@ -2,8 +2,12 @@ allprojects { ext { shortVersion = '4.3.3' isSnapshot = true - jxmppVersion = '[0.6, 0.7)' - miniDnsVersion = '[0.3, 0.4)' + // When using dynamic versions for those, do *not* use [1.0, + // 2.0), since this will also pull in 2.0-alpha1. Instead use + // [1.0, 1.0.99]. + // See also https://issues.apache.org/jira/browse/MNG-6232 + jxmppVersion = '[0.6, 0.6.999]' + miniDnsVersion = '[0.3, 0.3.999]' smackMinAndroidSdk = 9 } } From 78ee22c2611ee49c989092d9686d521b719df88a Mon Sep 17 00:00:00 2001 From: Florian Schmaus Date: Sat, 2 Mar 2019 14:42:19 +0100 Subject: [PATCH 08/20] Revert "Do not set com.sun.jndi.dns.DnsContextFactory in JavaxResolver" This reverts commit ac9641f09187ae4a393c5f1acb1e94e66f24a64f. Reverted because now an NoInitialContextException is now thrown. Related to SMACK-856. --- .../jivesoftware/smack/util/dns/javax/JavaxResolver.java | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/smack-resolver-javax/src/main/java/org/jivesoftware/smack/util/dns/javax/JavaxResolver.java b/smack-resolver-javax/src/main/java/org/jivesoftware/smack/util/dns/javax/JavaxResolver.java index e83c588f6..58da92432 100644 --- a/smack-resolver-javax/src/main/java/org/jivesoftware/smack/util/dns/javax/JavaxResolver.java +++ b/smack-resolver-javax/src/main/java/org/jivesoftware/smack/util/dns/javax/JavaxResolver.java @@ -1,6 +1,6 @@ /** * - * Copyright 2013-2019 Florian Schmaus + * Copyright 2013-2018 Florian Schmaus * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -18,6 +18,7 @@ package org.jivesoftware.smack.util.dns.javax; import java.net.InetAddress; import java.util.ArrayList; +import java.util.Hashtable; import java.util.List; import java.util.logging.Level; @@ -51,7 +52,9 @@ public class JavaxResolver extends DNSResolver implements SmackInitializer { static { try { - dirContext = new InitialDirContext(); + Hashtable env = new Hashtable<>(); + env.put("java.naming.factory.initial", "com.sun.jndi.dns.DnsContextFactory"); + dirContext = new InitialDirContext(env); } catch (NamingException e) { LOGGER.log(Level.SEVERE, "Could not construct InitialDirContext", e); } From f4ebd530e6de4cafad8fe6b12546fc1bd71b2754 Mon Sep 17 00:00:00 2001 From: Florian Schmaus Date: Sat, 2 Mar 2019 14:47:28 +0100 Subject: [PATCH 09/20] Add note about module dependencies to JavaxResolver Related to SMACK-856. --- .../org/jivesoftware/smack/util/dns/javax/JavaxResolver.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/smack-resolver-javax/src/main/java/org/jivesoftware/smack/util/dns/javax/JavaxResolver.java b/smack-resolver-javax/src/main/java/org/jivesoftware/smack/util/dns/javax/JavaxResolver.java index 58da92432..957d8006c 100644 --- a/smack-resolver-javax/src/main/java/org/jivesoftware/smack/util/dns/javax/JavaxResolver.java +++ b/smack-resolver-javax/src/main/java/org/jivesoftware/smack/util/dns/javax/JavaxResolver.java @@ -1,6 +1,6 @@ /** * - * Copyright 2013-2018 Florian Schmaus + * Copyright 2013-2019 Florian Schmaus * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -41,6 +41,7 @@ import org.minidns.dnsname.DnsName; /** * A DNS resolver (mostly for SRV records), which makes use of the API provided in the javax.* namespace. + * Note that using JavaxResovler requires applications using newer Java versions (at least 11) to declare a dependency on the "sun.jdk" module. * * @author Florian Schmaus * From f602de8771db507f4538a60302a0f2d2bca218f6 Mon Sep 17 00:00:00 2001 From: Florian Schmaus Date: Mon, 4 Mar 2019 17:08:53 +0100 Subject: [PATCH 10/20] Call shutdown() in connect() on exception to clean up the state build up by connect(). Related to SMACK-855 there is the possiblitiy of a stray (writer) thread if, for example, tlsHandled.checkifSuccessOrWaitorThrow() in XMPPTCPConnection.connectInternal() throws. This commit should prevent that. --- .../smack/bosh/XMPPBOSHConnection.java | 5 ++++ .../smack/AbstractXMPPConnection.java | 25 +++++++++++++------ .../jivesoftware/smack/DummyConnection.java | 6 +++++ .../smack/tcp/XMPPTCPConnection.java | 4 +-- 4 files changed, 29 insertions(+), 11 deletions(-) diff --git a/smack-bosh/src/main/java/org/jivesoftware/smack/bosh/XMPPBOSHConnection.java b/smack-bosh/src/main/java/org/jivesoftware/smack/bosh/XMPPBOSHConnection.java index 8fba8051a..68dd48fcb 100644 --- a/smack-bosh/src/main/java/org/jivesoftware/smack/bosh/XMPPBOSHConnection.java +++ b/smack-bosh/src/main/java/org/jivesoftware/smack/bosh/XMPPBOSHConnection.java @@ -263,6 +263,11 @@ public class XMPPBOSHConnection extends AbstractXMPPConnection { client = null; } + instantShutdown(); + } + + @Override + public void instantShutdown() { setWasAuthenticated(); sessionID = null; done = true; 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 7a9bf2ee1..697ebfc3b 100644 --- a/smack-core/src/main/java/org/jivesoftware/smack/AbstractXMPPConnection.java +++ b/smack-core/src/main/java/org/jivesoftware/smack/AbstractXMPPConnection.java @@ -405,15 +405,19 @@ public abstract class AbstractXMPPConnection implements XMPPConnection { tlsHandled.init(); streamId = null; - // Perform the actual connection to the XMPP service - connectInternal(); + try { + // Perform the actual connection to the XMPP service + connectInternal(); - // If TLS is required but the server doesn't offer it, disconnect - // from the server and throw an error. First check if we've already negotiated TLS - // and are secure, however (features get parsed a second time after TLS is established). - if (!isSecureConnection() && getConfiguration().getSecurityMode() == SecurityMode.required) { - shutdown(); - throw new SecurityRequiredByClientException(); + // If TLS is required but the server doesn't offer it, disconnect + // from the server and throw an error. First check if we've already negotiated TLS + // and are secure, however (features get parsed a second time after TLS is established). + if (!isSecureConnection() && getConfiguration().getSecurityMode() == SecurityMode.required) { + throw new SecurityRequiredByClientException(); + } + } catch (SmackException | IOException | XMPPException | InterruptedException e) { + instantShutdown(); + throw e; } // Make note of the fact that we're now connected. @@ -763,6 +767,11 @@ public abstract class AbstractXMPPConnection implements XMPPConnection { */ protected abstract void shutdown(); + /** + * Performs an unclean disconnect and shutdown of the connection. Does not send a closing stream stanza. + */ + public abstract void instantShutdown(); + @Override public void addConnectionListener(ConnectionListener connectionListener) { if (connectionListener == null) { diff --git a/smack-core/src/test/java/org/jivesoftware/smack/DummyConnection.java b/smack-core/src/test/java/org/jivesoftware/smack/DummyConnection.java index a3ff3246f..ea39e1233 100644 --- a/smack-core/src/test/java/org/jivesoftware/smack/DummyConnection.java +++ b/smack-core/src/test/java/org/jivesoftware/smack/DummyConnection.java @@ -100,6 +100,11 @@ public class DummyConnection extends AbstractXMPPConnection { callConnectionClosedListener(); } + @Override + public void instantShutdown() { + shutdown(); + } + @Override public boolean isSecureConnection() { return false; @@ -226,4 +231,5 @@ public class DummyConnection extends AbstractXMPPConnection { } } } + } diff --git a/smack-tcp/src/main/java/org/jivesoftware/smack/tcp/XMPPTCPConnection.java b/smack-tcp/src/main/java/org/jivesoftware/smack/tcp/XMPPTCPConnection.java index 8b1bdb70f..0d340cfcd 100644 --- a/smack-tcp/src/main/java/org/jivesoftware/smack/tcp/XMPPTCPConnection.java +++ b/smack-tcp/src/main/java/org/jivesoftware/smack/tcp/XMPPTCPConnection.java @@ -478,9 +478,7 @@ public class XMPPTCPConnection extends AbstractXMPPConnection { shutdown(false); } - /** - * Performs an unclean disconnect and shutdown of the connection. Does not send a closing stream stanza. - */ + @Override public synchronized void instantShutdown() { shutdown(true); } From 7f0932a481016505d58daf6cc4dbcbd5991573e0 Mon Sep 17 00:00:00 2001 From: Florian Schmaus Date: Mon, 4 Mar 2019 23:00:45 +0100 Subject: [PATCH 11/20] Reset the MUC self-presence collector on presence stanzas on join To prevent timeouts when joining very large MUCs we now reset the self-presence collector's timeout for every other (occupant) presence we receive. Fixes SMACK-859. --- .../smackx/muc/MultiUserChat.java | 19 ++++++++++++++++--- 1 file changed, 16 insertions(+), 3 deletions(-) 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 77a859e09..ce00484a3 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 @@ -340,8 +340,9 @@ public class MultiUserChat { // Setup the messageListeners and presenceListeners *before* the join presence is send. connection.addSyncStanzaListener(messageListener, fromRoomGroupchatFilter); - connection.addSyncStanzaListener(presenceListener, new AndFilter(fromRoomFilter, - StanzaTypeFilter.PRESENCE)); + StanzaFilter presenceFromRoomFilter = new AndFilter(fromRoomFilter, + StanzaTypeFilter.PRESENCE); + connection.addSyncStanzaListener(presenceListener, presenceFromRoomFilter); // @formatter:off connection.addSyncStanzaListener(subjectListener, new AndFilter(fromRoomFilter, @@ -370,15 +371,27 @@ public class MultiUserChat { ) ); // @formatter:on + StanzaCollector presenceStanzaCollector = null; Presence presence; try { - presence = connection.createStanzaCollectorAndSend(responseFilter, joinPresence).nextResultOrThrow(conf.getTimeout()); + // This stanza collector will collect the final self presence from the MUC, which also signals that we have successful entered the MUC. + StanzaCollector selfPresenceCollector = connection.createStanzaCollectorAndSend(responseFilter, joinPresence); + StanzaCollector.Configuration presenceStanzaCollectorConfguration = StanzaCollector.newConfiguration().setCollectorToReset( + selfPresenceCollector).setStanzaFilter(presenceFromRoomFilter); + // This stanza collector is used to reset the timeout of the selfPresenceCollector. + presenceStanzaCollector = connection.createStanzaCollector(presenceStanzaCollectorConfguration); + presence = selfPresenceCollector.nextResultOrThrow(conf.getTimeout()); } catch (NotConnectedException | InterruptedException | NoResponseException | XMPPErrorException e) { // Ensure that all callbacks are removed if there is an exception removeConnectionCallbacks(); throw e; } + finally { + if (presenceStanzaCollector != null) { + presenceStanzaCollector.cancel(); + } + } // This presence must be send from a full JID. We use the resourcepart of this JID as nick, since the room may // performed roomnick rewriting From 3d1a781a2228d9d2fe31f6b8c96687ca3dc56eb5 Mon Sep 17 00:00:00 2001 From: Florian Schmaus Date: Tue, 5 Mar 2019 08:21:59 +0100 Subject: [PATCH 12/20] Show correct reply timeout value in StanzaCollector's NoResponseException --- .../org/jivesoftware/smack/SmackException.java | 18 ++++++++++++++++-- .../jivesoftware/smack/StanzaCollector.java | 2 +- 2 files changed, 17 insertions(+), 3 deletions(-) diff --git a/smack-core/src/main/java/org/jivesoftware/smack/SmackException.java b/smack-core/src/main/java/org/jivesoftware/smack/SmackException.java index a84b75211..3011a1288 100644 --- a/smack-core/src/main/java/org/jivesoftware/smack/SmackException.java +++ b/smack-core/src/main/java/org/jivesoftware/smack/SmackException.java @@ -93,13 +93,24 @@ public class SmackException extends Exception { return new NoResponseException(sb.toString()); } + @Deprecated + // TODO: Remove in Smack 4.4. public static NoResponseException newWith(XMPPConnection connection, StanzaCollector collector) { return newWith(connection, collector.getStanzaFilter()); } + public static NoResponseException newWith(long timeout, + StanzaCollector collector) { + return newWith(timeout, collector.getStanzaFilter()); + } + public static NoResponseException newWith(XMPPConnection connection, StanzaFilter filter) { - final StringBuilder sb = getWaitingFor(connection); + return newWith(connection.getReplyTimeout(), filter); + } + + public static NoResponseException newWith(long timeout, StanzaFilter filter) { + final StringBuilder sb = getWaitingFor(timeout); sb.append(" Waited for response using: "); if (filter != null) { sb.append(filter.toString()); @@ -112,7 +123,10 @@ public class SmackException extends Exception { } private static StringBuilder getWaitingFor(XMPPConnection connection) { - final long replyTimeout = connection.getReplyTimeout(); + return getWaitingFor(connection.getReplyTimeout()); + } + + private static StringBuilder getWaitingFor(final long replyTimeout) { final StringBuilder sb = new StringBuilder(256); sb.append("No response received within reply timeout. Timeout was " + replyTimeout + "ms (~" diff --git a/smack-core/src/main/java/org/jivesoftware/smack/StanzaCollector.java b/smack-core/src/main/java/org/jivesoftware/smack/StanzaCollector.java index 0b933909e..f6aa5225c 100644 --- a/smack-core/src/main/java/org/jivesoftware/smack/StanzaCollector.java +++ b/smack-core/src/main/java/org/jivesoftware/smack/StanzaCollector.java @@ -262,7 +262,7 @@ public class StanzaCollector { if (!connection.isConnected()) { throw new NotConnectedException(connection, packetFilter); } - throw NoResponseException.newWith(connection, this); + throw NoResponseException.newWith(timeout, this); } XMPPErrorException.ifHasErrorThenThrow(result); From 7d2c3ac9f9ca28b172b3acb41d95ba9aa2583a63 Mon Sep 17 00:00:00 2001 From: Florian Schmaus Date: Wed, 6 Mar 2019 22:11:45 +0100 Subject: [PATCH 13/20] Do not call synchronized methods in reader/writer thread This may cause deadlocks with a call to acquire(2) on the introduced readerWriterSemaphore in initConnection(), which is also synchronized. --- .../smack/AbstractXMPPConnection.java | 2 +- .../smack/tcp/XMPPTCPConnection.java | 57 ++++++++++++++----- 2 files changed, 43 insertions(+), 16 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 697ebfc3b..d19b680ef 100644 --- a/smack-core/src/main/java/org/jivesoftware/smack/AbstractXMPPConnection.java +++ b/smack-core/src/main/java/org/jivesoftware/smack/AbstractXMPPConnection.java @@ -297,7 +297,7 @@ public abstract class AbstractXMPPConnection implements XMPPConnection { } }); - private static final AsyncButOrdered ASYNC_BUT_ORDERED = new AsyncButOrdered<>(); + protected static final AsyncButOrdered ASYNC_BUT_ORDERED = new AsyncButOrdered<>(); /** * The used host to establish the connection to diff --git a/smack-tcp/src/main/java/org/jivesoftware/smack/tcp/XMPPTCPConnection.java b/smack-tcp/src/main/java/org/jivesoftware/smack/tcp/XMPPTCPConnection.java index 0d340cfcd..77245f285 100644 --- a/smack-tcp/src/main/java/org/jivesoftware/smack/tcp/XMPPTCPConnection.java +++ b/smack-tcp/src/main/java/org/jivesoftware/smack/tcp/XMPPTCPConnection.java @@ -921,23 +921,46 @@ public class XMPPTCPConnection extends AbstractXMPPConnection { * * @param e the exception that causes the connection close event. */ - private synchronized void notifyConnectionError(Exception e) { - // Listeners were already notified of the exception, return right here. - if (packetReader.done && packetWriter.done()) return; + private synchronized void notifyConnectionError(final Exception e) { + ASYNC_BUT_ORDERED.performAsyncButOrdered(this, new Runnable() { + @Override + public void run() { + // Listeners were already notified of the exception, return right here. + if (packetReader.done || packetWriter.done()) return; - SmackWrappedException smackWrappedException = new SmackWrappedException(e); - tlsHandled.reportGenericFailure(smackWrappedException); - saslFeatureReceived.reportGenericFailure(smackWrappedException); - maybeCompressFeaturesReceived.reportGenericFailure(smackWrappedException); - lastFeaturesReceived.reportGenericFailure(smackWrappedException); + // Report the failure outside the synchronized block, so that a thread waiting within a synchronized + // function like connect() throws the wrapped exception. + SmackWrappedException smackWrappedException = new SmackWrappedException(e); + tlsHandled.reportGenericFailure(smackWrappedException); + saslFeatureReceived.reportGenericFailure(smackWrappedException); + maybeCompressFeaturesReceived.reportGenericFailure(smackWrappedException); + lastFeaturesReceived.reportGenericFailure(smackWrappedException); - // Closes the connection temporary. A reconnection is possible - // Note that a connection listener of XMPPTCPConnection will drop the SM state in - // case the Exception is a StreamErrorException. - instantShutdown(); + synchronized (XMPPTCPConnection.this) { + // Within this synchronized block, either *both* reader and writer threads must be terminated, or + // none. + assert ((packetReader.done && packetWriter.done()) + || (!packetReader.done && !packetWriter.done())); - // Notify connection listeners of the error. - callConnectionClosedOnErrorListener(e); + // Closes the connection temporary. A reconnection is possible + // Note that a connection listener of XMPPTCPConnection will drop the SM state in + // case the Exception is a StreamErrorException. + instantShutdown(); + + // Wait for reader and writer threads to be terminated. + readerWriterSemaphore.acquireUninterruptibly(2); + readerWriterSemaphore.release(2); + } + + Async.go(new Runnable() { + @Override + public void run() { + // Notify connection listeners of the error. + callConnectionClosedOnErrorListener(e); + } + }, XMPPTCPConnection.this + " callConnectionClosedOnErrorListener()"); + } + }); } /** @@ -1248,7 +1271,11 @@ public class XMPPTCPConnection extends AbstractXMPPConnection { LOGGER.info(XMPPTCPConnection.this + " received closing element." + " Server wants to terminate the connection, calling disconnect()"); - disconnect(); + ASYNC_BUT_ORDERED.performAsyncButOrdered(XMPPTCPConnection.this, new Runnable() { + @Override + public void run() { + disconnect(); + }}); } } break; From 5da6dea138cf57a96e6bd4d9298fdb497be5b64a Mon Sep 17 00:00:00 2001 From: Florian Schmaus Date: Thu, 7 Mar 2019 09:49:16 +0100 Subject: [PATCH 14/20] Throw exception to reduce call sites of notifyConnectionError() in XMPPTCPConnection. --- .../java/org/jivesoftware/smack/tcp/XMPPTCPConnection.java | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/smack-tcp/src/main/java/org/jivesoftware/smack/tcp/XMPPTCPConnection.java b/smack-tcp/src/main/java/org/jivesoftware/smack/tcp/XMPPTCPConnection.java index 77245f285..8d53dc5e8 100644 --- a/smack-tcp/src/main/java/org/jivesoftware/smack/tcp/XMPPTCPConnection.java +++ b/smack-tcp/src/main/java/org/jivesoftware/smack/tcp/XMPPTCPConnection.java @@ -973,14 +973,13 @@ public class XMPPTCPConnection extends AbstractXMPPConnection { } @Override - protected void afterFeaturesReceived() throws NotConnectedException, InterruptedException { + protected void afterFeaturesReceived() throws NotConnectedException, InterruptedException, SecurityRequiredByServerException { StartTls startTlsFeature = getFeature(StartTls.ELEMENT, StartTls.NAMESPACE); if (startTlsFeature != null) { if (startTlsFeature.required() && config.getSecurityMode() == SecurityMode.disabled) { - SmackException smackException = new SecurityRequiredByServerException(); + SecurityRequiredByServerException smackException = new SecurityRequiredByServerException(); tlsHandled.reportFailure(smackException); - notifyConnectionError(smackException); - return; + throw smackException; } if (config.getSecurityMode() != ConnectionConfiguration.SecurityMode.disabled) { From 7518bf9a25a8c8ac7e25fa5daff9d50ab4801bd7 Mon Sep 17 00:00:00 2001 From: Florian Schmaus Date: Fri, 8 Mar 2019 13:50:54 +0100 Subject: [PATCH 15/20] Add descriptive text to StanzaError.toString() --- .../java/org/jivesoftware/smack/packet/StanzaError.java | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/smack-core/src/main/java/org/jivesoftware/smack/packet/StanzaError.java b/smack-core/src/main/java/org/jivesoftware/smack/packet/StanzaError.java index fad9f5253..d5a2dd890 100644 --- a/smack-core/src/main/java/org/jivesoftware/smack/packet/StanzaError.java +++ b/smack-core/src/main/java/org/jivesoftware/smack/packet/StanzaError.java @@ -198,6 +198,12 @@ public class StanzaError extends AbstractError implements ExtensionElement { public String toString() { StringBuilder sb = new StringBuilder("XMPPError: "); sb.append(condition.toString()).append(" - ").append(type.toString()); + + String descriptiveText = getDescriptiveText(); + if (descriptiveText != null) { + sb.append(" [").append(descriptiveText).append(']'); + } + if (errorGenerator != null) { sb.append(". Generated by ").append(errorGenerator); } From c4289b2c1819bd7b62dca9ed7743410b19665296 Mon Sep 17 00:00:00 2001 From: Florian Schmaus Date: Sat, 9 Mar 2019 17:13:54 +0100 Subject: [PATCH 16/20] Add AbstractXMPPConnection.initState() and init/reset the sychronization points there. This method is called right at the beginning of connect() and at the end of shutdown(). --- .../org/jivesoftware/smack/AbstractXMPPConnection.java | 10 +++++++--- .../org/jivesoftware/smack/tcp/XMPPTCPConnection.java | 6 ++++++ 2 files changed, 13 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 d19b680ef..5800c6dd7 100644 --- a/smack-core/src/main/java/org/jivesoftware/smack/AbstractXMPPConnection.java +++ b/smack-core/src/main/java/org/jivesoftware/smack/AbstractXMPPConnection.java @@ -381,6 +381,12 @@ public abstract class AbstractXMPPConnection implements XMPPConnection { @Override public abstract boolean isUsingCompression(); + protected void initState() { + saslFeatureReceived.init(); + lastFeaturesReceived.init(); + tlsHandled.init(); + } + /** * Establishes a connection to the XMPP server. It basically * creates and maintains a connection to the server. @@ -399,10 +405,8 @@ public abstract class AbstractXMPPConnection implements XMPPConnection { throwAlreadyConnectedExceptionIfAppropriate(); // Reset the connection state + initState(); saslAuthentication.init(); - saslFeatureReceived.init(); - lastFeaturesReceived.init(); - tlsHandled.init(); streamId = null; try { diff --git a/smack-tcp/src/main/java/org/jivesoftware/smack/tcp/XMPPTCPConnection.java b/smack-tcp/src/main/java/org/jivesoftware/smack/tcp/XMPPTCPConnection.java index 8d53dc5e8..713bf77bd 100644 --- a/smack-tcp/src/main/java/org/jivesoftware/smack/tcp/XMPPTCPConnection.java +++ b/smack-tcp/src/main/java/org/jivesoftware/smack/tcp/XMPPTCPConnection.java @@ -534,6 +534,12 @@ public class XMPPTCPConnection extends AbstractXMPPConnection { reader = null; writer = null; + initState(); + } + + @Override + protected void initState() { + super.initState(); maybeCompressFeaturesReceived.init(); compressSyncPoint.init(); smResumedSyncPoint.init(); From 569f7417a8e2bc59462f0112334ae1cddab73dde Mon Sep 17 00:00:00 2001 From: Florian Schmaus Date: Sat, 9 Mar 2019 19:19:20 +0100 Subject: [PATCH 17/20] Add AuthenticatedConnectionInitiallyEstablished timestamp --- .../smack/AbstractXMPPConnection.java | 18 ++++++++++++++++++ .../smack/tcp/XMPPTCPConnection.java | 2 ++ 2 files changed, 20 insertions(+) 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 5800c6dd7..38e188ef9 100644 --- a/smack-core/src/main/java/org/jivesoftware/smack/AbstractXMPPConnection.java +++ b/smack-core/src/main/java/org/jivesoftware/smack/AbstractXMPPConnection.java @@ -314,6 +314,9 @@ public abstract class AbstractXMPPConnection implements XMPPConnection { */ protected boolean authenticated = false; + // TODO: Migrate to ZonedDateTime once Smack's minimum required Android SDK level is 26 (8.0, Oreo) or higher. + protected long authenticatedConnectionInitiallyEstablishedTimestamp; + /** * Flag that indicates if the user was authenticated with the server when the connection * to the server was closed (abruptly or not). @@ -590,6 +593,9 @@ public abstract class AbstractXMPPConnection implements XMPPConnection { } protected void afterSuccessfulLogin(final boolean resumed) throws NotConnectedException, InterruptedException { + if (!resumed) { + authenticatedConnectionInitiallyEstablishedTimestamp = System.currentTimeMillis(); + } // Indicate that we're now authenticated. this.authenticated = true; @@ -1729,6 +1735,18 @@ public abstract class AbstractXMPPConnection implements XMPPConnection { return lastStanzaReceived; } + /** + * Get the timestamp when the connection was the first time authenticated, i.e., when the first successful login was + * performed. Note that this value is not reset on disconnect, so it represents the timestamp from the last + * authenticated connection. The value is also not reset on stream resumption. + * + * @return the timestamp or {@code null}. + * @since 4.3.3 + */ + public final long getAuthenticatedConnectionInitiallyEstablishedTimestamp() { + return authenticatedConnectionInitiallyEstablishedTimestamp; + } + /** * Install a parsing exception callback, which will be invoked once an exception is encountered while parsing a * stanza. diff --git a/smack-tcp/src/main/java/org/jivesoftware/smack/tcp/XMPPTCPConnection.java b/smack-tcp/src/main/java/org/jivesoftware/smack/tcp/XMPPTCPConnection.java index 713bf77bd..9f2c2d048 100644 --- a/smack-tcp/src/main/java/org/jivesoftware/smack/tcp/XMPPTCPConnection.java +++ b/smack-tcp/src/main/java/org/jivesoftware/smack/tcp/XMPPTCPConnection.java @@ -527,6 +527,8 @@ public class XMPPTCPConnection extends AbstractXMPPConnection { // Reset the stream management session id to null, since if the stream is cleanly closed, i.e. sending a closing // stream tag, there is no longer a stream to resume. smSessionId = null; + // Note that we deliberately do not reset authenticatedConnectionInitiallyEstablishedTimestamp here, so that the + // information is available in the connectionClosedOnError() listeners. } authenticated = false; connected = false; From e5bbd19ef11e4111d8927f44be5061e765975962 Mon Sep 17 00:00:00 2001 From: Georg Lukas Date: Wed, 28 Nov 2018 17:55:46 +0100 Subject: [PATCH 18/20] StanzaDroppedListener for XEP-0198 resumption failures If a stream resume fails, smack will re-send all queued stanzas after a reconnect. However, it does not make sense to re-send them: * IQs / IQ responses have probably timed out * MUC messages and PMs will be rejected as you haven't rejoined yet * regular messages should be amended with a element This patch adds a StanzaDroppedListener interface to the XMPPTCPConnection. If at least one StanzaDroppedListener is provided, all queued messages will be drained into the StanzaDroppedListener(s). Otherwise, the original behavior of attempting to transmit them will be followed. Discussion: https://discourse.igniterealtime.org/t/xep-0198-resume-failure-reconnect-resending-of-muc-messages/83510/3 Signed-off-by: Georg Lukas --- .../smack/tcp/XMPPTCPConnection.java | 56 ++++++++++++++++++- 1 file changed, 53 insertions(+), 3 deletions(-) diff --git a/smack-tcp/src/main/java/org/jivesoftware/smack/tcp/XMPPTCPConnection.java b/smack-tcp/src/main/java/org/jivesoftware/smack/tcp/XMPPTCPConnection.java index 9f2c2d048..61de855d6 100644 --- a/smack-tcp/src/main/java/org/jivesoftware/smack/tcp/XMPPTCPConnection.java +++ b/smack-tcp/src/main/java/org/jivesoftware/smack/tcp/XMPPTCPConnection.java @@ -283,6 +283,15 @@ public class XMPPTCPConnection extends AbstractXMPPConnection { */ private final Collection stanzaAcknowledgedListeners = new ConcurrentLinkedQueue<>(); + /** + * These listeners are invoked for every stanza that got dropped. + *

+ * We use a {@link ConcurrentLinkedQueue} here in order to allow the listeners to remove + * themselves after they have been invoked. + *

+ */ + private final Collection stanzaDroppedListeners = new ConcurrentLinkedQueue<>(); + /** * This listeners are invoked for a acknowledged stanza that has the given stanza ID. They will * only be invoked once and automatically removed after that. @@ -447,9 +456,24 @@ public class XMPPTCPConnection extends AbstractXMPPConnection { } } } - // (Re-)send the stanzas *after* we tried to enable SM - for (Stanza stanza : previouslyUnackedStanzas) { - sendStanzaInternal(stanza); + // Inform client about failed resumption if possible, resend stanzas otherwise + // Process the stanzas synchronously so a client can re-queue them for transmission + // before it is informed about connection success + if (!stanzaDroppedListeners.isEmpty()) { + for (Stanza stanza : previouslyUnackedStanzas) { + for (StanzaListener listener : stanzaDroppedListeners) { + try { + listener.processStanza(stanza); + } + catch (InterruptedException | NotConnectedException | NotLoggedInException e) { + LOGGER.log(Level.FINER, "StanzaDroppedListener received exception", e); + } + } + } + } else { + for (Stanza stanza : previouslyUnackedStanzas) { + sendStanzaInternal(stanza); + } } afterSuccessfulLogin(false); @@ -1787,6 +1811,32 @@ public class XMPPTCPConnection extends AbstractXMPPConnection { stanzaAcknowledgedListeners.clear(); } + /** + * Add a Stanza dropped listener. + *

+ * Those listeners will be invoked every time a Stanza has been dropped due to a failed SM resume. They will not get + * automatically removed. If at least one StanzaDroppedListener is configured, no attempt will be made to retransmit + * the Stanzas. + *

+ * + * @param listener the listener to add. + * @since 4.3.3 + */ + public void addStanzaDroppedListener(StanzaListener listener) { + stanzaDroppedListeners.add(listener); + } + + /** + * Remove the given Stanza dropped listener. + * + * @param listener the listener. + * @return true if the listener was removed. + * @since 4.3.3 + */ + public boolean removeStanzaDroppedListener(StanzaListener listener) { + return stanzaDroppedListeners.remove(listener); + } + /** * Add a new Stanza ID acknowledged listener for the given ID. *

From 0de0873abb3b34618bb9b771800c505609a2113f Mon Sep 17 00:00:00 2001 From: Florian Schmaus Date: Thu, 14 Mar 2019 14:29:23 +0100 Subject: [PATCH 19/20] version.gradle: Add link to SMACK-858 --- version.gradle | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/version.gradle b/version.gradle index a478e9a52..4634e860a 100644 --- a/version.gradle +++ b/version.gradle @@ -5,7 +5,9 @@ allprojects { // When using dynamic versions for those, do *not* use [1.0, // 2.0), since this will also pull in 2.0-alpha1. Instead use // [1.0, 1.0.99]. - // See also https://issues.apache.org/jira/browse/MNG-6232 + // See also: + // - https://issues.apache.org/jira/browse/MNG-6232 + // - https://issues.igniterealtime.org/browse/SMACK-858 jxmppVersion = '[0.6, 0.6.999]' miniDnsVersion = '[0.3, 0.3.999]' smackMinAndroidSdk = 9 From b054c4fe77435cc9cdbdd2722e9bb923815acdc7 Mon Sep 17 00:00:00 2001 From: Florian Schmaus Date: Thu, 14 Mar 2019 14:31:09 +0100 Subject: [PATCH 20/20] Smack 4.3.3 --- resources/releasedocs/changelog.html | 18 ++++++++++++++++++ version.gradle | 2 +- 2 files changed, 19 insertions(+), 1 deletion(-) diff --git a/resources/releasedocs/changelog.html b/resources/releasedocs/changelog.html index 5d249c249..706a4e2c6 100644 --- a/resources/releasedocs/changelog.html +++ b/resources/releasedocs/changelog.html @@ -141,6 +141,24 @@ hr {

+

4.3.3 -- 2019-03-14

+ +

Bug +

+
    +
  • [SMACK-856] - Smack fails under JDK 11 because com.sun.jndi.dns.DnsContextFactory is not inaccessible +
    • +
+ +

Improvement +

+
    +
  • [SMACK-858] - Dependency version specifier of jxmpp and MiniDNS include alpha/beta/... versions of the follow up version when Maven is used +
    • +
  • [SMACK-859] - MultiUserChat enter() should reset the timeout of the collector waiting for the final self presence to prevent timeouts for large MUCs +
    • +
+

4.3.2 -- 2019-02-22

Bug diff --git a/version.gradle b/version.gradle index 4634e860a..85012ef33 100644 --- a/version.gradle +++ b/version.gradle @@ -1,7 +1,7 @@ allprojects { ext { shortVersion = '4.3.3' - isSnapshot = true + isSnapshot = false // When using dynamic versions for those, do *not* use [1.0, // 2.0), since this will also pull in 2.0-alpha1. Instead use // [1.0, 1.0.99].