From d53d9d8a1db1de93bf823ee59805940ac1a2e4f9 Mon Sep 17 00:00:00 2001 From: Simone Bordet Date: Fri, 7 Aug 2020 15:53:19 +0200 Subject: [PATCH 1/4] Fixes #5079 - :authority header for IPv6 address not having square brackets. 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 --- .../eclipse/jetty/embedded/OneWebAppTest.java | 3 + .../org/eclipse/jetty/client/HttpClient.java | 8 +- .../org/eclipse/jetty/client/HttpRequest.java | 2 +- .../java/org/eclipse/jetty/client/Origin.java | 3 +- .../eclipse/jetty/client/HttpClientTest.java | 15 +-- .../jetty/client/HttpClientURITest.java | 6 +- .../jetty/proxy/EmptyServerHandler.java | 41 +++++++ .../jetty/proxy/ForwardProxyServerTest.java | 100 ++++++++++++++++-- .../proxy/ForwardProxyTLSServerTest.java | 28 +++++ .../org/eclipse/jetty/server/Request.java | 43 +++++--- .../ForwardedRequestCustomizerTest.java | 2 +- .../jetty/server/ProxyConnectionTest.java | 8 +- .../eclipse/jetty/server/ResponseTest.java | 8 +- .../test/resources/jetty-logging.properties | 3 +- .../org/eclipse/jetty/servlets/DoSFilter.java | 8 +- .../java/org/eclipse/jetty/util/HostPort.java | 42 ++++---- .../jetty/http/client/HttpClientTest.java | 57 ++++++++++ 17 files changed, 303 insertions(+), 74 deletions(-) create mode 100644 jetty-proxy/src/test/java/org/eclipse/jetty/proxy/EmptyServerHandler.java diff --git a/examples/embedded/src/test/java/org/eclipse/jetty/embedded/OneWebAppTest.java b/examples/embedded/src/test/java/org/eclipse/jetty/embedded/OneWebAppTest.java index 576ff00b9572..587d4dba1e46 100644 --- a/examples/embedded/src/test/java/org/eclipse/jetty/embedded/OneWebAppTest.java +++ b/examples/embedded/src/test/java/org/eclipse/jetty/embedded/OneWebAppTest.java @@ -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; @@ -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) diff --git a/jetty-client/src/main/java/org/eclipse/jetty/client/HttpClient.java b/jetty-client/src/main/java/org/eclipse/jetty/client/HttpClient.java index 545b13cf4b56..7a5903a46ee7 100644 --- a/jetty-client/src/main/java/org/eclipse/jetty/client/HttpClient.java +++ b/jetty-client/src/main/java/org/eclipse/jetty/client/HttpClient.java @@ -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; } diff --git a/jetty-client/src/main/java/org/eclipse/jetty/client/HttpRequest.java b/jetty-client/src/main/java/org/eclipse/jetty/client/HttpRequest.java index 9e488d5cd4b3..53750ad0f2a6 100644 --- a/jetty-client/src/main/java/org/eclipse/jetty/client/HttpRequest.java +++ b/jetty-client/src/main/java/org/eclipse/jetty/client/HttpRequest.java @@ -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(); diff --git a/jetty-client/src/main/java/org/eclipse/jetty/client/Origin.java b/jetty-client/src/main/java/org/eclipse/jetty/client/Origin.java index ac5e476146af..2c964a952332 100644 --- a/jetty-client/src/main/java/org/eclipse/jetty/client/Origin.java +++ b/jetty-client/src/main/java/org/eclipse/jetty/client/Origin.java @@ -20,6 +20,7 @@ import java.util.Objects; +import org.eclipse.jetty.util.HostPort; import org.eclipse.jetty.util.URIUtil; public class Origin @@ -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; } diff --git a/jetty-client/src/test/java/org/eclipse/jetty/client/HttpClientTest.java b/jetty-client/src/test/java/org/eclipse/jetty/client/HttpClientTest.java index 5b5a42486015..21603d3c8163 100644 --- a/jetty-client/src/test/java/org/eclipse/jetty/client/HttpClientTest.java +++ b/jetty-client/src/test/java/org/eclipse/jetty/client/HttpClientTest.java @@ -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 diff --git a/jetty-client/src/test/java/org/eclipse/jetty/client/HttpClientURITest.java b/jetty-client/src/test/java/org/eclipse/jetty/client/HttpClientURITest.java index 277b141c1820..383327b8c90b 100644 --- a/jetty-client/src/test/java/org/eclipse/jetty/client/HttpClientURITest.java +++ b/jetty-client/src/test/java/org/eclipse/jetty/client/HttpClientURITest.java @@ -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); diff --git a/jetty-proxy/src/test/java/org/eclipse/jetty/proxy/EmptyServerHandler.java b/jetty-proxy/src/test/java/org/eclipse/jetty/proxy/EmptyServerHandler.java new file mode 100644 index 000000000000..995f3b44ff74 --- /dev/null +++ b/jetty-proxy/src/test/java/org/eclipse/jetty/proxy/EmptyServerHandler.java @@ -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 + { + } +} diff --git a/jetty-proxy/src/test/java/org/eclipse/jetty/proxy/ForwardProxyServerTest.java b/jetty-proxy/src/test/java/org/eclipse/jetty/proxy/ForwardProxyServerTest.java index e64e08dc2a0d..bedcefb7b234 100644 --- a/jetty-proxy/src/test/java/org/eclipse/jetty/proxy/ForwardProxyServerTest.java +++ b/jetty-proxy/src/test/java/org/eclipse/jetty/proxy/ForwardProxyServerTest.java @@ -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; @@ -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; @@ -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; @@ -73,7 +85,7 @@ 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(); @@ -81,10 +93,11 @@ protected void startServer(SslContextFactory.Server serverTLS, ConnectionFactory 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"); @@ -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(); } @@ -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 @@ -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); @@ -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()); + } } diff --git a/jetty-proxy/src/test/java/org/eclipse/jetty/proxy/ForwardProxyTLSServerTest.java b/jetty-proxy/src/test/java/org/eclipse/jetty/proxy/ForwardProxyTLSServerTest.java index c4b7d451dde4..c579f265159a 100644 --- a/jetty-proxy/src/test/java/org/eclipse/jetty/proxy/ForwardProxyTLSServerTest.java +++ b/jetty-proxy/src/test/java/org/eclipse/jetty/proxy/ForwardProxyTLSServerTest.java @@ -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 diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/Request.java b/jetty-server/src/main/java/org/eclipse/jetty/server/Request.java index b8dddded7ac3..9417e5c37a33 100644 --- a/jetty-server/src/main/java/org/eclipse/jetty/server/Request.java +++ b/jetty-server/src/main/java/org/eclipse/jetty/server/Request.java @@ -27,6 +27,7 @@ import java.io.UnsupportedEncodingException; import java.net.InetAddress; import java.net.InetSocketAddress; +import java.net.UnknownHostException; import java.nio.charset.Charset; import java.nio.charset.StandardCharsets; import java.nio.charset.UnsupportedCharsetException; @@ -83,6 +84,7 @@ import org.eclipse.jetty.server.session.SessionHandler; import org.eclipse.jetty.util.Attributes; import org.eclipse.jetty.util.AttributesMap; +import org.eclipse.jetty.util.HostPort; import org.eclipse.jetty.util.IO; import org.eclipse.jetty.util.MultiMap; import org.eclipse.jetty.util.StringUtil; @@ -984,11 +986,12 @@ public String getLocalAddr() String name = InetAddress.getLocalHost().getHostAddress(); if (StringUtil.ALL_INTERFACES.equals(name)) return null; - return name; + return HostPort.normalizeHost(name); } - catch (java.net.UnknownHostException e) + catch (UnknownHostException e) { LOG.ignore(e); + return null; } } @@ -996,9 +999,10 @@ public String getLocalAddr() if (local == null) return ""; InetAddress address = local.getAddress(); - if (address == null) - return local.getHostString(); - return address.getHostAddress(); + String result = address == null + ? local.getHostString() + : address.getHostAddress(); + return HostPort.normalizeHost(result); } /* @@ -1011,7 +1015,7 @@ public String getLocalName() { InetSocketAddress local = _channel.getLocalAddress(); if (local != null) - return local.getHostString(); + return HostPort.normalizeHost(local.getHostString()); } try @@ -1019,9 +1023,9 @@ public String getLocalName() String name = InetAddress.getLocalHost().getHostName(); if (StringUtil.ALL_INTERFACES.equals(name)) return null; - return name; + return HostPort.normalizeHost(name); } - catch (java.net.UnknownHostException e) + catch (UnknownHostException e) { LOG.ignore(e); } @@ -1236,15 +1240,17 @@ public String getRemoteAddr() InetSocketAddress remote = _remote; if (remote == null) remote = _channel.getRemoteAddress(); - if (remote == null) return ""; InetAddress address = remote.getAddress(); - if (address == null) - return remote.getHostString(); - - return address.getHostAddress(); + String result = address == null + ? remote.getHostString() + : address.getHostAddress(); + // Add IPv6 brackets if necessary, to be consistent + // with cases where _remote has been built from other + // sources such as forward headers or PROXY protocol. + return HostPort.normalizeHost(result); } /* @@ -1256,7 +1262,10 @@ public String getRemoteHost() InetSocketAddress remote = _remote; if (remote == null) remote = _channel.getRemoteAddress(); - return remote == null ? "" : remote.getHostString(); + if (remote == null) + return ""; + // We want the URI host, so add IPv6 brackets if necessary. + return HostPort.normalizeHost(remote.getHostString()); } /* @@ -1411,14 +1420,14 @@ private String findServerName() // Return host from connection String name = getLocalName(); if (name != null) - return name; + return HostPort.normalizeHost(name); // Return the local host try { - return InetAddress.getLocalHost().getHostAddress(); + return HostPort.normalizeHost(InetAddress.getLocalHost().getHostAddress()); } - catch (java.net.UnknownHostException e) + catch (UnknownHostException e) { LOG.ignore(e); } diff --git a/jetty-server/src/test/java/org/eclipse/jetty/server/ForwardedRequestCustomizerTest.java b/jetty-server/src/test/java/org/eclipse/jetty/server/ForwardedRequestCustomizerTest.java index a55fcf82ef46..6c477eb10ac1 100644 --- a/jetty-server/src/test/java/org/eclipse/jetty/server/ForwardedRequestCustomizerTest.java +++ b/jetty-server/src/test/java/org/eclipse/jetty/server/ForwardedRequestCustomizerTest.java @@ -703,7 +703,7 @@ private static class Expectations implements Consumer public void accept(Actual actual) { assertThat("scheme", actual.scheme.get(), is(expectedScheme)); - if (actual.scheme.equals("https")) + if (actual.scheme.get().equals("https")) { assertTrue(actual.wasSecure.get(), "wasSecure"); } diff --git a/jetty-server/src/test/java/org/eclipse/jetty/server/ProxyConnectionTest.java b/jetty-server/src/test/java/org/eclipse/jetty/server/ProxyConnectionTest.java index 92908ed8f343..13af2c9052d6 100644 --- a/jetty-server/src/test/java/org/eclipse/jetty/server/ProxyConnectionTest.java +++ b/jetty-server/src/test/java/org/eclipse/jetty/server/ProxyConnectionTest.java @@ -109,8 +109,8 @@ public void testIPv6(RequestProcessor p) throws Exception assertThat(response, Matchers.containsString("HTTP/1.1 200")); assertThat(response, Matchers.containsString("pathInfo=/path")); - assertThat(response, Matchers.containsString("remote=eeee:eeee:eeee:eeee:eeee:eeee:eeee:eeee:65535")); - assertThat(response, Matchers.containsString("local=ffff:ffff:ffff:ffff:ffff:ffff:ffff:ffff:65535")); + assertThat(response, Matchers.containsString("remote=[eeee:eeee:eeee:eeee:eeee:eeee:eeee:eeee]:65535")); + assertThat(response, Matchers.containsString("local=[ffff:ffff:ffff:ffff:ffff:ffff:ffff:ffff]:65535")); } @ParameterizedTest @@ -147,8 +147,8 @@ public void testIPv6V2(RequestProcessor p) throws Exception assertThat(response, Matchers.containsString("HTTP/1.1 200")); assertThat(response, Matchers.containsString("pathInfo=/path")); - assertThat(response, Matchers.containsString("local=eeee:eeee:eeee:eeee:eeee:eeee:eeee:eeee:8080")); - assertThat(response, Matchers.containsString("remote=ffff:ffff:ffff:ffff:ffff:ffff:ffff:ffff:12345")); + assertThat(response, Matchers.containsString("local=[eeee:eeee:eeee:eeee:eeee:eeee:eeee:eeee]:8080")); + assertThat(response, Matchers.containsString("remote=[ffff:ffff:ffff:ffff:ffff:ffff:ffff:ffff]:12345")); } @ParameterizedTest diff --git a/jetty-server/src/test/java/org/eclipse/jetty/server/ResponseTest.java b/jetty-server/src/test/java/org/eclipse/jetty/server/ResponseTest.java index b6282ae10e7f..1cabd60b38db 100644 --- a/jetty-server/src/test/java/org/eclipse/jetty/server/ResponseTest.java +++ b/jetty-server/src/test/java/org/eclipse/jetty/server/ResponseTest.java @@ -813,7 +813,7 @@ public void testSendRedirect() }; int[] ports = new int[]{8080, 80}; - String[] hosts = new String[]{null, "myhost", "192.168.0.1", "0::1"}; + String[] hosts = new String[]{null, "myhost", "192.168.0.1", "[0::1]"}; for (int port : ports) { for (String host : hosts) @@ -850,7 +850,7 @@ public void testSendRedirect() String location = response.getHeader("Location"); String expected = tests[i][1] - .replace("@HOST@", host == null ? request.getLocalAddr() : (host.contains(":") ? ("[" + host + "]") : host)) + .replace("@HOST@", host == null ? request.getLocalAddr() : host) .replace("@PORT@", host == null ? ":8888" : (port == 80 ? "" : (":" + port))); assertEquals(expected, location, "test-" + i + " " + host + ":" + port); } @@ -887,7 +887,7 @@ public void testSendRedirectRelative() }; int[] ports = new int[]{8080, 80}; - String[] hosts = new String[]{null, "myhost", "192.168.0.1", "0::1"}; + String[] hosts = new String[]{null, "myhost", "192.168.0.1", "[0::1]"}; for (int port : ports) { for (String host : hosts) @@ -925,7 +925,7 @@ public void testSendRedirectRelative() String location = response.getHeader("Location"); String expected = tests[i][1] - .replace("@HOST@", host == null ? request.getLocalAddr() : (host.contains(":") ? ("[" + host + "]") : host)) + .replace("@HOST@", host == null ? request.getLocalAddr() : host) .replace("@PORT@", host == null ? ":8888" : (port == 80 ? "" : (":" + port))); assertEquals(expected, location, "test-" + i + " " + host + ":" + port); } diff --git a/jetty-servlet/src/test/resources/jetty-logging.properties b/jetty-servlet/src/test/resources/jetty-logging.properties index 37f092141fc4..f89e3ab36fa8 100644 --- a/jetty-servlet/src/test/resources/jetty-logging.properties +++ b/jetty-servlet/src/test/resources/jetty-logging.properties @@ -1,8 +1,7 @@ org.eclipse.jetty.util.log.class=org.eclipse.jetty.util.log.StdErrLog -org.eclipse.jetty.LEVEL=INFO #org.eclipse.jetty.LEVEL=DEBUG #org.eclipse.jetty.server.LEVEL=DEBUG #org.eclipse.jetty.servlet.LEVEL=DEBUG #org.eclipse.jetty.io.ChannelEndPoint.LEVEL=DEBUG #org.eclipse.jetty.server.DebugListener.LEVEL=DEBUG -#org.eclipse.jetty.server.HttpChannelState.LEVEL=DEBUG \ No newline at end of file +#org.eclipse.jetty.server.HttpChannelState.LEVEL=DEBUG diff --git a/jetty-servlets/src/main/java/org/eclipse/jetty/servlets/DoSFilter.java b/jetty-servlets/src/main/java/org/eclipse/jetty/servlets/DoSFilter.java index d9f1ad711ef5..d1ee5261212f 100644 --- a/jetty-servlets/src/main/java/org/eclipse/jetty/servlets/DoSFilter.java +++ b/jetty-servlets/src/main/java/org/eclipse/jetty/servlets/DoSFilter.java @@ -1369,12 +1369,10 @@ public void onTimeout(AsyncEvent event) throws IOException } } - private String createRemotePortId(final ServletRequest request) + private String createRemotePortId(ServletRequest request) { - final String addr = request.getRemoteAddr(); - final int port = request.getRemotePort(); - if (addr.contains(":")) - return "[" + addr + "]:" + port; + String addr = request.getRemoteAddr(); + int port = request.getRemotePort(); return addr + ":" + port; } } diff --git a/jetty-util/src/main/java/org/eclipse/jetty/util/HostPort.java b/jetty-util/src/main/java/org/eclipse/jetty/util/HostPort.java index 546d9bc6d009..d3b5d01e2bef 100644 --- a/jetty-util/src/main/java/org/eclipse/jetty/util/HostPort.java +++ b/jetty-util/src/main/java/org/eclipse/jetty/util/HostPort.java @@ -19,22 +19,18 @@ package org.eclipse.jetty.util; /** - * Parse an authority string into Host and Port - *

Parse a string in the form "host:port", handling IPv4 an IPv6 hosts

- * - *

The System property "org.eclipse.jetty.util.HostPort.STRIP_IPV6" can be set to a boolean - * value to control of the square brackets are stripped off IPv6 addresses (default false).

+ *

Parse an authority string (in the form {@code host:port}) into + * {@code host} and {@code port}, handling IPv4 and IPv6 host formats + * as defined in https://www.ietf.org/rfc/rfc2732.txt

*/ public class HostPort { - private static final boolean STRIP_IPV6 = Boolean.parseBoolean(System.getProperty("org.eclipse.jetty.util.HostPort.STRIP_IPV6", "false")); - private final String _host; private final int _port; public HostPort(String host, int port) { - _host = host; + _host = normalizeHost(host); _port = port; } @@ -55,7 +51,7 @@ else if (authority.charAt(0) == '[') int close = authority.lastIndexOf(']'); if (close < 0) throw new IllegalArgumentException("Bad IPv6 host"); - _host = STRIP_IPV6 ? authority.substring(1, close) : authority.substring(0, close + 1); + _host = authority.substring(0, close + 1); if (authority.length() > close + 1) { @@ -64,14 +60,17 @@ else if (authority.charAt(0) == '[') _port = parsePort(authority.substring(close + 2)); } else + { _port = 0; + } } else { - // ipv4address or hostname + // ipv6address or ipv4address or hostname int c = authority.lastIndexOf(':'); if (c >= 0) { + // ipv6address if (c != authority.indexOf(':')) { _host = "[" + authority + "]"; @@ -94,14 +93,9 @@ else if (authority.charAt(0) == '[') { throw iae; } - catch (final Exception ex) + catch (Exception ex) { - throw new IllegalArgumentException("Bad HostPort") - { - { - initCause(ex); - } - }; + throw new IllegalArgumentException("Bad HostPort", ex); } } @@ -126,7 +120,7 @@ public int getPort() } /** - * Get the port. + * Get the port or the given default port. * * @param defaultPort, the default port to return if a port is not specified * @return the port @@ -140,15 +134,17 @@ public int getPort(int defaultPort) public String toString() { if (_port > 0) - return normalizeHost(_host) + ":" + _port; + return _host + ":" + _port; return _host; } /** - * Normalize IPv6 address as per https://www.ietf.org/rfc/rfc2732.txt + * Normalizes IPv6 address as per https://tools.ietf.org/html/rfc2732 + * and https://tools.ietf.org/html/rfc6874, + * surrounding with square brackets if they are absent. * - * @param host A host name - * @return Host name surrounded by '[' and ']' as needed. + * @param host a host name, IPv4 address, IPv6 address or IPv6 literal + * @return a host name or an IPv4 address or an IPv6 literal (not an IPv6 address) */ public static String normalizeHost(String host) { @@ -165,7 +161,7 @@ public static String normalizeHost(String host) * * @param rawPort the port string. * @return the integer value for the port. - * @throws IllegalArgumentException + * @throws IllegalArgumentException if the port is invalid */ public static int parsePort(String rawPort) throws IllegalArgumentException { diff --git a/tests/test-http-client-transport/src/test/java/org/eclipse/jetty/http/client/HttpClientTest.java b/tests/test-http-client-transport/src/test/java/org/eclipse/jetty/http/client/HttpClientTest.java index d96f83a281bb..ccc977159cc9 100644 --- a/tests/test-http-client-transport/src/test/java/org/eclipse/jetty/http/client/HttpClientTest.java +++ b/tests/test-http-client-transport/src/test/java/org/eclipse/jetty/http/client/HttpClientTest.java @@ -21,6 +21,7 @@ import java.io.IOException; import java.io.InputStream; import java.io.InterruptedIOException; +import java.nio.charset.StandardCharsets; import java.util.List; import java.util.Random; import java.util.concurrent.CountDownLatch; @@ -47,10 +48,12 @@ import org.eclipse.jetty.http2.FlowControlStrategy; import org.eclipse.jetty.server.Request; import org.eclipse.jetty.server.handler.AbstractHandler; +import org.eclipse.jetty.toolchain.test.Net; import org.eclipse.jetty.util.Callback; import org.eclipse.jetty.util.IO; import org.eclipse.jetty.util.ssl.SslContextFactory; import org.eclipse.jetty.util.thread.QueuedThreadPool; +import org.hamcrest.Matchers; import org.junit.jupiter.api.Assumptions; import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.ArgumentsSource; @@ -60,6 +63,7 @@ import static org.junit.jupiter.api.Assertions.assertArrayEquals; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertNotNull; import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; import static org.junit.jupiter.api.Assertions.fail; @@ -676,6 +680,59 @@ public void testOneDestinationPerUser(Transport transport) throws Exception assertEquals(users, destinations.size()); } + @ParameterizedTest + @ArgumentsSource(TransportProvider.class) + public void testIPv6Host(Transport transport) throws Exception + { + Assumptions.assumeTrue(Net.isIpv6InterfaceAvailable()); + + init(transport); + scenario.start(new EmptyServerHandler() + { + @Override + protected void service(String target, Request jettyRequest, HttpServletRequest request, HttpServletResponse response) throws IOException + { + response.setContentType("text/plain"); + response.getOutputStream().print(request.getHeader("Host")); + } + }); + + // Test with a full URI. + String hostAddress = "::1"; + String host = "[" + hostAddress + "]"; + int port = Integer.parseInt(scenario.getNetworkConnectorLocalPort().get()); + String uri = scenario.getScheme() + "://" + host + ":" + port + "/path"; + ContentResponse response = scenario.client.newRequest(uri) + .method(HttpMethod.PUT) + .timeout(5, TimeUnit.SECONDS) + .send(); + assertNotNull(response); + assertEquals(200, response.getStatus()); + assertThat(new String(response.getContent(), StandardCharsets.ISO_8859_1), Matchers.startsWith("[::1]:")); + + // Test with host address. + response = scenario.client.newRequest(hostAddress, port) + .scheme(scenario.getScheme()) + .method(HttpMethod.PUT) + .timeout(5, TimeUnit.SECONDS) + .send(); + assertNotNull(response); + assertEquals(200, response.getStatus()); + assertThat(new String(response.getContent(), StandardCharsets.ISO_8859_1), Matchers.startsWith("[::1]:")); + + // Test with host. + response = scenario.client.newRequest(host, port) + .scheme(scenario.getScheme()) + .method(HttpMethod.PUT) + .timeout(5, TimeUnit.SECONDS) + .send(); + assertNotNull(response); + assertEquals(200, response.getStatus()); + assertThat(new String(response.getContent(), StandardCharsets.ISO_8859_1), Matchers.startsWith("[::1]:")); + + assertEquals(1, scenario.client.getDestinations().size()); + } + private void sleep(long time) throws IOException { try From 867621af89650b380350ddb625f711ee62ea3a13 Mon Sep 17 00:00:00 2001 From: Simone Bordet Date: Mon, 10 Aug 2020 14:52:57 +0200 Subject: [PATCH 2/4] Fixes #5079 - :authority header for IPv6 address not having square brackets. Fixed Jenkins failures by disabling tests that require IPv6 if it is not available. Signed-off-by: Simone Bordet --- .../org/eclipse/jetty/proxy/ForwardProxyServerTest.java | 6 ++++++ .../org/eclipse/jetty/proxy/ForwardProxyTLSServerTest.java | 4 ++++ 2 files changed, 10 insertions(+) diff --git a/jetty-proxy/src/test/java/org/eclipse/jetty/proxy/ForwardProxyServerTest.java b/jetty-proxy/src/test/java/org/eclipse/jetty/proxy/ForwardProxyServerTest.java index bedcefb7b234..b334bc1f0d61 100644 --- a/jetty-proxy/src/test/java/org/eclipse/jetty/proxy/ForwardProxyServerTest.java +++ b/jetty-proxy/src/test/java/org/eclipse/jetty/proxy/ForwardProxyServerTest.java @@ -46,6 +46,7 @@ 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; @@ -53,6 +54,7 @@ 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; @@ -226,6 +228,8 @@ public void onFillable() @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); @@ -263,6 +267,8 @@ protected void addProxyHeaders(HttpServletRequest clientRequest, Request proxyRe @Test public void testIPv6WithForwardedHeader() throws Exception { + Assumptions.assumeTrue(Net.isIpv6InterfaceAvailable()); + HttpConfiguration httpConfig = new HttpConfiguration(); httpConfig.addCustomizer(new ForwardedRequestCustomizer()); ConnectionFactory http = new HttpConnectionFactory(httpConfig); diff --git a/jetty-proxy/src/test/java/org/eclipse/jetty/proxy/ForwardProxyTLSServerTest.java b/jetty-proxy/src/test/java/org/eclipse/jetty/proxy/ForwardProxyTLSServerTest.java index c579f265159a..d3ebd2803f74 100644 --- a/jetty-proxy/src/test/java/org/eclipse/jetty/proxy/ForwardProxyTLSServerTest.java +++ b/jetty-proxy/src/test/java/org/eclipse/jetty/proxy/ForwardProxyTLSServerTest.java @@ -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; @@ -467,6 +469,8 @@ protected void handleConnect(Request baseRequest, HttpServletRequest request, Ht @MethodSource("proxyTLS") public void testIPv6(SslContextFactory.Server proxyTLS) throws Exception { + Assumptions.assumeTrue(Net.isIpv6InterfaceAvailable()); + startTLSServer(new ServerHandler()); startProxy(proxyTLS); From 897e766f24becb5c1a3ddc0b0d3970e45c08d134 Mon Sep 17 00:00:00 2001 From: Simone Bordet Date: Tue, 11 Aug 2020 19:18:11 +0200 Subject: [PATCH 3/4] Fixes #5079 - :authority header for IPv6 address not having square brackets. Updates after review. Signed-off-by: Simone Bordet --- .../src/main/java/org/eclipse/jetty/client/HttpClient.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/jetty-client/src/main/java/org/eclipse/jetty/client/HttpClient.java b/jetty-client/src/main/java/org/eclipse/jetty/client/HttpClient.java index 7a5903a46ee7..88cba2ebd1f4 100644 --- a/jetty-client/src/main/java/org/eclipse/jetty/client/HttpClient.java +++ b/jetty-client/src/main/java/org/eclipse/jetty/client/HttpClient.java @@ -64,6 +64,7 @@ import org.eclipse.jetty.io.MappedByteBufferPool; import org.eclipse.jetty.io.ssl.SslClientConnectionFactory; import org.eclipse.jetty.util.Fields; +import org.eclipse.jetty.util.HostPort; import org.eclipse.jetty.util.Jetty; import org.eclipse.jetty.util.ProcessorUtils; import org.eclipse.jetty.util.Promise; @@ -1163,7 +1164,7 @@ protected HttpField getAcceptEncodingField() @Deprecated protected String normalizeHost(String host) { - return host; + return HostPort.normalizeHost(host); } public static int normalizePort(String scheme, int port) From 2e73f80d34d7946c13451baf7f5280bb83aca50c Mon Sep 17 00:00:00 2001 From: Simone Bordet Date: Tue, 11 Aug 2020 20:03:42 +0200 Subject: [PATCH 4/4] Fixes #5079 - :authority header for IPv6 address not having square brackets. Reverted code changes to HttpClient.normalizeHost(). Signed-off-by: Simone Bordet --- .../src/main/java/org/eclipse/jetty/client/HttpClient.java | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/jetty-client/src/main/java/org/eclipse/jetty/client/HttpClient.java b/jetty-client/src/main/java/org/eclipse/jetty/client/HttpClient.java index 88cba2ebd1f4..7a5903a46ee7 100644 --- a/jetty-client/src/main/java/org/eclipse/jetty/client/HttpClient.java +++ b/jetty-client/src/main/java/org/eclipse/jetty/client/HttpClient.java @@ -64,7 +64,6 @@ import org.eclipse.jetty.io.MappedByteBufferPool; import org.eclipse.jetty.io.ssl.SslClientConnectionFactory; import org.eclipse.jetty.util.Fields; -import org.eclipse.jetty.util.HostPort; import org.eclipse.jetty.util.Jetty; import org.eclipse.jetty.util.ProcessorUtils; import org.eclipse.jetty.util.Promise; @@ -1164,7 +1163,7 @@ protected HttpField getAcceptEncodingField() @Deprecated protected String normalizeHost(String host) { - return HostPort.normalizeHost(host); + return host; } public static int normalizePort(String scheme, int port)