From 49ebe8c58719ce2fb7a830595d6c17f0b4ac7f75 Mon Sep 17 00:00:00 2001 From: Florian Schmaus Date: Mon, 17 Aug 2020 21:56:26 +0200 Subject: [PATCH 1/6] [tcp] Drop Stream Management state on clean shutdown We previously only set the SM session ID to zero, but that is not enough. On a clean shutdown, i.e. where we send a close tag, we also have to nullify the unacknowledgedStanzas queue. --- .../smack/tcp/XMPPTCPConnection.java | 19 ++++++++++++++----- 1 file changed, 14 insertions(+), 5 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 4e5ff2974..2d283d802 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 @@ -540,13 +540,22 @@ public class XMPPTCPConnection extends AbstractXMPPConnection { // If we are able to resume the stream, then don't set // connected/authenticated/usingTLS to false since we like behave like we are still // connected (e.g. sendStanza should not throw a NotConnectedException). - if (isSmResumptionPossible() && instant) { - disconnectedButResumeable = true; + if (instant) { + disconnectedButResumeable = isSmResumptionPossible(); + if (!disconnectedButResumeable) { + // Reset the stream management session id to null, since the stream is no longer resumable. Note that we + // keep the unacknowledgedStanzas queue, because we want to resend them when we are reconnected. + smSessionId = null; + } } else { disconnectedButResumeable = false; - // 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; + + // Drop the stream management state if this is not an instant shutdown. We send + // a close tag and now the stream management state is no longer valid. + // This also prevents that we will potentially (re-)send any unavailable presence we + // may have send, because it got put into the unacknowledged queue and was not acknowledged before the + // connection terminated. + dropSmState(); // Note that we deliberately do not reset authenticatedConnectionInitiallyEstablishedTimestamp here, so that the // information is available in the connectionClosedOnError() listeners. } From cf4c9725b7eed3edc6a5c835504eb4093f2d2008 Mon Sep 17 00:00:00 2001 From: Florian Schmaus Date: Fri, 28 Aug 2020 09:37:29 +0200 Subject: [PATCH 2/6] [core] Add ProviderManager.getExtensionProvider(QName) --- .../org/jivesoftware/smack/provider/ProviderManager.java | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/smack-core/src/main/java/org/jivesoftware/smack/provider/ProviderManager.java b/smack-core/src/main/java/org/jivesoftware/smack/provider/ProviderManager.java index 7527b330f..f9e2bb560 100644 --- a/smack-core/src/main/java/org/jivesoftware/smack/provider/ProviderManager.java +++ b/smack-core/src/main/java/org/jivesoftware/smack/provider/ProviderManager.java @@ -243,7 +243,11 @@ public final class ProviderManager { */ public static ExtensionElementProvider getExtensionProvider(String elementName, String namespace) { QName key = getQName(elementName, namespace); - return extensionProviders.get(key); + return getExtensionProvider(key); + } + + public static ExtensionElementProvider getExtensionProvider(QName qname) { + return extensionProviders.get(qname); } /** From 1aab0b8aac197147f81b9c3731da847cbd3f152e Mon Sep 17 00:00:00 2001 From: Florian Schmaus Date: Fri, 28 Aug 2020 09:47:09 +0200 Subject: [PATCH 3/6] [core] Add cache to XmppElementUtil.getQNameFor(Class) --- .../smack/util/XmppElementUtil.java | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-) diff --git a/smack-core/src/main/java/org/jivesoftware/smack/util/XmppElementUtil.java b/smack-core/src/main/java/org/jivesoftware/smack/util/XmppElementUtil.java index b78f62ab9..83d09cf21 100644 --- a/smack-core/src/main/java/org/jivesoftware/smack/util/XmppElementUtil.java +++ b/smack-core/src/main/java/org/jivesoftware/smack/util/XmppElementUtil.java @@ -25,15 +25,26 @@ import javax.xml.namespace.QName; import org.jivesoftware.smack.packet.FullyQualifiedElement; +import org.jxmpp.util.cache.LruCache; + public class XmppElementUtil { + private static final LruCache, QName> CLASS_TO_QNAME_CACHE = new LruCache<>(512); + public static final Logger LOGGER = Logger.getLogger(XmppElementUtil.class.getName()); public static QName getQNameFor(Class fullyQualifiedElement) { + QName qname = CLASS_TO_QNAME_CACHE.get(fullyQualifiedElement); + if (qname != null) { + return qname; + } + try { Object qnameObject = fullyQualifiedElement.getField("QNAME").get(null); if (QName.class.isAssignableFrom(qnameObject.getClass())) { - return (QName) qnameObject; + qname = (QName) qnameObject; + CLASS_TO_QNAME_CACHE.put(fullyQualifiedElement, qname); + return qname; } LOGGER.warning("The QNAME field of " + fullyQualifiedElement + " is not of type QNAME."); } catch (NoSuchFieldException e) { @@ -52,7 +63,9 @@ public class XmppElementUtil { throw new IllegalArgumentException("The " + fullyQualifiedElement + " has no ELEMENT, NAMESPACE or QNAME member. Consider adding QNAME", e); } - return new QName(namespace, element); + qname = new QName(namespace, element); + CLASS_TO_QNAME_CACHE.put(fullyQualifiedElement, qname); + return qname; } public static List getElementsFrom( From 99297e5a765c23caaa69d1ab4a5c4b025b55f69a Mon Sep 17 00:00:00 2001 From: Florian Schmaus Date: Fri, 28 Aug 2020 09:47:54 +0200 Subject: [PATCH 4/6] [mam] Improve MamResultExtension: use MessageView in from() and add QNAME --- .../smackx/mam/element/MamElements.java | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/smack-experimental/src/main/java/org/jivesoftware/smackx/mam/element/MamElements.java b/smack-experimental/src/main/java/org/jivesoftware/smackx/mam/element/MamElements.java index 9fcff7b9c..2423b0c1e 100644 --- a/smack-experimental/src/main/java/org/jivesoftware/smackx/mam/element/MamElements.java +++ b/smack-experimental/src/main/java/org/jivesoftware/smackx/mam/element/MamElements.java @@ -18,9 +18,11 @@ package org.jivesoftware.smackx.mam.element; import java.util.List; +import javax.xml.namespace.QName; + import org.jivesoftware.smack.packet.Element; import org.jivesoftware.smack.packet.ExtensionElement; -import org.jivesoftware.smack.packet.Message; +import org.jivesoftware.smack.packet.MessageView; import org.jivesoftware.smack.util.StringUtils; import org.jivesoftware.smack.util.XmlStringBuilder; @@ -54,6 +56,11 @@ public class MamElements { */ public static final String ELEMENT = "result"; + /** + * The qualified name of the MAM result extension element. + */ + public static final QName QNAME = new QName(NAMESPACE, ELEMENT); + /** * id of the result. */ @@ -139,8 +146,8 @@ public class MamElements { return xml; } - public static MamResultExtension from(Message message) { - return (MamResultExtension) message.getExtensionElement(ELEMENT, NAMESPACE); + public static MamResultExtension from(MessageView message) { + return message.getExtension(MamResultExtension.class); } } From d06e9499e86f8f7273d144e18a0953f76dd90b1d Mon Sep 17 00:00:00 2001 From: Florian Schmaus Date: Fri, 28 Aug 2020 09:51:07 +0200 Subject: [PATCH 5/6] [core] Add XmppElementUtil.castOrThrow(ExtensionElement, Class) This method throws an IllegalStateException if the provided extension element is not of the expected type and hints users towards potential causes. --- .../smack/util/XmppElementUtil.java | 41 ++++++++++++++++--- 1 file changed, 35 insertions(+), 6 deletions(-) diff --git a/smack-core/src/main/java/org/jivesoftware/smack/util/XmppElementUtil.java b/smack-core/src/main/java/org/jivesoftware/smack/util/XmppElementUtil.java index 83d09cf21..1e4eabbc3 100644 --- a/smack-core/src/main/java/org/jivesoftware/smack/util/XmppElementUtil.java +++ b/smack-core/src/main/java/org/jivesoftware/smack/util/XmppElementUtil.java @@ -23,7 +23,10 @@ import java.util.logging.Logger; import javax.xml.namespace.QName; +import org.jivesoftware.smack.packet.ExtensionElement; import org.jivesoftware.smack.packet.FullyQualifiedElement; +import org.jivesoftware.smack.packet.StandardExtensionElement; +import org.jivesoftware.smack.provider.ProviderManager; import org.jxmpp.util.cache.LruCache; @@ -68,21 +71,47 @@ public class XmppElementUtil { return qname; } - public static List getElementsFrom( - MultiMap elementMap, Class extensionElementClass) { + public static List getElementsFrom( + MultiMap elementMap, Class extensionElementClass) { QName qname = XmppElementUtil.getQNameFor(extensionElementClass); - List extensionElements = elementMap.getAll(qname); + List extensionElements = elementMap.getAll(qname); if (extensionElements.isEmpty()) { return Collections.emptyList(); } - List res = new ArrayList<>(extensionElements.size()); - for (E extensionElement : extensionElements) { - R e = extensionElementClass.cast(extensionElement); + List res = new ArrayList<>(extensionElements.size()); + for (ExtensionElement extensionElement : extensionElements) { + E e = castOrThrow(extensionElement, extensionElementClass); res.add(e); } return res; } + + public static E castOrThrow(ExtensionElement extensionElement, Class extensionElementClass) { + if (!extensionElementClass.isInstance(extensionElement)) { + final QName qname = getQNameFor(extensionElementClass); + + final String detailMessage; + if (extensionElement instanceof StandardExtensionElement) { + detailMessage = "because there is no according extension element provider registered with ProviderManager for " + + qname + + ". WARNING: This indicates a serious problem with your Smack setup, probably causing Smack not being able to properly initialize itself."; + } else { + Object provider = ProviderManager.getExtensionProvider(qname); + detailMessage = "because there is an inconsistency with the provider registered with ProviderManager: the active provider for " + + qname + " '" + provider.getClass() + + "' does not return instances of type " + extensionElementClass + + ", but instead returns instances of type " + extensionElement.getClass() + "."; + } + + String message = "Extension element is not of expected class '" + extensionElementClass.getName() + "', " + + detailMessage; + throw new IllegalStateException(message); + } + + return extensionElementClass.cast(extensionElement); + } + } From b09cd0605322d27367371f066d15275f5c0b05d5 Mon Sep 17 00:00:00 2001 From: Florian Schmaus Date: Fri, 28 Aug 2020 09:52:28 +0200 Subject: [PATCH 6/6] [core] Use XmppElementUtil.castOrThrow() in StanzaView.getExtension(Class) This means that users get now exceptions with helpful error messages instead of the dreaded ClassCastException, like java.lang.ClassCastException: org.jivesoftware.smack.packet.StandardExtensionElement cannot be cast to org.jivesoftware.smackx.mam.element.MamElements$MamResultExtension at when StanzaView.getExtension(Class) is used to retrieve the extension. --- .../main/java/org/jivesoftware/smack/packet/StanzaView.java | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/smack-core/src/main/java/org/jivesoftware/smack/packet/StanzaView.java b/smack-core/src/main/java/org/jivesoftware/smack/packet/StanzaView.java index 7745ecc2a..003ddbeef 100644 --- a/smack-core/src/main/java/org/jivesoftware/smack/packet/StanzaView.java +++ b/smack-core/src/main/java/org/jivesoftware/smack/packet/StanzaView.java @@ -90,11 +90,12 @@ public interface StanzaView extends XmlLangElement { default E getExtension(Class extensionElementClass) { QName qname = XmppElementUtil.getQNameFor(extensionElementClass); ExtensionElement extensionElement = getExtension(qname); - if (!extensionElementClass.isInstance(extensionElement)) { + + if (extensionElement == null) { return null; } - return extensionElementClass.cast(extensionElement); + return XmppElementUtil.castOrThrow(extensionElement, extensionElementClass); } /**