Fix net.i2p.util.Addresses::getIP empty string

The standard library behavior is to return the localhost when null or an empty string is passed.
getIP seeked to override that behavior, but didn't treat the empty string case.

core/java/src/net/i2p/util/Addresses.java

@@ -40,7 +40,7 @@ import net.i2p.data.DataHelper;
  * @author zzz
  */
 public abstract class Addresses {
-    
+
     private static final File IF_INET6_FILE = new File("/proc/net/if_inet6");
     private static final long INET6_CACHE_EXPIRE = 10*60*1000;
     private static final boolean INET6_CACHE_ENABLED = !SystemVersion.isMac() && !SystemVersion.isWindows() &&
@@ -122,10 +122,9 @@ public abstract class Addresses {
      *
      *  Warning, very slow on Windows, appx. 200ms + 50ms/interface
      *
-     *  @return a sorted set of all addresses including wildcard
      *  @param includeLocal whether to include local
      *  @param includeIPv6 whether to include IPV6
-     *  @return a Set of all addresses
+     *  @return a sorted set of all addresses including wildcard
      *  @since 0.8.3
      */
     public static SortedSet<String> getAddresses(boolean includeLocal, boolean includeIPv6) {
@@ -141,11 +140,10 @@ public abstract class Addresses {
      *
      *  Warning, very slow on Windows, appx. 200ms + 50ms/interface
      *
-     *  @return a sorted set of all addresses
      *  @param includeSiteLocal whether to include private like 192.168.x.x
      *  @param includeLoopbackAndWildcard whether to include 127.x.x.x and 0.0.0.0
      *  @param includeIPv6 whether to include IPV6
-     *  @return a Set of all addresses
+     *  @return a sorted set of all addresses
      *  @since 0.9.4
      */
     public static SortedSet<String> getAddresses(boolean includeSiteLocal,
@@ -163,12 +161,11 @@ public abstract class Addresses {
      *
      *  Warning, very slow on Windows, appx. 200ms + 50ms/interface
      *
-     *  @return a sorted set of all addresses
      *  @param includeSiteLocal whether to include private like 192.168.x.x
      *  @param includeLoopbackAndWildcard whether to include 127.x.x.x and 0.0.0.0
      *  @param includeIPv6 whether to include IPV6
      *  @param includeIPv6Temporary whether to include IPV6 temporary addresses
-     *  @return a Set of all addresses
+     *  @return a sorted set of all addresses
      *  @since 0.9.46
      */
     public static SortedSet<String> getAddresses(boolean includeSiteLocal,
@@ -343,7 +340,7 @@ public abstract class Addresses {
             return "(bad IP length " + addr.length + "):" + port;
         }
     }
-    
+
     /**
      *  Convenience method to convert and validate a port String
      *  without throwing an exception.
@@ -402,7 +399,7 @@ public abstract class Addresses {
      *  @since 0.9.3
      */
     public static byte[] getIP(String host) {
-        if (host == null)
+        if (host == null || host.isEmpty())
             return null;
         byte[] rv;
         synchronized (_IPAddress) {

core/java/test/junit/net/i2p/socks/SOCKS4ClientTest.java

@@ -1,195 +0,0 @@
-package net.i2p.socks;
-
-import org.junit.After;
-import org.junit.Before;
-import org.junit.Test;
-import org.mockito.Mockito;
-import sun.net.util.IPAddressUtil;
-
-import java.io.ByteArrayInputStream;
-import java.io.ByteArrayOutputStream;
-import java.io.DataOutputStream;
-import java.io.IOException;
-import java.net.Socket;
-import java.nio.charset.StandardCharsets;
-
-import static net.i2p.socks.SOCKS4Constants.SOCKS_VERSION_4;
-import static org.junit.Assert.assertArrayEquals;
-import static org.junit.Assert.assertThrows;
-
-/**
- * @since 0.9.49
- */
-public class SOCKS4ClientTest {
-
-    ByteArrayInputStream inputStream;
-    ByteArrayOutputStream outputStream;
-
-    @Before
-    public void openStreams(){
-        outputStream = new ByteArrayOutputStream();
-    }
-
-    @After
-    public void closeStreams() throws IOException {
-        if (inputStream != null) {
-            inputStream.close();
-        }
-        if (outputStream != null) {
-            outputStream.close();
-        }
-    }
-
-    /**
-     * A successful connection to an IPv4 host
-     */
-    @Test
-    public void connect() throws IOException {
-        _testConnect(false);
-    }
-
-    /**
-     * A successful connection to an IPv4 host using a socket
-     */
-    @Test
-    public void connect__withSocket() throws IOException {
-        _testConnect(true);
-    }
-
-    private void _testConnect(boolean useSocket) throws IOException {
-        String hostIPv4 = "11.22.33.44";
-        int connectionPort = 8080;
-        byte[] hostIPv4Bin = IPAddressUtil.textToNumericFormatV4(hostIPv4);
-
-        // Build sequence of bytes to be expected
-        ByteArrayOutputStream expectedByteStream = new ByteArrayOutputStream();
-        DataOutputStream writerStream = new DataOutputStream(expectedByteStream);
-        writerStream.writeByte(SOCKS_VERSION_4);
-        writerStream.writeByte(SOCKS4Constants.Command.CONNECT);
-        writerStream.writeShort(connectionPort);
-        writerStream.write(hostIPv4Bin);
-        writerStream.write((byte) 0);
-
-        inputStream = new ByteArrayInputStream(new byte[]{
-            0, // dummy
-            SOCKS4Constants.Reply.SUCCEEDED, // Connection succeeded
-            0, 0, 0, 0, 0, 0 // filler
-        });
-        outputStream = new ByteArrayOutputStream();
-
-        // Test overloaded function
-        try {
-            if (useSocket) {
-                Socket socket = Mockito.mock(Socket.class);
-                Mockito.when(socket.getInputStream()).thenReturn(inputStream);
-                Mockito.when(socket.getOutputStream()).thenReturn(outputStream);
-
-                SOCKS4Client.connect(socket, hostIPv4, connectionPort);
-            } else {
-                SOCKS4Client.connect(inputStream, outputStream, hostIPv4, connectionPort);
-            }
-        } finally {
-            writerStream.close();
-        }
-        assertArrayEquals(expectedByteStream.toByteArray(), outputStream.toByteArray());
-    }
-
-    /**
-     * Connect proxy with a domain name
-     */
-    @Test
-    public void connect__host() throws IOException {
-        String host = "stats.i2p";
-        int connectionPort = 80;
-
-        // Build sequence of bytes to be expected
-        ByteArrayOutputStream expectedByteStream = new ByteArrayOutputStream();
-        DataOutputStream writerStream = new DataOutputStream(expectedByteStream);
-        writerStream.writeByte(SOCKS_VERSION_4);
-        writerStream.writeByte(SOCKS4Constants.Command.CONNECT);
-        writerStream.writeShort(connectionPort);
-        writerStream.write(new byte[]{0,0,0,1});  // 0.0.0.1
-        writerStream.write((byte) 0); // empty userID
-        writerStream.write(host.getBytes(StandardCharsets.ISO_8859_1));
-        writerStream.write((byte) 0);
-
-        inputStream = new ByteArrayInputStream(new byte[]{
-            0, // dummy
-            SOCKS4Constants.Reply.SUCCEEDED, // Connection succeeded
-            0, 0, 0, 0, 0, 0 // filler
-        });
-        try {
-            SOCKS4Client.connect(SOCKS4ClientTest.this.inputStream, outputStream, host, connectionPort);
-        } finally {
-            expectedByteStream.close();
-        }
-        assertArrayEquals(expectedByteStream.toByteArray(), outputStream.toByteArray());
-    }
-
-    /**
-     * Run into IOException while trying to connect due to no input/response
-     */
-    @Test
-    public void connect__ioException() {
-        inputStream = new ByteArrayInputStream(new byte[]{});
-        assertThrows(IOException.class, () -> {
-            SOCKS4Client.connect(
-                inputStream,
-                outputStream,
-                "127.0.0.1",
-                80);
-        });
-    }
-
-    /**
-     * Run into IOException while trying to connect due to closed input stream
-     */
-    @Test
-    public void connect__ioExceptionWithSocket() {
-        inputStream = new ByteArrayInputStream(new byte[]{});
-        assertThrows(IOException.class, () -> {
-            Socket socket = Mockito.mock(Socket.class);
-            Mockito.when(socket.getInputStream()).thenReturn(inputStream);
-            Mockito.when(socket.getOutputStream()).thenReturn(outputStream);
-
-            SOCKS4Client.connect(
-                socket,
-                "127.0.0.1",
-                80
-            );
-        });
-    }
-
-    /**
-     * Check that CONNECTION_REFUSED throws exception
-     */
-    @Test
-    public void connect__responseCONNECTION_REFUSED() throws IOException {
-        inputStream = new ByteArrayInputStream(new byte[]{
-            0, // dummy
-            SOCKS4Constants.Reply.CONNECTION_REFUSED, // Connection succeeded
-        });
-        assertThrows(SOCKSException.class, () -> {
-            SOCKS4Client.connect(inputStream,
-                outputStream,
-                "1.1.1.1",
-                80
-            );
-        });
-    }
-
-    /**
-     * IPv6 is not supported by this SOCKS client so it just throws an exception
-     */
-    @Test
-    public void connect__IPv6() {
-        inputStream = new ByteArrayInputStream(new byte[]{});
-        assertThrows(SOCKSException.class, () -> {
-            SOCKS4Client.connect(
-                inputStream,
-                outputStream,
-                "::1",
-                80);
-        });
-    }
-}

core/java/test/junit/net/i2p/util/AddressesTest.java

@@ -0,0 +1,145 @@
+package net.i2p.util;
+
+import org.junit.Test;
+
+import java.net.Inet6Address;
+import java.net.UnknownHostException;
+
+import static org.junit.Assert.*;
+
+/**
+ * @since 0.9.49
+ */
+public class AddressesTest {
+
+    @Test
+    public void getIPNull() {
+        assertNull(Addresses.getIP(null));
+    }
+
+    @Test
+    public void getIPEmptyString() {
+        assertNull(Addresses.getIP(""));
+    }
+
+    @Test
+    public void getIPWithIPString() {
+        byte[] address = {
+            1, 2, 3, 4
+        };
+        assertArrayEquals(address, Addresses.getIP("1.2.3.4"));
+    }
+
+    @Test
+    public void getPort() {
+        assertEquals(80, Addresses.getPort("80"));
+    }
+
+    @Test
+    public void getPort__invalidPort() {
+        String[] strings = {
+            "",
+            " 80",
+            "-100",
+            "a",
+            "99999",
+            null
+        };
+        for (String string : strings) {
+            assertEquals(0, Addresses.getPort(string));
+        }
+    }
+
+    @Test
+    public void isIPAddress() {
+        assertTrue(Addresses.isIPAddress("127.0.0.1"));
+        assertTrue(Addresses.isIPAddress("::1"));
+    }
+
+    @Test
+    public void isIPv6Address() {
+        assertTrue(Addresses.isIPv6Address("::1"));
+        assertFalse(Addresses.isIPv6Address(""));
+    }
+
+    @Test
+    public void isIPv4Address() {
+        assertTrue(Addresses.isIPv4Address("127.0.0.1"));
+        assertFalse(Addresses.isIPv4Address(""));
+    }
+
+    /**
+     * Should always return false when the address isn't in the cache
+     */
+    @Test
+    public void isDynamic() throws UnknownHostException {
+        String host = "localhost";
+        byte[] address = {0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1};
+        assertFalse(Addresses.isDynamic((Inet6Address) Inet6Address.getByAddress(host, address)));
+    }
+
+    /**
+     * Should always return false when the address isn't in the cache
+     */
+    @Test
+    public void isDeprecated() throws UnknownHostException {
+        String host = "localhost";
+        byte[] address = {0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1};
+        assertFalse(Addresses.isDeprecated((Inet6Address) Inet6Address.getByAddress(host, address)));
+    }
+
+    @Test
+    public void testToString() {
+        byte[] address = {127, 0, 0, 1};
+        assertEquals("127.0.0.1", Addresses.toString(address));
+    }
+
+    @Test
+    public void testToString__ipv4withPort() {
+        byte[] address = {127, 0, 0, 1};
+        assertEquals("127.0.0.1:80", Addresses.toString(address, 80));
+    }
+
+    @Test
+    public void testToString__ipv6withPort() {
+        byte[] address = {
+            0, 0, 0, 0,
+            0, 0, 0, 0,
+            0, 0, 0, 0,
+            0, 0, 0, 1,
+        };
+        assertEquals("[0:0:0:0:0:0:0:1]:80", Addresses.toString(address, 80));
+    }
+
+    @Test
+    public void testToString__null() {
+        assertEquals("null", Addresses.toString(null));
+    }
+
+    @Test
+    public void testToString__nullWithPort() {
+        assertEquals("null:80", Addresses.toString(null, 80));
+    }
+
+    @Test
+    public void testToString__badLength() {
+        byte[] address = {1};
+        assertTrue(Addresses.toString(address).startsWith("bad IP length"));
+    }
+
+    @Test
+    public void testToString__badLengthWithPort() {
+        byte[] address = {1};
+        String string = Addresses.toString(address, 80);
+        String expectedStartString = "(bad IP length";
+        assertTrue(
+            String.format("%s doesn't start with: %s", string, expectedStartString),
+            string.startsWith(expectedStartString)
+        );
+        String expectedEndString = "80";
+        assertTrue(
+            String.format("%s doesn't end with: %s", string, expectedEndString),
+            string.endsWith(expectedEndString)
+        );
+    }
+}