HttpProxyHandler generates invalid CONNECT url and Host header when address is resolved
Motivation: HttpProxyHandler uses `NetUtil#toSocketAddressString` to compute CONNECT url and Host header. The url is correct when the address is unresolved, as `NetUtil#toSocketAddressString` will then use `getHoststring`/`getHostname`. If the address is already resolved, the url will be based on the IP instead of the hostname. There’s an additional minor issue with the Host header: default port 443 should be omitted. Modifications: * Introduce NetUtil#getHostname * Introduce HttpUtil#formatHostnameForHttp to format an InetSocketAddress to HTTP format * Change url computation to favor hostname instead of IP * Introduce HttpProxyHandler ignoreDefaultPortsInConnectHostHeader parameter to ignore 80 and 443 ports in Host header Result: HttpProxyHandler performs properly when connecting to a resolved address
This commit is contained in:
parent
fc3b145cbb
commit
d60cd0231d
@ -17,7 +17,9 @@ package io.netty.handler.codec.http;
|
||||
|
||||
import io.netty.util.AsciiString;
|
||||
import io.netty.util.CharsetUtil;
|
||||
import io.netty.util.NetUtil;
|
||||
|
||||
import java.net.InetSocketAddress;
|
||||
import java.net.URI;
|
||||
import java.util.ArrayList;
|
||||
import java.nio.charset.Charset;
|
||||
@ -511,4 +513,21 @@ public final class HttpUtil {
|
||||
return contentTypeValue.length() > 0 ? contentTypeValue : null;
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Formats the host string of an address so it can be used for computing an HTTP component
|
||||
* such as an URL or a Host header
|
||||
* @param addr the address
|
||||
* @return the formatted String
|
||||
*/
|
||||
public static String formatHostnameForHttp(InetSocketAddress addr) {
|
||||
String hostString = NetUtil.getHostname(addr);
|
||||
if (NetUtil.isValidIpV6Address(hostString)) {
|
||||
if (!addr.isUnresolved()) {
|
||||
hostString = NetUtil.toAddressString(addr.getAddress());
|
||||
}
|
||||
return "[" + hostString + "]";
|
||||
}
|
||||
return hostString;
|
||||
}
|
||||
}
|
||||
|
@ -19,13 +19,14 @@ import io.netty.util.CharsetUtil;
|
||||
import io.netty.util.ReferenceCountUtil;
|
||||
import org.junit.Test;
|
||||
|
||||
import java.net.InetAddress;
|
||||
import java.net.InetSocketAddress;
|
||||
import java.nio.charset.StandardCharsets;
|
||||
import java.util.ArrayList;
|
||||
import java.util.Collections;
|
||||
import java.util.List;
|
||||
|
||||
import static io.netty.handler.codec.http.HttpHeadersTestUtils.of;
|
||||
import static org.hamcrest.Matchers.containsString;
|
||||
import static org.hamcrest.Matchers.hasToString;
|
||||
import static org.junit.Assert.assertEquals;
|
||||
import static org.junit.Assert.assertFalse;
|
||||
@ -256,4 +257,39 @@ public class HttpUtilTest {
|
||||
ReferenceCountUtil.release(message);
|
||||
}
|
||||
|
||||
@Test
|
||||
public void testFormatHostnameForHttpFromResolvedAddressWithHostname() throws Exception {
|
||||
InetSocketAddress socketAddress = new InetSocketAddress(InetAddress.getByName("localhost"), 8080);
|
||||
assertEquals("localhost", HttpUtil.formatHostnameForHttp(socketAddress));
|
||||
}
|
||||
|
||||
@Test
|
||||
public void testFormatHostnameForHttpFromUnesolvedAddressWithHostname() {
|
||||
InetSocketAddress socketAddress = InetSocketAddress.createUnresolved("localhost", 80);
|
||||
assertEquals("localhost", HttpUtil.formatHostnameForHttp(socketAddress));
|
||||
}
|
||||
|
||||
@Test
|
||||
public void testIpv6() throws Exception {
|
||||
InetSocketAddress socketAddress = new InetSocketAddress(InetAddress.getByName("::1"), 8080);
|
||||
assertEquals("[::1]", HttpUtil.formatHostnameForHttp(socketAddress));
|
||||
}
|
||||
|
||||
@Test
|
||||
public void testIpv6Unresolved() {
|
||||
InetSocketAddress socketAddress = InetSocketAddress.createUnresolved("::1", 8080);
|
||||
assertEquals("[::1]", HttpUtil.formatHostnameForHttp(socketAddress));
|
||||
}
|
||||
|
||||
@Test
|
||||
public void testIpv4() throws Exception {
|
||||
InetSocketAddress socketAddress = new InetSocketAddress(InetAddress.getByName("10.0.0.1"), 8080);
|
||||
assertEquals("10.0.0.1", HttpUtil.formatHostnameForHttp(socketAddress));
|
||||
}
|
||||
|
||||
@Test
|
||||
public void testIpv4Unresolved() {
|
||||
InetSocketAddress socketAddress = InetSocketAddress.createUnresolved("10.0.0.1", 8080);
|
||||
assertEquals("10.0.0.1", HttpUtil.formatHostnameForHttp(socketAddress));
|
||||
}
|
||||
}
|
||||
|
@ -897,8 +897,8 @@ public final class NetUtil {
|
||||
final StringBuilder sb;
|
||||
|
||||
if (addr.isUnresolved()) {
|
||||
String hostString = PlatformDependent.javaVersion() >= 7 ? addr.getHostString() : addr.getHostName();
|
||||
sb = newSocketAddressStringBuilder(hostString, port, !isValidIpV6Address(hostString));
|
||||
String hostname = getHostname(addr);
|
||||
sb = newSocketAddressStringBuilder(hostname, port, !isValidIpV6Address(hostname));
|
||||
} else {
|
||||
InetAddress address = addr.getAddress();
|
||||
String hostString = toAddressString(address);
|
||||
@ -1068,6 +1068,16 @@ public final class NetUtil {
|
||||
return b.toString();
|
||||
}
|
||||
|
||||
/**
|
||||
* Returns {@link InetSocketAddress#getHostString()} if Java >= 7,
|
||||
* or {@link InetSocketAddress#getHostName()} otherwise.
|
||||
* @param addr The address
|
||||
* @return the host string
|
||||
*/
|
||||
public static String getHostname(InetSocketAddress addr) {
|
||||
return PlatformDependent.javaVersion() >= 7 ? addr.getHostString() : addr.getHostName();
|
||||
}
|
||||
|
||||
/**
|
||||
* Does a range check on {@code value} if is within {@code start} (inclusive) and {@code end} (exclusive).
|
||||
* @param value The value to checked if is within {@code start} (inclusive) and {@code end} (exclusive)
|
||||
|
@ -29,11 +29,11 @@ import io.netty.handler.codec.http.HttpHeaders;
|
||||
import io.netty.handler.codec.http.HttpMethod;
|
||||
import io.netty.handler.codec.http.HttpResponse;
|
||||
import io.netty.handler.codec.http.HttpResponseStatus;
|
||||
import io.netty.handler.codec.http.HttpUtil;
|
||||
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;
|
||||
@ -47,6 +47,7 @@ public final class HttpProxyHandler extends ProxyHandler {
|
||||
private final String username;
|
||||
private final String password;
|
||||
private final CharSequence authorization;
|
||||
private final boolean ignoreDefaultPortsInConnectHostHeader;
|
||||
private HttpResponseStatus status;
|
||||
private HttpHeaders headers;
|
||||
|
||||
@ -55,11 +56,18 @@ public final class HttpProxyHandler extends ProxyHandler {
|
||||
}
|
||||
|
||||
public HttpProxyHandler(SocketAddress proxyAddress, HttpHeaders headers) {
|
||||
this(proxyAddress, headers, false);
|
||||
}
|
||||
|
||||
public HttpProxyHandler(SocketAddress proxyAddress,
|
||||
HttpHeaders headers,
|
||||
boolean ignoreDefaultPortsInConnectHostHeader) {
|
||||
super(proxyAddress);
|
||||
username = null;
|
||||
password = null;
|
||||
authorization = null;
|
||||
this.headers = headers;
|
||||
this.ignoreDefaultPortsInConnectHostHeader = ignoreDefaultPortsInConnectHostHeader;
|
||||
}
|
||||
|
||||
public HttpProxyHandler(SocketAddress proxyAddress, String username, String password) {
|
||||
@ -68,6 +76,14 @@ public final class HttpProxyHandler extends ProxyHandler {
|
||||
|
||||
public HttpProxyHandler(SocketAddress proxyAddress, String username, String password,
|
||||
HttpHeaders headers) {
|
||||
this(proxyAddress, username, password, headers, false);
|
||||
}
|
||||
|
||||
public HttpProxyHandler(SocketAddress proxyAddress,
|
||||
String username,
|
||||
String password,
|
||||
HttpHeaders headers,
|
||||
boolean ignoreDefaultPortsInConnectHostHeader) {
|
||||
super(proxyAddress);
|
||||
if (username == null) {
|
||||
throw new NullPointerException("username");
|
||||
@ -87,6 +103,7 @@ public final class HttpProxyHandler extends ProxyHandler {
|
||||
authzBase64.release();
|
||||
|
||||
this.headers = headers;
|
||||
this.ignoreDefaultPortsInConnectHostHeader = ignoreDefaultPortsInConnectHostHeader;
|
||||
}
|
||||
|
||||
@Override
|
||||
@ -127,13 +144,20 @@ public final class HttpProxyHandler extends ProxyHandler {
|
||||
@Override
|
||||
protected Object newInitialMessage(ChannelHandlerContext ctx) throws Exception {
|
||||
InetSocketAddress raddr = destinationAddress();
|
||||
final String host = NetUtil.toSocketAddressString(raddr);
|
||||
|
||||
String hostString = HttpUtil.formatHostnameForHttp(raddr);
|
||||
int port = raddr.getPort();
|
||||
String url = hostString + ":" + port;
|
||||
String hostHeader = (ignoreDefaultPortsInConnectHostHeader && (port == 80 || port == 443)) ?
|
||||
hostString :
|
||||
url;
|
||||
|
||||
FullHttpRequest req = new DefaultFullHttpRequest(
|
||||
HttpVersion.HTTP_1_1, HttpMethod.CONNECT,
|
||||
host,
|
||||
url,
|
||||
Unpooled.EMPTY_BUFFER, false);
|
||||
|
||||
req.headers().set(HttpHeaderNames.HOST, host);
|
||||
req.headers().set(HttpHeaderNames.HOST, hostHeader);
|
||||
|
||||
if (authorization != null) {
|
||||
req.headers().set(HttpHeaderNames.PROXY_AUTHORIZATION, authorization);
|
||||
|
@ -34,39 +34,130 @@ import static org.mockito.Mockito.*;
|
||||
public class HttpProxyHandlerTest {
|
||||
|
||||
@Test
|
||||
public void testIpv6() throws Exception {
|
||||
public void testHostname() throws Exception {
|
||||
InetSocketAddress socketAddress = new InetSocketAddress(InetAddress.getByName("localhost"), 8080);
|
||||
testInitialMessage(
|
||||
socketAddress,
|
||||
"localhost:8080",
|
||||
"localhost:8080",
|
||||
null,
|
||||
true);
|
||||
}
|
||||
|
||||
@Test
|
||||
public void testHostnameUnresolved() throws Exception {
|
||||
InetSocketAddress socketAddress = InetSocketAddress.createUnresolved("localhost", 8080);
|
||||
testInitialMessage(
|
||||
socketAddress,
|
||||
"localhost:8080",
|
||||
"localhost:8080",
|
||||
null,
|
||||
true);
|
||||
}
|
||||
|
||||
@Test
|
||||
public void testHostHeaderWithHttpDefaultPort() throws Exception {
|
||||
InetSocketAddress socketAddress = new InetSocketAddress(InetAddress.getByName("localhost"), 80);
|
||||
testInitialMessage(socketAddress,
|
||||
"localhost:80",
|
||||
"localhost:80", null,
|
||||
false);
|
||||
}
|
||||
|
||||
@Test
|
||||
public void testHostHeaderWithHttpDefaultPortIgnored() throws Exception {
|
||||
InetSocketAddress socketAddress = InetSocketAddress.createUnresolved("localhost", 80);
|
||||
testInitialMessage(
|
||||
socketAddress,
|
||||
"localhost:80",
|
||||
"localhost",
|
||||
null,
|
||||
true);
|
||||
}
|
||||
|
||||
@Test
|
||||
public void testHostHeaderWithHttpsDefaultPort() throws Exception {
|
||||
InetSocketAddress socketAddress = new InetSocketAddress(InetAddress.getByName("localhost"), 443);
|
||||
testInitialMessage(
|
||||
socketAddress,
|
||||
"localhost:443",
|
||||
"localhost:443",
|
||||
null,
|
||||
false);
|
||||
}
|
||||
|
||||
@Test
|
||||
public void testHostHeaderWithHttpsDefaultPortIgnored() throws Exception {
|
||||
InetSocketAddress socketAddress = InetSocketAddress.createUnresolved("localhost", 443);
|
||||
testInitialMessage(
|
||||
socketAddress,
|
||||
"localhost:443",
|
||||
"localhost",
|
||||
null,
|
||||
true);
|
||||
}
|
||||
|
||||
@Test
|
||||
public void testIpv6() throws Exception {
|
||||
InetSocketAddress socketAddress = new InetSocketAddress(InetAddress.getByName("::1"), 8080);
|
||||
testInitialMessage(socketAddress, "[::1]:8080", null);
|
||||
testInitialMessage(
|
||||
socketAddress,
|
||||
"[::1]:8080",
|
||||
"[::1]:8080",
|
||||
null,
|
||||
true);
|
||||
}
|
||||
|
||||
@Test
|
||||
public void testIpv6Unresolved() throws Exception {
|
||||
public void testIpv6Unresolved() throws Exception {
|
||||
InetSocketAddress socketAddress = InetSocketAddress.createUnresolved("::1", 8080);
|
||||
testInitialMessage(socketAddress, "[::1]:8080", null);
|
||||
testInitialMessage(
|
||||
socketAddress,
|
||||
"[::1]:8080",
|
||||
"[::1]:8080",
|
||||
null,
|
||||
true);
|
||||
}
|
||||
|
||||
@Test
|
||||
public void testIpv4() throws Exception {
|
||||
public void testIpv4() throws Exception {
|
||||
InetSocketAddress socketAddress = new InetSocketAddress(InetAddress.getByName("10.0.0.1"), 8080);
|
||||
testInitialMessage(socketAddress, "10.0.0.1:8080", null);
|
||||
testInitialMessage(socketAddress,
|
||||
"10.0.0.1:8080",
|
||||
"10.0.0.1:8080",
|
||||
null,
|
||||
true);
|
||||
}
|
||||
|
||||
@Test
|
||||
public void testIpv4Unresolved() throws Exception {
|
||||
public void testIpv4Unresolved() throws Exception {
|
||||
InetSocketAddress socketAddress = InetSocketAddress.createUnresolved("10.0.0.1", 8080);
|
||||
testInitialMessage(socketAddress, "10.0.0.1:8080", null);
|
||||
testInitialMessage(
|
||||
socketAddress,
|
||||
"10.0.0.1:8080",
|
||||
"10.0.0.1:8080",
|
||||
null,
|
||||
true);
|
||||
}
|
||||
|
||||
@Test
|
||||
public void testCustomHeaders() throws Exception {
|
||||
InetSocketAddress socketAddress = InetSocketAddress.createUnresolved("10.0.0.1", 8080);
|
||||
testInitialMessage(socketAddress, "10.0.0.1:8080",
|
||||
new DefaultHttpHeaders().add("CUSTOM_HEADER", "CUSTOM_VALUE1")
|
||||
.add("CUSTOM_HEADER", "CUSTOM_VALUE2"));
|
||||
testInitialMessage(
|
||||
socketAddress,
|
||||
"10.0.0.1:8080",
|
||||
"10.0.0.1:8080",
|
||||
new DefaultHttpHeaders()
|
||||
.add("CUSTOM_HEADER", "CUSTOM_VALUE1")
|
||||
.add("CUSTOM_HEADER", "CUSTOM_VALUE2"),
|
||||
true);
|
||||
}
|
||||
|
||||
private static void testInitialMessage(InetSocketAddress socketAddress, String hostPort,
|
||||
HttpHeaders headers) throws Exception {
|
||||
private static void testInitialMessage(InetSocketAddress socketAddress,
|
||||
String expectedUrl,
|
||||
String expectedHostHeader,
|
||||
HttpHeaders headers,
|
||||
boolean ignoreDefaultPortsInConnectHostHeader) throws Exception {
|
||||
InetSocketAddress proxyAddress = new InetSocketAddress(NetUtil.LOCALHOST, 8080);
|
||||
|
||||
ChannelPromise promise = mock(ChannelPromise.class);
|
||||
@ -75,15 +166,18 @@ public class HttpProxyHandlerTest {
|
||||
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), headers);
|
||||
HttpProxyHandler handler = new HttpProxyHandler(
|
||||
new InetSocketAddress(NetUtil.LOCALHOST, 8080),
|
||||
headers,
|
||||
ignoreDefaultPortsInConnectHostHeader);
|
||||
handler.connect(ctx, socketAddress, null, promise);
|
||||
|
||||
FullHttpRequest request = (FullHttpRequest) handler.newInitialMessage(ctx);
|
||||
try {
|
||||
assertEquals(HttpVersion.HTTP_1_1, request.protocolVersion());
|
||||
assertEquals(hostPort, request.uri());
|
||||
assertEquals(expectedUrl, request.uri());
|
||||
HttpHeaders actualHeaders = request.headers();
|
||||
assertEquals(hostPort, actualHeaders.get(HttpHeaderNames.HOST));
|
||||
assertEquals(expectedHostHeader, actualHeaders.get(HttpHeaderNames.HOST));
|
||||
|
||||
if (headers != null) {
|
||||
// The actual request header is a strict superset of the custom header
|
||||
|
Loading…
Reference in New Issue
Block a user