Improve Socks5 Bytestreams

- determine all local IPv4 and IPv6 addresses
- prevent loopback addresses from appearing as streamhost

Some unit tests where changed because they assumed that a host only has
one local address. But nowadays hosts often have more, at least because
they are IPv4 and IPv6 multi-homed.

smack-extensions/src/main/java/org/jivesoftware/smackx/bytestreams/socks5/Socks5BytestreamManager.java

@@ -652,26 +652,36 @@ public final class Socks5BytestreamManager implements BytestreamManager {
         // get local proxy singleton
         Socks5Proxy socks5Server = Socks5Proxy.getSocks5Proxy();
 
-        if (socks5Server.isRunning()) {
+        if (!socks5Server.isRunning()) {
+            // server is not running
+            return null;
+        }
         List<String> addresses = socks5Server.getLocalAddresses();
-            int port = socks5Server.getPort();
+        if (addresses.isEmpty()) {
+            // local address could not be determined
+            return null;
+        }
+        final int port = socks5Server.getPort();
 
-            if (addresses.size() >= 1) {
         List<StreamHost> streamHosts = new ArrayList<StreamHost>();
-                for (String address : addresses) {
+        outerloop: for (String address : addresses) {
-                    StreamHost streamHost = new StreamHost(this.connection.getUser(), address);
+            // Prevent loopback addresses from appearing as streamhost
+            final String[] loopbackAddresses = { "127.0.0.1", "0:0:0:0:0:0:0:1" };
+            for (String loopbackAddress : loopbackAddresses) {
+                // Use 'startsWith' here since IPv6 addresses may have scope ID,
+                // ie. the part after the '%' sign.
+                if (address.startsWith(loopbackAddress)) {
+                    continue outerloop;
+                }
+            }
+            StreamHost streamHost = new StreamHost(this.connection.getUser(),
+                            address);
             streamHost.setPort(port);
             streamHosts.add(streamHost);
         }
         return streamHosts;
     }
 
-        }
-
-        // server is not running or local address could not be determined
-        return null;
-    }
-
     /**
      * Returns a SOCKS5 Bytestream initialization request packet with the given session ID
      * containing the given stream hosts for the given target JID.

smack-extensions/src/main/java/org/jivesoftware/smackx/bytestreams/socks5/Socks5Proxy.java

@@ -20,12 +20,14 @@ import java.io.DataInputStream;
 import java.io.DataOutputStream;
 import java.io.IOException;
 import java.net.InetAddress;
+import java.net.NetworkInterface;
 import java.net.ServerSocket;
 import java.net.Socket;
 import java.net.SocketException;
-import java.net.UnknownHostException;
+import java.util.Collection;
-import java.util.ArrayList;
 import java.util.Collections;
+import java.util.Enumeration;
+import java.util.HashSet;
 import java.util.LinkedHashSet;
 import java.util.LinkedList;
 import java.util.List;
@@ -47,7 +49,7 @@ import org.jivesoftware.smack.SmackException;
  * <p>
  * If your application is running on a machine with multiple network interfaces or if you want to
  * provide your public address in case you are behind a NAT router, invoke
- * {@link #addLocalAddress(String)} or {@link #replaceLocalAddresses(List)} to modify the list of
+ * {@link #addLocalAddress(String)} or {@link #replaceLocalAddresses(Collection)} to modify the list of
  * local network addresses used for outgoing SOCKS5 Bytestream requests.
  * <p>
  * The local SOCKS5 proxy server refuses all connections except the ones that are explicitly allowed
@@ -95,7 +97,7 @@ public class Socks5Proxy {
     /* list of digests connections should be stored */
     private final List<String> allowedConnections = Collections.synchronizedList(new LinkedList<String>());
 
-    private final Set<String> localAddresses = Collections.synchronizedSet(new LinkedHashSet<String>());
+    private final Set<String> localAddresses = new LinkedHashSet<String>(4);
 
     /**
      * Private constructor.
@@ -103,14 +105,27 @@ public class Socks5Proxy {
     private Socks5Proxy() {
         this.serverProcess = new Socks5ServerProcess();
 
-        // add default local address
+        Enumeration<NetworkInterface> networkInterfaces;
         try {
-            this.localAddresses.add(InetAddress.getLocalHost().getHostAddress());
+            networkInterfaces = NetworkInterface.getNetworkInterfaces();
+        } catch (SocketException e) {
+            throw new IllegalStateException(e);
         }
-        catch (UnknownHostException e) {
+        Set<String> localHostAddresses = new HashSet<String>();
-            // do nothing
+        for (NetworkInterface networkInterface : Collections.list(networkInterfaces)) {
+            // We can't use NetworkInterface.getInterfaceAddresses here, which
+            // would return a List instead the deprecated Enumeration, because
+            // it's Android API 9 and Smack currently uses 8. Change that when
+            // we raise Smack's minimum Android API.
+            Enumeration<InetAddress> inetAddresses = networkInterface.getInetAddresses();
+            for (InetAddress address : Collections.list(inetAddresses)) {
+                localHostAddresses.add(address.getHostAddress());
             }
-
+        }
+        if (localHostAddresses.isEmpty()) {
+            throw new IllegalStateException("Could not determine any local host address");
+        }
+        replaceLocalAddresses(localHostAddresses);
     }
 
    /**
@@ -243,35 +258,42 @@ public class Socks5Proxy {
      * <p>
      * Note that the list of addresses initially contains the address returned by
      * <code>InetAddress.getLocalHost().getHostAddress()</code>. You can replace the list of
-     * addresses by invoking {@link #replaceLocalAddresses(List)}.
+     * addresses by invoking {@link #replaceLocalAddresses(Collection)}.
      * 
      * @param address the local network address to add
      */
     public void addLocalAddress(String address) {
         if (address == null) {
-            throw new IllegalArgumentException("address may not be null");
+            return;
         }
+        synchronized (localAddresses) {
             this.localAddresses.add(address);
         }
+    }
 
     /**
      * Removes the given address from the list of local network addresses. This address will then no
      * longer be used of outgoing SOCKS5 Bytestream requests.
      * 
      * @param address the local network address to remove
+     * @return true if the address was removed.
      */
-    public void removeLocalAddress(String address) {
+    public boolean removeLocalAddress(String address) {
-        this.localAddresses.remove(address);
+        synchronized(localAddresses) {
+            return localAddresses.remove(address);
+        }
     }
 
     /**
-     * Returns an unmodifiable list of the local network addresses that will be used for streamhost
+     * Returns an set of the local network addresses that will be used for streamhost
      * candidates of outgoing SOCKS5 Bytestream requests.
      * 
-     * @return unmodifiable list of the local network addresses
+     * @return set of the local network addresses
      */
     public List<String> getLocalAddresses() {
-        return Collections.unmodifiableList(new ArrayList<String>(this.localAddresses));
+        synchronized (localAddresses) {
+            return new LinkedList<String>(localAddresses);
+        }
     }
 
     /**
@@ -284,13 +306,14 @@ public class Socks5Proxy {
      * 
      * @param addresses the new list of local network addresses
      */
-    public void replaceLocalAddresses(List<String> addresses) {
+    public void replaceLocalAddresses(Collection<String> addresses) {
         if (addresses == null) {
             throw new IllegalArgumentException("list must not be null");
         }
-        this.localAddresses.clear();
+        synchronized(localAddresses) {
-        this.localAddresses.addAll(addresses);
+            localAddresses.clear();
-
+            localAddresses.addAll(addresses);
+        }
     }
 
     /**

smack-extensions/src/main/java/org/jivesoftware/smackx/bytestreams/socks5/packet/Bytestream.java

@@ -17,7 +17,6 @@
 package org.jivesoftware.smackx.bytestreams.socks5.packet;
 
 import java.util.ArrayList;
-import java.util.Collection;
 import java.util.Collections;
 import java.util.List;
 
@@ -145,8 +144,8 @@ public class Bytestream extends IQ {
      * 
      * @return Returns the list of stream hosts contained in the packet.
      */
-    public Collection<StreamHost> getStreamHosts() {
+    public List<StreamHost> getStreamHosts() {
-        return Collections.unmodifiableCollection(streamHosts);
+        return Collections.unmodifiableList(streamHosts);
     }
 
     /**

smack-extensions/src/test/java/org/jivesoftware/smackx/bytestreams/socks5/Socks5ByteStreamManagerTest.java

@@ -799,10 +799,9 @@ public class Socks5ByteStreamManagerTest {
 
             public void verify(Bytestream request, Bytestream response) {
                 assertEquals(response.getSessionID(), request.getSessionID());
-                assertEquals(2, request.getStreamHosts().size());
+                StreamHost streamHost1 = request.getStreamHosts().get(0);
-                StreamHost streamHost1 = (StreamHost) request.getStreamHosts().toArray()[0];
                 assertEquals(response.getUsedHost().getJID(), streamHost1.getJID());
-                StreamHost streamHost2 = (StreamHost) request.getStreamHosts().toArray()[1];
+                StreamHost streamHost2 = request.getStreamHosts().get(request.getStreamHosts().size() - 1);
                 assertEquals(response.getUsedHost().getJID(), streamHost2.getJID());
                 assertEquals("localAddress", streamHost2.getAddress());
             }

smack-extensions/src/test/java/org/jivesoftware/smackx/bytestreams/socks5/Socks5ProxyTest.java

@@ -30,6 +30,7 @@ import java.net.Socket;
 import java.net.SocketException;
 import java.net.UnknownHostException;
 import java.util.ArrayList;
+import java.util.Arrays;
 import java.util.List;
 
 import org.junit.After;
@@ -144,7 +145,13 @@ public class Socks5ProxyTest {
         proxy.addLocalAddress("same");
         proxy.addLocalAddress("same");
 
-        assertEquals(2, proxy.getLocalAddresses().size());
+        int sameCount = 0;
+        for(String localAddress : proxy.getLocalAddresses()) {
+            if ("same".equals(localAddress)) {
+                sameCount++;
+            }
+        }
+        assertEquals(1, sameCount);
     }
 
     /**
@@ -297,7 +304,6 @@ public class Socks5ProxyTest {
         proxy.start();
 
         assertTrue(proxy.isRunning());
-
         String digest = new String(new byte[] { (byte) 0xAA });
 
         // add digest to allow connection