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 <[email protected]> * Issue #5304 HTTP2 HostHeader + Found and fixed bug in HttpFields + Added port normalization support to HttpScheme + added test Signed-off-by: Greg Wilkins <[email protected]> * blank line Signed-off-by: Greg Wilkins <[email protected]> * Issue #5304 HTTP2 HostHeader + refixed bug in HttpFields Signed-off-by: Greg Wilkins <[email protected]> * Issue #5304 HTTP2 HostHeader + still fixing HttpFields bug Signed-off-by: Greg Wilkins <[email protected]> * Issue #5304 HTTP2 Host Header updates from review
jetty · Sep 23, 2020 · 0ac34ff · 0ac34ff
1 parent 5a00eba
commit 0ac34ff
Show file tree

Hide file tree

Showing 9 changed files with 161 additions and 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
@@ -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
@@ -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
@@ -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<HttpScheme> CACHE = new ArrayTrie<HttpScheme>();
 
@@ -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,17 +62,39 @@ public ByteBuffer asByteBuffer()
 
     public boolean is(String s)
     {
-        return s != null && _string.equalsIgnoreCase(s);
+        return _string.equalsIgnoreCase(s);
     }
 
     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
@@ -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
@@ -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
@@ -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).
- * <p>
- * 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).
  * <p>
- * 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.
+ * </p>
  */
 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
@@ -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
@@ -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
@@ -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();
+        }
+    }
 }