[MRESOLVER-449] Step toward testing all HTTP transports (#391)

Right now, the HTTP test suite is pulled out and reused for all HTTP transports. It passes for ApacheTransporter (as it was pulled from it), but Jetty and JDK show some signs of some problems... Current results (note: base class HttpTransporterTest has 70 tests, while some transport specific UTs add more): * apache ✔️ 74 test OK * jdk 🔴 62 test OK, 9 test FAIL * jetty 🔴 64 test OK, 6 FAIL The "basics" are overall good (basics pass everywhere), failures stems from small differences. Jetty: it seems it consumes the body (pulls bytes out of it) even before all the HTTP auth happens, this causes that `TransportListener` gets notified "transport started" but in fact is not and test asserts it to not. Also, Jetty throws in tests where server is set up to abruptly close the connection. Retries are not properly done (low level, like abrupt connection drop). JDK: same problem with body (pulls bytes beforehand) and similarly as with Jetty, "transport started" events fails the test. Preemptive auth is not done (as it seems it is either "manual" handling of auth or Authenticator is used but then preemptive auth becomes impossible as headers are removed, no way to set them). It also throws in tests where server abruptly drops connection. Same for retries (in case of TCP issue). --- https://issues.apache.org/jira/browse/MRESOLVER-449
apache · Dec 11, 2023 · 08f102a · 08f102a
1 parent 5e2ae01
commit 08f102a
Show file tree

Hide file tree

Showing 30 changed files with 1,908 additions and 1,308 deletions.
diff --git a/...ether/transport/jetty/JettyException.java → ...ector/transport/http/HttpTransporter.java b/...ether/transport/jetty/JettyException.java → ...ector/transport/http/HttpTransporter.java
@@ -16,22 +16,13 @@
  * specific language governing permissions and limitations
  * under the License.
  */
-package org.eclipse.aether.transport.jetty;
+package org.eclipse.aether.spi.connector.transport.http;
+
+import org.eclipse.aether.spi.connector.transport.Transporter;
 
 /**
- * Exception thrown by {@link JettyTransporter} in case of errors.
+ * A transporter using HTTP protocol.
  *
  * @since 2.0.0
  */
-final class JettyException extends Exception {
-    private final int statusCode;
-
-    JettyException(int statusCode) {
-        super("HTTP Status: " + statusCode);
-        this.statusCode = statusCode;
-    }
-
-    public int getStatusCode() {
-        return statusCode;
-    }
-}
+public interface HttpTransporter extends Transporter {}
diff --git a/...se/aether/transport/jdk/JdkException.java → ...nsport/http/HttpTransporterException.java b/...se/aether/transport/jdk/JdkException.java → ...nsport/http/HttpTransporterException.java
@@ -16,17 +16,17 @@
  * specific language governing permissions and limitations
  * under the License.
  */
-package org.eclipse.aether.transport.jdk;
+package org.eclipse.aether.spi.connector.transport.http;
 
 /**
- * Exception thrown by {@link JdkTransporter} in case of errors.
+ * Exception thrown by {@link HttpTransporter} in case of errors.
  *
  * @since 2.0.0
  */
-final class JdkException extends Exception {
+public class HttpTransporterException extends Exception {
     private final int statusCode;
 
-    JdkException(int statusCode) {
+    public HttpTransporterException(int statusCode) {
         super("HTTP Status: " + statusCode);
         this.statusCode = statusCode;
     }

diff --git a/...src/main/java/org/eclipse/aether/spi/connector/transport/http/HttpTransporterFactory.java b/...src/main/java/org/eclipse/aether/spi/connector/transport/http/HttpTransporterFactory.java
@@ -0,0 +1,45 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.eclipse.aether.spi.connector.transport.http;
+
+import org.eclipse.aether.RepositorySystemSession;
+import org.eclipse.aether.repository.RemoteRepository;
+import org.eclipse.aether.spi.connector.transport.TransporterFactory;
+import org.eclipse.aether.transfer.NoTransporterException;
+
+/**
+ * A factory for {@link HttpTransporter}.
+ *
+ * @since 2.0.0
+ */
+public interface HttpTransporterFactory extends TransporterFactory {
+
+    /**
+     * Tries to create HTTP transporter for the specified remote repository.
+     *
+     * @param session The repository system session from which to configure the transporter, must not be {@code null}.
+     *            In particular, a transporter should obey the timeouts configured for the session.
+     * @param repository The remote repository to create a transporter for, must not be {@code null}.
+     * @return The transporter for the given repository, never {@code null}.
+     * @throws NoTransporterException If the factory cannot create a transporter for the specified remote repository.
+     */
+    @Override
+    HttpTransporter newInstance(RepositorySystemSession session, RemoteRepository repository)
+            throws NoTransporterException;
+}
diff --git a/maven-resolver-test-http/pom.xml b/maven-resolver-test-http/pom.xml
@@ -68,6 +68,18 @@
       <groupId>org.eclipse.jetty</groupId>
       <artifactId>jetty-http</artifactId>
     </dependency>
+    <dependency>
+      <groupId>org.eclipse.jetty</groupId>
+      <artifactId>jetty-alpn-server</artifactId>
+    </dependency>
+    <dependency>
+      <groupId>org.eclipse.jetty</groupId>
+      <artifactId>jetty-alpn-java-server</artifactId>
+    </dependency>
+    <dependency>
+      <groupId>org.eclipse.jetty.http2</groupId>
+      <artifactId>http2-server</artifactId>
+    </dependency>
     <dependency>
       <groupId>org.slf4j</groupId>
       <artifactId>slf4j-api</artifactId>

diff --git a/...solver-test-http/src/main/java/org/eclipse/aether/internal/test/util/http/HttpServer.java b/...solver-test-http/src/main/java/org/eclipse/aether/internal/test/util/http/HttpServer.java
@@ -39,15 +39,11 @@
 
 import org.eclipse.aether.internal.impl.checksum.Sha1ChecksumAlgorithmFactory;
 import org.eclipse.aether.spi.connector.checksum.ChecksumAlgorithmHelper;
+import org.eclipse.jetty.alpn.server.ALPNServerConnectionFactory;
 import org.eclipse.jetty.http.HttpHeader;
 import org.eclipse.jetty.http.HttpMethod;
-import org.eclipse.jetty.server.HttpConfiguration;
-import org.eclipse.jetty.server.HttpConnectionFactory;
-import org.eclipse.jetty.server.Request;
-import org.eclipse.jetty.server.Response;
-import org.eclipse.jetty.server.SecureRequestCustomizer;
-import org.eclipse.jetty.server.Server;
-import org.eclipse.jetty.server.ServerConnector;
+import org.eclipse.jetty.http2.server.HTTP2ServerConnectionFactory;
+import org.eclipse.jetty.server.*;
 import org.eclipse.jetty.server.handler.AbstractHandler;
 import org.eclipse.jetty.server.handler.HandlerList;
 import org.eclipse.jetty.util.IO;
@@ -162,24 +158,37 @@ public HttpServer addSelfSignedSslConnector() {
     private HttpServer addSslConnector(boolean needClientAuth) {
         if (httpsConnector == null) {
             SslContextFactory.Server ssl = new SslContextFactory.Server();
-            if (needClientAuth) {
-                ssl.setNeedClientAuth(true);
-                ssl.setKeyStorePath(new File("src/test/resources/ssl/server-store").getAbsolutePath());
+            ssl.setNeedClientAuth(needClientAuth);
+            if (!needClientAuth) {
+                ssl.setKeyStorePath(HttpTransporterTest.KEY_STORE_SELF_SIGNED_PATH
+                        .toAbsolutePath()
+                        .toString());
                 ssl.setKeyStorePassword("server-pwd");
-                ssl.setTrustStorePath(new File("src/test/resources/ssl/client-store").getAbsolutePath());
-                ssl.setTrustStorePassword("client-pwd");
                 ssl.setSniRequired(false);
             } else {
-                ssl.setNeedClientAuth(false);
-                ssl.setKeyStorePath(new File("src/test/resources/ssl/server-store-selfsigned").getAbsolutePath());
+                ssl.setKeyStorePath(
+                        HttpTransporterTest.KEY_STORE_PATH.toAbsolutePath().toString());
                 ssl.setKeyStorePassword("server-pwd");
+                ssl.setTrustStorePath(
+                        HttpTransporterTest.TRUST_STORE_PATH.toAbsolutePath().toString());
+                ssl.setTrustStorePassword("client-pwd");
                 ssl.setSniRequired(false);
             }
+
             HttpConfiguration httpsConfig = new HttpConfiguration();
             SecureRequestCustomizer customizer = new SecureRequestCustomizer();
             customizer.setSniHostCheck(false);
             httpsConfig.addCustomizer(customizer);
-            httpsConnector = new ServerConnector(server, ssl, new HttpConnectionFactory(httpsConfig));
+
+            HttpConnectionFactory http1 = new HttpConnectionFactory(httpsConfig);
+
+            HTTP2ServerConnectionFactory http2 = new HTTP2ServerConnectionFactory(httpsConfig);
+
+            ALPNServerConnectionFactory alpn = new ALPNServerConnectionFactory();
+            alpn.setDefaultProtocol(http1.getProtocol());
+
+            SslConnectionFactory tls = new SslConnectionFactory(ssl, alpn.getProtocol());
+            httpsConnector = new ServerConnector(server, tls, alpn, http2, http1);
             server.addConnector(httpsConnector);
             try {
                 httpsConnector.start();
@@ -268,6 +277,7 @@ public void stop() throws Exception {
     }
 
     private class ConnectionClosingHandler extends AbstractHandler {
+        @Override
         public void handle(String target, Request req, HttpServletRequest request, HttpServletResponse response) {
             if (connectionsToClose.getAndDecrement() > 0) {
                 Response jettyResponse = (Response) response;
@@ -277,7 +287,7 @@ public void handle(String target, Request req, HttpServletRequest request, HttpS
     }
 
     private class LogHandler extends AbstractHandler {
-
+        @Override
         public void handle(String target, Request req, HttpServletRequest request, HttpServletResponse response) {
             LOGGER.info(
                     "{} {}{}",
@@ -304,7 +314,7 @@ public void handle(String target, Request req, HttpServletRequest request, HttpS
     private static final Pattern SIMPLE_RANGE = Pattern.compile("bytes=([0-9])+-");
 
     private class RepoHandler extends AbstractHandler {
-
+        @Override
         public void handle(String target, Request req, HttpServletRequest request, HttpServletResponse response)
                 throws IOException {
             String path = req.getPathInfo().substring(1);
@@ -438,7 +448,7 @@ public void handle(String target, Request req, HttpServletRequest request, HttpS
     }
 
     private class RedirectHandler extends AbstractHandler {
-
+        @Override
         public void handle(String target, Request req, HttpServletRequest request, HttpServletResponse response) {
             String path = req.getPathInfo();
             if (!path.startsWith("/redirect/")) {
@@ -465,7 +475,7 @@ public void handle(String target, Request req, HttpServletRequest request, HttpS
     }
 
     private class AuthHandler extends AbstractHandler {
-
+        @Override
         public void handle(String target, Request req, HttpServletRequest request, HttpServletResponse response)
                 throws IOException {
             if (ExpectContinue.BROKEN.equals(expectContinue)
@@ -485,7 +495,7 @@ public void handle(String target, Request req, HttpServletRequest request, HttpS
     }
 
     private class ProxyAuthHandler extends AbstractHandler {
-
+        @Override
         public void handle(String target, Request req, HttpServletRequest request, HttpServletResponse response) {
             if (proxyUsername != null && proxyPassword != null) {
                 if (checkBasicAuth(