From cfb0656456cb76285fd0f992bcac14b77c4bb0b5 Mon Sep 17 00:00:00 2001 From: Florian Schmaus Date: Mon, 14 Aug 2017 20:45:06 +0200 Subject: [PATCH 01/15] Smack 4.2.2-SNAPSHOT --- version.gradle | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/version.gradle b/version.gradle index 1988ed131..d43855ac2 100644 --- a/version.gradle +++ b/version.gradle @@ -1,7 +1,7 @@ allprojects { ext { - shortVersion = '4.2.1' - isSnapshot = false + shortVersion = '4.2.2' + isSnapshot = true jxmppVersion = '0.5.0' smackMinAndroidSdk = 8 } From f5ccf61c5099591e9fc3a2e0e9644e45c91fdc57 Mon Sep 17 00:00:00 2001 From: Florian Schmaus Date: Mon, 14 Aug 2017 20:50:41 +0200 Subject: [PATCH 02/15] Disable clirr for now --- build.gradle | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/build.gradle b/build.gradle index a8b0bb1ec..9a2a06d27 100644 --- a/build.gradle +++ b/build.gradle @@ -396,6 +396,12 @@ subprojects { } clirr { + // 2018-08-14: Disabled Clirr because + // - It reports an breaking change in android.jar (seems right, but there is nothing we can do about it) + // - Only the first smack-* projects are correctly checked, + // the other ones have the output of a clirr report from a previous project + // (Look at the clirr reports). + enabled false semver false } } From 8052ee752b16cd5faad962fa126d1c2c6beb41ce Mon Sep 17 00:00:00 2001 From: vanitasvitae Date: Wed, 9 Aug 2017 21:00:01 +0200 Subject: [PATCH 03/15] Add missing cleanup in Omemo integrationtest --- .../java/org/jivesoftware/smackx/omemo/OmemoStoreTest.java | 3 +++ 1 file changed, 3 insertions(+) diff --git a/smack-integration-test/src/main/java/org/jivesoftware/smackx/omemo/OmemoStoreTest.java b/smack-integration-test/src/main/java/org/jivesoftware/smackx/omemo/OmemoStoreTest.java index 91dca385b..e79e65c34 100644 --- a/smack-integration-test/src/main/java/org/jivesoftware/smackx/omemo/OmemoStoreTest.java +++ b/smack-integration-test/src/main/java/org/jivesoftware/smackx/omemo/OmemoStoreTest.java @@ -22,6 +22,7 @@ import static junit.framework.TestCase.assertNotNull; import static junit.framework.TestCase.assertNotSame; import static junit.framework.TestCase.assertNull; import static junit.framework.TestCase.assertTrue; +import static org.jivesoftware.smackx.omemo.OmemoIntegrationTestHelper.cleanServerSideTraces; import static org.jivesoftware.smackx.omemo.OmemoIntegrationTestHelper.deletePath; import static org.jivesoftware.smackx.omemo.OmemoIntegrationTestHelper.setUpOmemoManager; @@ -154,6 +155,8 @@ public class OmemoStoreTest extends AbstractOmemoIntegrationTest { @Override public void after() { + cleanServerSideTraces(alice); + cleanServerSideTraces(bob); alice.shutdown(); bob.shutdown(); } From 199311eda16b7b6a3457129fe0f1585bb6289293 Mon Sep 17 00:00:00 2001 From: Ingo Bauersachs Date: Sat, 5 Aug 2017 17:05:40 +0200 Subject: [PATCH 04/15] Fix error Condition to Type mappings to match RFC 6120 Synchronize the Javadoc table to match the actual implementation. --- .../org/jivesoftware/smack/packet/XMPPError.java | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/smack-core/src/main/java/org/jivesoftware/smack/packet/XMPPError.java b/smack-core/src/main/java/org/jivesoftware/smack/packet/XMPPError.java index 3913fe374..fc96cbbfa 100644 --- a/smack-core/src/main/java/org/jivesoftware/smack/packet/XMPPError.java +++ b/smack-core/src/main/java/org/jivesoftware/smack/packet/XMPPError.java @@ -39,14 +39,14 @@ import org.jivesoftware.smack.util.XmlStringBuilder; * conflictCANCEL8.3.3.2 * feature-not-implementedCANCEL8.3.3.3 * forbiddenAUTH8.3.3.4 - * goneMODIFY8.3.3.5 + * goneCANCEL8.3.3.5 * internal-server-errorWAIT8.3.3.6 * item-not-foundCANCEL8.3.3.7 * jid-malformedMODIFY8.3.3.8 - * not-acceptable MODIFY8.3.3.9 + * not-acceptableMODIFY8.3.3.9 * not-allowedCANCEL8.3.3.10 * not-authorizedAUTH8.3.3.11 - * policy-violationAUTH8.3.3.12 + * policy-violationMODIFY8.3.3.12 * recipient-unavailableWAIT8.3.3.13 * redirectMODIFY8.3.3.14 * registration-requiredAUTH8.3.3.15 @@ -55,7 +55,7 @@ import org.jivesoftware.smack.util.XmlStringBuilder; * resource-constraintWAIT8.3.3.18 * service-unavailableCANCEL8.3.3.19 * subscription-requiredAUTH8.3.3.20 - * undefined-conditionWAIT8.3.3.21 + * undefined-conditionMODIFY8.3.3.21 * unexpected-requestWAIT8.3.3.22 * * @@ -91,9 +91,10 @@ public class XMPPError extends AbstractError { CONDITION_TO_TYPE.put(Condition.remote_server_not_found, Type.CANCEL); CONDITION_TO_TYPE.put(Condition.remote_server_timeout, Type.WAIT); CONDITION_TO_TYPE.put(Condition.resource_constraint, Type.WAIT); - CONDITION_TO_TYPE.put(Condition.service_unavailable, Type.WAIT); - CONDITION_TO_TYPE.put(Condition.subscription_required, Type.WAIT); - CONDITION_TO_TYPE.put(Condition.unexpected_request, Type.MODIFY); + CONDITION_TO_TYPE.put(Condition.service_unavailable, Type.CANCEL); + CONDITION_TO_TYPE.put(Condition.subscription_required, Type.AUTH); + CONDITION_TO_TYPE.put(Condition.undefined_condition, Type.MODIFY); + CONDITION_TO_TYPE.put(Condition.unexpected_request, Type.WAIT); } private final Condition condition; From 47c936e50861da3bc50c173cf0c8fde23cc6820a Mon Sep 17 00:00:00 2001 From: Ingo Bauersachs Date: Sun, 6 Aug 2017 14:25:57 +0200 Subject: [PATCH 05/15] Make UnknownIqRequestReplyMode public If it is not made public, then (g|s)etUnknownIqRequestReplyMode cannot be used. --- .../main/java/org/jivesoftware/smack/SmackConfiguration.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/smack-core/src/main/java/org/jivesoftware/smack/SmackConfiguration.java b/smack-core/src/main/java/org/jivesoftware/smack/SmackConfiguration.java index 245e8f0e9..332dfca62 100644 --- a/smack-core/src/main/java/org/jivesoftware/smack/SmackConfiguration.java +++ b/smack-core/src/main/java/org/jivesoftware/smack/SmackConfiguration.java @@ -371,7 +371,7 @@ public final class SmackConfiguration { return defaultHostnameVerififer; } - enum UnknownIqRequestReplyMode { + public enum UnknownIqRequestReplyMode { doNotReply, replyFeatureNotImplemented, replyServiceUnavailable, From b7542bbde5546b8c794a391963b34f67d852dea5 Mon Sep 17 00:00:00 2001 From: Ingo Bauersachs Date: Mon, 14 Aug 2017 21:47:53 +0200 Subject: [PATCH 06/15] Fix too shorts indents of two Javadoc comments --- .../main/java/org/jivesoftware/smack/SmackConfiguration.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/smack-core/src/main/java/org/jivesoftware/smack/SmackConfiguration.java b/smack-core/src/main/java/org/jivesoftware/smack/SmackConfiguration.java index 245e8f0e9..5096a1e44 100644 --- a/smack-core/src/main/java/org/jivesoftware/smack/SmackConfiguration.java +++ b/smack-core/src/main/java/org/jivesoftware/smack/SmackConfiguration.java @@ -179,7 +179,7 @@ public final class SmackConfiguration { } } - /** + /** * Add a Collection of SASL mechanisms to the list to be used. * * @param mechs the Collection of SASL mechanisms to be added @@ -236,7 +236,7 @@ public final class SmackConfiguration { defaultMechs.remove(mech); } - /** + /** * Remove a Collection of SASL mechanisms to the list to be used. * * @param mechs the Collection of SASL mechanisms to be removed From 25114b3fc1b3510a734524139080e970eb8b45ee Mon Sep 17 00:00:00 2001 From: Ingo Bauersachs Date: Sat, 5 Aug 2017 17:06:13 +0200 Subject: [PATCH 07/15] Add a test to ensure all Conditions have a Type mapping --- .../jivesoftware/smack/packet/XMPPError.java | 2 +- .../smack/packet/XMPPErrorTest.java | 35 +++++++++++++++++++ 2 files changed, 36 insertions(+), 1 deletion(-) create mode 100644 smack-core/src/test/java/org/jivesoftware/smack/packet/XMPPErrorTest.java diff --git a/smack-core/src/main/java/org/jivesoftware/smack/packet/XMPPError.java b/smack-core/src/main/java/org/jivesoftware/smack/packet/XMPPError.java index fc96cbbfa..54d40a47a 100644 --- a/smack-core/src/main/java/org/jivesoftware/smack/packet/XMPPError.java +++ b/smack-core/src/main/java/org/jivesoftware/smack/packet/XMPPError.java @@ -70,7 +70,7 @@ public class XMPPError extends AbstractError { public static final String ERROR = "error"; private static final Logger LOGGER = Logger.getLogger(XMPPError.class.getName()); - private static final Map CONDITION_TO_TYPE = new HashMap(); + static final Map CONDITION_TO_TYPE = new HashMap(); static { CONDITION_TO_TYPE.put(Condition.bad_request, Type.MODIFY); diff --git a/smack-core/src/test/java/org/jivesoftware/smack/packet/XMPPErrorTest.java b/smack-core/src/test/java/org/jivesoftware/smack/packet/XMPPErrorTest.java new file mode 100644 index 000000000..f2e84a3f7 --- /dev/null +++ b/smack-core/src/test/java/org/jivesoftware/smack/packet/XMPPErrorTest.java @@ -0,0 +1,35 @@ +/** + * + * Copyright © 2017 Ingo Bauersachs + * + * 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.packet.XMPPError.Condition; +import static org.jivesoftware.smack.packet.XMPPError.Type; +import static org.junit.Assert.assertEquals; + +import java.util.Map; + +import org.junit.Test; + +public class XMPPErrorTest { + @Test + public void testConditionHasDefaultTypeMapping() throws NoSuchFieldException, IllegalAccessException { + Map conditionToTypeMap = XMPPError.CONDITION_TO_TYPE; + assertEquals("CONDITION_TO_TYPE map is likely out of sync with Condition enum", + Condition.values().length, + conditionToTypeMap.size()); + } +} From c92a95136faa4f227cbffe3431bb3c80bbca0a48 Mon Sep 17 00:00:00 2001 From: iachimoe Date: Sat, 26 Aug 2017 12:16:07 -0500 Subject: [PATCH 08/15] Updated comments to indicate that reject_all is default SubscriptionMode --- .../java/org/jivesoftware/smack/roster/Roster.java | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/smack-im/src/main/java/org/jivesoftware/smack/roster/Roster.java b/smack-im/src/main/java/org/jivesoftware/smack/roster/Roster.java index dfd35a636..3fc27d62f 100644 --- a/smack-im/src/main/java/org/jivesoftware/smack/roster/Roster.java +++ b/smack-im/src/main/java/org/jivesoftware/smack/roster/Roster.java @@ -213,7 +213,7 @@ public final class Roster extends Manager { * Returns the default subscription processing mode to use when a new Roster is created. The * subscription processing mode dictates what action Smack will take when subscription * requests from other users are made. The default subscription mode - * is {@link SubscriptionMode#accept_all}. + * is {@link SubscriptionMode#reject_all}. * * @return the default subscription mode to use for new Rosters */ @@ -225,7 +225,7 @@ public final class Roster extends Manager { * Sets the default subscription processing mode to use when a new Roster is created. The * subscription processing mode dictates what action Smack will take when subscription * requests from other users are made. The default subscription mode - * is {@link SubscriptionMode#accept_all}. + * is {@link SubscriptionMode#reject_all}. * * @param subscriptionMode the default subscription mode to use for new Rosters. */ @@ -383,7 +383,7 @@ public final class Roster extends Manager { /** * Returns the subscription processing mode, which dictates what action * Smack will take when subscription requests from other users are made. - * The default subscription mode is {@link SubscriptionMode#accept_all}. + * The default subscription mode is {@link SubscriptionMode#reject_all}. *

* If using the manual mode, a PacketListener should be registered that * listens for Presence packets that have a type of @@ -399,7 +399,7 @@ public final class Roster extends Manager { /** * Sets the subscription processing mode, which dictates what action * Smack will take when subscription requests from other users are made. - * The default subscription mode is {@link SubscriptionMode#accept_all}. + * The default subscription mode is {@link SubscriptionMode#reject_all}. *

* If using the manual mode, a PacketListener should be registered that * listens for Presence packets that have a type of @@ -1402,14 +1402,14 @@ public final class Roster extends Manager { public enum SubscriptionMode { /** - * Automatically accept all subscription and unsubscription requests. This is - * the default mode and is suitable for simple client. More complex client will + * Automatically accept all subscription and unsubscription requests. + * This is suitable for simple clients. More complex clients will * likely wish to handle subscription requests manually. */ accept_all, /** - * Automatically reject all subscription requests. + * Automatically reject all subscription requests. This is the default mode. */ reject_all, From 699145ee5fdc4fb18140fb2e385c6d8fe60f81d1 Mon Sep 17 00:00:00 2001 From: Ingo Bauersachs Date: Wed, 2 Aug 2017 01:54:29 +0200 Subject: [PATCH 09/15] Get descriptive text from error without lang A stanza with Some text, i.e. without a xml:lang attribute, did not return the value 'Some text' in getDescriptiveText(). Insert the empty string as key when the attribute is not present while parsing the xml. When writing, omit the attribute if the key is the empty string. --- .../smack/packet/AbstractError.java | 14 +++++- .../smack/util/PacketParserUtils.java | 6 +++ .../smack/util/XmlStringBuilder.java | 2 +- .../smack/util/PacketParserUtilsTest.java | 43 +++++++++++++++++++ 4 files changed, 63 insertions(+), 2 deletions(-) diff --git a/smack-core/src/main/java/org/jivesoftware/smack/packet/AbstractError.java b/smack-core/src/main/java/org/jivesoftware/smack/packet/AbstractError.java index d04c41ad3..fbb68eb52 100644 --- a/smack-core/src/main/java/org/jivesoftware/smack/packet/AbstractError.java +++ b/smack-core/src/main/java/org/jivesoftware/smack/packet/AbstractError.java @@ -23,6 +23,7 @@ import java.util.List; import java.util.Locale; import java.util.Map; +import org.jivesoftware.smack.util.Objects; import org.jivesoftware.smack.util.PacketUtil; import org.jivesoftware.smack.util.XmlStringBuilder; @@ -85,6 +86,7 @@ public class AbstractError { * @return the descriptive text or null. */ public String getDescriptiveText(String xmllang) { + Objects.requireNonNull(xmllang, "xmllang must not be null"); return descriptiveTexts.get(xmllang); } @@ -105,7 +107,8 @@ public class AbstractError { String xmllang = entry.getKey(); String text = entry.getValue(); xml.halfOpenElement("text").xmlnsAttribute(textNamespace) - .xmllangAttribute(xmllang).rightAngleBracket(); + .optXmlLangAttribute(xmllang) + .rightAngleBracket(); xml.escape(text); xml.closeElement("text"); } @@ -120,6 +123,15 @@ public class AbstractError { protected List extensions; public B setDescriptiveTexts(Map descriptiveTexts) { + if (descriptiveTexts == null) { + this.descriptiveTexts = null; + return getThis(); + } + for (String key : descriptiveTexts.keySet()) { + if (key == null) { + throw new IllegalArgumentException("descriptiveTexts cannot contain null key"); + } + } if (this.descriptiveTexts == null) { this.descriptiveTexts = descriptiveTexts; } 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 02a6c96a3..4c02ee080 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 @@ -749,6 +749,12 @@ public class PacketParserUtils { descriptiveTexts = new HashMap(); } String xmllang = getLanguageAttribute(parser); + if (xmllang == null) { + // XMPPError assumes the default locale, 'en', or the empty string. + // Establish the invariant that there is never null as a key. + xmllang = ""; + } + String text = parser.nextText(); String previousValue = descriptiveTexts.put(xmllang, text); assert (previousValue == null); 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 2579bf522..e0b4a3847 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 @@ -361,7 +361,7 @@ public class XmlStringBuilder implements Appendable, CharSequence { } public XmlStringBuilder optXmlLangAttribute(String lang) { - if (lang != null) { + if (!StringUtils.isNullOrEmpty(lang)) { xmllangAttribute(lang); } return this; diff --git a/smack-core/src/test/java/org/jivesoftware/smack/util/PacketParserUtilsTest.java b/smack-core/src/test/java/org/jivesoftware/smack/util/PacketParserUtilsTest.java index 780e0bb23..bdbeb4e1a 100644 --- a/smack-core/src/test/java/org/jivesoftware/smack/util/PacketParserUtilsTest.java +++ b/smack-core/src/test/java/org/jivesoftware/smack/util/PacketParserUtilsTest.java @@ -25,7 +25,9 @@ import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; import java.io.IOException; +import java.util.HashMap; import java.util.Locale; +import java.util.Map; import java.util.Properties; import javax.xml.parsers.FactoryConfigurationError; @@ -35,6 +37,7 @@ import javax.xml.transform.TransformerException; import org.jivesoftware.smack.packet.Message; import org.jivesoftware.smack.packet.Presence; import org.jivesoftware.smack.packet.Stanza; +import org.jivesoftware.smack.packet.XMPPError; import org.jivesoftware.smack.sasl.SASLError; import org.jivesoftware.smack.sasl.packet.SaslStreamElements; import org.jivesoftware.smack.sasl.packet.SaslStreamElements.SASLFailure; @@ -867,4 +870,44 @@ public class PacketParserUtilsTest { return otherLanguage; } + @Test(expected = IllegalArgumentException.class) + public void descriptiveTextNullLangPassedMap() throws Exception { + final String text = "Dummy descriptive text"; + Map texts = new HashMap<>(); + texts.put(null, text); + XMPPError + .getBuilder(XMPPError.Condition.internal_server_error) + .setDescriptiveTexts(texts) + .build(); + } + + @Test + public void ensureNoEmptyLangInDescriptiveText() throws Exception { + final String text = "Dummy descriptive text"; + Map texts = new HashMap<>(); + texts.put("", text); + XMPPError error = XMPPError + .getBuilder(XMPPError.Condition.internal_server_error) + .setDescriptiveTexts(texts) + .build(); + final String errorXml = XMLBuilder + .create(XMPPError.ERROR).a("type", "cancel").up() + .element("internal-server-error", XMPPError.NAMESPACE).up() + .element("text", XMPPError.NAMESPACE).t(text).up() + .asString(); + XmlUnitUtils.assertSimilar(errorXml, error.toXML()); + } + + @Test + public void ensureNoNullLangInParsedDescriptiveTexts() throws Exception { + final String text = "Dummy descriptive text"; + final String errorXml = XMLBuilder + .create(XMPPError.ERROR).a("type", "cancel").up() + .element("internal-server-error", XMPPError.NAMESPACE).up() + .element("text", XMPPError.NAMESPACE).t(text).up() + .asString(); + XmlPullParser parser = TestUtils.getParser(errorXml); + XMPPError error = PacketParserUtils.parseError(parser).build(); + assertEquals(text, error.getDescriptiveText()); + } } From 941f29e92842f6f787bf8d504d5a8eab4fcc3105 Mon Sep 17 00:00:00 2001 From: Florian Schmaus Date: Fri, 29 Sep 2017 15:19:10 +0200 Subject: [PATCH 10/15] Improve ReconnectionManager Fixes SMACK-778. --- .../smack/ReconnectionManager.java | 70 +++++++++++++------ 1 file changed, 49 insertions(+), 21 deletions(-) diff --git a/smack-core/src/main/java/org/jivesoftware/smack/ReconnectionManager.java b/smack-core/src/main/java/org/jivesoftware/smack/ReconnectionManager.java index c2b35110d..2cd0b7c8d 100644 --- a/smack-core/src/main/java/org/jivesoftware/smack/ReconnectionManager.java +++ b/smack-core/src/main/java/org/jivesoftware/smack/ReconnectionManager.java @@ -43,7 +43,10 @@ import org.jivesoftware.smack.util.Async; * * * {@link ReconnectionPolicy#FIXED_DELAY} - The reconnection mechanism will try to reconnect after a fixed delay - * independently from the number of reconnection attempts already performed + * independently from the number of reconnection attempts already performed. + *

+ * Interrupting the reconnection thread will abort the reconnection mechanism. + *

* * @author Francisco Vives * @author Luca Stucchi @@ -163,7 +166,7 @@ public final class ReconnectionManager { private ReconnectionManager(AbstractXMPPConnection connection) { weakRefConnection = new WeakReference(connection); - reconnectionRunnable = new Thread() { + reconnectionRunnable = new Runnable() { /** * Holds the current number of reconnection attempts @@ -211,6 +214,10 @@ public final class ReconnectionManager { if (connection == null) { return; } + + // Reset attempts to zero since a new reconnection cycle is started once this runs. + attempts = 0; + // The process will try to reconnect until the connection is established or // the user cancel the reconnection process AbstractXMPPConnection.disconnect(). while (isReconnectionPossible(connection)) { @@ -219,7 +226,10 @@ public final class ReconnectionManager { // Sleep until we're ready for the next reconnection attempt. Notify // listeners once per second about how much time remains before the next // reconnection attempt. - while (isReconnectionPossible(connection) && remainingSeconds > 0) { + while (remainingSeconds > 0) { + if (!isReconnectionPossible(connection)) { + return; + } try { Thread.sleep(1000); remainingSeconds--; @@ -228,8 +238,9 @@ public final class ReconnectionManager { } } catch (InterruptedException e) { - LOGGER.log(Level.FINE, "waiting for reconnection interrupted", e); - break; + LOGGER.log(Level.FINE, "Reconnection Thread was interrupted, aborting reconnection mechanism", e); + // Exit the reconnection thread in case it was interrupted. + return; } } @@ -237,24 +248,18 @@ public final class ReconnectionManager { listener.reconnectingIn(0); } + if (!isReconnectionPossible(connection)) { + return; + } // Makes a reconnection attempt try { - if (isReconnectionPossible(connection)) { - try { - connection.connect(); - } catch (SmackException.AlreadyConnectedException e) { - LOGGER.log(Level.FINER, "Connection was already connected on reconnection attempt", e); - } + try { + connection.connect(); } - // TODO Starting with Smack 4.2, connect() will no - // longer login automatically. So change this and the - // previous lines to connection.connect().login() in the - // 4.2, or any later, branch. - if (!connection.isAuthenticated()) { - connection.login(); + catch (SmackException.AlreadyConnectedException e) { + LOGGER.log(Level.FINER, "Connection was already connected on reconnection attempt", e); } - // Successfully reconnected. - attempts = 0; + connection.login(); } catch (SmackException.AlreadyLoggedInException e) { // This can happen if another thread concurrently triggers a reconnection @@ -262,12 +267,21 @@ public final class ReconnectionManager { // failure. See also SMACK-725. LOGGER.log(Level.FINER, "Reconnection not required, was already logged in", e); } - catch (SmackException | IOException | XMPPException | InterruptedException e) { + catch (SmackException | IOException | XMPPException e) { // Fires the failed reconnection notification for (ConnectionListener listener : connection.connectionListeners) { listener.reconnectionFailed(e); } + // Failed to reconnect, try again. + continue; + } catch (InterruptedException e) { + LOGGER.log(Level.FINE, "Reconnection Thread was interrupted, aborting reconnection mechanism", e); + // Exit the reconnection thread in case it was interrupted. + return; } + + // Successfully reconnected . + return; } } }; @@ -314,7 +328,7 @@ public final class ReconnectionManager { * * @return true, if the reconnection mechanism is enabled. */ - public boolean isAutomaticReconnectEnabled() { + public synchronized boolean isAutomaticReconnectEnabled() { return automaticReconnectEnabled; } @@ -348,6 +362,20 @@ public final class ReconnectionManager { "Smack Reconnection Manager (" + connection.getConnectionCounter() + ')'); } + /** + * Abort a possibly running reconnection mechanism. + * + * @since 4.2.2 + */ + public synchronized void abortPossiblyRunningReconnection() { + if (reconnectionThread == null) { + return; + } + + reconnectionThread.interrupt(); + reconnectionThread = null; + } + private final ConnectionListener connectionListener = new AbstractConnectionListener() { @Override From 58181bab08045fb0f4319416fef061e46f589c32 Mon Sep 17 00:00:00 2001 From: Florian Schmaus Date: Fri, 29 Sep 2017 15:36:28 +0200 Subject: [PATCH 11/15] OMEMO: Unspaghetti PEP listener code While I like Spaghetti, I don't like them in my code. 4 insertions, 24 deletions, *kaboom*. :) --- .../smackx/omemo/OmemoManager.java | 4 ++++ .../smackx/omemo/OmemoService.java | 24 ------------------- 2 files changed, 4 insertions(+), 24 deletions(-) diff --git a/smack-omemo/src/main/java/org/jivesoftware/smackx/omemo/OmemoManager.java b/smack-omemo/src/main/java/org/jivesoftware/smackx/omemo/OmemoManager.java index d17582172..f635d2acb 100644 --- a/smack-omemo/src/main/java/org/jivesoftware/smackx/omemo/OmemoManager.java +++ b/smack-omemo/src/main/java/org/jivesoftware/smackx/omemo/OmemoManager.java @@ -19,6 +19,7 @@ package org.jivesoftware.smackx.omemo; import static org.jivesoftware.smackx.omemo.util.OmemoConstants.BODY_OMEMO_HINT; import static org.jivesoftware.smackx.omemo.util.OmemoConstants.OMEMO; import static org.jivesoftware.smackx.omemo.util.OmemoConstants.OMEMO_NAMESPACE_V_AXOLOTL; +import static org.jivesoftware.smackx.omemo.util.OmemoConstants.PEP_NODE_DEVICE_LIST_NOTIFY; import java.security.NoSuchAlgorithmException; import java.util.ArrayList; @@ -132,6 +133,9 @@ public final class OmemoManager extends Manager { } }); + PEPManager.getInstanceFor(connection).addPEPListener(deviceListUpdateListener); + ServiceDiscoveryManager.getInstanceFor(connection).addFeature(PEP_NODE_DEVICE_LIST_NOTIFY); + service = OmemoService.getInstance(); } diff --git a/smack-omemo/src/main/java/org/jivesoftware/smackx/omemo/OmemoService.java b/smack-omemo/src/main/java/org/jivesoftware/smackx/omemo/OmemoService.java index 093767166..3c357b7ed 100644 --- a/smack-omemo/src/main/java/org/jivesoftware/smackx/omemo/OmemoService.java +++ b/smack-omemo/src/main/java/org/jivesoftware/smackx/omemo/OmemoService.java @@ -19,7 +19,6 @@ package org.jivesoftware.smackx.omemo; import static org.jivesoftware.smackx.omemo.util.OmemoConstants.OMEMO_NAMESPACE_V_AXOLOTL; import static org.jivesoftware.smackx.omemo.util.OmemoConstants.PEP_NODE_BUNDLE_FROM_DEVICE_ID; import static org.jivesoftware.smackx.omemo.util.OmemoConstants.PEP_NODE_DEVICE_LIST; -import static org.jivesoftware.smackx.omemo.util.OmemoConstants.PEP_NODE_DEVICE_LIST_NOTIFY; import java.io.UnsupportedEncodingException; import java.security.InvalidAlgorithmParameterException; @@ -55,7 +54,6 @@ import org.jivesoftware.smack.util.Async; import org.jivesoftware.smackx.carbons.CarbonCopyReceivedListener; import org.jivesoftware.smackx.carbons.CarbonManager; import org.jivesoftware.smackx.carbons.packet.CarbonExtension; -import org.jivesoftware.smackx.disco.ServiceDiscoveryManager; import org.jivesoftware.smackx.forward.packet.Forwarded; import org.jivesoftware.smackx.mam.MamManager; import org.jivesoftware.smackx.muc.MultiUserChat; @@ -79,7 +77,6 @@ import org.jivesoftware.smackx.omemo.internal.OmemoMessageInformation; import org.jivesoftware.smackx.omemo.internal.OmemoSession; import org.jivesoftware.smackx.omemo.util.OmemoConstants; import org.jivesoftware.smackx.omemo.util.OmemoMessageBuilder; -import org.jivesoftware.smackx.pep.PEPManager; import org.jivesoftware.smackx.pubsub.LeafNode; import org.jivesoftware.smackx.pubsub.PayloadItem; import org.jivesoftware.smackx.pubsub.PubSubException; @@ -228,7 +225,6 @@ public abstract class OmemoService Date: Fri, 29 Sep 2017 15:43:29 +0200 Subject: [PATCH 12/15] Use Async.go() in OMEMO PEPListener to avoid a deadlock, since the PEP listener is synchronous. --- .../smackx/omemo/OmemoManager.java | 25 ++++++++++++------- 1 file changed, 16 insertions(+), 9 deletions(-) diff --git a/smack-omemo/src/main/java/org/jivesoftware/smackx/omemo/OmemoManager.java b/smack-omemo/src/main/java/org/jivesoftware/smackx/omemo/OmemoManager.java index f635d2acb..dffd0c94b 100644 --- a/smack-omemo/src/main/java/org/jivesoftware/smackx/omemo/OmemoManager.java +++ b/smack-omemo/src/main/java/org/jivesoftware/smackx/omemo/OmemoManager.java @@ -814,16 +814,23 @@ public final class OmemoManager extends Manager { Set deviceListIds = omemoDeviceListElement.copyDeviceIds(); //enroll at the deviceList deviceListIds.add(ourDeviceId); - omemoDeviceListElement = new OmemoDeviceListVAxolotlElement(deviceListIds); + final OmemoDeviceListVAxolotlElement newOmemoDeviceListElement = new OmemoDeviceListVAxolotlElement(deviceListIds); - try { - OmemoService.publishDeviceIds(OmemoManager.this, omemoDeviceListElement); - } catch (SmackException | InterruptedException | XMPPException.XMPPErrorException e) { - //TODO: It might be dangerous NOT to retry publishing our deviceId - LOGGER.log(Level.SEVERE, - "Could not publish our device list after an update without our id was received: " - + e.getMessage()); - } + // PEPListener is a synchronous listener. Avoid any deadlocks by using an async task to update the device list. + Async.go(new Runnable() { + @Override + public void run() { + try { + OmemoService.publishDeviceIds(OmemoManager.this, newOmemoDeviceListElement); + } + catch (SmackException | InterruptedException | XMPPException.XMPPErrorException e) { + // TODO: It might be dangerous NOT to retry publishing our deviceId + LOGGER.log(Level.SEVERE, + "Could not publish our device list after an update without our id was received: " + + e.getMessage()); + } + } + }); } } } From 01aa6d9c18241d8705ec52cbefa7bd8c3b0d7171 Mon Sep 17 00:00:00 2001 From: Florian Schmaus Date: Sat, 14 Oct 2017 13:29:46 +0200 Subject: [PATCH 13/15] DNS: Correctly handle broken SRV records where the SRV RR points to a target DNS name with no associated A or AAAA RRs. Fixes SMACK-781. --- .../jivesoftware/smack/util/dns/DNSResolver.java | 14 ++++++++++++++ .../smack/util/dns/dnsjava/DNSJavaResolver.java | 9 ++++++++- .../smack/util/dns/javax/JavaxResolver.java | 8 +++++++- .../smack/util/dns/minidns/MiniDnsResolver.java | 9 ++++++++- 4 files changed, 37 insertions(+), 3 deletions(-) diff --git a/smack-core/src/main/java/org/jivesoftware/smack/util/dns/DNSResolver.java b/smack-core/src/main/java/org/jivesoftware/smack/util/dns/DNSResolver.java index f5e4518f9..c2823213c 100644 --- a/smack-core/src/main/java/org/jivesoftware/smack/util/dns/DNSResolver.java +++ b/smack-core/src/main/java/org/jivesoftware/smack/util/dns/DNSResolver.java @@ -59,6 +59,20 @@ public abstract class DNSResolver { return new HostAddress(name, port, inetAddresses); } + /** + * Lookup the IP addresses of a given host name. Returns null if there was an error, in which the error + * reason will be added in form of a HostAddress to failedAddresses. Returns a empty list + * in case the DNS name exists but has no associated A or AAAA resource records. Otherwise, if the resolution was + * successful and there is at least one A or AAAA resource record, then a non-empty list will be returned. + *

+ * Concrete DNS resolver implementations are free to overwrite this, but have to stick to the interface contract. + *

+ * + * @param name the DNS name to lookup + * @param failedAddresses a list with the failed addresses + * @param dnssecMode the selected DNSSEC mode + * @return A list, either empty or non-empty, or null + */ protected List lookupHostAddress0(String name, List failedAddresses, DnssecMode dnssecMode) { // Default implementation of a DNS name lookup for A/AAAA records. It is assumed that this method does never // support DNSSEC. Subclasses are free to override this method. diff --git a/smack-resolver-dnsjava/src/main/java/org/jivesoftware/smack/util/dns/dnsjava/DNSJavaResolver.java b/smack-resolver-dnsjava/src/main/java/org/jivesoftware/smack/util/dns/dnsjava/DNSJavaResolver.java index 0d29ed344..dd449fce3 100644 --- a/smack-resolver-dnsjava/src/main/java/org/jivesoftware/smack/util/dns/dnsjava/DNSJavaResolver.java +++ b/smack-resolver-dnsjava/src/main/java/org/jivesoftware/smack/util/dns/dnsjava/DNSJavaResolver.java @@ -19,6 +19,7 @@ package org.jivesoftware.smack.util.dns.dnsjava; import java.net.InetAddress; import java.util.ArrayList; import java.util.List; +import java.util.logging.Level; import org.jivesoftware.smack.ConnectionConfiguration.DnssecMode; import org.jivesoftware.smack.initializer.SmackInitializer; @@ -73,7 +74,13 @@ public class DNSJavaResolver extends DNSResolver implements SmackInitializer { int weight = srvRecord.getWeight(); List hostAddresses = lookupHostAddress0(host, failedAddresses, dnssecMode); - if (hostAddresses == null) { + if (hostAddresses == null || hostAddresses.isEmpty()) { + // If hostAddresses is not null but empty, then the DNS resolution was successful but the domain did not + // have any A or AAAA resource records. + if (hostAddresses.isEmpty()) { + LOGGER.log(Level.INFO, "The DNS name " + name + ", points to a hostname (" + host + + ") which has neither A or AAAA resource records. This is an indication of a broken DNS setup."); + } continue; } 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 9ebec30a2..1afce4198 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 @@ -111,7 +111,13 @@ public class JavaxResolver extends DNSResolver implements SmackInitializer { String host = srvRecordEntries[srvRecordEntries.length - 1]; List hostAddresses = lookupHostAddress0(host, failedAddresses, dnssecMode); - if (hostAddresses == null) { + if (hostAddresses == null || hostAddresses.isEmpty()) { + // If hostAddresses is not null but empty, then the DNS resolution was successful but the domain did not + // have any A or AAAA resource records. + if (hostAddresses.isEmpty()) { + LOGGER.log(Level.INFO, "The DNS name " + name + ", points to a hostname (" + host + + ") which has neither A or AAAA resource records. This is an indication of a broken DNS setup."); + } continue; } diff --git a/smack-resolver-minidns/src/main/java/org/jivesoftware/smack/util/dns/minidns/MiniDnsResolver.java b/smack-resolver-minidns/src/main/java/org/jivesoftware/smack/util/dns/minidns/MiniDnsResolver.java index ff5906ff7..24e4c985c 100644 --- a/smack-resolver-minidns/src/main/java/org/jivesoftware/smack/util/dns/minidns/MiniDnsResolver.java +++ b/smack-resolver-minidns/src/main/java/org/jivesoftware/smack/util/dns/minidns/MiniDnsResolver.java @@ -24,6 +24,7 @@ import java.util.Collections; import java.util.LinkedList; import java.util.List; import java.util.Set; +import java.util.logging.Level; import org.jivesoftware.smack.ConnectionConfiguration.DnssecMode; import org.jivesoftware.smack.initializer.SmackInitializer; @@ -90,7 +91,13 @@ public class MiniDnsResolver extends DNSResolver implements SmackInitializer { for (SRV srv : result.getAnswers()) { String hostname = srv.name.ace; List hostAddresses = lookupHostAddress0(hostname, failedAddresses, dnssecMode); - if (hostAddresses == null) { + if (hostAddresses == null || hostAddresses.isEmpty()) { + // If hostAddresses is not null but empty, then the DNS resolution was successful but the domain did not + // have any A or AAAA resource records. + if (hostAddresses.isEmpty()) { + LOGGER.log(Level.INFO, "The DNS name " + name + ", points to a hostname (" + hostname + + ") which has neither A or AAAA resource records. This is an indication of a broken DNS setup."); + } continue; } From e1e12031ac3d1f28683b90cc9a6e628f2693b57c Mon Sep 17 00:00:00 2001 From: Florian Schmaus Date: Sat, 14 Oct 2017 13:38:24 +0200 Subject: [PATCH 14/15] REPL: Add support to enable a JDWP debug link --- repl | 18 ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-) diff --git a/repl b/repl index 889022260..f11456edb 100755 --- a/repl +++ b/repl @@ -3,15 +3,29 @@ set -e set -u set -o pipefail +JDWP=false +JDWP_PORT=8000 -while getopts d OPTION "$@"; do +while getopts djp: OPTION "$@"; do case $OPTION in d) set -x ;; + j) + JDWP=true + ;; + p) + JDWP_PORT=$OPTARG + ;; esac done +EXTRA_JAVA_ARGS=() +if $JDWP; then + EXTRA_JAVA_ARGS+=("-Xdebug") + EXTRA_JAVA_ARGS+=("-Xrunjdwp:server=y,transport=dt_socket,address=${JDWP_PORT},suspend=n") +fi + PROJECT_ROOT=$(dirname "${BASH_SOURCE[0]}") cd "${PROJECT_ROOT}" @@ -27,7 +41,7 @@ GRADLE_CLASSPATH="$(gradle :smack-repl:printClasspath --quiet |\ tail -n1)" echo "Finished, starting REPL" -java \ +java "${EXTRA_JAVA_ARGS[@]}" \ -Dscala.usejavacp=true \ -classpath "${GRADLE_CLASSPATH}" \ ammonite.Main \ From 0729392ab830936094d5f6eb28959602d709559e Mon Sep 17 00:00:00 2001 From: Florian Schmaus Date: Sat, 14 Oct 2017 14:12:28 +0200 Subject: [PATCH 15/15] Fix isSupported discovery of "Push Notifications" Fixes SMACK-780. --- .../PushNotificationsManager.java | 19 ++++++++++ .../smackx/disco/ServiceDiscoveryManager.java | 36 +++++++++++++++++++ 2 files changed, 55 insertions(+) diff --git a/smack-experimental/src/main/java/org/jivesoftware/smackx/push_notifications/PushNotificationsManager.java b/smack-experimental/src/main/java/org/jivesoftware/smackx/push_notifications/PushNotificationsManager.java index 0a9e2303b..c3afb086e 100644 --- a/smack-experimental/src/main/java/org/jivesoftware/smackx/push_notifications/PushNotificationsManager.java +++ b/smack-experimental/src/main/java/org/jivesoftware/smackx/push_notifications/PushNotificationsManager.java @@ -87,13 +87,32 @@ public final class PushNotificationsManager extends Manager { * @throws XMPPErrorException * @throws NotConnectedException * @throws InterruptedException + * @deprecated Use {@link #isSupported()} instead. */ + @Deprecated + // TODO: Remove in Smack 4.3 public boolean isSupportedByServer() throws NoResponseException, XMPPErrorException, NotConnectedException, InterruptedException { return ServiceDiscoveryManager.getInstanceFor(connection()) .serverSupportsFeature(PushNotificationsElements.NAMESPACE); } + /** + * Returns true if Push Notifications are supported by this account. + * + * @return true if Push Notifications are supported by this account. + * @throws NoResponseException + * @throws XMPPErrorException + * @throws NotConnectedException + * @throws InterruptedException + * @since 4.2.2 + */ + public boolean isSupported() + throws NoResponseException, XMPPErrorException, NotConnectedException, InterruptedException { + return ServiceDiscoveryManager.getInstanceFor(connection()).accountSupportsFeatures( + PushNotificationsElements.NAMESPACE); + } + /** * Enable push notifications. * diff --git a/smack-extensions/src/main/java/org/jivesoftware/smackx/disco/ServiceDiscoveryManager.java b/smack-extensions/src/main/java/org/jivesoftware/smackx/disco/ServiceDiscoveryManager.java index f67f16a5c..2d3b7b83e 100644 --- a/smack-extensions/src/main/java/org/jivesoftware/smackx/disco/ServiceDiscoveryManager.java +++ b/smack-extensions/src/main/java/org/jivesoftware/smackx/disco/ServiceDiscoveryManager.java @@ -52,6 +52,7 @@ import org.jivesoftware.smackx.disco.packet.DiscoverItems; import org.jivesoftware.smackx.xdata.packet.DataForm; import org.jxmpp.jid.DomainBareJid; +import org.jxmpp.jid.EntityBareJid; import org.jxmpp.jid.Jid; import org.jxmpp.util.cache.Cache; import org.jxmpp.util.cache.ExpirationCache; @@ -680,6 +681,41 @@ public final class ServiceDiscoveryManager extends Manager { return supportsFeatures(connection().getXMPPServiceDomain(), features); } + /** + * Check if the given features are supported by the connection account. This means that the discovery information + * lookup will be performed on the bare JID of the connection managed by this ServiceDiscoveryManager. + * + * @param features the features to check + * @return true if all features are supported by the connection account, false otherwise + * @throws NoResponseException + * @throws XMPPErrorException + * @throws NotConnectedException + * @throws InterruptedException + * @since 4.2.2 + */ + public boolean accountSupportsFeatures(CharSequence... features) + throws NoResponseException, XMPPErrorException, NotConnectedException, InterruptedException { + return accountSupportsFeatures(Arrays.asList(features)); + } + + /** + * Check if the given collection of features are supported by the connection account. This means that the discovery + * information lookup will be performed on the bare JID of the connection managed by this ServiceDiscoveryManager. + * + * @param features a collection of features + * @return true if all features are supported by the connection account, false otherwise + * @throws NoResponseException + * @throws XMPPErrorException + * @throws NotConnectedException + * @throws InterruptedException + * @since 4.2.2 + */ + public boolean accountSupportsFeatures(Collection features) + throws NoResponseException, XMPPErrorException, NotConnectedException, InterruptedException { + EntityBareJid accountJid = connection().getUser().asEntityBareJid(); + return supportsFeatures(accountJid, features); + } + /** * Queries the remote entity for it's features and returns true if the given feature is found. *