From 48e31271720d65824e0ffff3f39abe9a2ef1980f Mon Sep 17 00:00:00 2001 From: Florian Schmaus Date: Thu, 20 Dec 2018 16:52:44 +0100 Subject: [PATCH 1/4] Always wait for stream features after authentication The purpose of the "maybeCompressFeaturesReceived" synchronization point is to wait for the stream features after authentication. This is not really reflected by its name and furthermore its checkIfSuccessOrWait() method was only invoked if compression was enabled, but we always want to wait for the stream features after authentication. Hence move the call before the isCompressionEnabled() check and one layer up in the call stack. Fixes SMACK-846. --- .../java/org/jivesoftware/smack/tcp/XMPPTCPConnection.java | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) 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 6ba5e64fd..c5183fa70 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 @@ -387,6 +387,11 @@ public class XMPPTCPConnection extends AbstractXMPPConnection { SSLSession sslSession = secureSocket != null ? secureSocket.getSession() : null; saslAuthentication.authenticate(username, password, config.getAuthzid(), sslSession); + // Wait for stream features after the authentication. + // TODO: The name of this synchronization point "maybeCompressFeaturesReceived" is not perfect. It should be + // renamed to "streamFeaturesAfterAuthenticationReceived". + maybeCompressFeaturesReceived.checkIfSuccessOrWait(); + // If compression is enabled then request the server to use stream compression. XEP-170 // recommends to perform stream compression before resource binding. maybeEnableCompression(); @@ -859,7 +864,7 @@ public class XMPPTCPConnection extends AbstractXMPPConnection { if (!config.isCompressionEnabled()) { return; } - maybeCompressFeaturesReceived.checkIfSuccessOrWait(); + Compress.Feature compression = getFeature(Compress.Feature.ELEMENT, Compress.NAMESPACE); if (compression == null) { // Server does not support compression From d1c73eba8d4cdac363204a95da2d93905c0da6d0 Mon Sep 17 00:00:00 2001 From: Florian Schmaus Date: Thu, 20 Dec 2018 17:00:00 +0100 Subject: [PATCH 2/4] Fix MamManager javadoc of methods which have been renamed. --- .../src/main/java/org/jivesoftware/smackx/mam/MamManager.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/smack-experimental/src/main/java/org/jivesoftware/smackx/mam/MamManager.java b/smack-experimental/src/main/java/org/jivesoftware/smackx/mam/MamManager.java index 61fd861df..ba3af51aa 100644 --- a/smack-experimental/src/main/java/org/jivesoftware/smackx/mam/MamManager.java +++ b/smack-experimental/src/main/java/org/jivesoftware/smackx/mam/MamManager.java @@ -114,8 +114,8 @@ import org.jxmpp.jid.Jid; * 
  * {@code
  * MamQueryArgs mamQueryArgs = MamQueryArgs.builder()
- *                                 .withJid(jid)
- *                                 .setResultPageSize(10)
+ *                                 .limitResultsToJid(jid)
+ *                                 .setResultPageSizeTo(10)
  *                                 .queryLastPage()
  *                                 .build();
  * MamQuery mamQuery = mamManager.queryArchive(mamQueryArgs);

From a2743549b8a4126e17f61adcb369f72d86ef58e1 Mon Sep 17 00:00:00 2001
From: Florian Schmaus 
Date: Mon, 17 Dec 2018 21:16:03 +0100
Subject: [PATCH 3/4] Make TCP socket connection attempt interruptable

by introducing SmackFuture.SocketFuture.

Fixes SMACK-847.
---
 .../org/jivesoftware/smack/SmackFuture.java   | 82 ++++++++++++++++++-
 .../smack/tcp/XMPPTCPConnection.java          | 12 ++-
 2 files changed, 87 insertions(+), 7 deletions(-)

diff --git a/smack-core/src/main/java/org/jivesoftware/smack/SmackFuture.java b/smack-core/src/main/java/org/jivesoftware/smack/SmackFuture.java
index 88a951677..9a6511d5f 100644
--- a/smack-core/src/main/java/org/jivesoftware/smack/SmackFuture.java
+++ b/smack-core/src/main/java/org/jivesoftware/smack/SmackFuture.java
@@ -16,11 +16,18 @@
  */
 package org.jivesoftware.smack;
 
+import java.io.IOException;
+import java.net.Socket;
+import java.net.SocketAddress;
 import java.util.concurrent.CancellationException;
 import java.util.concurrent.ExecutionException;
 import java.util.concurrent.Future;
 import java.util.concurrent.TimeUnit;
 import java.util.concurrent.TimeoutException;
+import java.util.logging.Level;
+import java.util.logging.Logger;
+
+import javax.net.SocketFactory;
 
 import org.jivesoftware.smack.packet.Stanza;
 import org.jivesoftware.smack.util.CallbackRecipient;
@@ -29,6 +36,8 @@ import org.jivesoftware.smack.util.SuccessCallback;
 
 public abstract class SmackFuture implements Future, CallbackRecipient {
 
+    private static final Logger LOGGER = Logger.getLogger(SmackFuture.class.getName());
+
     private boolean cancelled;
 
     protected V result;
@@ -94,7 +103,7 @@ public abstract class SmackFuture implements Future,
     @Override
     public final synchronized V get() throws InterruptedException, ExecutionException {
         while (result == null && exception == null && !cancelled) {
-            wait();
+            futureWait();
         }
 
         return getOrThrowExecutionException();
@@ -102,7 +111,7 @@ public abstract class SmackFuture implements Future,
 
     public final synchronized V getOrThrow() throws E, InterruptedException {
         while (result == null && exception == null && !cancelled) {
-            wait();
+            futureWait();
         }
 
         if (exception != null) {
@@ -124,7 +133,7 @@ public abstract class SmackFuture implements Future,
         while (result != null && exception != null) {
             final long waitTimeRemaining = deadline - System.currentTimeMillis();
             if (waitTimeRemaining > 0) {
-                wait(waitTimeRemaining);
+                futureWait(waitTimeRemaining);
             }
         }
 
@@ -162,6 +171,15 @@ public abstract class SmackFuture implements Future,
         }
     }
 
+    protected final void futureWait() throws InterruptedException {
+        futureWait(0);
+    }
+
+    @SuppressWarnings("WaitNotInLoop")
+    protected void futureWait(long timeout) throws InterruptedException {
+        wait(timeout);
+    }
+
     public static class InternalSmackFuture extends SmackFuture {
         public final synchronized void setResult(V result) {
             this.result = result;
@@ -178,6 +196,64 @@ public abstract class SmackFuture implements Future,
         }
     }
 
+    public static class SocketFuture extends InternalSmackFuture {
+        private final Socket socket;
+
+        private final Object wasInterruptedLock = new Object();
+
+        private boolean wasInterrupted;
+
+        public SocketFuture(SocketFactory socketFactory) throws IOException {
+            socket = socketFactory.createSocket();
+        }
+
+        @Override
+        protected void futureWait(long timeout) throws InterruptedException {
+            try {
+                super.futureWait(timeout);
+            } catch (InterruptedException interruptedException) {
+                synchronized (wasInterruptedLock) {
+                    wasInterrupted = true;
+                    if (!socket.isClosed()) {
+                        closeSocket();
+                    }
+                }
+                throw interruptedException;
+            }
+        }
+
+        public void connectAsync(final SocketAddress socketAddress, final int timeout) {
+            AbstractXMPPConnection.asyncGo(new Runnable() {
+                @Override
+                public void run() {
+                    try {
+                        socket.connect(socketAddress, timeout);
+                    }
+                    catch (IOException e) {
+                        setException(e);
+                        return;
+                    }
+                    synchronized (wasInterruptedLock) {
+                        if (wasInterrupted) {
+                            closeSocket();
+                            return;
+                        }
+                    }
+                    setResult(socket);
+                }
+            });
+        }
+
+        private void closeSocket() {
+            try {
+                socket.close();
+            }
+            catch (IOException ioException) {
+                LOGGER.log(Level.WARNING, "Could not close socket", ioException);
+            }
+        }
+    }
+
     public abstract static class InternalProcessStanzaSmackFuture extends InternalSmackFuture
                     implements StanzaListener, ExceptionCallback {
 
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 c5183fa70..5d940a679 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
@@ -83,6 +83,7 @@ import org.jivesoftware.smack.SmackException.NoResponseException;
 import org.jivesoftware.smack.SmackException.NotConnectedException;
 import org.jivesoftware.smack.SmackException.NotLoggedInException;
 import org.jivesoftware.smack.SmackException.SecurityRequiredByServerException;
+import org.jivesoftware.smack.SmackFuture;
 import org.jivesoftware.smack.StanzaListener;
 import org.jivesoftware.smack.SynchronizationPoint;
 import org.jivesoftware.smack.XMPPConnection;
@@ -560,7 +561,7 @@ public class XMPPTCPConnection extends AbstractXMPPConnection {
         }
     }
 
-    private void connectUsingConfiguration() throws ConnectionException, IOException {
+    private void connectUsingConfiguration() throws ConnectionException, IOException, InterruptedException {
         List failedAddresses = populateHostAddresses();
         SocketFactory socketFactory = config.getSocketFactory();
         ProxyInfo proxyInfo = config.getProxyInfo();
@@ -579,14 +580,17 @@ public class XMPPTCPConnection extends AbstractXMPPConnection {
                 innerloop: while (inetAddresses.hasNext()) {
                     // Create a *new* Socket before every connection attempt, i.e. connect() call, since Sockets are not
                     // re-usable after a failed connection attempt. See also SMACK-724.
-                    socket = socketFactory.createSocket();
+                    SmackFuture.SocketFuture socketFuture = new SmackFuture.SocketFuture(socketFactory);
 
                     final InetAddress inetAddress = inetAddresses.next();
                     final String inetAddressAndPort = inetAddress + " at port " + port;
+                    final InetSocketAddress inetSocketAddress = new InetSocketAddress(inetAddress, port);
                     LOGGER.finer("Trying to establish TCP connection to " + inetAddressAndPort);
+                    socketFuture.connectAsync(inetSocketAddress, timeout);
+
                     try {
-                        socket.connect(new InetSocketAddress(inetAddress, port), timeout);
-                    } catch (Exception e) {
+                        socket = socketFuture.getOrThrow();
+                    } catch (IOException e) {
                         hostAddress.setException(inetAddress, e);
                         if (inetAddresses.hasNext()) {
                             continue innerloop;

From b9c12d44c3d8e5697d892c2938fe4e031c90a137 Mon Sep 17 00:00:00 2001
From: Florian Schmaus 
Date: Fri, 21 Dec 2018 12:05:14 +0100
Subject: [PATCH 4/4] Use InetSocketAddress in log message in XMPPTCPConnection

The inetAddressAndPort String is redundant since
a2743549b8a4126e17f61adcb369f72d86ef58e1, because we now construct the
InetSocketAddress earlier and can hence use it in the log statement.
---
 .../java/org/jivesoftware/smack/tcp/XMPPTCPConnection.java   | 5 ++---
 1 file changed, 2 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 5d940a679..24fcf37bc 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
@@ -583,9 +583,8 @@ public class XMPPTCPConnection extends AbstractXMPPConnection {
                     SmackFuture.SocketFuture socketFuture = new SmackFuture.SocketFuture(socketFactory);
 
                     final InetAddress inetAddress = inetAddresses.next();
-                    final String inetAddressAndPort = inetAddress + " at port " + port;
                     final InetSocketAddress inetSocketAddress = new InetSocketAddress(inetAddress, port);
-                    LOGGER.finer("Trying to establish TCP connection to " + inetAddressAndPort);
+                    LOGGER.finer("Trying to establish TCP connection to " + inetSocketAddress);
                     socketFuture.connectAsync(inetSocketAddress, timeout);
 
                     try {
@@ -598,7 +597,7 @@ public class XMPPTCPConnection extends AbstractXMPPConnection {
                             break innerloop;
                         }
                     }
-                    LOGGER.finer("Established TCP connection to " + inetAddressAndPort);
+                    LOGGER.finer("Established TCP connection to " + inetSocketAddress);
                     // We found a host to connect to, return here
                     this.host = host;
                     this.port = port;