From 1a2a613112712580bedbf4374fba84dfe0f44517 Mon Sep 17 00:00:00 2001 From: Florian Schmaus Date: Thu, 6 Aug 2020 10:28:07 +0200 Subject: [PATCH] Set 'connected' to 'true' as early as possible MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit We previously only set 'connected' after connectInternal() returned. This could lead to notifyConnectionError() ignoring stream error exceptions, e.g. when establishing TLS which happens also in connectInternal(), because 'connected' was still 'false'. 2020-08-06 13:08:06.265 19830-20423/org.atalk.android D/SMACK: SENT (0): 2020-08-06 13:08:06.333 19830-20424/org.atalk.android D/SMACK: RECV (0): ?xml version='1.0'?> Too many (20) failed authentications from this IP address (::ffff:42.60.7.13). The address will be unblocked at 05:15:34 06.08.2020 UTC 2020-08-06 13:08:06.346 19830-20424/org.atalk.android I/aTalk: [241896] org.jivesoftware.smack.AbstractXMPPConnection.notifyConnectionError() Connection was already disconnected when attempting to handle org.jivesoftware.smack.XMPPException$StreamErrorException: policy-violation You can read more about the meaning of this stream error at http://xmpp.org/rfcs/rfc6120.html#streams-error-conditions Too many (20) failed authentications from this IP address (::ffff:42.60.7.13). The address will be unblocked at 05:15:34 06.08.2020 UTC org.jivesoftware.smack.XMPPException$StreamErrorException: policy-violation You can read more about the meaning of this stream error at http://xmpp.org/rfcs/rfc6120.html#streams-error-conditions Too many (20) failed authentications from this IP address (::ffff:42.60.7.13). The address will be unblocked at 05:15:34 06.08.2020 UTC at org.jivesoftware.smack.tcp.XMPPTCPConnection$PacketReader.parsePackets(XMPPTCPConnection.java:966) at org.jivesoftware.smack.tcp.XMPPTCPConnection$PacketReader.access$700(XMPPTCPConnection.java:898) at org.jivesoftware.smack.tcp.XMPPTCPConnection$PacketReader$1.run(XMPPTCPConnection.java:921) at java.lang.Thread.run(Thread.java:919) Which eventually leads to a NoResponseException org.jivesoftware.smack.SmackException$NoResponseException: No response received within reply timeout. Timeout was 30000ms (~30s). While waiting for establishing TLS [XMPPTCPConnection[not-authenticated] (4)] We now set 'connected' to 'true' as soon as the transport (e.g. TCP, BOSH, …) is connected. While this is in other ways also sensible, it also allows notifyConnectionError() to handle exceptions in the early connection stage. Thanks to Eng Chong Meng for reporting this. --- .../org/jivesoftware/smack/AbstractXMPPConnection.java | 8 ++++++-- .../smack/c2s/ModularXmppClientToServerConnection.java | 1 + .../ModularXmppClientToServerConnectionInternal.java | 6 ++++++ .../org/jivesoftware/smack/tcp/XMPPTCPConnection.java | 2 ++ 4 files changed, 15 insertions(+), 2 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 bd3d47554..530bf7b11 100644 --- a/smack-core/src/main/java/org/jivesoftware/smack/AbstractXMPPConnection.java +++ b/smack-core/src/main/java/org/jivesoftware/smack/AbstractXMPPConnection.java @@ -522,6 +522,9 @@ public abstract class AbstractXMPPConnection implements XMPPConnection { closingStreamReceived = false; streamId = null; + // The connection should not be connected nor marked as such prior calling connectInternal(). + assert !connected; + try { // Perform the actual connection to the XMPP service connectInternal(); @@ -537,8 +540,9 @@ public abstract class AbstractXMPPConnection implements XMPPConnection { throw e; } - // Make note of the fact that we're now connected. - connected = true; + // If connectInternal() did not throw, then this connection must now be marked as connected. + assert connected; + callConnectionConnectedListener(); return this; diff --git a/smack-core/src/main/java/org/jivesoftware/smack/c2s/ModularXmppClientToServerConnection.java b/smack-core/src/main/java/org/jivesoftware/smack/c2s/ModularXmppClientToServerConnection.java index f62caf8ee..f12180ce7 100644 --- a/smack-core/src/main/java/org/jivesoftware/smack/c2s/ModularXmppClientToServerConnection.java +++ b/smack-core/src/main/java/org/jivesoftware/smack/c2s/ModularXmppClientToServerConnection.java @@ -217,6 +217,7 @@ public final class ModularXmppClientToServerConnection extends AbstractXMPPConne @Override public void setTransport(XmppClientToServerTransport xmppTransport) { ModularXmppClientToServerConnection.this.activeTransport = xmppTransport; + ModularXmppClientToServerConnection.this.connected = true; } }; diff --git a/smack-core/src/main/java/org/jivesoftware/smack/c2s/internal/ModularXmppClientToServerConnectionInternal.java b/smack-core/src/main/java/org/jivesoftware/smack/c2s/internal/ModularXmppClientToServerConnectionInternal.java index 81a485771..db41feafb 100644 --- a/smack-core/src/main/java/org/jivesoftware/smack/c2s/internal/ModularXmppClientToServerConnectionInternal.java +++ b/smack-core/src/main/java/org/jivesoftware/smack/c2s/internal/ModularXmppClientToServerConnectionInternal.java @@ -116,5 +116,11 @@ public abstract class ModularXmppClientToServerConnectionInternal { public abstract void setCompressionEnabled(boolean compressionEnabled); + /** + * Set the active transport (TCP, BOSH, WebSocket, …) to be used for the XMPP connection. Also marks the connection + * as connected. + * + * @param xmppTransport the active transport. + */ public abstract void setTransport(XmppClientToServerTransport xmppTransport); } 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 2dbb44a04..4e5ff2974 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 @@ -836,6 +836,8 @@ public class XMPPTCPConnection extends AbstractXMPPConnection { // there is an error establishing the connection connectUsingConfiguration(); + connected = true; + // We connected successfully to the servers TCP port initConnection();