Skip to content

Commit

Permalink
SocksCmdRequest and SocksCmdResponse are trying to convert host from …
Browse files Browse the repository at this point in the history 
…IDN for the non-DOMAIN address types

Motivation:

In the SocksCmdRequest and SocksCmdResponse constructors a host param converts from IDN to ascii compatible form regardless address type.

Modifications:

Use `IDN#toASCII` only for `DOMAIN` address type.

Result:

More correct host handling in socks commands.
  • Loading branch information
@fenik17 @normanmaurer
fenik17 authored and normanmaurer committed Jun 28, 2017
1 parent f350477 commit 8d0e092
Show file tree
Hide file tree
Showing 4 changed files with 109 additions and 13 deletions.
10 changes: 6 additions & 4 deletions codec-socks/src/main/java/io/netty/handler/codec/socks/SocksCmdRequest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -51,9 +51,11 @@ public SocksCmdRequest(SocksCmdType cmdType, SocksAddressType addressType, Strin
}
break;
case DOMAIN:
if (IDN.toASCII(host).length() > 255) {
throw new IllegalArgumentException(host + " IDN: " + IDN.toASCII(host) + " exceeds 255 char limit");
String asciiHost = IDN.toASCII(host);
if (asciiHost.length() > 255) {
throw new IllegalArgumentException(host + " IDN: " + asciiHost + " exceeds 255 char limit");
}
host = asciiHost;
break;
case IPv6:
if (!NetUtil.isValidIpV6Address(host)) {
Expand All @@ -68,7 +70,7 @@ public SocksCmdRequest(SocksCmdType cmdType, SocksAddressType addressType, Strin
}
this.cmdType = cmdType;
this.addressType = addressType;
this.host = IDN.toASCII(host);
this.host = host;
this.port = port;
}

Expand Down Expand Up @@ -96,7 +98,7 @@ public SocksAddressType addressType() {
* @return host that is used as a parameter in {@link SocksCmdType}
*/
public String host() {
return IDN.toUnicode(host);
return addressType == SocksAddressType.DOMAIN ? IDN.toUnicode(host) : host;
}

/**
Expand Down
14 changes: 5 additions & 9 deletions codec-socks/src/main/java/io/netty/handler/codec/socks/SocksCmdResponse.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -75,10 +75,11 @@ public SocksCmdResponse(SocksCmdStatus cmdStatus, SocksAddressType addressType,
}
break;
case DOMAIN:
if (IDN.toASCII(host).length() > 255) {
throw new IllegalArgumentException(host + " IDN: " +
IDN.toASCII(host) + " exceeds 255 char limit");
String asciiHost = IDN.toASCII(host);
if (asciiHost.length() > 255) {
throw new IllegalArgumentException(host + " IDN: " + asciiHost + " exceeds 255 char limit");
}
host = asciiHost;
break;
case IPv6:
if (!NetUtil.isValidIpV6Address(host)) {
Expand All @@ -88,7 +89,6 @@ public SocksCmdResponse(SocksCmdStatus cmdStatus, SocksAddressType addressType,
case UNKNOWN:
break;
}
host = IDN.toASCII(host);
}
if (port < 0 || port > 65535) {
throw new IllegalArgumentException(port + " is not in bounds 0 <= x <= 65535");
Expand Down Expand Up @@ -126,11 +126,7 @@ public SocksAddressType addressType() {
* or null when there was no host specified during response construction
*/
public String host() {
if (host != null) {
return IDN.toUnicode(host);
} else {
return null;
}
return host != null && addressType == SocksAddressType.DOMAIN ? IDN.toUnicode(host) : host;
}

/**
Expand Down
50 changes: 50 additions & 0 deletions codec-socks/src/test/java/io/netty/handler/codec/socks/SocksCmdRequestTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,13 @@
*/
package io.netty.handler.codec.socks;

import io.netty.buffer.ByteBuf;
import io.netty.buffer.Unpooled;
import io.netty.util.CharsetUtil;
import org.junit.Test;

import java.net.IDN;

import static org.junit.Assert.*;

public class SocksCmdRequestTest {
Expand Down Expand Up @@ -72,6 +77,51 @@ public void testIDNNotExceeds255CharsLimit() {
}
}

@Test
public void testHostNotEncodedForUnknown() {
String asciiHost = "xn--e1aybc.xn--p1ai";
short port = 10000;

SocksCmdRequest rq = new SocksCmdRequest(SocksCmdType.BIND, SocksAddressType.UNKNOWN, asciiHost, port);
assertEquals(asciiHost, rq.host());

ByteBuf buffer = Unpooled.buffer(16);
rq.encodeAsByteBuf(buffer);

buffer.resetReaderIndex();
assertEquals(SocksProtocolVersion.SOCKS5.byteValue(), buffer.readByte());
assertEquals(SocksCmdType.BIND.byteValue(), buffer.readByte());
assertEquals((byte) 0x00, buffer.readByte());
assertEquals(SocksAddressType.UNKNOWN.byteValue(), buffer.readByte());
assertFalse(buffer.isReadable());

buffer.release();
}

@Test
public void testIDNEncodeToAsciiForDomain() {
String host = "тест.рф";
String asciiHost = IDN.toASCII(host);
short port = 10000;

SocksCmdRequest rq = new SocksCmdRequest(SocksCmdType.BIND, SocksAddressType.DOMAIN, host, port);
assertEquals(host, rq.host());

ByteBuf buffer = Unpooled.buffer(24);
rq.encodeAsByteBuf(buffer);

buffer.resetReaderIndex();
assertEquals(SocksProtocolVersion.SOCKS5.byteValue(), buffer.readByte());
assertEquals(SocksCmdType.BIND.byteValue(), buffer.readByte());
assertEquals((byte) 0x00, buffer.readByte());
assertEquals(SocksAddressType.DOMAIN.byteValue(), buffer.readByte());
assertEquals((byte) asciiHost.length(), buffer.readUnsignedByte());
assertEquals(asciiHost, buffer.readCharSequence(asciiHost.length(), CharsetUtil.US_ASCII));
assertEquals(port, buffer.readUnsignedShort());

buffer.release();
}

@Test
public void testValidPortRange() {
try {
Expand Down
48 changes: 48 additions & 0 deletions codec-socks/src/test/java/io/netty/handler/codec/socks/SocksCmdResponseTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,11 @@

import io.netty.buffer.ByteBuf;
import io.netty.buffer.Unpooled;
import io.netty.util.CharsetUtil;
import org.junit.Test;

import java.net.IDN;

import static org.junit.Assert.*;

public class SocksCmdResponseTest {
Expand Down Expand Up @@ -108,6 +111,51 @@ public void testEmptyBoundAddress() {
assertByteBufEquals(expected, buffer);
}

@Test
public void testHostNotEncodedForUnknown() {
String asciiHost = "xn--e1aybc.xn--p1ai";
short port = 10000;

SocksCmdResponse rs = new SocksCmdResponse(SocksCmdStatus.SUCCESS, SocksAddressType.UNKNOWN, asciiHost, port);
assertEquals(asciiHost, rs.host());

ByteBuf buffer = Unpooled.buffer(16);
rs.encodeAsByteBuf(buffer);

buffer.resetReaderIndex();
assertEquals(SocksProtocolVersion.SOCKS5.byteValue(), buffer.readByte());
assertEquals(SocksCmdStatus.SUCCESS.byteValue(), buffer.readByte());
assertEquals((byte) 0x00, buffer.readByte());
assertEquals(SocksAddressType.UNKNOWN.byteValue(), buffer.readByte());
assertFalse(buffer.isReadable());

buffer.release();
}

@Test
public void testIDNEncodeToAsciiForDomain() {
String host = "тест.рф";
String asciiHost = IDN.toASCII(host);
short port = 10000;

SocksCmdResponse rs = new SocksCmdResponse(SocksCmdStatus.SUCCESS, SocksAddressType.DOMAIN, host, port);
assertEquals(host, rs.host());

ByteBuf buffer = Unpooled.buffer(24);
rs.encodeAsByteBuf(buffer);

buffer.resetReaderIndex();
assertEquals(SocksProtocolVersion.SOCKS5.byteValue(), buffer.readByte());
assertEquals(SocksCmdStatus.SUCCESS.byteValue(), buffer.readByte());
assertEquals((byte) 0x00, buffer.readByte());
assertEquals(SocksAddressType.DOMAIN.byteValue(), buffer.readByte());
assertEquals((byte) asciiHost.length(), buffer.readUnsignedByte());
assertEquals(asciiHost, buffer.readCharSequence(asciiHost.length(), CharsetUtil.US_ASCII));
assertEquals(port, buffer.readUnsignedShort());

buffer.release();
}

/**
* Verifies that Response cannot be constructed with invalid IP.
*/
Expand Down

0 comments on commit 8d0e092

Please sign in to comment.