Skip to content

Commit

Permalink
NetUtil.bytesToIpAddress bug
Browse files Browse the repository at this point in the history 
Motivation:
NetUtil.bytesToIpAddress does not correctly translate IPv4 address to String. Also IPv6 addresses may not follow minimization conventions when converting to a String (see rfc 5952).

Modifications:
- NetUtil.bytesToIpAddress should correctly handle negative byte values for IPv4
- NetUtil.bytesToIpAddress should leverage existing to string conversion code in NetUtil

Result:
Fixes netty#5821
  • Loading branch information
@Scottmitch
Scottmitch committed Sep 23, 2016
1 parent dd1ba2a commit 506ac2c
Show file tree
Hide file tree
Showing 5 changed files with 62 additions and 44 deletions.
2 changes: 1 addition & 1 deletion codec-socks/src/main/java/io/netty/handler/codec/socksx/v5/Socks5AddressDecoder.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ public String decodeAddress(Socks5AddressType addrType, ByteBuf in) throws Excep
} else {
byte[] tmp = new byte[IPv6_LEN];
in.readBytes(tmp);
return NetUtil.bytesToIpAddress(tmp, 0, IPv6_LEN);
return NetUtil.bytesToIpAddress(tmp);
}
} else {
throw new DecoderException("unsupported address type: " + (addrType.byteValue() & 0xFF));
Expand Down
2 changes: 1 addition & 1 deletion ...socks/src/test/java/io/netty/handler/codec/socksx/v5/Socks5CommandRequestDecoderTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ public void testCmdRequestDecoderIPv4() {
@Test
public void testCmdRequestDecoderIPv6() {
String[] hosts = {
NetUtil.bytesToIpAddress(IPAddressUtil.textToNumericFormatV6("::1"), 0, 16) };
NetUtil.bytesToIpAddress(IPAddressUtil.textToNumericFormatV6("::1")) };
int[] ports = {1, 32769, 65535};
for (Socks5CommandType cmdType: Arrays.asList(Socks5CommandType.BIND,
Socks5CommandType.CONNECT,
Expand Down
5 changes: 3 additions & 2 deletions ...ocks/src/test/java/io/netty/handler/codec/socksx/v5/Socks5CommandResponseDecoderTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,8 @@

import java.util.Arrays;

import static org.junit.Assert.*;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertNull;

public class Socks5CommandResponseDecoderTest {

Expand Down Expand Up @@ -92,7 +93,7 @@ public void testSocksCmdResponseDecoderIncludingHost() {
test(cmdStatus, Socks5AddressType.IPv6,
"2001:db8:85a3:42:1000:8a2e:370:7334", 80);
test(cmdStatus, Socks5AddressType.IPv6,
"1111:111:11:1:0:0:0:1", 80);
"1111:111:11:1::1", 80);
}
}
}
61 changes: 31 additions & 30 deletions common/src/main/java/io/netty/util/NetUtil.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@
package io.netty.util;

import io.netty.util.internal.PlatformDependent;
import io.netty.util.internal.StringUtil;
import io.netty.util.internal.logging.InternalLogger;
import io.netty.util.internal.logging.InternalLoggerFactory;

Expand Down Expand Up @@ -506,35 +505,33 @@ public static String intToIpAddress(int i) {
* @throws IllegalArgumentException
* if {@code length} is not {@code 4} nor {@code 16}
*/
public static String bytesToIpAddress(byte[] bytes, int offset, int length) {
if (length == 4) {
StringBuilder buf = new StringBuilder(15);

buf.append(bytes[offset ++] >> 24 & 0xff);
buf.append('.');
buf.append(bytes[offset ++] >> 16 & 0xff);
buf.append('.');
buf.append(bytes[offset ++] >> 8 & 0xff);
buf.append('.');
buf.append(bytes[offset] & 0xff);

return buf.toString();
}

if (length == 16) {
final StringBuilder sb = new StringBuilder(39);
final int endOffset = offset + 14;
public static String bytesToIpAddress(byte[] bytes) {
return bytesToIpAddress(bytes, 0, bytes.length);
}

for (; offset < endOffset; offset += 2) {
StringUtil.toHexString(sb, bytes, offset, 2);
sb.append(':');
/**
* Converts 4-byte or 16-byte data into an IPv4 or IPv6 string respectively.
*
* @throws IllegalArgumentException
* if {@code length} is not {@code 4} nor {@code 16}
*/
public static String bytesToIpAddress(byte[] bytes, int offset, int length) {
switch (length) {
case 4: {
return new StringBuilder(15)
.append(bytes[offset] & 0xff)
.append('.')
.append(bytes[offset + 1] & 0xff)
.append('.')
.append(bytes[offset + 2] & 0xff)
.append('.')
.append(bytes[offset + 3] & 0xff).toString();
}
StringUtil.toHexString(sb, bytes, offset, 2);

return sb.toString();
case 16:
return toAddressString(bytes, offset, false);
default:
throw new IllegalArgumentException("length: " + length + " (expected: 4 or 16)");
}

throw new IllegalArgumentException("length: " + length + " (expected: 4 or 16)");
}

public static boolean isValidIpV6Address(String ipAddress) {
Expand Down Expand Up @@ -981,13 +978,17 @@ public static String toAddressString(InetAddress ip, boolean ipv4Mapped) {
return ip.getHostAddress();
}
if (!(ip instanceof Inet6Address)) {
throw new IllegalArgumentException("Unhandled type: " + ip.getClass());
throw new IllegalArgumentException("Unhandled type: " + ip);
}

final byte[] bytes = ip.getAddress();
return toAddressString(ip.getAddress(), 0, ipv4Mapped);
}

private static String toAddressString(byte[] bytes, int offset, boolean ipv4Mapped) {
final int[] words = new int[IPV6_WORD_COUNT];
int i;
for (i = 0; i < words.length; ++i) {
final int end = offset + words.length;
for (i = offset; i < end; ++i) {
words[i] = ((bytes[i << 1] & 0xff) << 8) | (bytes[(i << 1) + 1] & 0xff);
}

Expand Down
36 changes: 26 additions & 10 deletions common/src/test/java/io/netty/util/NetUtilTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,12 +17,17 @@

import org.junit.Test;

import java.net.Inet6Address;
import java.net.InetAddress;
import java.net.UnknownHostException;
import java.util.HashMap;
import java.util.Map;
import java.util.Map.Entry;

import static io.netty.util.NetUtil.bytesToIpAddress;
import static io.netty.util.NetUtil.createByteArrayFromIpAddressString;
import static io.netty.util.NetUtil.getByName;
import static io.netty.util.NetUtil.toAddressString;
import static org.junit.Assert.*;

public class NetUtilTest {
Expand All @@ -44,7 +49,8 @@ private static final class TestMap extends HashMap<String, String> {
"10.255.255.254", "0afffffe",
"172.18.5.4", "ac120504",
"0.0.0.0", "00000000",
"127.0.0.1", "7f000001");
"127.0.0.1", "7f000001",
"1.2.3.4", "01020304");

private static final Map<String, String> invalidIpV4Hosts = new TestMap(
"1.256.3.4", null,
Expand Down Expand Up @@ -438,30 +444,40 @@ public void testIsValidIpV6Address() {
@Test
public void testCreateByteArrayFromIpAddressString() {
for (Entry<String, String> e : validIpV4Hosts.entrySet()) {
assertHexDumpEquals(e.getValue(), NetUtil.createByteArrayFromIpAddressString(e.getKey()));
assertHexDumpEquals(e.getValue(), createByteArrayFromIpAddressString(e.getKey()));
}
for (Entry<String, String> e : invalidIpV4Hosts.entrySet()) {
assertHexDumpEquals(e.getValue(), NetUtil.createByteArrayFromIpAddressString(e.getKey()));
assertHexDumpEquals(e.getValue(), createByteArrayFromIpAddressString(e.getKey()));
}
for (Entry<String, String> e : validIpV6Hosts.entrySet()) {
assertHexDumpEquals(e.getValue(), NetUtil.createByteArrayFromIpAddressString(e.getKey()));
assertHexDumpEquals(e.getValue(), createByteArrayFromIpAddressString(e.getKey()));
}
for (Entry<String, String> e : invalidIpV6Hosts.entrySet()) {
assertHexDumpEquals(e.getValue(), NetUtil.createByteArrayFromIpAddressString(e.getKey()));
assertHexDumpEquals(e.getValue(), createByteArrayFromIpAddressString(e.getKey()));
}
}

@Test
public void testBytesToIpAddress() throws UnknownHostException {
for (Entry<String, String> e : validIpV4Hosts.entrySet()) {
assertEquals(e.getKey(), bytesToIpAddress(createByteArrayFromIpAddressString(e.getKey())));
}
for (Entry<byte[], String> testEntry : ipv6ToAddressStrings.entrySet()) {
assertEquals(testEntry.getValue(), bytesToIpAddress(testEntry.getKey()));
}
}

@Test
public void testIp6AddressToString() throws UnknownHostException {
for (Entry<byte[], String> testEntry : ipv6ToAddressStrings.entrySet()) {
assertEquals(testEntry.getValue(), NetUtil.toAddressString(InetAddress.getByAddress(testEntry.getKey())));
assertEquals(testEntry.getValue(), toAddressString(InetAddress.getByAddress(testEntry.getKey())));
}
}

@Test
public void testIp4AddressToString() throws UnknownHostException {
for (Entry<String, String> e : validIpV4Hosts.entrySet()) {
assertEquals(e.getKey(), NetUtil.toAddressString(InetAddress.getByAddress(unhex(e.getValue()))));
assertEquals(e.getKey(), toAddressString(InetAddress.getByAddress(unhex(e.getValue()))));
}
}

Expand All @@ -470,18 +486,18 @@ public void testIpv4MappedIp6GetByName() {
for (Entry<String, String> testEntry : ipv4MappedToIPv6AddressStrings.entrySet()) {
assertEquals(
testEntry.getValue(),
NetUtil.toAddressString(NetUtil.getByName(testEntry.getKey(), true), true));
toAddressString(getByName(testEntry.getKey(), true), true));
}
}

@Test
public void testinvalidIpv4MappedIp6GetByName() {
for (String testEntry : invalidIpV4Hosts.keySet()) {
assertNull(NetUtil.getByName(testEntry, true));
assertNull(getByName(testEntry, true));
}

for (String testEntry : invalidIpV6Hosts.keySet()) {
assertNull(NetUtil.getByName(testEntry, true));
assertNull(getByName(testEntry, true));
}
}

Expand Down

0 comments on commit 506ac2c

Please sign in to comment.