Revert "Merge pull request #18772 from vespa-engine/revert-18759-bjorncs/http2"

This reverts commit 4b5d08ebfd7456a820d6eec25704a27d56612b69, reversing
changes made to fed02e0b81cedd76962da597d73462d0d23e0bf3.

6 files changed, 137 insertions, 80 deletions

diff --git a/container-core/src/main/java/com/yahoo/jdisc/http/server/jetty/ConnectorFactory.java b/container-core/src/main/java/com/yahoo/jdisc/http/server/jetty/ConnectorFactory.java
index 92d2cc5d1cd..f4f33afe535 100644
--- a/container-core/src/main/java/com/yahoo/jdisc/http/server/jetty/ConnectorFactory.java
+++ b/container-core/src/main/java/com/yahoo/jdisc/http/server/jetty/ConnectorFactory.java
@@ -8,7 +8,8 @@ import com.yahoo.jdisc.http.ssl.SslContextFactoryProvider;
 import com.yahoo.security.tls.MixedMode;
 import com.yahoo.security.tls.TransportSecurityUtils;
 import org.eclipse.jetty.alpn.server.ALPNServerConnectionFactory;
-import org.eclipse.jetty.http2.parser.RateControl;
+import org.eclipse.jetty.http2.server.AbstractHTTP2ServerConnectionFactory;
+import org.eclipse.jetty.http2.server.HTTP2CServerConnectionFactory;
 import org.eclipse.jetty.http2.server.HTTP2ServerConnectionFactory;
 import org.eclipse.jetty.server.ConnectionFactory;
 import org.eclipse.jetty.server.DetectorConnectionFactory;
@@ -21,7 +22,13 @@ import org.eclipse.jetty.server.ServerConnector;
 import org.eclipse.jetty.server.SslConnectionFactory;
 import org.eclipse.jetty.util.ssl.SslContextFactory;
 
+import java.util.ArrayList;
 import java.util.List;
+import java.util.logging.Logger;
+
+import static com.yahoo.security.tls.MixedMode.DISABLED;
+import static com.yahoo.security.tls.MixedMode.PLAINTEXT_CLIENT_MIXED_SERVER;
+import static com.yahoo.security.tls.MixedMode.TLS_CLIENT_MIXED_SERVER;
 
 /**
  * @author Einar M R Rosenvinge
@@ -29,6 +36,8 @@ import java.util.List;
  */
 public class ConnectorFactory {
 
+    private static final Logger log = Logger.getLogger(ConnectorFactory.class.getName());
+
     private final ConnectorConfig connectorConfig;
     private final SslContextFactoryProvider sslContextFactoryProvider;
 
@@ -50,7 +59,7 @@ public class ConnectorFactory {
     private static void validateProxyProtocolConfiguration(ConnectorConfig config) {
         ConnectorConfig.ProxyProtocol proxyProtocolConfig = config.proxyProtocol();
         if (proxyProtocolConfig.enabled()) {
-            boolean tlsMixedModeEnabled = TransportSecurityUtils.getInsecureMixedMode() != MixedMode.DISABLED;
+            boolean tlsMixedModeEnabled = TransportSecurityUtils.getInsecureMixedMode() != DISABLED;
             if (!isSslEffectivelyEnabled(config) || tlsMixedModeEnabled) {
                 throw new IllegalArgumentException("Proxy protocol can only be enabled if connector is effectively HTTPS only");
             }
@@ -81,61 +90,51 @@ public class ConnectorFactory {
     }
 
     private List<ConnectionFactory> createConnectionFactories(Metric metric) {
-        if (!isSslEffectivelyEnabled(connectorConfig)) {
-            return List.of(newHttp1ConnectionFactory());
-        } else if (connectorConfig.ssl().enabled()) {
+        boolean vespaTlsEnabled = TransportSecurityUtils.isTransportSecurityEnabled();
+        MixedMode tlsMixedMode = TransportSecurityUtils.getInsecureMixedMode();
+        if (connectorConfig.ssl().enabled() || (vespaTlsEnabled && tlsMixedMode == DISABLED)) {
             return connectionFactoriesForHttps(metric);
-        } else if (TransportSecurityUtils.isTransportSecurityEnabled()) {
-            switch (TransportSecurityUtils.getInsecureMixedMode()) {
-                case TLS_CLIENT_MIXED_SERVER:
-                case PLAINTEXT_CLIENT_MIXED_SERVER:
-                    return connectionFactoriesForHttpsMixedMode(metric);
-                case DISABLED:
-                    return connectionFactoriesForHttps(metric);
-                default:
-                    throw new IllegalStateException();
+        } else if (vespaTlsEnabled) {
+            if (tlsMixedMode != TLS_CLIENT_MIXED_SERVER && tlsMixedMode != PLAINTEXT_CLIENT_MIXED_SERVER) {
+                throw new IllegalArgumentException("Unknown mixed mode " + tlsMixedMode);
             }
+            return connectionFactoriesForTlsMixedMode(metric);
         } else {
-            return List.of(newHttp1ConnectionFactory());
+            return connectorConfig.http2Enabled()
+                    ? List.of(newHttp1ConnectionFactory(), newHttp2ClearTextConnectionFactory())
+                    : List.of(newHttp1ConnectionFactory());
         }
     }
 
     private List<ConnectionFactory> connectionFactoriesForHttps(Metric metric) {
+        List<ConnectionFactory> factories = new ArrayList<>();
         ConnectorConfig.ProxyProtocol proxyProtocolConfig = connectorConfig.proxyProtocol();
         HttpConnectionFactory http1Factory = newHttp1ConnectionFactory();
+        ALPNServerConnectionFactory alpnFactory;
+        SslConnectionFactory sslFactory;
         if (connectorConfig.http2Enabled()) {
-            HTTP2ServerConnectionFactory http2Factory = newHttp2ConnectionFactory();
-            ALPNServerConnectionFactory alpnFactory = newAlpnConnectionFactory();
-            SslConnectionFactory sslFactory = newSslConnectionFactory(metric, alpnFactory);
-            if (proxyProtocolConfig.enabled()) {
-                ProxyConnectionFactory proxyProtocolFactory = newProxyProtocolConnectionFactory(sslFactory);
-                if (proxyProtocolConfig.mixedMode()) {
-                    DetectorConnectionFactory detectorFactory = newDetectorConnectionFactory(sslFactory);
-                    return List.of(detectorFactory, proxyProtocolFactory, sslFactory, alpnFactory, http1Factory, http2Factory);
-                } else {
-                    return List.of(proxyProtocolFactory, sslFactory, alpnFactory, http1Factory, http2Factory);
-                }
-            } else {
-                return List.of(sslFactory, alpnFactory, http1Factory, http2Factory);
-            }
+            alpnFactory = newAlpnConnectionFactory();
+            sslFactory = newSslConnectionFactory(metric, alpnFactory);
         } else {
-            SslConnectionFactory sslFactory = newSslConnectionFactory(metric, http1Factory);
-            if (proxyProtocolConfig.enabled()) {
-                ProxyConnectionFactory proxyProtocolFactory = newProxyProtocolConnectionFactory(sslFactory);
-                if (proxyProtocolConfig.mixedMode()) {
-                    DetectorConnectionFactory detectorFactory = newDetectorConnectionFactory(sslFactory);
-                    return List.of(detectorFactory, proxyProtocolFactory, sslFactory, http1Factory);
-                } else {
-                    return List.of(proxyProtocolFactory, sslFactory, http1Factory);
-                }
-            } else {
-                return List.of(sslFactory, http1Factory);
+            alpnFactory = null;
+            sslFactory = newSslConnectionFactory(metric, http1Factory);
+        }
+        if (proxyProtocolConfig.enabled()) {
+            if (proxyProtocolConfig.mixedMode()) {
+                factories.add(newDetectorConnectionFactory(sslFactory));
             }
+            factories.add(newProxyProtocolConnectionFactory(sslFactory));
         }
+        factories.add(sslFactory);
+        if (connectorConfig.http2Enabled()) factories.add(alpnFactory);
+        factories.add(http1Factory);
+        if (connectorConfig.http2Enabled()) factories.add(newHttp2ConnectionFactory());
+        return List.copyOf(factories);
     }
 
-    private List<ConnectionFactory> connectionFactoriesForHttpsMixedMode(Metric metric) {
-        // No support for proxy-protocol/http2 when using HTTP with TLS mixed mode
+    private List<ConnectionFactory> connectionFactoriesForTlsMixedMode(Metric metric) {
+        log.warning(String.format("TLS mixed mode enabled for port %d - HTTP/2 and proxy-protocol are not supported",
+                connectorConfig.listenPort()));
         HttpConnectionFactory httpFactory = newHttp1ConnectionFactory();
         SslConnectionFactory sslFactory = newSslConnectionFactory(metric, httpFactory);
         DetectorConnectionFactory detectorFactory = newDetectorConnectionFactory(sslFactory);
@@ -163,11 +162,21 @@ public class ConnectorFactory {
 
     private HTTP2ServerConnectionFactory newHttp2ConnectionFactory() {
         HTTP2ServerConnectionFactory factory = new HTTP2ServerConnectionFactory(newHttpConfiguration());
+        setHttp2Config(factory);
+        return factory;
+    }
+
+    private HTTP2CServerConnectionFactory newHttp2ClearTextConnectionFactory() {
+        HTTP2CServerConnectionFactory factory = new HTTP2CServerConnectionFactory(newHttpConfiguration());
+        setHttp2Config(factory);
+        return factory;
+    }
+
+    private void setHttp2Config(AbstractHTTP2ServerConnectionFactory factory) {
         factory.setStreamIdleTimeout(toMillis(connectorConfig.http2().streamIdleTimeout()));
         factory.setMaxConcurrentStreams(connectorConfig.http2().maxConcurrentStreams());
         factory.setInitialSessionRecvWindow(1 << 24);
         factory.setInitialStreamRecvWindow(1 << 20);
-        return factory;
     }
 
     private SslConnectionFactory newSslConnectionFactory(Metric metric, ConnectionFactory wrappedFactory) {
diff --git a/container-core/src/test/java/com/yahoo/jdisc/http/server/jetty/Http2Test.java b/container-core/src/test/java/com/yahoo/jdisc/http/server/jetty/Http2Test.java
new file mode 100644
index 00000000000..40b1881bc49
--- /dev/null
+++ b/container-core/src/test/java/com/yahoo/jdisc/http/server/jetty/Http2Test.java
@@ -0,0 +1,67 @@
+// Copyright Verizon Media. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
+package com.yahoo.jdisc.http.server.jetty;
+
+import com.yahoo.container.logging.ConnectionLog;
+import com.yahoo.container.logging.ConnectionLogEntry;
+import com.yahoo.jdisc.http.ConnectorConfig;
+import com.yahoo.jdisc.http.ServerConfig;
+import org.apache.hc.client5.http.async.methods.SimpleHttpResponse;
+import org.apache.hc.client5.http.async.methods.SimpleRequestBuilder;
+import org.apache.hc.client5.http.impl.async.CloseableHttpAsyncClient;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.io.TempDir;
+
+import java.nio.file.Path;
+
+import static com.yahoo.jdisc.Response.Status.OK;
+import static com.yahoo.jdisc.http.server.jetty.Utils.createHttp2Client;
+import static com.yahoo.jdisc.http.server.jetty.Utils.createSslTestDriver;
+import static com.yahoo.jdisc.http.server.jetty.Utils.generatePrivateKeyAndCertificate;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertNull;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+/**
+ * @author bjorncs
+ */
+class Http2Test {
+    @Test
+    void requireThatServerCanRespondToHttp2Request(@TempDir Path tmpFolder) throws Exception {
+        Path privateKeyFile = tmpFolder.resolve("private-key.pem");
+        Path certificateFile = tmpFolder.resolve("certificate.pem");
+        generatePrivateKeyAndCertificate(privateKeyFile, certificateFile);
+
+        MetricConsumerMock metricConsumer = new MetricConsumerMock();
+        InMemoryConnectionLog connectionLog = new InMemoryConnectionLog();
+        JettyTestDriver driver = createSslTestDriver(certificateFile, privateKeyFile, metricConsumer, connectionLog);
+        try (CloseableHttpAsyncClient client = createHttp2Client(driver)) {
+            String uri = "https://localhost:" + driver.server().getListenPort() + "/status.html";
+            SimpleHttpResponse response = client.execute(SimpleRequestBuilder.get(uri).build(), null).get();
+            assertNull(response.getBodyText());
+            assertEquals(OK, response.getCode());
+        }
+        assertTrue(driver.close());
+        ConnectionLogEntry entry = connectionLog.logEntries().get(0);
+        assertEquals("HTTP/2.0", entry.httpProtocol().get());
+    }
+
+    @Test
+    void requireThatServerCanRespondToHttp2PlainTextRequest() throws Exception {
+        InMemoryConnectionLog connectionLog = new InMemoryConnectionLog();
+        JettyTestDriver driver = JettyTestDriver.newConfiguredInstance(
+                new EchoRequestHandler(),
+                new ServerConfig.Builder().connectionLog(new ServerConfig.ConnectionLog.Builder().enabled(true)),
+                new ConnectorConfig.Builder(),
+                binder -> binder.bind(ConnectionLog.class).toInstance(connectionLog));
+        try (CloseableHttpAsyncClient client = createHttp2Client(driver)) {
+            String uri = "http://localhost:" + driver.server().getListenPort() + "/status.html";
+            SimpleHttpResponse response = client.execute(SimpleRequestBuilder.get(uri).build(), null).get();
+            assertNull(response.getBodyText());
+            assertEquals(OK, response.getCode());
+        }
+        assertTrue(driver.close());
+        ConnectionLogEntry entry = connectionLog.logEntries().get(0);
+        assertEquals("HTTP/2.0", entry.httpProtocol().get());
+    }
+
+}
diff --git a/container-core/src/test/java/com/yahoo/jdisc/http/server/jetty/HttpServerTest.java b/container-core/src/test/java/com/yahoo/jdisc/http/server/jetty/HttpServerTest.java
index be96fc2332d..40f02ed676e 100644
--- a/container-core/src/test/java/com/yahoo/jdisc/http/server/jetty/HttpServerTest.java
+++ b/container-core/src/test/java/com/yahoo/jdisc/http/server/jetty/HttpServerTest.java
@@ -34,10 +34,7 @@ import org.apache.hc.client5.http.entity.mime.FormBodyPart;
 import org.apache.hc.client5.http.entity.mime.FormBodyPartBuilder;
 import org.apache.hc.client5.http.entity.mime.StringBody;
 import org.apache.hc.client5.http.impl.async.CloseableHttpAsyncClient;
-import org.apache.hc.client5.http.impl.async.H2AsyncClientBuilder;
-import org.apache.hc.client5.http.ssl.ClientTlsStrategyBuilder;
 import org.apache.hc.core5.http.ContentType;
-import org.apache.hc.core5.http.nio.ssl.TlsStrategy;
 import org.assertj.core.api.Assertions;
 import org.eclipse.jetty.server.handler.AbstractHandlerContainer;
 import org.junit.Rule;
@@ -79,6 +76,7 @@ import static com.yahoo.jdisc.http.HttpHeaders.Names.X_DISABLE_CHUNKING;
 import static com.yahoo.jdisc.http.HttpHeaders.Values.APPLICATION_X_WWW_FORM_URLENCODED;
 import static com.yahoo.jdisc.http.HttpHeaders.Values.CLOSE;
 import static com.yahoo.jdisc.http.server.jetty.SimpleHttpClient.ResponseValidator;
+import static com.yahoo.jdisc.http.server.jetty.Utils.createHttp2Client;
 import static com.yahoo.jdisc.http.server.jetty.Utils.createSslTestDriver;
 import static com.yahoo.jdisc.http.server.jetty.Utils.generatePrivateKeyAndCertificate;
 import static org.cthul.matchers.CthulMatchers.containsPattern;
@@ -90,7 +88,6 @@ import static org.hamcrest.CoreMatchers.startsWith;
 import static org.hamcrest.MatcherAssert.assertThat;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertNotEquals;
-import static org.junit.Assert.assertNull;
 import static org.junit.Assert.assertTrue;
 import static org.junit.Assert.fail;
 import static org.mockito.Mockito.atLeast;
@@ -518,25 +515,6 @@ public class HttpServerTest {
         assertTrue(driver.close());
     }
 
-    @Test
-    public void requireThatServerCanRespondToHttp2Request() throws Exception {
-        Path privateKeyFile = tmpFolder.newFile().toPath();
-        Path certificateFile = tmpFolder.newFile().toPath();
-        generatePrivateKeyAndCertificate(privateKeyFile, certificateFile);
-
-        MetricConsumerMock metricConsumer = new MetricConsumerMock();
-        InMemoryConnectionLog connectionLog = new InMemoryConnectionLog();
-        JettyTestDriver driver = createSslTestDriver(certificateFile, privateKeyFile, metricConsumer, connectionLog);
-        try (CloseableHttpAsyncClient client = createHttp2Client(driver)) {
-            String uri = "https://localhost:" + driver.server().getListenPort() + "/status.html";
-            SimpleHttpResponse response = client.execute(SimpleRequestBuilder.get(uri).build(), null).get();
-            assertNull(response.getBodyText());
-            assertEquals(OK, response.getCode());
-        }
-        assertTrue(driver.close());
-        ConnectionLogEntry entry = connectionLog.logEntries().get(0);
-        assertEquals("HTTP/2.0", entry.httpProtocol().get());
-    }
 
     @Test
     public void requireThatTlsClientAuthenticationEnforcerRejectsRequestsForNonWhitelistedPaths() throws IOException {
@@ -759,18 +737,6 @@ public class HttpServerTest {
         assertTrue(driver.close());
     }
 
-    private static CloseableHttpAsyncClient createHttp2Client(JettyTestDriver driver) {
-        TlsStrategy tlsStrategy = ClientTlsStrategyBuilder.create()
-                                                          .setSslContext(driver.sslContext())
-                                                          .build();
-        var client = H2AsyncClientBuilder.create()
-                                         .disableAutomaticRetries()
-                                         .setTlsStrategy(tlsStrategy)
-                                         .build();
-        client.start();
-        return client;
-    }
-
     private static JettyTestDriver createSslWithTlsClientAuthenticationEnforcer(Path certificateFile, Path privateKeyFile) {
         ConnectorConfig.Builder connectorConfig = new ConnectorConfig.Builder()
                 .tlsClientAuthEnforcer(
diff --git a/container-core/src/test/java/com/yahoo/jdisc/http/server/jetty/JettyTestDriver.java b/container-core/src/test/java/com/yahoo/jdisc/http/server/jetty/JettyTestDriver.java
index 57438cbe207..cf226d7cd18 100644
--- a/container-core/src/test/java/com/yahoo/jdisc/http/server/jetty/JettyTestDriver.java
+++ b/container-core/src/test/java/com/yahoo/jdisc/http/server/jetty/JettyTestDriver.java
@@ -75,7 +75,6 @@ public class JettyTestDriver {
                 requestHandler,
                 new ServerConfig.Builder().connectionLog(new ServerConfig.ConnectionLog.Builder().enabled(true)),
                 new ConnectorConfig.Builder()
-                        .http2Enabled(true)
                         .ssl(new ConnectorConfig.Ssl.Builder()
                                 .enabled(true)
                                 .clientAuth(tlsClientAuth == TlsClientAuth.NEED
diff --git a/container-core/src/test/java/com/yahoo/jdisc/http/server/jetty/ProxyProtocolTest.java b/container-core/src/test/java/com/yahoo/jdisc/http/server/jetty/ProxyProtocolTest.java
index d29abea024e..fe62a9641de 100644
--- a/container-core/src/test/java/com/yahoo/jdisc/http/server/jetty/ProxyProtocolTest.java
+++ b/container-core/src/test/java/com/yahoo/jdisc/http/server/jetty/ProxyProtocolTest.java
@@ -119,7 +119,6 @@ class ProxyProtocolTest {
             Path certificateFile, Path privateKeyFile, RequestLog requestLog,
             ConnectionLog connectionLog, boolean mixedMode) {
         ConnectorConfig.Builder connectorConfig = new ConnectorConfig.Builder()
-                .http2Enabled(true)
                 .proxyProtocol(new ConnectorConfig.ProxyProtocol.Builder()
                         .enabled(true)
                         .mixedMode(mixedMode))
diff --git a/container-core/src/test/java/com/yahoo/jdisc/http/server/jetty/Utils.java b/container-core/src/test/java/com/yahoo/jdisc/http/server/jetty/Utils.java
index 626ab521773..e2949d96b08 100644
--- a/container-core/src/test/java/com/yahoo/jdisc/http/server/jetty/Utils.java
+++ b/container-core/src/test/java/com/yahoo/jdisc/http/server/jetty/Utils.java
@@ -9,6 +9,10 @@ import com.yahoo.security.Pkcs10Csr;
 import com.yahoo.security.Pkcs10CsrBuilder;
 import com.yahoo.security.X509CertificateBuilder;
 import com.yahoo.security.X509CertificateUtils;
+import org.apache.hc.client5.http.impl.async.CloseableHttpAsyncClient;
+import org.apache.hc.client5.http.impl.async.H2AsyncClientBuilder;
+import org.apache.hc.client5.http.ssl.ClientTlsStrategyBuilder;
+import org.apache.hc.core5.http.nio.ssl.TlsStrategy;
 
 import javax.security.auth.x500.X500Principal;
 import java.io.IOException;
@@ -65,4 +69,17 @@ class Utils {
                 .build();
         Files.writeString(certificateFile, X509CertificateUtils.toPem(certificate));
     }
+
+    static CloseableHttpAsyncClient createHttp2Client(JettyTestDriver driver) {
+        TlsStrategy tlsStrategy = ClientTlsStrategyBuilder.create()
+                .setSslContext(driver.sslContext())
+                .build();
+        var client = H2AsyncClientBuilder.create()
+                .disableAutomaticRetries()
+                .setTlsStrategy(tlsStrategy)
+                .build();
+        client.start();
+        return client;
+    }
+
 }