[JENKINS-75278] Upgrade Jetty 12.0.17 which restore UriCompliance.LEGACY previous behaviour

pom.xml

@@ -37,7 +37,7 @@
   <properties>
     <revision>8.5</revision>
     <changelist>-SNAPSHOT</changelist>
-    <jetty.version>12.0.16</jetty.version>
+    <jetty.version>12.0.17</jetty.version>
     <gitHubRepo>jenkinsci/winstone</gitHubRepo>
     <spotless.check.skip>false</spotless.check.skip>
   </properties>
@@ -145,6 +145,16 @@
       <artifactId>jetty-client</artifactId>
       <scope>test</scope>
     </dependency>
+    <dependency>
+      <groupId>org.eclipse.jetty.http2</groupId>
+      <artifactId>jetty-http2-client</artifactId>
+      <scope>test</scope>
+    </dependency>
+    <dependency>
+      <groupId>org.eclipse.jetty.http2</groupId>
+      <artifactId>jetty-http2-client-transport</artifactId>
+      <scope>test</scope>
+    </dependency>
     <dependency>
       <groupId>org.jenkins-ci</groupId>
       <artifactId>test-annotations</artifactId>

src/main/java/winstone/HostConfiguration.java

@@ -177,6 +177,7 @@
                 }
                 setMaxFormKeys(Option.MAX_PARAM_COUNT.get(args));
                 setMaxFormContentSize(Option.REQUEST_FORM_CONTENT_SIZE.get(args));
+                // TODO ee10 getServletHandler().setDecodeAmbiguousURIs(true);
             }
 
             @Override

src/main/java/winstone/Http2ConnectorFactory.java

@@ -92,7 +92,6 @@ public Connector start(Map<String, String> args, Server server) throws IOExcepti
             http2Connector.setHost(listenAddress);
             server.addConnector(http2Connector);
             server.setDumpAfterStart(Boolean.getBoolean("dumpAfterStart"));
-
             return http2Connector;
         } catch (IllegalStateException e) {
             Logger.log(Level.WARNING, Launcher.RESOURCES, "Http2ConnectorFactory.FailedStart.ALPN", e);

src/test/java/winstone/AbstractWinstoneTest.java

@@ -8,9 +8,16 @@
 import java.nio.file.Path;
 import org.eclipse.jetty.client.ContentResponse;
 import org.eclipse.jetty.client.HttpClient;
+import org.eclipse.jetty.client.Request;
+import org.eclipse.jetty.client.transport.HttpClientConnectionFactory;
 import org.eclipse.jetty.client.transport.HttpClientTransportDynamic;
 import org.eclipse.jetty.http.HttpStatus;
+import org.eclipse.jetty.http.HttpVersion;
+import org.eclipse.jetty.http2.client.HTTP2Client;
+import org.eclipse.jetty.http2.client.transport.ClientConnectionFactoryOverHTTP2;
+import org.eclipse.jetty.io.ClientConnectionFactory;
 import org.eclipse.jetty.io.ClientConnector;
+import org.eclipse.jetty.util.ssl.SslContextFactory;
 import org.junit.After;
 
 /**
@@ -26,29 +33,94 @@
         }
     }
 
+    enum Protocol {
+        HTTP_1,
+        HTTP_2;
+    }
+
+    /**
+     * please use {@link #makeRequest(String, String, Protocol)}
+     */
+    @Deprecated
     public String makeRequest(String url) throws Exception {
-        return makeRequest(null, url);
+        return makeRequest(null, url, Protocol.HTTP_1);
     }
 
+    /**
+     * please use {@link #makeRequest(String, String, Protocol)}
+     */
+    @Deprecated
     public String makeRequest(String path, String url) throws Exception {
+        return makeRequest(path, url, Protocol.HTTP_1);
+    }
+
+    /**
+     *
+     * @param path path to unix domain socket (can be null if not using unix domain socket)
+     * @param url the URL to request
+     * @param protocol see #Protocol
+     * @return the response
+     * @throws Exception
+     */
+    public String makeRequest(String path, String url, Protocol protocol) throws Exception {
+        return makeRequest(path, url, HttpStatus.OK_200, protocol);
+    }
+
+    /**
+     *
+     * @param path path to unix domain socket (can be null if not using unix domain socket)
+     * @param url the URL to request
+     * @param expectedHttpStatus the expected http response code from the server
+     * @param protocol see #Protocol
+     * @return the response
+     * @throws Exception
+     */
+    public String makeRequest(String path, String url, int expectedHttpStatus, Protocol protocol) throws Exception {
+
+        HttpClient httpClient = getHttpClient(path);
+
+        Request request = httpClient.newRequest(url);
+
+        switch (protocol) {
+            case HTTP_1 -> request.version(HttpVersion.HTTP_1_1);
+            case HTTP_2 -> request.version(HttpVersion.HTTP_2);
+            default -> throw new Exception("Unsupported Http Version: " + protocol);
+        }
+
+        ContentResponse response = request.send();
 
+        httpClient.stop();
+
+        assertEquals(expectedHttpStatus, response.getStatus());
+        return response.getContentAsString();
+    }
+
+    protected HttpClient getHttpClient(String path) throws Exception {
         HttpClient httpClient;
 
+        ClientConnector connector;
+
         if (path != null) {
             Path unixDomainPath = Path.of(path);
-            ClientConnector clientConnector = ClientConnector.forUnixDomain(unixDomainPath);
-            httpClient = new HttpClient(new HttpClientTransportDynamic(clientConnector));
+            connector = ClientConnector.forUnixDomain(unixDomainPath);
         } else {
-            httpClient = new HttpClient();
+            SslContextFactory.Client sslContextFactory = new SslContextFactory.Client(true);
+            sslContextFactory.setHostnameVerifier((hostname, session) -> true);
+
+            connector = new ClientConnector();
+            connector.setSslContextFactory(sslContextFactory);
         }
-        httpClient.start();
 
-        ContentResponse response = httpClient.GET(url);
+        ClientConnectionFactory.Info http1 = HttpClientConnectionFactory.HTTP11;
 
-        httpClient.stop();
+        HTTP2Client http2Client = new HTTP2Client(connector);
+        ClientConnectionFactoryOverHTTP2.HTTP2 http2 = new ClientConnectionFactoryOverHTTP2.HTTP2(http2Client);
 
-        assertEquals(HttpStatus.OK_200, response.getStatus());
-        return response.getContentAsString();
+        HttpClientTransportDynamic transport = new HttpClientTransportDynamic(connector, http1, http2);
+        httpClient = new HttpClient(transport);
+        httpClient.setFollowRedirects(false);
+        httpClient.start();
+        return httpClient;
     }
 
     protected void assertConnectionRefused(String host, int port) {

src/test/java/winstone/Http2ConnectorFactoryTest.java

@@ -5,46 +5,16 @@
 import static org.junit.Assert.assertNotNull;
 import static org.junit.Assert.assertTrue;
 
-import java.net.HttpURLConnection;
-import java.net.URI;
-import java.net.http.HttpClient;
-import java.net.http.HttpRequest;
-import java.net.http.HttpResponse;
+import java.net.URLEncoder;
+import java.nio.charset.StandardCharsets;
 import java.util.HashMap;
 import java.util.Map;
-import javax.net.ssl.SSLContext;
-import javax.net.ssl.X509TrustManager;
 import org.eclipse.jetty.server.LowResourceMonitor;
 import org.eclipse.jetty.server.ServerConnector;
 import org.junit.Test;
 
 public class Http2ConnectorFactoryTest extends AbstractWinstoneTest {
 
-    private static final String DISABLE_HOSTNAME_VERIFICATION = "jdk.internal.httpclient.disableHostnameVerification";
-
-    private String request(X509TrustManager tm, int port) throws Exception {
-        String disableHostnameVerification = System.getProperty(DISABLE_HOSTNAME_VERIFICATION);
-        try {
-            System.setProperty(DISABLE_HOSTNAME_VERIFICATION, Boolean.TRUE.toString());
-            HttpRequest request = HttpRequest.newBuilder(new URI("https://localhost:" + port + "/CountRequestsServlet"))
-                    .version(HttpClient.Version.HTTP_2)
-                    .GET()
-                    .build();
-            SSLContext sslContext = SSLContext.getInstance("SSL");
-            sslContext.init(null, new X509TrustManager[] {tm}, null);
-            HttpClient client = HttpClient.newBuilder().sslContext(sslContext).build();
-            HttpResponse<String> response = client.send(request, HttpResponse.BodyHandlers.ofString());
-            assertEquals(HttpURLConnection.HTTP_OK, response.statusCode());
-            return response.body();
-        } finally {
-            if (disableHostnameVerification != null) {
-                System.setProperty(DISABLE_HOSTNAME_VERIFICATION, disableHostnameVerification);
-            } else {
-                System.clearProperty(DISABLE_HOSTNAME_VERIFICATION);
-            }
-        }
-    }
-
     @Test
     public void wildcard() throws Exception {
         Map<String, String> args = new HashMap<>();
@@ -60,10 +30,36 @@ public void wildcard() throws Exception {
         assertConnectionRefused("127.0.0.2", port);
         assertEquals(
                 "<html><body>This servlet has been accessed via GET 1001 times</body></html>\r\n",
-                request(new TrustEveryoneManager(), port));
+                makeRequest(null, "https://localhost:" + port + "/CountRequestsServlet", Protocol.HTTP_2));
         LowResourceMonitor lowResourceMonitor = winstone.server.getBean(LowResourceMonitor.class);
         assertNotNull(lowResourceMonitor);
         assertFalse(lowResourceMonitor.isLowOnResources());
         assertTrue(lowResourceMonitor.isAcceptingInLowResources());
     }
+
+    @Test
+    public void helloSuspiciousPathCharacters() throws Exception {
+        Map<String, String> args = new HashMap<>();
+        args.put("warfile", "target/test-classes/test.war");
+        args.put("prefix", "/");
+        args.put("httpPort", "-1");
+        args.put("http2Port", "0");
+        args.put("http2ListenAddress", "localhost");
+        args.put("httpsKeyStore", "src/ssl/wildcard.jks");
+        args.put("httpsKeyStorePassword", "changeit");
+        winstone = new Launcher(args);
+        int port = ((ServerConnector) winstone.server.getConnectors()[0]).getLocalPort();
+
+        assertEquals(
+                "<html><body>Hello winstone </body></html>\r\n",
+                makeRequest(null, "https://127.0.0.1:" + port + "/hello/winstone", Protocol.HTTP_2));
+
+        assertEquals(
+                "<html><body>Hello win\\stone </body></html>\r\n",
+                makeRequest(
+                        null,
+                        "https://127.0.0.1:" + port + "/hello/"
+                                + URLEncoder.encode("win\\stone", StandardCharsets.UTF_8),
+                        Protocol.HTTP_2)); // %5C == \
+    }
 }

src/test/java/winstone/HttpConnectorFactoryTest.java

@@ -8,6 +8,7 @@
 import static winstone.Launcher.WINSTONE_PORT_FILE_NAME_PROPERTY;
 
 import java.io.IOException;
+import java.net.URLEncoder;
 import java.nio.charset.StandardCharsets;
 import java.nio.file.Files;
 import java.nio.file.Path;
@@ -112,4 +113,23 @@ public void writePortInFile() throws Exception {
         assertEquals(Integer.toString(futurePort.get()), portFileString);
         assertNotEquals(8080, futurePort.get().longValue());
     }
+
+    @Test
+    public void helloSuspiciousPathCharacters() throws Exception {
+        Map<String, String> args = new HashMap<>();
+        args.put("warfile", "target/test-classes/test.war");
+        args.put("prefix", "/");
+        args.put("httpPort", "0");
+        winstone = new Launcher(args);
+        int port = ((ServerConnector) winstone.server.getConnectors()[0]).getLocalPort();
+
+        assertEquals(
+                "<html><body>Hello winstone </body></html>\r\n",
+                makeRequest("http://127.0.0.1:" + port + "/hello/winstone"));
+
+        assertEquals(
+                "<html><body>Hello win\\stone </body></html>\r\n",
+                makeRequest("http://127.0.0.1:" + port + "/hello/"
+                        + URLEncoder.encode("win\\stone", StandardCharsets.UTF_8))); // %5C == \
+    }
 }