From 0ac34ff2b8a73d6d2f99bd8751ffac34791d78f3 Mon Sep 17 00:00:00 2001 From: Greg Wilkins Date: Wed, 23 Sep 2020 15:05:52 +0200 Subject: [PATCH] Issue #5304 HTTP2 HostHeader (#5307) * Issue #5304 HTTP2 HostHeader Updated HostHeaderCustomizer to actually add the Host header, either from values passed in the custructor or from the getServerName and getServerPort methods. The HttpURI is no longer updated. Signed-off-by: Greg Wilkins * Issue #5304 HTTP2 HostHeader + Found and fixed bug in HttpFields + Added port normalization support to HttpScheme + added test Signed-off-by: Greg Wilkins * blank line Signed-off-by: Greg Wilkins * Issue #5304 HTTP2 HostHeader + refixed bug in HttpFields Signed-off-by: Greg Wilkins * Issue #5304 HTTP2 HostHeader + still fixing HttpFields bug Signed-off-by: Greg Wilkins * Issue #5304 HTTP2 Host Header updates from review --- .../org/eclipse/jetty/client/HttpClient.java | 10 +--- .../org/eclipse/jetty/http/HttpFields.java | 18 ++++-- .../org/eclipse/jetty/http/HttpScheme.java | 38 +++++++++--- .../java/org/eclipse/jetty/http/HttpURI.java | 9 +-- .../eclipse/jetty/http/HttpFieldsTest.java | 19 ++++++ .../jetty/server/HostHeaderCustomizer.java | 52 +++++++++++------ .../org/eclipse/jetty/server/Request.java | 6 +- .../org/eclipse/jetty/server/Response.java | 2 +- .../server/HostHeaderCustomizerTest.java | 58 ++++++++++++++++++- 9 files changed, 161 insertions(+), 51 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 83add784126..0f25ab0151d 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 @@ -1126,18 +1126,12 @@ public class HttpClient extends ContainerLifeCycle { if (port > 0) return port; - else if (isSchemeSecure(scheme)) - return 443; - else - return 80; + return HttpScheme.getDefaultPort(scheme); } public boolean isDefaultPort(String scheme, int port) { - if (isSchemeSecure(scheme)) - return port == 443; - else - return port == 80; + return HttpScheme.getDefaultPort(scheme) == port; } public static boolean isSchemeSecure(String scheme) diff --git a/jetty-http/src/main/java/org/eclipse/jetty/http/HttpFields.java b/jetty-http/src/main/java/org/eclipse/jetty/http/HttpFields.java index 285953681a2..7bf249f2206 100644 --- a/jetty-http/src/main/java/org/eclipse/jetty/http/HttpFields.java +++ b/jetty-http/src/main/java/org/eclipse/jetty/http/HttpFields.java @@ -639,22 +639,28 @@ public interface HttpFields extends Iterable public Mutable add(HttpFields fields) { + if (_fields == null) + _fields = new HttpField[fields.size() + 4]; + else if (_size + fields.size() >= _fields.length) + _fields = Arrays.copyOf(_fields, _size + fields.size() + 4); + + if (fields.size() == 0) + return this; + if (fields instanceof Immutable) { Immutable b = (Immutable)fields; - _fields = Arrays.copyOf(b._fields, b._fields.length + 4); - _size = b._fields.length; + System.arraycopy(b._fields, 0, _fields, _size, b._fields.length); + _size += b._fields.length; } else if (fields instanceof Mutable) { Mutable b = (Mutable)fields; - _fields = Arrays.copyOf(b._fields, b._fields.length); - _size = b._size; + System.arraycopy(b._fields, 0, _fields, _size, b._size); + _size += b._size; } else { - _fields = new HttpField[fields.size() + 4]; - _size = 0; for (HttpField f : fields) _fields[_size++] = f; } diff --git a/jetty-http/src/main/java/org/eclipse/jetty/http/HttpScheme.java b/jetty-http/src/main/java/org/eclipse/jetty/http/HttpScheme.java index b80b5d4e879..01f85c1491a 100644 --- a/jetty-http/src/main/java/org/eclipse/jetty/http/HttpScheme.java +++ b/jetty-http/src/main/java/org/eclipse/jetty/http/HttpScheme.java @@ -25,14 +25,14 @@ import org.eclipse.jetty.util.BufferUtil; import org.eclipse.jetty.util.Trie; /** - * + * HTTP and WebSocket Schemes */ public enum HttpScheme { - HTTP("http"), - HTTPS("https"), - WS("ws"), - WSS("wss"); + HTTP("http", 80), + HTTPS("https", 443), + WS("ws", 80), + WSS("wss", 443); public static final Trie CACHE = new ArrayTrie(); @@ -46,11 +46,13 @@ public enum HttpScheme private final String _string; private final ByteBuffer _buffer; + private final int _defaultPort; - HttpScheme(String s) + HttpScheme(String s, int port) { _string = s; _buffer = BufferUtil.toBuffer(s); + _defaultPort = port; } public ByteBuffer asByteBuffer() @@ -60,7 +62,7 @@ public enum HttpScheme public boolean is(String s) { - return s != null && _string.equalsIgnoreCase(s); + return _string.equalsIgnoreCase(s); } public String asString() @@ -68,9 +70,31 @@ public enum HttpScheme return _string; } + public int getDefaultPort() + { + return _defaultPort; + } + + public int normalizePort(int port) + { + return port == _defaultPort ? 0 : port; + } + @Override public String toString() { return _string; } + + public static int getDefaultPort(String scheme) + { + HttpScheme httpScheme = scheme == null ? null : CACHE.get(scheme); + return httpScheme == null ? HTTP.getDefaultPort() : httpScheme.getDefaultPort(); + } + + public static int normalizePort(String scheme, int port) + { + HttpScheme httpScheme = scheme == null ? null : CACHE.get(scheme); + return httpScheme == null ? port : httpScheme.normalizePort(port); + } } diff --git a/jetty-http/src/main/java/org/eclipse/jetty/http/HttpURI.java b/jetty-http/src/main/java/org/eclipse/jetty/http/HttpURI.java index f740a2086a0..9a1f017f06a 100644 --- a/jetty-http/src/main/java/org/eclipse/jetty/http/HttpURI.java +++ b/jetty-http/src/main/java/org/eclipse/jetty/http/HttpURI.java @@ -638,11 +638,12 @@ public interface HttpURI public Mutable normalize() { - if (_port == 80 && HttpScheme.HTTP.is(_scheme)) + HttpScheme scheme = _scheme == null ? null : HttpScheme.CACHE.get(_scheme); + if (scheme != null && _port == scheme.getDefaultPort()) + { _port = 0; - if (_port == 443 && HttpScheme.HTTPS.is(_scheme)) - _port = 0; - _uri = null; + _uri = null; + } return this; } diff --git a/jetty-http/src/test/java/org/eclipse/jetty/http/HttpFieldsTest.java b/jetty-http/src/test/java/org/eclipse/jetty/http/HttpFieldsTest.java index 4a77a4cdf49..04ebd3905d2 100644 --- a/jetty-http/src/test/java/org/eclipse/jetty/http/HttpFieldsTest.java +++ b/jetty-http/src/test/java/org/eclipse/jetty/http/HttpFieldsTest.java @@ -742,6 +742,25 @@ public class HttpFieldsTest assertThat(fields.size(), is(0)); } + @Test + public void testAddHttpFields() + { + HttpFields.Mutable fields = new HttpFields.Mutable(new HttpFields.Mutable()); + fields.add("One", "1"); + + fields = new HttpFields.Mutable(fields); + + fields.add(HttpFields.build().add("two", "2").add("three", "3")); + fields.add(HttpFields.build().add("four", "4").add("five", "5").asImmutable()); + + assertThat(fields.size(), is(5)); + assertThat(fields.get("one"), is("1")); + assertThat(fields.get("two"), is("2")); + assertThat(fields.get("three"), is("3")); + assertThat(fields.get("four"), is("4")); + assertThat(fields.get("five"), is("5")); + } + @Test public void testPutNullName() { diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/HostHeaderCustomizer.java b/jetty-server/src/main/java/org/eclipse/jetty/server/HostHeaderCustomizer.java index adf0d08f169..f1491e0c8a9 100644 --- a/jetty-server/src/main/java/org/eclipse/jetty/server/HostHeaderCustomizer.java +++ b/jetty-server/src/main/java/org/eclipse/jetty/server/HostHeaderCustomizer.java @@ -18,31 +18,34 @@ package org.eclipse.jetty.server; -import java.util.Objects; -import javax.servlet.http.HttpServletRequest; - +import org.eclipse.jetty.http.HostPortHttpField; +import org.eclipse.jetty.http.HttpFields; +import org.eclipse.jetty.http.HttpHeader; +import org.eclipse.jetty.http.HttpScheme; import org.eclipse.jetty.http.HttpURI; +import org.eclipse.jetty.http.HttpVersion; /** - * Customizes requests that lack the {@code Host} header (for example, HTTP 1.0 requests). + * Adds a missing {@code Host} header (for example, HTTP 1.0 or 2.0 requests). *

- * In case of HTTP 1.0 requests that lack the {@code Host} header, the application may issue - * a redirect, and the {@code Location} header is usually constructed from the {@code Host} - * header; if the {@code Host} header is missing, the server may query the connector for its - * IP address in order to construct the {@code Location} header, and thus leak to clients - * internal IP addresses. - *

- * This {@link HttpConfiguration.Customizer} is configured with a {@code serverName} and - * optionally a {@code serverPort}. - * If the {@code Host} header is absent, the configured {@code serverName} will be set on - * the request so that {@link HttpServletRequest#getServerName()} will return that value, - * and likewise for {@code serverPort} and {@link HttpServletRequest#getServerPort()}. + * The host and port may be provided in the constructor or taken from the + * {@link Request#getServerName()} and {@link Request#getServerPort()} methods. + *

*/ public class HostHeaderCustomizer implements HttpConfiguration.Customizer { private final String serverName; private final int serverPort; + /** + * Construct customizer that uses {@link Request#getServerName()} and + * {@link Request#getServerPort()} to create a host header. + */ + public HostHeaderCustomizer() + { + this(null, 0); + } + /** * @param serverName the {@code serverName} to set on the request (the {@code serverPort} will not be set) */ @@ -57,15 +60,26 @@ public class HostHeaderCustomizer implements HttpConfiguration.Customizer */ public HostHeaderCustomizer(String serverName, int serverPort) { - this.serverName = Objects.requireNonNull(serverName); + this.serverName = serverName; this.serverPort = serverPort; } @Override public void customize(Connector connector, HttpConfiguration channelConfig, Request request) { - if (request.getHeader("Host") == null) - // TODO set the field as well? - request.setHttpURI(HttpURI.build(request.getHttpURI()).host(serverName).port(serverPort)); + if (request.getHttpVersion() != HttpVersion.HTTP_1_1 && !request.getHttpFields().contains(HttpHeader.HOST)) + { + String host = serverName == null ? request.getServerName() : serverName; + int port = HttpScheme.normalizePort(request.getScheme(), serverPort == 0 ? request.getServerPort() : serverPort); + + if (serverName != null || serverPort > 0) + request.setHttpURI(HttpURI.build(request.getHttpURI()).authority(host, port)); + + HttpFields original = request.getHttpFields(); + HttpFields.Mutable httpFields = HttpFields.build(original.size() + 1); + httpFields.add(new HostPortHttpField(host, port)); + httpFields.add(request.getHttpFields()); + request.setHttpFields(httpFields); + } } } 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 977c9956597..bcaf9c2c897 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 @@ -1355,11 +1355,7 @@ public class Request implements HttpServletRequest // If no port specified, return the default port for the scheme if (port <= 0) - { - if (getScheme().equalsIgnoreCase(URIUtil.HTTPS)) - return 443; - return 80; - } + return HttpScheme.getDefaultPort(getScheme()); // return a specific port return port; diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/Response.java b/jetty-server/src/main/java/org/eclipse/jetty/server/Response.java index 4a3e65bee7d..628c1098b60 100644 --- a/jetty-server/src/main/java/org/eclipse/jetty/server/Response.java +++ b/jetty-server/src/main/java/org/eclipse/jetty/server/Response.java @@ -330,7 +330,7 @@ public class Response implements HttpServletResponse path = (path == null ? "" : path); int port = uri.getPort(); if (port < 0) - port = HttpScheme.HTTPS.asString().equalsIgnoreCase(uri.getScheme()) ? 443 : 80; + port = HttpScheme.getDefaultPort(uri.getScheme()); // Is it the same server? if (!request.getServerName().equalsIgnoreCase(uri.getHost())) diff --git a/jetty-server/src/test/java/org/eclipse/jetty/server/HostHeaderCustomizerTest.java b/jetty-server/src/test/java/org/eclipse/jetty/server/HostHeaderCustomizerTest.java index ce3985292e7..d302fdc91b2 100644 --- a/jetty-server/src/test/java/org/eclipse/jetty/server/HostHeaderCustomizerTest.java +++ b/jetty-server/src/test/java/org/eclipse/jetty/server/HostHeaderCustomizerTest.java @@ -37,7 +37,7 @@ import static org.junit.jupiter.api.Assertions.assertTrue; public class HostHeaderCustomizerTest { @Test - public void testHostHeaderCustomizer() throws Exception + public void testFixedHostPort() throws Exception { Server server = new Server(); HttpConfiguration httpConfig = new HttpConfiguration(); @@ -55,6 +55,7 @@ public class HostHeaderCustomizerTest baseRequest.setHandled(true); assertEquals(serverName, request.getServerName()); assertEquals(serverPort, request.getServerPort()); + assertEquals(serverName + ":" + serverPort, request.getHeader("Host")); response.sendRedirect(redirectPath); } }); @@ -89,4 +90,59 @@ public class HostHeaderCustomizerTest server.stop(); } } + + @Test + public void testHostPort() throws Exception + { + Server server = new Server(); + HttpConfiguration httpConfig = new HttpConfiguration(); + final String serverName = "127.0.0.1"; + final String redirectPath = "/redirect"; + httpConfig.addCustomizer(new HostHeaderCustomizer()); + final ServerConnector connector = new ServerConnector(server, new HttpConnectionFactory(httpConfig)); + server.addConnector(connector); + server.setHandler(new AbstractHandler() + { + @Override + public void handle(String target, Request baseRequest, HttpServletRequest request, HttpServletResponse response) throws IOException, ServletException + { + baseRequest.setHandled(true); + assertEquals(serverName, request.getServerName()); + assertEquals(connector.getLocalPort(), request.getServerPort()); + assertEquals(serverName + ":" + connector.getLocalPort(), request.getHeader("Host")); + response.sendRedirect(redirectPath); + } + }); + server.start(); + + try + { + try (Socket socket = new Socket("localhost", connector.getLocalPort())) + { + try (OutputStream output = socket.getOutputStream()) + { + String request = + "GET / HTTP/1.0\r\n" + + "\r\n"; + output.write(request.getBytes(StandardCharsets.UTF_8)); + output.flush(); + + HttpTester.Input input = HttpTester.from(socket.getInputStream()); + HttpTester.Response response = HttpTester.parseResponse(input); + + String location = response.get("location"); + assertNotNull(location); + String schemePrefix = "http://"; + assertTrue(location.startsWith(schemePrefix)); + assertTrue(location.endsWith(redirectPath)); + String hostPort = location.substring(schemePrefix.length(), location.length() - redirectPath.length()); + assertEquals(serverName + ":" + connector.getLocalPort(), hostPort); + } + } + } + finally + { + server.stop(); + } + } }