#2177 Adding support for bound host and port for the SOCKS5 command response. Changes are fully backward compatible.

codec-socks/src/main/java/io/netty/handler/codec/socks/SocksCmdResponse.java

@@ -16,9 +16,13 @@
 package io.netty.handler.codec.socks;
 
 import io.netty.buffer.ByteBuf;
+import io.netty.util.CharsetUtil;
+import io.netty.util.NetUtil;
+
+import java.net.IDN;
 
 /**
- * An socks cmd response.
+ * A socks cmd response.
  *
  * @see SocksCmdRequest
  * @see SocksCmdResponseDecoder
@@ -27,7 +31,11 @@ public final class SocksCmdResponse extends SocksResponse {
     private final SocksCmdStatus cmdStatus;
 
     private final SocksAddressType addressType;
+    private final String host;
+    private final int port;
+
     // All arrays are initialized on construction time to 0/false/null remove array Initialization
+    private static final byte[] DOMAIN_ZEROED = {0x00};
     private static final byte[] IPv4_HOSTNAME_ZEROED = {0x00, 0x00, 0x00, 0x00};
     private static final byte[] IPv6_HOSTNAME_ZEROED = {0x00, 0x00, 0x00, 0x00,
             0x00, 0x00, 0x00, 0x00,
@@ -35,6 +43,23 @@ public final class SocksCmdResponse extends SocksResponse {
             0x00, 0x00, 0x00, 0x00};
 
     public SocksCmdResponse(SocksCmdStatus cmdStatus, SocksAddressType addressType) {
+        this(cmdStatus, addressType, null, 0);
+    }
+
+    /**
+     * Constructs new response and includes provided host and port as part of it.
+     *
+     * @param cmdStatus status of the response
+     * @param addressType type of host parameter
+     * @param host host (BND.ADDR field) is address that server used when connecting to the target host.
+     *             When null a value of 4/8 0x00 octets will be used for IPv4/IPv6 and a single 0x00 byte will be
+     *             used for domain addressType. Value is converted to ASCII using {@link IDN#toASCII(String)}.
+     * @param port port (BND.PORT field) that the server assigned to connect to the target host
+     * @throws NullPointerException in case cmdStatus or addressType are missing
+     * @throws IllegalArgumentException in case host or port cannot be validated
+     * @see IDN#toASCII(String)
+     */
+    public SocksCmdResponse(SocksCmdStatus cmdStatus, SocksAddressType addressType, String host, int port) {
         super(SocksResponseType.CMD);
         if (cmdStatus == null) {
             throw new NullPointerException("cmdStatus");
@@ -42,8 +67,36 @@ public final class SocksCmdResponse extends SocksResponse {
         if (addressType == null) {
             throw new NullPointerException("addressType");
         }
+        if (host != null) {
+            switch (addressType) {
+                case IPv4:
+                    if (!NetUtil.isValidIpV4Address(host)) {
+                        throw new IllegalArgumentException(host + " is not a valid IPv4 address");
+                    }
+                    break;
+                case DOMAIN:
+                    if (IDN.toASCII(host).length() > 255) {
+                        throw new IllegalArgumentException(host + " IDN: " +
+                                IDN.toASCII(host) + " exceeds 255 char limit");
+                    }
+                    break;
+                case IPv6:
+                    if (!NetUtil.isValidIpV6Address(host)) {
+                        throw new IllegalArgumentException(host + " is not a valid IPv6 address");
+                    }
+                    break;
+                case UNKNOWN:
+                    break;
+            }
+            host = IDN.toASCII(host);
+        }
+        if (port < 0 && port >= 65535) {
+            throw new IllegalArgumentException(port + " is not in bounds 0 < x < 65536");
+        }
         this.cmdStatus = cmdStatus;
         this.addressType = addressType;
+        this.host = host;
+        this.port = port;
     }
 
     /**
@@ -64,6 +117,32 @@ public final class SocksCmdResponse extends SocksResponse {
         return addressType;
     }
 
+    /**
+     * Returns host that is used as a parameter in {@link io.netty.handler.codec.socks.SocksCmdType}.
+     * Host (BND.ADDR field in response) is address that server used when connecting to the target host.
+     * This is typically different from address which client uses to connect to the SOCKS server.
+     *
+     * @return host that is used as a parameter in {@link io.netty.handler.codec.socks.SocksCmdType}
+     *         or null when there was no host specified during response construction
+     */
+    public String host() {
+        if (host != null) {
+            return IDN.toUnicode(host);
+        } else {
+            return null;
+        }
+    }
+
+    /**
+     * Returns port that is used as a parameter in {@link io.netty.handler.codec.socks.SocksCmdType}.
+     * Port (BND.PORT field in response) is port that the server assigned to connect to the target host.
+     *
+     * @return port that is used as a parameter in {@link io.netty.handler.codec.socks.SocksCmdType}
+     */
+    public int port() {
+        return port;
+    }
+
     @Override
     public void encodeAsByteBuf(ByteBuf byteBuf) {
         byteBuf.writeByte(protocolVersion().byteValue());
@@ -72,19 +151,25 @@ public final class SocksCmdResponse extends SocksResponse {
         byteBuf.writeByte(addressType.byteValue());
         switch (addressType) {
             case IPv4: {
-                byteBuf.writeBytes(IPv4_HOSTNAME_ZEROED);
+                byte[] hostContent = host == null ?
-                byteBuf.writeShort(0);
+                        IPv4_HOSTNAME_ZEROED : NetUtil.createByteArrayFromIpAddressString(host);
+                byteBuf.writeBytes(hostContent);
+                byteBuf.writeShort(port);
                 break;
             }
             case DOMAIN: {
-                byteBuf.writeByte(1);   // domain length
+                byte[] hostContent = host == null ?
-                byteBuf.writeByte(0);   // domain value
+                        DOMAIN_ZEROED : host.getBytes(CharsetUtil.US_ASCII);
-                byteBuf.writeShort(0);  // port value
+                byteBuf.writeByte(hostContent.length);   // domain length
+                byteBuf.writeBytes(hostContent);   // domain value
+                byteBuf.writeShort(port);  // port value
                 break;
             }
             case IPv6: {
-                byteBuf.writeBytes(IPv6_HOSTNAME_ZEROED);
+                byte[] hostContent = host == null
-                byteBuf.writeShort(0);
+                        ? IPv6_HOSTNAME_ZEROED : NetUtil.createByteArrayFromIpAddressString(host);
+                byteBuf.writeBytes(hostContent);
+                byteBuf.writeShort(port);
                 break;
             }
         }

codec-socks/src/main/java/io/netty/handler/codec/socks/SocksCmdResponseDecoder.java

@@ -67,20 +67,20 @@ public class SocksCmdResponseDecoder extends ReplayingDecoder<SocksCmdResponseDe
                     case IPv4: {
                         host = SocksCommonUtils.intToIp(byteBuf.readInt());
                         port = byteBuf.readUnsignedShort();
-                        msg = new SocksCmdResponse(cmdStatus, addressType);
+                        msg = new SocksCmdResponse(cmdStatus, addressType, host, port);
                         break;
                     }
                     case DOMAIN: {
                         fieldLength = byteBuf.readByte();
                         host = byteBuf.readBytes(fieldLength).toString(CharsetUtil.US_ASCII);
                         port = byteBuf.readUnsignedShort();
-                        msg = new SocksCmdResponse(cmdStatus, addressType);
+                        msg = new SocksCmdResponse(cmdStatus, addressType, host, port);
                         break;
                     }
                     case IPv6: {
                         host = SocksCommonUtils.ipv6toStr(byteBuf.readBytes(16).array());
                         port = byteBuf.readUnsignedShort();
-                        msg = new SocksCmdResponse(cmdStatus, addressType);
+                        msg = new SocksCmdResponse(cmdStatus, addressType, host, port);
                         break;
                     }
                     case UNKNOWN:

codec-socks/src/test/java/io/netty/handler/codec/socks/SocksCmdResponseDecoderTest.java

@@ -26,9 +26,9 @@ public class SocksCmdResponseDecoderTest {
     private static final InternalLogger logger = InternalLoggerFactory.getInstance(SocksCmdResponseDecoderTest.class);
 
     private static void testSocksCmdResponseDecoderWithDifferentParams(
-            SocksCmdStatus cmdStatus, SocksAddressType addressType) {
+            SocksCmdStatus cmdStatus, SocksAddressType addressType, String host, int port) {
         logger.debug("Testing cmdStatus: " + cmdStatus + " addressType: " + addressType);
-        SocksResponse msg = new SocksCmdResponse(cmdStatus, addressType);
+        SocksResponse msg = new SocksCmdResponse(cmdStatus, addressType, host, port);
         SocksCmdResponseDecoder decoder = new SocksCmdResponseDecoder();
         EmbeddedChannel embedder = new EmbeddedChannel(decoder);
         SocksCommonTestUtils.writeMessageIntoEmbedder(embedder, msg);
@@ -37,16 +37,47 @@ public class SocksCmdResponseDecoderTest {
         } else {
             msg = (SocksResponse) embedder.readInbound();
             assertEquals(((SocksCmdResponse) msg).cmdStatus(), cmdStatus);
+            if (host != null) {
+                assertEquals(((SocksCmdResponse) msg).host(), host);
+            }
+            assertEquals(((SocksCmdResponse) msg).port(), port);
         }
         assertNull(embedder.readInbound());
     }
 
+    /**
+     * Verifies that sent socks messages are decoded correctly.
+     */
     @Test
     public void testSocksCmdResponseDecoder() {
-        for (SocksCmdStatus cmdStatus: SocksCmdStatus.values()) {
+        for (SocksCmdStatus cmdStatus : SocksCmdStatus.values()) {
-            for (SocksAddressType addressType: SocksAddressType.values()) {
+            for (SocksAddressType addressType : SocksAddressType.values()) {
-                testSocksCmdResponseDecoderWithDifferentParams(cmdStatus, addressType);
+                testSocksCmdResponseDecoderWithDifferentParams(cmdStatus, addressType, null, 0);
             }
         }
     }
+
+    /**
+     * Verifies that invalid bound host will fail with IllegalArgumentException during encoding.
+     */
+    @Test(expected = IllegalArgumentException.class)
+    public void testInvalidAddress() {
+        testSocksCmdResponseDecoderWithDifferentParams(SocksCmdStatus.SUCCESS, SocksAddressType.IPv4, "1", 80);
+    }
+
+    /**
+     * Verifies that send socks messages are decoded correctly when bound host and port are set.
+     */
+    @Test
+    public void testSocksCmdResponseDecoderIncludingHost() {
+        for (SocksCmdStatus cmdStatus : SocksCmdStatus.values()) {
+            testSocksCmdResponseDecoderWithDifferentParams(cmdStatus, SocksAddressType.IPv4,
+                    "127.0.0.1", 80);
+            testSocksCmdResponseDecoderWithDifferentParams(cmdStatus, SocksAddressType.DOMAIN,
+                    "testDomain.com", 80);
+            testSocksCmdResponseDecoderWithDifferentParams(cmdStatus, SocksAddressType.IPv6,
+                    "2001:db8:85a3:42:1000:8a2e:370:7334", 80);
+        }
+    }
+
 }

codec-socks/src/test/java/io/netty/handler/codec/socks/SocksCmdResponseTest.java

@@ -15,6 +15,8 @@
  */
 package io.netty.handler.codec.socks;
 
+import io.netty.buffer.ByteBuf;
+import io.netty.buffer.Unpooled;
 import org.junit.Test;
 
 import static org.junit.Assert.*;
@@ -33,4 +35,92 @@ public class SocksCmdResponseTest {
             assertTrue(e instanceof NullPointerException);
         }
     }
+
+    /**
+     * Verifies content of the response when domain is not specified.
+     */
+    @Test
+    public void testEmptyDomain() {
+        SocksCmdResponse socksCmdResponse = new SocksCmdResponse(SocksCmdStatus.SUCCESS, SocksAddressType.DOMAIN);
+        assertNull(socksCmdResponse.host());
+        assertEquals(0, socksCmdResponse.port());
+        ByteBuf buffer = Unpooled.buffer(20);
+        socksCmdResponse.encodeAsByteBuf(buffer);
+        byte[] expected = {
+                0x05, // version
+                0x00, // success reply
+                0x00, // reserved
+                0x03, // address type domain
+                0x01, // length of domain
+                0x00, // domain value
+                0x00, // port value
+                0x00
+        };
+        assertByteBufEquals(expected, buffer);
+    }
+
+    /**
+     * Verifies content of the response when IPv4 address is specified.
+     */
+    @Test
+    public void testIPv4Host() {
+        SocksCmdResponse socksCmdResponse = new SocksCmdResponse(SocksCmdStatus.SUCCESS, SocksAddressType.IPv4,
+                "127.0.0.1", 80);
+        assertEquals("127.0.0.1", socksCmdResponse.host());
+        assertEquals(80, socksCmdResponse.port());
+        ByteBuf buffer = Unpooled.buffer(20);
+        socksCmdResponse.encodeAsByteBuf(buffer);
+        byte[] expected = {
+                0x05, // version
+                0x00, // success reply
+                0x00, // reserved
+                0x01, // address type IPv4
+                0x7F, // address 127.0.0.1
+                0x00,
+                0x00,
+                0x01,
+                0x00, // port
+                0x50
+                };
+        assertByteBufEquals(expected, buffer);
+    }
+
+    /**
+     * Verifies that empty domain is allowed Response.
+     */
+    @Test
+    public void testEmptyBoundAddress() {
+        SocksCmdResponse socksCmdResponse = new SocksCmdResponse(SocksCmdStatus.SUCCESS, SocksAddressType.DOMAIN,
+                "", 80);
+        assertEquals("", socksCmdResponse.host());
+        assertEquals(80, socksCmdResponse.port());
+        ByteBuf buffer = Unpooled.buffer(20);
+        socksCmdResponse.encodeAsByteBuf(buffer);
+        byte[] expected = {
+                0x05, // version
+                0x00, // success reply
+                0x00, // reserved
+                0x03, // address type domain
+                0x00, // domain length
+                0x00, // port
+                0x50
+        };
+        assertByteBufEquals(expected, buffer);
+    }
+
+    /**
+     * Verifies that Response cannot be constructed with invalid IP.
+     */
+    @Test(expected = IllegalArgumentException.class)
+    public void testInvalidBoundAddress() {
+        new SocksCmdResponse(SocksCmdStatus.SUCCESS, SocksAddressType.IPv4, "127.0.0", 1000);
+    }
+
+    private static void assertByteBufEquals(byte[] expected, ByteBuf actual) {
+        byte[] actualBytes = new byte[actual.readableBytes()];
+        actual.readBytes(actualBytes);
+        assertEquals("Generated response has incorrect length", expected.length, actualBytes.length);
+        assertArrayEquals("Generated response differs from expected", expected, actualBytes);
+    }
+
 }