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 83add7841268..0f25ab0151d2 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 static int normalizePort(String scheme, int port) { 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 285953681a22..7bf249f22064 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 Mutable add(HttpField field) 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 b80b5d4e8791..01f85c1491a0 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.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 ByteBuffer asByteBuffer() public boolean is(String s) { - return s != null && _string.equalsIgnoreCase(s); + return _string.equalsIgnoreCase(s); } public String asString() @@ -68,9 +70,31 @@ public String asString() 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 f740a2086a02..9a1f017f06ac 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 boolean isAbsolute() public Mutable normalize() { - if (_port == 80 && HttpScheme.HTTP.is(_scheme)) - _port = 0; - if (_port == 443 && HttpScheme.HTTPS.is(_scheme)) + HttpScheme scheme = _scheme == null ? null : HttpScheme.CACHE.get(_scheme); + if (scheme != null && _port == scheme.getDefaultPort()) + { _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 4a77a4cdf496..04ebd3905d25 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 void testAddNullName() 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 adf0d08f169b..f1491e0c8a97 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). - *

- * 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. + * Adds a missing {@code Host} header (for example, HTTP 1.0 or 2.0 requests). *

- * 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 HostHeaderCustomizer(String serverName) */ 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 977c99565979..bcaf9c2c897a 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 int getServerPort() // 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 4a3e65bee7d5..628c1098b609 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 String encodeURL(String url) 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 ce3985292e7b..d302fdc91b2e 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 @@ 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 void handle(String target, Request baseRequest, HttpServletRequest reques 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 void handle(String target, Request baseRequest, HttpServletRequest reques 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(); + } + } }