Issue 5425: Added configurable validate-path and max-concurrent-strea…

…ms to HTTP 2. (#5981)

* Added configurable validate-path and max-concurrent-streams to HTTP 2.
* Removed reflection from the tests.

Signed-off-by: Tomáš Kraus <tomas.kraus@oracle.com>

nima/http2/webserver/src/main/java/io/helidon/nima/http2/webserver/Http2Config.java

@@ -53,6 +53,18 @@ public interface Http2Config {
     @ConfiguredOption("16384")
     int maxClientFrameSize();
 
+    /**
+     * Maximum number of concurrent streams that the server will allow.
+     * Defaults to {@code 8192}. This limit is directional: it applies to the number of streams that the sender
+     * permits the receiver to create.
+     * It is recommended that this value be no smaller than 100 to not unnecessarily limit parallelism
+     * See RFC 9113 section 6.5.2 for details.
+     *
+     * @return maximal number of concurrent streams
+     */
+    @ConfiguredOption("8192")
+    long maxConcurrentStreams();
+
     /**
      * Whether to send error message over HTTP to client.
      * Defaults to {@code false}, as exception message may contain internal information that could be used as an
@@ -63,4 +75,13 @@ public interface Http2Config {
      */
     @ConfiguredOption("false")
     boolean sendErrorDetails();
+
+    /**
+     * If set to false, any path is accepted (even containing illegal characters).
+     *
+     * @return whether to validate path
+     */
+    @ConfiguredOption("true")
+    boolean validatePath();
+
 }

nima/http2/webserver/src/main/java/io/helidon/nima/http2/webserver/Http2Connection.java

@@ -104,6 +104,7 @@ public class Http2Connection implements ServerConnection {
     private int continuationExpectedStreamId;
     private int lastStreamId;
     private long maxClientFrameSize;
+    private long maxClientConcurrentStreams;
     private int streamInitialWindowSize = WindowSize.DEFAULT_WIN_SIZE;
 
     Http2Connection(ConnectionContext ctx, Http2Config http2Config, List<Http2SubProtocolSelector> subProviders) {
@@ -121,6 +122,7 @@ public class Http2Connection implements ServerConnection {
         this.reader = ctx.dataReader();
         this.sendErrorDetails = http2Config.sendErrorDetails();
         this.maxClientFrameSize = http2Config.maxClientFrameSize();
+        this.maxClientConcurrentStreams = http2Config.maxConcurrentStreams();
     }
 
     @Override
@@ -207,6 +209,7 @@ Http2Settings serverSettings() {
     private static void settingsUpdate(Http2Config config, Http2Settings.Builder builder) {
         applySetting(builder, config.maxFrameSize(), Http2Setting.MAX_FRAME_SIZE);
         applySetting(builder, config.maxHeaderListSize(), Http2Setting.MAX_HEADER_LIST_SIZE);
+        applySetting(builder, config.maxConcurrentStreams(), Http2Setting.MAX_CONCURRENT_STREAMS);
     }
 
     // Add value to the builder only when differs from default
@@ -421,6 +424,23 @@ private void doSettings() {
                         + maxClientFrameSize);
             }
 
+            // Set server MAX_CONCURRENT_STREAMS limit when client sends number lower than hard limit
+            // from configuration. Refuse settings if client sends larger number than is configured.
+            this.clientSettings.presentValue(Http2Setting.MAX_CONCURRENT_STREAMS)
+                    .ifPresent(it -> {
+                        if (http2Config.maxConcurrentStreams() >= it) {
+                            maxClientConcurrentStreams = it;
+                        } else {
+                            Http2GoAway frame = new Http2GoAway(0, Http2ErrorCode.PROTOCOL,
+                                    "Value of maximum concurrent streams limit " + it
+                                          + " exceeded hard limit value " + http2Config.maxConcurrentStreams());
+                            connectionWriter.write(
+                                    frame.toFrameData(clientSettings, 0, Http2Flag.NoFlags.create()),
+                                    FlowControl.NOOP);
+
+                        }
+                    });
+
             // TODO for each
             //        Http2Setting.MAX_CONCURRENT_STREAMS;
             //        Http2Setting.MAX_HEADER_LIST_SIZE;
@@ -523,15 +543,14 @@ private void doHeaders() {
 
         receiveFrameListener.headers(ctx, headers);
         headers.validateRequest();
-        // todo configure path validation
         String path = headers.path();
         Http.Method method = headers.method();
         HttpPrologue httpPrologue = HttpPrologue.create(FULL_PROTOCOL,
                                                         PROTOCOL,
                                                         PROTOCOL_VERSION,
                                                         method,
                                                         path,
-                                                        true);
+                                                        http2Config.validatePath());
         stream.prologue(httpPrologue);
         stream.headers(headers, endOfStream);
         state = State.READ_FRAME;
@@ -655,6 +674,12 @@ private StreamContext stream(int streamId) {
                 }
             }
 
+            // MAX_CONCURRENT_STREAMS limit check - according to RFC 9113 section 5.1.2 endpoint MUST treat this
+            // as a stream error (section 5.4.2) of type PROTOCOL_ERROR or REFUSED_STREAM.
+            if (streams.size() > maxClientConcurrentStreams) {
+                throw new Http2Exception(Http2ErrorCode.REFUSED_STREAM,
+                        "Maximum concurrent streams limit " + maxClientConcurrentStreams + " exceeded");
+            }
             streamContext = new StreamContext(streamId,
                                               new Http2Stream(ctx,
                                                               routing,

nima/http2/webserver/src/test/java/io/helidon/nima/http2/webserver/ConnectionConfigTest.java

@@ -16,9 +16,8 @@
 
 package io.helidon.nima.http2.webserver;
 
-import java.lang.reflect.InvocationTargetException;
-import java.lang.reflect.Method;
 import java.util.List;
+import java.util.function.Function;
 
 import org.junit.jupiter.api.Test;
 
@@ -28,6 +27,7 @@
 import io.helidon.nima.webserver.Router;
 import io.helidon.nima.webserver.ServerContext;
 import io.helidon.nima.webserver.WebServer;
+import io.helidon.nima.webserver.spi.ServerConnectionProvider;
 import io.helidon.nima.webserver.spi.ServerConnectionSelector;
 
 import static org.hamcrest.CoreMatchers.is;
@@ -39,40 +39,14 @@ class ConnectionConfigTest {
 
     // Verify that HTTP/2 connection provider is properly configured from config file
     @Test
-    void testConnectionConfig()
-            throws NoSuchMethodException, InvocationTargetException, IllegalAccessException {
-
+    void testConnectionConfig() {
         // This will pick up application.yaml from the classpath as default configuration file
-        Config config = Config.create();
-
-        // Builds LoomServer instance including connectionProviders list.
-        WebServer.Builder wsBuilder = WebServer.builder()
-                .config(config.get("server"));
-
-        // Call wsBuilder.connectionProviders() trough reflection
-        Method connectionProviders
-                = WebServer.Builder.class.getDeclaredMethod("connectionProviders", (Class<?>[]) null);
-        connectionProviders.setAccessible(true);
-        @SuppressWarnings("unchecked")
-        List<ServerConnectionSelector> providers
-                = (List<ServerConnectionSelector>) connectionProviders.invoke(wsBuilder, (Object[]) null);
-
-        // Check whether at least one Http2ConnectionProvider was found
-        boolean haveHttp2Provider = false;
-
-        for (ServerConnectionSelector provider : providers) {
-            if (provider instanceof Http2ConnectionSelector) {
-                haveHttp2Provider = true;
-                Http2Connection conn = (Http2Connection) provider.connection(mockContext());
-                // Verify values to be updated from configuration file
-                assertThat(conn.config().maxFrameSize(), is(8192L));
-                assertThat(conn.config().maxHeaderListSize(), is(4096L));
-                // Verify Http2Settings values to be updated from configuration file
-                assertThat(conn.serverSettings().value(Http2Setting.MAX_FRAME_SIZE), is(8192L));
-                assertThat(conn.serverSettings().value(Http2Setting.MAX_HEADER_LIST_SIZE), is(4096L));
-            }
-        }
-        assertThat("No Http2ConnectionProvider was found", haveHttp2Provider, is(true));
+        TestProvider provider = new TestProvider();
+        WebServer.builder().addConnectionProvider(provider).build();
+        assertThat(provider.isConfig(), is(true));
+        Http2Config http2Config = provider.config();
+        assertThat(http2Config.maxFrameSize(), is(8192L));
+        assertThat(http2Config.maxHeaderListSize(), is(4096L));
     }
 
     // Verify that HTTP/2 connection provider is properly configured from builder
@@ -97,6 +71,54 @@ void testProviderConfigBuilder() {
         assertThat(conn.serverSettings().value(Http2Setting.MAX_HEADER_LIST_SIZE), is(2048L));
     }
 
+    // Verify that HTTP/2 MAX_CONCURRENT_STREAMS is properly configured from builder
+    @Test
+    void testConfigMaxConcurrentStreams()  {
+        // This will pick up application.yaml from the classpath as default configuration file
+        TestProvider provider = new TestProvider();
+        WebServer.builder().addConnectionProvider(provider).build();
+        assertThat(provider.isConfig(), is(true));
+        Http2Config http2Config = provider.config();
+        assertThat(http2Config.maxConcurrentStreams(), is(16384L));
+    }
+
+    // Verify that HTTP/2 validatePath is properly configured from builder
+    @Test
+    void testConfigValidatePath() {
+        // This will pick up application.yaml from the classpath as default configuration file
+        TestProvider provider = new TestProvider();
+        WebServer.builder().addConnectionProvider(provider).build();
+        assertThat(provider.isConfig(), is(true));
+        Http2Config http2Config = provider.config();
+        assertThat(http2Config.validatePath(), is(false));
+    }
+
+    private static class TestProvider implements ServerConnectionProvider {
+
+        private Http2Config http2Config = null;
+
+        @Override
+        public Iterable<String> configKeys() {
+            return List.of("http_2");
+        }
+
+        @Override
+        public ServerConnectionSelector create(Function<String, Config> configs) {
+            Config config = configs.apply("http_2");
+            http2Config = DefaultHttp2Config.toBuilder(config).build();
+            return mock(ServerConnectionSelector.class);
+        }
+
+        private Http2Config config() {
+            return http2Config;
+        }
+
+        private boolean isConfig() {
+            return http2Config != null;
+        }
+
+    }
+
     private static ConnectionContext mockContext() {
         ConnectionContext ctx = mock(ConnectionContext.class);
         when(ctx.router()).thenReturn(Router.empty());

nima/http2/webserver/src/test/resources/application.yaml

@@ -22,3 +22,5 @@ server:
     http_2:
       max-frame-size: 8192
       max-header-list-size: 4096
+      max-concurrent-streams: 16384
+      validate-path: false