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]>
jetty · Sep 22, 2020 · 38e6949 · 38e6949
1 parent 256c669
commit 38e6949
Show file tree

Hide file tree

Showing 9 changed files with 134 additions and 39 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,13 @@ public static int normalizePort(String scheme, int port)
     {
         if (port > 0)
             return port;
-        else if (isSchemeSecure(scheme))
-            return 443;
-        else
-            return 80;
+        return HttpScheme.normalizePort(scheme, port);
     }
 
     public boolean isDefaultPort(String scheme, int port)
     {
-        if (isSchemeSecure(scheme))
-            return port == 443;
-        else
-            return port == 80;
+        HttpScheme httpScheme = scheme == null ? null : HttpScheme.CACHE.get(scheme);
+        return httpScheme != null && port == httpScheme.getNormalPort();
     }
 
     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,26 @@ public Mutable add(HttpField field)
 
         public Mutable add(HttpFields fields)
         {
+            if (fields.size() == 0)
+                return this;
+
+            if (_size + fields.size() >= _fields.length)
+                _fields = Arrays.copyOf(_fields, _size + fields.size() + 4);
+
             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 _normalPort;
 
-    HttpScheme(String s)
+    HttpScheme(String s, int port)
     {
         _string = s;
         _buffer = BufferUtil.toBuffer(s);
+        _normalPort = 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 getNormalPort()
+    {
+        return _normalPort;
+    }
+
+    public int normalizePort(int port)
+    {
+        return port == _normalPort ? 0 : port;
+    }
+
     @Override
     public String toString()
     {
         return _string;
     }
+
+    public static int getNormalPort(String scheme)
+    {
+        HttpScheme httpScheme = scheme == null ? null : CACHE.get(scheme);
+        return httpScheme == null ? HTTP.getNormalPort() : httpScheme.getNormalPort();
+    }
+
+    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.getNormalPort())
+            {
                 _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,22 @@ public void testAddNullName()
         assertThat(fields.size(), is(0));
     }
 
+    @Test
+    public void testAddHttpFields()
+    {
+        HttpFields.Mutable fields = HttpFields.build();
+        fields.add("One", "1");
+        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,12 +18,11 @@
 
 package org.eclipse.jetty.server;
 
-import java.util.Objects;
-
 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;
 
 /**
@@ -38,6 +37,10 @@ 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);
@@ -57,7 +60,7 @@ public HostHeaderCustomizer(String serverName)
      */
     public HostHeaderCustomizer(String serverName, int serverPort)
     {
-        this.serverName = Objects.requireNonNull(serverName);
+        this.serverName = serverName;
         this.serverPort = serverPort;
     }
 
@@ -67,11 +70,10 @@ public void customize(Connector connector, HttpConfiguration channelConfig, Requ
         if (request.getHttpVersion() != HttpVersion.HTTP_1_1 && !request.getHttpFields().contains(HttpHeader.HOST))
         {
             String host = serverName == null ? request.getServerName() : serverName;
-            int port = serverPort == 0 ? request.getServerPort() : 0;
-            String proto = request.getProtocol();
+            int port = HttpScheme.normalizePort(request.getScheme(), serverPort == 0 ? request.getServerPort() : serverPort);
 
-            if ((HttpScheme.HTTPS.is(proto) && port == 443) || (HttpScheme.HTTP.is(proto) && port == 80))
-                port = 0;
+            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);

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.getNormalPort(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.getNormalPort(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,60 @@ 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();
+        }
+    }
 }