Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixes #5079 - :authority header for IPv6 address not having square br… #5128

Merged
merged 5 commits into from Aug 11, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
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
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should the impl at least call HostPort.normalize so that it works if somebody does call it?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, perhaps add some apidoc text telling developers what the options are now that it's deprecated ...

/**
 * @deprecated use {@link HostPort#normalize(String)} instead.
 */
@Deprecated

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nope, the method should not be used. It's protected so only subclasses could use it, but they should not - there is no need to normalize the host anymore.

I initially followed @gregw suggestion to call HostPort.normalizeHost(), but actually this method was doing exactly the opposite. So either restore the old behavior, or keep this new one that does nothing.

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,21 +37,31 @@
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.toolchain.test.Net;
import org.eclipse.jetty.util.BufferUtil;
import org.eclipse.jetty.util.Callback;
import org.eclipse.jetty.util.Utf8StringBuilder;
import org.eclipse.jetty.util.ssl.SslContextFactory;
import org.eclipse.jetty.util.thread.QueuedThreadPool;
import org.hamcrest.Matchers;
import org.junit.jupiter.api.AfterEach;
import org.junit.jupiter.api.Assumptions;
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 +87,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 +113,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 +180,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 +200,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 +223,83 @@ public void onFillable()
httpClient.stop();
}
}

@ParameterizedTest
@ValueSource(strings = {"::2", "[::3]"})
public void testIPv6WithXForwardedForHeader(String ipv6) throws Exception
{
Assumptions.assumeTrue(Net.isIpv6InterfaceAvailable());

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
{
Assumptions.assumeTrue(Net.isIpv6InterfaceAvailable());

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 @@ -59,11 +59,13 @@
import org.eclipse.jetty.server.ServerConnector;
import org.eclipse.jetty.server.handler.AbstractHandler;
import org.eclipse.jetty.toolchain.test.MavenTestingUtils;
import org.eclipse.jetty.toolchain.test.Net;
import org.eclipse.jetty.util.Promise;
import org.eclipse.jetty.util.ssl.SslContextFactory;
import org.eclipse.jetty.util.thread.QueuedThreadPool;
import org.hamcrest.Matchers;
import org.junit.jupiter.api.AfterEach;
import org.junit.jupiter.api.Assumptions;
import org.junit.jupiter.api.Disabled;
import org.junit.jupiter.api.Tag;
import org.junit.jupiter.api.Test;
Expand Down Expand Up @@ -463,6 +465,36 @@ protected void handleConnect(Request baseRequest, HttpServletRequest request, Ht
httpClient.stop();
}

@ParameterizedTest
@MethodSource("proxyTLS")
public void testIPv6(SslContextFactory.Server proxyTLS) throws Exception
{
Assumptions.assumeTrue(Net.isIpv6InterfaceAvailable());

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