Skip to content

Commit

Permalink
Fixes #5079 - :authority header for IPv6 address not having square br…
Browse files Browse the repository at this point in the history
…ackets.

On the client:
* Origin.Address.host is passed through HostPort.normalizeHost(),
so that if it is IPv6 is bracketed.
Now the ipv6 address passed to an `HttClient` request is bracketed.
* HttpRequest was de-bracketing the host, but now it does not anymore.

On the server:
* Request.getLocalAddr(), getLocalName(), getRemoteAddr(),
getRemoteHost(), getServerName(), when dealing with an IPv6 address,
return it bracketed.
The reason to return bracketed IPv6 also from *Addr() methods is that
if it is used with InetAddress/InetSocketAddress it still works, but
often it is interpreted as a URI host so brackets are necessary.
* DoSFilter was blindly bracketing - now it does not.

Added a number of test cases, and fixed those that expected
non-bracketed IPv6.

Signed-off-by: Simone Bordet <simone.bordet@gmail.com>
  • Loading branch information
sbordet committed Aug 7, 2020
1 parent 1f14dfa commit d53d9d8
Show file tree
Hide file tree
Showing 17 changed files with 303 additions and 74 deletions.
Expand Up @@ -27,6 +27,7 @@
import org.eclipse.jetty.util.component.LifeCycle;
import org.junit.jupiter.api.AfterEach;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Tag;
import org.junit.jupiter.api.Test;

import static org.hamcrest.MatcherAssert.assertThat;
Expand Down Expand Up @@ -54,8 +55,10 @@ public void stopServer() throws Exception
}

@Test
@Tag("external")
public void testGetAsyncRest() throws Exception
{
// The async rest webapp forwards the call to ebay.com.
URI uri = server.getURI().resolve("/testAsync?items=mouse,beer,gnome");
ContentResponse response = client.newRequest(uri)
.method(HttpMethod.GET)
Expand Down
Expand Up @@ -1155,10 +1155,14 @@ protected HttpField getAcceptEncodingField()
return encodingField;
}

/**
* @param host the host to normalize
* @return the host itself
* @deprecated no replacement, do not use it
*/
@Deprecated
protected String normalizeHost(String host)
{
if (host != null && host.startsWith("[") && host.endsWith("]"))
return host.substring(1, host.length() - 1);
return host;
}

Expand Down
Expand Up @@ -95,7 +95,7 @@ protected HttpRequest(HttpClient client, HttpConversation conversation, URI uri)
this.client = client;
this.conversation = conversation;
scheme = uri.getScheme();
host = client.normalizeHost(uri.getHost());
host = uri.getHost();
port = HttpClient.normalizePort(scheme, uri.getPort());
path = uri.getRawPath();
query = uri.getRawQuery();
Expand Down
Expand Up @@ -20,6 +20,7 @@

import java.util.Objects;

import org.eclipse.jetty.util.HostPort;
import org.eclipse.jetty.util.URIUtil;

public class Origin
Expand Down Expand Up @@ -107,7 +108,7 @@ public static class Address

public Address(String host, int port)
{
this.host = Objects.requireNonNull(host);
this.host = HostPort.normalizeHost(Objects.requireNonNull(host));
this.port = port;
}

Expand Down
Expand Up @@ -1623,29 +1623,32 @@ public void testCONNECTWithHTTP10(Scenario scenario) throws Exception

@ParameterizedTest
@ArgumentsSource(ScenarioProvider.class)
public void testIPv6Host(Scenario scenario) throws Exception
public void testIPv6HostWithHTTP10(Scenario scenario) throws Exception
{
Assumptions.assumeTrue(Net.isIpv6InterfaceAvailable());
start(scenario, new AbstractHandler()
start(scenario, new EmptyServerHandler()
{
@Override
public void handle(String target, org.eclipse.jetty.server.Request baseRequest, HttpServletRequest request, HttpServletResponse response) throws IOException
protected void service(String target, org.eclipse.jetty.server.Request jettyRequest, HttpServletRequest request, HttpServletResponse response) throws IOException
{
baseRequest.setHandled(true);
response.setContentType("text/plain");
response.getOutputStream().print(request.getHeader("Host"));
response.getOutputStream().print(request.getServerName());
}
});

URI uri = URI.create(scenario.getScheme() + "://[::1]:" + connector.getLocalPort() + "/path");
ContentResponse response = client.newRequest(uri)
.method(HttpMethod.PUT)
.version(HttpVersion.HTTP_1_0)
.onRequestBegin(r -> r.getHeaders().remove(HttpHeader.HOST))
.timeout(5, TimeUnit.SECONDS)
.send();

assertNotNull(response);
assertEquals(200, response.getStatus());
assertThat(new String(response.getContent(), StandardCharsets.ISO_8859_1), Matchers.startsWith("[::1]:"));
String content = response.getContentAsString();
assertThat(content, Matchers.startsWith("["));
assertThat(content, Matchers.endsWith(":1]"));
}

@ParameterizedTest
Expand Down
Expand Up @@ -67,8 +67,10 @@ public void testIPv6Host(Scenario scenario) throws Exception
Assumptions.assumeTrue(Net.isIpv6InterfaceAvailable());
start(scenario, new EmptyServerHandler());

String host = "::1";
Request request = client.newRequest(host, connector.getLocalPort())
String hostAddress = "::1";
String host = "[" + hostAddress + "]";
// Explicitly use a non-bracketed IPv6 host.
Request request = client.newRequest(hostAddress, connector.getLocalPort())
.scheme(scenario.getScheme())
.timeout(5, TimeUnit.SECONDS);

Expand Down
@@ -0,0 +1,41 @@
//
// ========================================================================
// Copyright (c) 1995-2020 Mort Bay Consulting Pty Ltd and others.
// ------------------------------------------------------------------------
// All rights reserved. This program and the accompanying materials
// are made available under the terms of the Eclipse Public License v1.0
// and Apache License v2.0 which accompanies this distribution.
//
// The Eclipse Public License is available at
// http://www.eclipse.org/legal/epl-v10.html
//
// The Apache License v2.0 is available at
// http://www.opensource.org/licenses/apache2.0.php
//
// You may elect to redistribute this code under either of these licenses.
// ========================================================================
//

package org.eclipse.jetty.proxy;

import java.io.IOException;
import javax.servlet.ServletException;
import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpServletResponse;

import org.eclipse.jetty.server.Request;
import org.eclipse.jetty.server.handler.AbstractHandler;

public class EmptyServerHandler extends AbstractHandler
{
@Override
public final void handle(String target, Request jettyRequest, HttpServletRequest request, HttpServletResponse response) throws IOException, ServletException
{
jettyRequest.setHandled(true);
service(target, jettyRequest, request, response);
}

protected void service(String target, Request jettyRequest, HttpServletRequest request, HttpServletResponse response) throws IOException, ServletException
{
}
}
Expand Up @@ -21,10 +21,14 @@
import java.nio.ByteBuffer;
import java.nio.charset.StandardCharsets;
import java.util.stream.Stream;
import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpServletResponse;

import org.eclipse.jetty.client.HttpClient;
import org.eclipse.jetty.client.HttpProxy;
import org.eclipse.jetty.client.api.ContentResponse;
import org.eclipse.jetty.client.api.Request;
import org.eclipse.jetty.http.HttpHeader;
import org.eclipse.jetty.http.HttpMethod;
import org.eclipse.jetty.http.HttpStatus;
import org.eclipse.jetty.io.AbstractConnection;
Expand All @@ -33,9 +37,14 @@
import org.eclipse.jetty.server.AbstractConnectionFactory;
import org.eclipse.jetty.server.ConnectionFactory;
import org.eclipse.jetty.server.Connector;
import org.eclipse.jetty.server.ForwardedRequestCustomizer;
import org.eclipse.jetty.server.Handler;
import org.eclipse.jetty.server.HttpConfiguration;
import org.eclipse.jetty.server.HttpConnectionFactory;
import org.eclipse.jetty.server.Server;
import org.eclipse.jetty.server.ServerConnector;
import org.eclipse.jetty.servlet.ServletContextHandler;
import org.eclipse.jetty.servlet.ServletHolder;
import org.eclipse.jetty.toolchain.test.MavenTestingUtils;
import org.eclipse.jetty.util.BufferUtil;
import org.eclipse.jetty.util.Callback;
Expand All @@ -44,10 +53,13 @@
import org.eclipse.jetty.util.thread.QueuedThreadPool;
import org.hamcrest.Matchers;
import org.junit.jupiter.api.AfterEach;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.MethodSource;
import org.junit.jupiter.params.provider.ValueSource;

import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.greaterThan;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertFalse;

Expand All @@ -73,18 +85,19 @@ private static SslContextFactory.Server newServerSslContextFactory()
private Server proxy;
private ServerConnector proxyConnector;

protected void startServer(SslContextFactory.Server serverTLS, ConnectionFactory connectionFactory) throws Exception
protected void startServer(SslContextFactory.Server serverTLS, ConnectionFactory connectionFactory, Handler handler) throws Exception
{
serverSslContextFactory = serverTLS;
QueuedThreadPool serverThreads = new QueuedThreadPool();
serverThreads.setName("server");
server = new Server(serverThreads);
serverConnector = new ServerConnector(server, serverSslContextFactory, connectionFactory);
server.addConnector(serverConnector);
server.setHandler(handler);
server.start();
}

protected void startProxy() throws Exception
protected void startProxy(ProxyServlet proxyServlet) throws Exception
{
QueuedThreadPool proxyThreads = new QueuedThreadPool();
proxyThreads.setName("proxy");
Expand All @@ -98,7 +111,7 @@ protected void startProxy() throws Exception
proxy.setHandler(connectHandler);

ServletContextHandler proxyHandler = new ServletContextHandler(connectHandler, "/");
proxyHandler.addServlet(ProxyServlet.class, "/*");
proxyHandler.addServlet(new ServletHolder(proxyServlet), "/*");

proxy.start();
}
Expand Down Expand Up @@ -165,7 +178,7 @@ public void onFillable()
// the client, and convert it to a relative URI.
// The ConnectHandler won't modify what the client
// sent, which must be a relative URI.
assertThat(request.length(), Matchers.greaterThan(0));
assertThat(request.length(), greaterThan(0));
if (serverSslContextFactory == null)
assertFalse(request.contains("http://"));
else
Expand All @@ -185,8 +198,8 @@ public void onFillable()
}
};
}
});
startProxy();
}, new EmptyServerHandler());
startProxy(new ProxyServlet());

SslContextFactory.Client clientTLS = new SslContextFactory.Client(true);
HttpClient httpClient = new HttpClient(clientTLS);
Expand All @@ -208,4 +221,79 @@ public void onFillable()
httpClient.stop();
}
}

@ParameterizedTest
@ValueSource(strings = {"::2", "[::3]"})
public void testIPv6WithXForwardedForHeader(String ipv6) throws Exception
{
HttpConfiguration httpConfig = new HttpConfiguration();
httpConfig.addCustomizer(new ForwardedRequestCustomizer());
ConnectionFactory http = new HttpConnectionFactory(httpConfig);
startServer(null, http, new EmptyServerHandler()
{
@Override
protected void service(String target, org.eclipse.jetty.server.Request jettyRequest, HttpServletRequest request, HttpServletResponse response)
{
String remoteHost = jettyRequest.getRemoteHost();
assertThat(remoteHost, Matchers.matchesPattern("\\[.+\\]"));
String remoteAddr = jettyRequest.getRemoteAddr();
assertThat(remoteAddr, Matchers.matchesPattern("\\[.+\\]"));
}
});
startProxy(new ProxyServlet()
{
@Override
protected void addProxyHeaders(HttpServletRequest clientRequest, Request proxyRequest)
{
proxyRequest.header(HttpHeader.X_FORWARDED_FOR, ipv6);
}
});

HttpClient httpClient = new HttpClient();
httpClient.getProxyConfiguration().getProxies().add(newHttpProxy());
httpClient.start();

ContentResponse response = httpClient.newRequest("[::1]", serverConnector.getLocalPort())
.scheme("http")
.send();

assertEquals(HttpStatus.OK_200, response.getStatus());
}

@Test
public void testIPv6WithForwardedHeader() throws Exception
{
HttpConfiguration httpConfig = new HttpConfiguration();
httpConfig.addCustomizer(new ForwardedRequestCustomizer());
ConnectionFactory http = new HttpConnectionFactory(httpConfig);
startServer(null, http, new EmptyServerHandler()
{
@Override
protected void service(String target, org.eclipse.jetty.server.Request jettyRequest, HttpServletRequest request, HttpServletResponse response)
{
String remoteHost = jettyRequest.getRemoteHost();
assertThat(remoteHost, Matchers.matchesPattern("\\[.+\\]"));
String remoteAddr = jettyRequest.getRemoteAddr();
assertThat(remoteAddr, Matchers.matchesPattern("\\[.+\\]"));
}
});
startProxy(new ProxyServlet()
{
@Override
protected void addProxyHeaders(HttpServletRequest clientRequest, Request proxyRequest)
{
proxyRequest.header(HttpHeader.FORWARDED, "for=\"[::2]\"");
}
});

HttpClient httpClient = new HttpClient();
httpClient.getProxyConfiguration().getProxies().add(newHttpProxy());
httpClient.start();

ContentResponse response = httpClient.newRequest("[::1]", serverConnector.getLocalPort())
.scheme("http")
.send();

assertEquals(HttpStatus.OK_200, response.getStatus());
}
}
Expand Up @@ -463,6 +463,34 @@ protected void handleConnect(Request baseRequest, HttpServletRequest request, Ht
httpClient.stop();
}

@ParameterizedTest
@MethodSource("proxyTLS")
public void testIPv6(SslContextFactory.Server proxyTLS) throws Exception
{
startTLSServer(new ServerHandler());
startProxy(proxyTLS);

HttpClient httpClient = new HttpClient(newClientSslContextFactory());
HttpProxy httpProxy = new HttpProxy(new Origin.Address("[::1]", proxyConnector.getLocalPort()), proxyTLS != null);
httpClient.getProxyConfiguration().getProxies().add(httpProxy);
httpClient.start();

try
{
ContentResponse response = httpClient.newRequest("[::1]", serverConnector.getLocalPort())
.scheme(HttpScheme.HTTPS.asString())
.path("/echo?body=")
.timeout(5, TimeUnit.SECONDS)
.send();

assertEquals(HttpStatus.OK_200, response.getStatus());
}
finally
{
httpClient.stop();
}
}

@ParameterizedTest
@MethodSource("proxyTLS")
public void testProxyAuthentication(SslContextFactory.Server proxyTLS) throws Exception
Expand Down

0 comments on commit d53d9d8

Please sign in to comment.