Set 'connected' to 'true' as early as possible

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):
    <stream:stream xmlns='jabber:client' to='atalk.sytes.net' xmlns:stream='http://etherx.jabber.org/streams' version='1.0' xml:lang='en'>
2020-08-06 13:08:06.333 19830-20424/org.atalk.android D/SMACK: RECV (0): ?xml version='1.0'?>
    <stream:stream id='16420577292739412012' version='1.0' xml:lang='en' xmlns:stream='http://etherx.jabber.org/streams' from='atalk.sytes.net' xmlns='jabber:client'>
    <stream:error>
      <policy-violation xmlns='urn:ietf:params:xml:ns:xmpp-streams'/>
      <text xml:lang='en' xmlns='urn:ietf:params:xml:ns:xmpp-streams'>
        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
      </text>
    </stream:error>
    </stream:stream>
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
    <stream:error><policy-violation xmlns='urn:ietf:params:xml:ns:xmpp-streams'/><text xml:lang='en'>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</text></stream:error>
    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
    <stream:error><policy-violation xmlns='urn:ietf:params:xml:ns:xmpp-streams'/><text xml:lang='en'>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</text></stream:error>
        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.
This commit is contained in:
Florian Schmaus 2020-08-06 10:28:07 +02:00
parent a356e91108
commit 1a2a613112
4 changed files with 15 additions and 2 deletions

View File

@ -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;

View File

@ -217,6 +217,7 @@ public final class ModularXmppClientToServerConnection extends AbstractXMPPConne
@Override
public void setTransport(XmppClientToServerTransport xmppTransport) {
ModularXmppClientToServerConnection.this.activeTransport = xmppTransport;
ModularXmppClientToServerConnection.this.connected = true;
}
};

View File

@ -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);
}

View File

@ -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();