From eb5dc4bcedc7503738e19ee8635f4df58f7062a2 Mon Sep 17 00:00:00 2001
From: Norman Maurer <norman_maurer@apple.com>
Date: Thu, 22 Dec 2016 10:27:03 +0100
Subject: [PATCH] Correctly handle IPV6 in HttpProxyHandler

Motivation:

The HttpProxyHandler is expected to be capable of issuing a valid CONNECT request for a tunneled connection to an IPv6 host.

Modifications:

- Correctly format the IPV6 address.
- Add unit tests

Result:

HttpProxyHandler works with IPV6 as well. Fixes [#6152].
---
 .../src/main/java/io/netty/util/NetUtil.java  | 33 ++++++++
 .../test/java/io/netty/util/NetUtilTest.java  | 18 +++++
 .../netty/handler/proxy/HttpProxyHandler.java | 10 +--
 .../handler/proxy/HttpProxyHandlerTest.java   | 80 +++++++++++++++++++
 4 files changed, 133 insertions(+), 8 deletions(-)
 create mode 100644 handler-proxy/src/test/java/io/netty/handler/proxy/HttpProxyHandlerTest.java

diff --git a/common/src/main/java/io/netty/util/NetUtil.java b/common/src/main/java/io/netty/util/NetUtil.java
index a645cd70d6..a442ff4ce9 100644
--- a/common/src/main/java/io/netty/util/NetUtil.java
+++ b/common/src/main/java/io/netty/util/NetUtil.java
@@ -25,6 +25,7 @@ import java.io.FileReader;
 import java.net.Inet4Address;
 import java.net.Inet6Address;
 import java.net.InetAddress;
+import java.net.InetSocketAddress;
 import java.net.NetworkInterface;
 import java.net.SocketException;
 import java.net.UnknownHostException;
@@ -933,6 +934,38 @@ public final class NetUtil {
         }
     }
 
+    /**
+     * Returns the {@link String} representation of an {@link InetSocketAddress}.
+     * <p>
+     * The output does not include Scope ID.
+     * @param addr {@link InetSocketAddress} to be converted to an address string
+     * @return {@code String} containing the text-formatted IP address
+     */
+    public static String toSocketAddressString(InetSocketAddress addr) {
+        String port = String.valueOf(addr.getPort());
+        final StringBuilder sb;
+
+        if (addr.isUnresolved()) {
+            String hostString = PlatformDependent.javaVersion() >= 7 ? addr.getHostString() : addr.getHostName();
+            sb = newSocketAddressStringBuilder(hostString, port, !isValidIpV6Address(hostString));
+        } else {
+            InetAddress address = addr.getAddress();
+            String hostString = toAddressString(address);
+            sb = newSocketAddressStringBuilder(hostString, port, address instanceof Inet4Address);
+        }
+        return sb.append(':').append(port).toString();
+    }
+
+    private static StringBuilder newSocketAddressStringBuilder(String hostString, String port, boolean ipv4) {
+        if (ipv4) {
+            // Need to include enough space for hostString:port.
+            return new StringBuilder(hostString.length() + 1 + port.length()).append(hostString);
+        }
+        // Need to include enough space for [hostString]:port.
+        return new StringBuilder(
+                hostString.length() + 3 + port.length()).append('[').append(hostString).append(']');
+    }
+
     /**
      * Returns the {@link String} representation of an {@link InetAddress}.
      * <ul>
diff --git a/common/src/test/java/io/netty/util/NetUtilTest.java b/common/src/test/java/io/netty/util/NetUtilTest.java
index aa04b2a4c0..09afacca35 100644
--- a/common/src/test/java/io/netty/util/NetUtilTest.java
+++ b/common/src/test/java/io/netty/util/NetUtilTest.java
@@ -18,6 +18,7 @@ package io.netty.util;
 import org.junit.Test;
 
 import java.net.InetAddress;
+import java.net.InetSocketAddress;
 import java.net.UnknownHostException;
 import java.util.HashMap;
 import java.util.Map;
@@ -27,6 +28,7 @@ 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 io.netty.util.NetUtil.toSocketAddressString;
 import static org.junit.Assert.*;
 
 public class NetUtilTest {
@@ -500,6 +502,22 @@ public class NetUtilTest {
         }
     }
 
+    @Test
+    public void testIp6InetSocketAddressToString() throws UnknownHostException {
+        for (Entry<byte[], String> testEntry : ipv6ToAddressStrings.entrySet()) {
+            assertEquals('[' + testEntry.getValue() + "]:9999",
+                    toSocketAddressString(new InetSocketAddress(InetAddress.getByAddress(testEntry.getKey()), 9999)));
+        }
+    }
+
+    @Test
+    public void testIp4SocketAddressToString() throws UnknownHostException {
+        for (Entry<String, String> e : validIpV4Hosts.entrySet()) {
+            assertEquals(e.getKey() + ":9999",
+                    toSocketAddressString(new InetSocketAddress(InetAddress.getByAddress(unhex(e.getValue())), 9999)));
+        }
+    }
+
     private static void assertHexDumpEquals(String expected, byte[] actual) {
         assertEquals(expected, hex(actual));
     }
diff --git a/handler-proxy/src/main/java/io/netty/handler/proxy/HttpProxyHandler.java b/handler-proxy/src/main/java/io/netty/handler/proxy/HttpProxyHandler.java
index 53d6fbb823..aecf5f3e7e 100644
--- a/handler-proxy/src/main/java/io/netty/handler/proxy/HttpProxyHandler.java
+++ b/handler-proxy/src/main/java/io/netty/handler/proxy/HttpProxyHandler.java
@@ -32,6 +32,7 @@ import io.netty.handler.codec.http.HttpVersion;
 import io.netty.handler.codec.http.LastHttpContent;
 import io.netty.util.AsciiString;
 import io.netty.util.CharsetUtil;
+import io.netty.util.NetUtil;
 
 import java.net.InetSocketAddress;
 import java.net.SocketAddress;
@@ -112,14 +113,7 @@ public final class HttpProxyHandler extends ProxyHandler {
     @Override
     protected Object newInitialMessage(ChannelHandlerContext ctx) throws Exception {
         InetSocketAddress raddr = destinationAddress();
-        String rhost;
-        if (raddr.isUnresolved()) {
-            rhost = raddr.getHostString();
-        } else {
-            rhost = raddr.getAddress().getHostAddress();
-        }
-
-        final String host = rhost + ':' + raddr.getPort();
+        final String host = NetUtil.toSocketAddressString(raddr);
         FullHttpRequest req = new DefaultFullHttpRequest(
                 HttpVersion.HTTP_1_1, HttpMethod.CONNECT,
                 host,
diff --git a/handler-proxy/src/test/java/io/netty/handler/proxy/HttpProxyHandlerTest.java b/handler-proxy/src/test/java/io/netty/handler/proxy/HttpProxyHandlerTest.java
new file mode 100644
index 0000000000..61fd6911a0
--- /dev/null
+++ b/handler-proxy/src/test/java/io/netty/handler/proxy/HttpProxyHandlerTest.java
@@ -0,0 +1,80 @@
+/*
+ * Copyright 2016 The Netty Project
+ *
+ * The Netty Project licenses this file to you under the Apache License,
+ * version 2.0 (the "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at:
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ */
+package io.netty.handler.proxy;
+
+import io.netty.channel.ChannelHandlerContext;
+import io.netty.channel.ChannelPromise;
+import io.netty.handler.codec.http.FullHttpRequest;
+import io.netty.handler.codec.http.HttpHeaderNames;
+import io.netty.handler.codec.http.HttpVersion;
+import io.netty.util.NetUtil;
+import org.junit.Test;
+
+import java.net.InetAddress;
+import java.net.InetSocketAddress;
+
+import static org.junit.Assert.assertEquals;
+import static org.mockito.Mockito.*;
+
+public class HttpProxyHandlerTest {
+
+    @Test
+    public void testIpv6() throws Exception  {
+        InetSocketAddress socketAddress = new InetSocketAddress(InetAddress.getByName("::1"), 8080);
+        testInitialMessage(socketAddress, "[::1]:8080");
+    }
+
+    @Test
+    public void testIpv6Unresolved() throws Exception  {
+        InetSocketAddress socketAddress = InetSocketAddress.createUnresolved("::1", 8080);
+        testInitialMessage(socketAddress, "[::1]:8080");
+    }
+
+    @Test
+    public void testIpv4() throws Exception  {
+        InetSocketAddress socketAddress = new InetSocketAddress(InetAddress.getByName("10.0.0.1"), 8080);
+        testInitialMessage(socketAddress, "10.0.0.1:8080");
+    }
+
+    @Test
+    public void testIpv4Unresolved() throws Exception  {
+        InetSocketAddress socketAddress = InetSocketAddress.createUnresolved("10.0.0.1", 8080);
+        testInitialMessage(socketAddress, "10.0.0.1:8080");
+    }
+
+    private static void testInitialMessage(InetSocketAddress socketAddress, String expected) throws Exception  {
+        InetSocketAddress proxyAddress = new InetSocketAddress(NetUtil.LOCALHOST, 8080);
+
+        ChannelPromise promise = mock(ChannelPromise.class);
+        verifyNoMoreInteractions(promise);
+
+        ChannelHandlerContext ctx = mock(ChannelHandlerContext.class);
+        when(ctx.connect(same(proxyAddress), isNull(InetSocketAddress.class), same(promise))).thenReturn(promise);
+
+        HttpProxyHandler handler = new HttpProxyHandler(new InetSocketAddress(NetUtil.LOCALHOST, 8080));
+        handler.connect(ctx, socketAddress, null, promise);
+
+        FullHttpRequest request = (FullHttpRequest) handler.newInitialMessage(ctx);
+        try {
+            assertEquals(HttpVersion.HTTP_1_1, request.protocolVersion());
+            assertEquals(expected, request.uri());
+            assertEquals(expected, request.headers().get(HttpHeaderNames.HOST));
+        } finally {
+            request.release();
+        }
+        verify(ctx).connect(proxyAddress, null, promise);
+    }
+}