Fix #11414 URI schema and port normalization (#11416)

* Issue #11414 - use HttpURI instead of URIUtil to have a single point of spec behavior * Issue #11414 - enforce lowercase scheme in HttpConfiguration.secureScheme * Issue #11414 - Scheme produced on `Location` header is lowercase * Issue #11414 - Scheme to lowercase * Issue #11414 - Scheme to lowercase * Revert change to HttpClient * Added schema port knowledge to URIUtil * Fixed tests for normalized URIs * updates from review * updates from review * Fix tests * Restored methods as deprecated * More testing --------- Co-authored-by: Joakim Erdfelt <joakim.erdfelt@gmail.com>
jetty · Feb 20, 2024 · f07d812 · f07d812
1 parent d02406c
commit f07d812
Show file tree

Hide file tree

Showing 36 changed files with 587 additions and 150 deletions.
diff --git a/jetty-core/jetty-client/src/main/java/org/eclipse/jetty/client/HttpClient.java b/jetty-core/jetty-client/src/main/java/org/eclipse/jetty/client/HttpClient.java
@@ -56,6 +56,7 @@
 import org.eclipse.jetty.util.ProcessorUtils;
 import org.eclipse.jetty.util.Promise;
 import org.eclipse.jetty.util.SocketAddressResolver;
+import org.eclipse.jetty.util.URIUtil;
 import org.eclipse.jetty.util.annotation.ManagedAttribute;
 import org.eclipse.jetty.util.annotation.ManagedObject;
 import org.eclipse.jetty.util.component.ContainerLifeCycle;
@@ -1099,11 +1100,17 @@ public ProxyConfiguration getProxyConfiguration()
         return proxyConfig;
     }
 
+    /**
+     * Return a normalized port suitable for use by Origin and Address
+     * @param scheme the scheme to use for the default port (if port is unspecified)
+     * @param port the port (0 or negative means the port is unspecified)
+     * @return the normalized port.
+     */
     public static int normalizePort(String scheme, int port)
     {
         if (port > 0)
             return port;
-        return HttpScheme.getDefaultPort(scheme);
+        return URIUtil.getDefaultPortForScheme(scheme);
     }
 
     public ClientConnectionFactory newSslClientConnectionFactory(SslContextFactory.Client sslContextFactory, ClientConnectionFactory connectionFactory)

diff --git a/jetty-core/jetty-client/src/main/java/org/eclipse/jetty/client/HttpRedirector.java b/jetty-core/jetty-client/src/main/java/org/eclipse/jetty/client/HttpRedirector.java
@@ -30,6 +30,7 @@
 import org.eclipse.jetty.http.HttpMethod;
 import org.eclipse.jetty.http.HttpStatus;
 import org.eclipse.jetty.util.NanoTime;
+import org.eclipse.jetty.util.URIUtil;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
@@ -278,7 +279,7 @@ private URI sanitize(String location)
             Matcher matcher = URI_PATTERN.matcher(location);
             if (matcher.matches())
             {
-                String scheme = matcher.group(2);
+                String scheme = URIUtil.normalizeScheme(matcher.group(2));
                 String authority = matcher.group(3);
                 String path = matcher.group(4);
                 String query = matcher.group(5);

diff --git a/jetty-core/jetty-client/src/main/java/org/eclipse/jetty/client/Origin.java b/jetty-core/jetty-client/src/main/java/org/eclipse/jetty/client/Origin.java
@@ -20,6 +20,7 @@
 import java.util.Objects;
 
 import org.eclipse.jetty.client.transport.HttpClientTransportDynamic;
+import org.eclipse.jetty.http.HttpURI;
 import org.eclipse.jetty.io.ClientConnectionFactory;
 import org.eclipse.jetty.io.EndPoint;
 import org.eclipse.jetty.util.HostPort;
@@ -74,7 +75,7 @@ public Origin(String scheme, Address address, Object tag)
 
     public Origin(String scheme, Address address, Object tag, Protocol protocol)
     {
-        this.scheme = Objects.requireNonNull(scheme);
+        this.scheme = URIUtil.normalizeScheme(Objects.requireNonNull(scheme));
         this.address = address;
         this.tag = tag;
         this.protocol = protocol;
@@ -122,9 +123,7 @@ public int hashCode()
 
     public String asString()
     {
-        StringBuilder result = new StringBuilder();
-        URIUtil.appendSchemeHostPort(result, scheme, address.host, address.port);
-        return result.toString();
+        return HttpURI.from(scheme, address.host, address.port, null).asString();
     }
 
     @Override

diff --git a/...y-core/jetty-client/src/main/java/org/eclipse/jetty/client/transport/HttpDestination.java b/...y-core/jetty-client/src/main/java/org/eclipse/jetty/client/transport/HttpDestination.java
@@ -39,6 +39,7 @@
 import org.eclipse.jetty.util.HostPort;
 import org.eclipse.jetty.util.NanoTime;
 import org.eclipse.jetty.util.Promise;
+import org.eclipse.jetty.util.URIUtil;
 import org.eclipse.jetty.util.annotation.ManagedAttribute;
 import org.eclipse.jetty.util.annotation.ManagedObject;
 import org.eclipse.jetty.util.component.ContainerLifeCycle;
@@ -80,7 +81,8 @@ public HttpDestination(HttpClient client, Origin origin, boolean intrinsicallySe
 
         String host = HostPort.normalizeHost(getHost());
         int port = getPort();
-        if (port != HttpScheme.getDefaultPort(getScheme()))
+        String scheme = getScheme();
+        if (port != URIUtil.getDefaultPortForScheme(scheme))
             host += ":" + port;
         hostField = new HttpField(HttpHeader.HOST, host);
 

diff --git a/jetty-core/jetty-client/src/main/java/org/eclipse/jetty/client/transport/HttpRequest.java b/jetty-core/jetty-client/src/main/java/org/eclipse/jetty/client/transport/HttpRequest.java
@@ -55,6 +55,7 @@
 import org.eclipse.jetty.http.HttpFields;
 import org.eclipse.jetty.http.HttpHeader;
 import org.eclipse.jetty.http.HttpMethod;
+import org.eclipse.jetty.http.HttpURI;
 import org.eclipse.jetty.http.HttpVersion;
 import org.eclipse.jetty.util.Fields;
 import org.eclipse.jetty.util.NanoTime;
@@ -122,9 +123,7 @@ public HttpRequest copy(URI newURI)
     {
         if (newURI == null)
         {
-            StringBuilder builder = new StringBuilder(64);
-            URIUtil.appendSchemeHostPort(builder, getScheme(), getHost(), getPort());
-            newURI = URI.create(builder.toString());
+            newURI = HttpURI.from(getScheme(), getHost(), getPort(), null).toURI();
         }
 
         HttpRequest newRequest = copyInstance(newURI);
@@ -184,7 +183,7 @@ public String getScheme()
     @Override
     public Request scheme(String scheme)
     {
-        this.scheme = scheme;
+        this.scheme = URIUtil.normalizeScheme(scheme);
         this.uri = null;
         return this;
     }

diff --git a/jetty-core/jetty-client/src/test/java/org/eclipse/jetty/client/HttpClientURITest.java b/jetty-core/jetty-client/src/test/java/org/eclipse/jetty/client/HttpClientURITest.java
@@ -32,13 +32,13 @@
 import org.eclipse.jetty.http.HttpHeader;
 import org.eclipse.jetty.http.HttpMethod;
 import org.eclipse.jetty.http.HttpStatus;
+import org.eclipse.jetty.http.HttpURI;
 import org.eclipse.jetty.http.UriCompliance;
 import org.eclipse.jetty.server.HttpConfiguration;
 import org.eclipse.jetty.server.Response;
 import org.eclipse.jetty.toolchain.test.Net;
 import org.eclipse.jetty.util.Fields;
 import org.eclipse.jetty.util.StringUtil;
-import org.eclipse.jetty.util.URIUtil;
 import org.junit.jupiter.api.Assumptions;
 import org.junit.jupiter.params.ParameterizedTest;
 import org.junit.jupiter.params.provider.ArgumentsSource;
@@ -68,9 +68,8 @@ public void testIPv6Host(Scenario scenario) throws Exception
             .timeout(5, TimeUnit.SECONDS);
 
         assertEquals(host, request.getHost());
-        StringBuilder uri = new StringBuilder();
-        URIUtil.appendSchemeHostPort(uri, scenario.getScheme(), host, connector.getLocalPort());
-        assertEquals(uri.toString(), request.getURI().toString());
+        HttpURI httpURI = HttpURI.from(scenario.getScheme(), host, connector.getLocalPort(), null);
+        assertEquals(httpURI.asString(), request.getURI().toString());
 
         assertEquals(HttpStatus.OK_200, request.send().getStatus());
     }

diff --git a/jetty-core/jetty-deploy/src/test/java/org/eclipse/jetty/deploy/test/XmlConfiguredJetty.java b/jetty-core/jetty-deploy/src/test/java/org/eclipse/jetty/deploy/test/XmlConfiguredJetty.java
@@ -35,6 +35,7 @@
 import java.util.stream.Collectors;
 
 import org.eclipse.jetty.http.HttpScheme;
+import org.eclipse.jetty.http.HttpURI;
 import org.eclipse.jetty.server.Connector;
 import org.eclipse.jetty.server.Handler;
 import org.eclipse.jetty.server.NetworkConnector;
@@ -249,9 +250,7 @@ public int getServerPort()
 
     public URI getServerURI() throws UnknownHostException
     {
-        StringBuilder uri = new StringBuilder();
-        URIUtil.appendSchemeHostPort(uri, getScheme(), InetAddress.getLocalHost().getHostAddress(), getServerPort());
-        return URI.create(uri.toString());
+        return HttpURI.from(getScheme(), InetAddress.getLocalHost().getHostAddress(), getServerPort(), null).toURI();
     }
 
     public Path getJettyBasePath()
@@ -332,7 +331,7 @@ public void setProperty(String key, String value)
 
     public void setScheme(String scheme)
     {
-        this._scheme = scheme;
+        this._scheme = URIUtil.normalizeScheme(scheme);
     }
 
     public void start() throws Exception

diff --git a/...fcgi/jetty-fcgi-proxy/src/main/java/org/eclipse/jetty/fcgi/proxy/FastCGIProxyHandler.java b/...fcgi/jetty-fcgi-proxy/src/main/java/org/eclipse/jetty/fcgi/proxy/FastCGIProxyHandler.java
@@ -38,6 +38,7 @@
 import org.eclipse.jetty.server.Response;
 import org.eclipse.jetty.server.handler.TryPathsHandler;
 import org.eclipse.jetty.util.Callback;
+import org.eclipse.jetty.util.URIUtil;
 import org.eclipse.jetty.util.thread.QueuedThreadPool;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
@@ -317,7 +318,7 @@ protected void sendProxyToServerRequest(Request clientToProxyRequest, org.eclips
         // If the Host header is missing, add it.
         if (!proxyToServerRequest.getHeaders().contains(HttpHeader.HOST))
         {
-            if (serverPort != HttpScheme.getDefaultPort(scheme))
+            if (serverPort != URIUtil.getDefaultPortForScheme(scheme))
                 serverName += ":" + serverPort;
             String host = serverName;
             proxyToServerRequest.headers(headers -> headers

diff --git a/jetty-core/jetty-http/src/main/java/org/eclipse/jetty/http/HttpScheme.java b/jetty-core/jetty-http/src/main/java/org/eclipse/jetty/http/HttpScheme.java
@@ -17,16 +17,17 @@
 
 import org.eclipse.jetty.util.BufferUtil;
 import org.eclipse.jetty.util.Index;
+import org.eclipse.jetty.util.URIUtil;
 
 /**
  * HTTP and WebSocket Schemes
  */
 public enum HttpScheme
 {
-    HTTP("http", 80),
-    HTTPS("https", 443),
-    WS("ws", 80),
-    WSS("wss", 443);
+    HTTP("http"),
+    HTTPS("https"),
+    WS("ws"),
+    WSS("wss");
 
     public static final Index<HttpScheme> CACHE = new Index.Builder<HttpScheme>()
         .caseSensitive(false)
@@ -37,11 +38,11 @@ public enum HttpScheme
     private final ByteBuffer _buffer;
     private final int _defaultPort;
 
-    HttpScheme(String s, int port)
+    HttpScheme(String s)
     {
         _string = s;
         _buffer = BufferUtil.toBuffer(s);
-        _defaultPort = port;
+        _defaultPort = URIUtil.getDefaultPortForScheme(s);
     }
 
     public ByteBuffer asByteBuffer()
@@ -75,16 +76,29 @@ public String toString()
         return _string;
     }
 
+    /**
+     * Get the default port for a URI scheme
+     * @param scheme The scheme
+     * @return Default port for URI scheme
+     * @deprecated Use {@link URIUtil#getDefaultPortForScheme(String)}
+     */
+    @Deprecated
     public static int getDefaultPort(String scheme)
     {
-        HttpScheme httpScheme = scheme == null ? null : CACHE.get(scheme);
-        return httpScheme == null ? HTTP.getDefaultPort() : httpScheme.getDefaultPort();
+        return URIUtil.getDefaultPortForScheme(scheme);
     }
 
+    /**
+     * Normalize a port for a URI scheme
+     * @param scheme the scheme
+     * @param port the port to normalize
+     * @return The normalized port
+     * @deprecated Use {@link URIUtil#normalizePortForScheme(String, int)}
+     */
+    @Deprecated
     public static int normalizePort(String scheme, int port)
     {
-        HttpScheme httpScheme = scheme == null ? null : CACHE.get(scheme);
-        return httpScheme == null ? port : httpScheme.normalizePort(port);
+        return URIUtil.normalizePortForScheme(scheme, port);
     }
 
     public static boolean isSecure(String scheme)

diff --git a/jetty-core/jetty-http/src/main/java/org/eclipse/jetty/http/HttpURI.java b/jetty-core/jetty-http/src/main/java/org/eclipse/jetty/http/HttpURI.java
@@ -151,6 +151,11 @@ static Immutable from(String scheme, String host, int port, String pathQuery)
         return new Mutable(scheme, host, port, pathQuery).asImmutable();
     }
 
+    static Immutable from(String scheme, String host, int port, String path, String query, String fragment)
+    {
+        return new Immutable(scheme, host, port, path, query, fragment);
+    }
+
     Immutable asImmutable();
 
     String asString();
@@ -302,18 +307,19 @@ private Immutable(Mutable builder)
                 _violations = Collections.unmodifiableSet(EnumSet.copyOf(builder._violations));
         }
 
-        private Immutable(String uri)
+        private Immutable(String scheme, String host, int port, String path, String query, String fragment)
         {
-            _scheme = null;
+            _uri = null;
+
+            _scheme = URIUtil.normalizeScheme(scheme);
             _user = null;
-            _host = null;
-            _port = -1;
-            _path = uri;
+            _host = host;
+            _port = port;
+            _path = path;
+            _canonicalPath = _path == null ? null : URIUtil.canonicalPath(_path);
             _param = null;
-            _query = null;
-            _fragment = null;
-            _uri = uri;
-            _canonicalPath = null;
+            _query = query;
+            _fragment = fragment;
         }
 
         @Override
@@ -340,19 +346,26 @@ public String asString()
                     out.append(_host);
                 }
 
-                if (_port > 0)
-                    out.append(':').append(_port);
+                int normalizedPort = URIUtil.normalizePortForScheme(_scheme, _port);
+                if (normalizedPort > 0)
+                    out.append(':').append(normalizedPort);
+
+                // we output even if the input is an empty string (to match java URI / URL behaviors)
+                boolean hasQuery = _query != null;
+                boolean hasFragment = _fragment != null;
 
                 if (_path != null)
                     out.append(_path);
+                else if (hasQuery || hasFragment)
+                    out.append('/');
 
-                if (_query != null)
+                if (hasQuery)
                     out.append('?').append(_query);
 
-                if (_fragment != null)
+                if (hasFragment)
                     out.append('#').append(_fragment);
 
-                if (out.length() > 0)
+                if (!out.isEmpty())
                     _uri = out.toString();
                 else
                     _uri = "";
@@ -504,7 +517,7 @@ public URI toURI()
         {
             try
             {
-                return new URI(_scheme, null, _host, _port, _path, _query == null ? null : UrlEncoded.decodeString(_query), _fragment);
+                return new URI(_scheme, null, _host, URIUtil.normalizePortForScheme(_scheme, _port), _path, _query == null ? null : UrlEncoded.decodeString(_query), _fragment);
             }
             catch (URISyntaxException x)
             {
@@ -616,7 +629,7 @@ private Mutable(URI uri)
         {
             _uri = null;
 
-            _scheme = uri.getScheme();
+            _scheme = URIUtil.normalizeScheme(uri.getScheme());
             _host = uri.getHost();
             if (_host == null && uri.getRawSchemeSpecificPart().startsWith("//"))
                 _host = "";
@@ -631,13 +644,9 @@ private Mutable(URI uri)
 
         private Mutable(String scheme, String host, int port, String pathQuery)
         {
-            // TODO review if this should be here
-            if (port == HttpScheme.getDefaultPort(scheme))
-                port = 0;
-
             _uri = null;
 
-            _scheme = scheme;
+            _scheme = URIUtil.normalizeScheme(scheme);
             _host = host;
             _port = port;
 
@@ -960,7 +969,7 @@ public Mutable scheme(HttpScheme scheme)
 
         public Mutable scheme(String scheme)
         {
-            _scheme = scheme;
+            _scheme = URIUtil.normalizeScheme(scheme);
             _uri = null;
             return this;
         }
@@ -1122,7 +1131,7 @@ private void parse(State state, final String uri)
                         {
                             case ':':
                                 // must have been a scheme
-                                _scheme = uri.substring(mark, i);
+                                _scheme = URIUtil.normalizeScheme(uri.substring(mark, i));
                                 // Start again with scheme set
                                 state = State.START;
                                 break;