Repository: helidon-io/helidon (java)

Issue:
Helidon 4.0.5 fails during HTTP/2 connection upgrade due to wrong Base64 decoding
## Environment Details
* Helidon Version: 4.0.5
* Helidon SE
* JDK version: Zulu 21.0.1
* OS: 6.1.71-1-MANJARO
* Docker version (if applicable): -

----------

I am trying to run a benchmark using Hyperfoil and Helidon 4.0.5 BUT Helidon returns 500 error during H2C connection upgrade (benchmark runs OK when using HTTP 1.1. HTTP2 server properly configured with dependency in pom.xml and tested with curl.


 Helidon shows this error message in the logs:

```
2024.02.20 09:09:28.091 Internal server error
io.helidon.http.RequestException: Internal error
	at io.helidon.http.RequestException$Builder.build(RequestException.java:139)
	at io.helidon.webserver.http1.Http1Connection.handle(Http1Connection.java:227)
	at io.helidon.webserver.ConnectionHandler.run(ConnectionHandler.java:165)
	at io.helidon.common.task.InterruptableTask.call(InterruptableTask.java:47)
	at io.helidon.webserver.ThreadPerTaskExecutor$ThreadBoundFuture.run(ThreadPerTaskExecutor.java:239)
	at java.base/java.lang.VirtualThread.run(VirtualThread.java:309)
Caused by: java.lang.IllegalArgumentException: Illegal base64 character 5f
	at java.base/java.util.Base64$Decoder.decode0(Base64.java:852)
	at java.base/java.util.Base64$Decoder.decode(Base64.java:570)
	at io.helidon.webserver.http2.Http2Upgrader.upgrade(Http2Upgrader.java:80)
	at io.helidon.webserver.http1.Http1Connection.handle(Http1Connection.java:177)
	... 4 more

```
I think the problem is that Helidon is NOT able to properly handle the HTTP2-Settings header. According to the RFC ( https://datatracker.ietf.org/doc/html/draft-ietf-httpbis-http2-07#page-10) , HTTP"-Settings is encoded as a URL- and filename-safe Base64 encoding. That means that "_" is a valid char in this alphabet. But looks to me that Helidon is using plain Base64 that is wrong according to the specs.


![wireshark](https://github.com/helidon-io/helidon/assets/3510476/4ddb8018-4d0d-47f3-8ac9-924c9c73c7d1)

In this case, the HTTP2-Setting headers as produced by Hyperfoil is AAEAABAAAAIAAAABAAN______AAQAAP__AAUAAEAAAAYAACAA, that is valid one according to specs. I attach a picture with the proper detail of the Wireshark capture to illustrate the issue.

I think that Http2Upgrader should use 

`private static final Base64.Decoder BASE_64_DECODER = Base64.getUrlDecoder();`

instead of 

`private static final Base64.Decoder BASE_64_DECODER = Base64.getDecoder();`

I am using hyperfoil 0.24.1 with this config file to easily reproduce the issue:


```
name: helidonConstantRate
threads: 2
http:
  host: http://localhost:8080
  sharedConnections: 100
  allowHttp1x: false
  maxHttp2Streams: 100
ergonomics: # Disable stopping the scenario on 4xx or 5xx response
  autoRangeCheck: false
phases:
- helidon:
    constantRate:
      usersPerSec: 2000
      duration: 120s
      maxDuration: 180s
      scenario:
      - search:
        - httpRequest:
            GET: /simple-greet

```

UPDATE: I tested the recommended fix and it works as expected!!!!

Pull request:
4.0.10: HTTP2-Settings needs to be encoded/decoded to Base64 with url dialect

Backport of #8845 to 4.0.10

Fixes #8399



Patch:
diff --git a/webclient/http2/src/main/java/io/helidon/webclient/http2/Http2ClientConnectionHandler.java b/webclient/http2/src/main/java/io/helidon/webclient/http2/Http2ClientConnectionHandler.java
index 539b1a378f..abb5a8f25d 100644
--- a/webclient/http2/src/main/java/io/helidon/webclient/http2/Http2ClientConnectionHandler.java
+++ b/webclient/http2/src/main/java/io/helidon/webclient/http2/Http2ClientConnectionHandler.java
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2022, 2023 Oracle and/or its affiliates.
+ * Copyright (c) 2022, 2024 Oracle and/or its affiliates.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -208,7 +208,7 @@ class Http2ClientConnectionHandler {
                 .data();
         byte[] b = new byte[settingsFrameData.available()];
         settingsFrameData.read(b);
-        return Base64.getEncoder().encodeToString(b);
+        return Base64.getUrlEncoder().encodeToString(b);
     }
 
     private Http2ConnectionAttemptResult http1(Http2ClientImpl http2Client,
diff --git a/webserver/http2/src/main/java/io/helidon/webserver/http2/Http2Connection.java b/webserver/http2/src/main/java/io/helidon/webserver/http2/Http2Connection.java
index 3713d68f4e..fa70fa15fb 100644
--- a/webserver/http2/src/main/java/io/helidon/webserver/http2/Http2Connection.java
+++ b/webserver/http2/src/main/java/io/helidon/webserver/http2/Http2Connection.java
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2022, 2023 Oracle and/or its affiliates.
+ * Copyright (c) 2022, 2024 Oracle and/or its affiliates.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -321,6 +321,11 @@ public class Http2Connection implements ServerConnection, InterruptableTask<Void
         return serverSettings;
     }
 
+    // jUnit Http2Settings pkg only visible test accessor.
+    Http2Settings clientSettings() {
+        return clientSettings;
+    }
+
     private void doHandle(Semaphore requestSemaphore) throws InterruptedException {
         myThread = Thread.currentThread();
         while (canRun && state != State.FINISHED) {
diff --git a/webserver/http2/src/main/java/io/helidon/webserver/http2/Http2Upgrader.java b/webserver/http2/src/main/java/io/helidon/webserver/http2/Http2Upgrader.java
index 8ec43b0bc5..23209f273e 100644
--- a/webserver/http2/src/main/java/io/helidon/webserver/http2/Http2Upgrader.java
+++ b/webserver/http2/src/main/java/io/helidon/webserver/http2/Http2Upgrader.java
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2022, 2023 Oracle and/or its affiliates.
+ * Copyright (c) 2022, 2024 Oracle and/or its affiliates.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -43,7 +43,6 @@ public class Http2Upgrader implements Http1Upgrader {
                     + "Upgrade: h2c\r\n\r\n")
             .getBytes(StandardCharsets.UTF_8);
     private static final HeaderName HTTP2_SETTINGS_HEADER_NAME = HeaderNames.create("HTTP2-Settings");
-    private static final Base64.Decoder BASE_64_DECODER = Base64.getDecoder();
 
     private final Http2Config config;
     private final List<Http2SubProtocolSelector> subProtocolProviders;
@@ -77,8 +76,7 @@ public class Http2Upgrader implements Http1Upgrader {
                                     WritableHeaders<?> headers) {
         Http2Connection connection = new Http2Connection(ctx, config, subProtocolProviders);
         if (headers.contains(HTTP2_SETTINGS_HEADER_NAME)) {
-            connection.clientSettings(Http2Settings.create(BufferData.create(BASE_64_DECODER.decode(headers.get(
-                    HTTP2_SETTINGS_HEADER_NAME).value().getBytes(StandardCharsets.US_ASCII)))));
+            connection.clientSettings(token68ToHttp2Settings(headers.get(HTTP2_SETTINGS_HEADER_NAME).valueBytes()));
         } else {
             throw new RuntimeException("Bad request -> not " + HTTP2_SETTINGS_HEADER_NAME + " header");
         }
@@ -86,7 +84,7 @@ public class Http2Upgrader implements Http1Upgrader {
         http2Headers.path(prologue.uriPath().rawPath());
         http2Headers.method(prologue.method());
         headers.remove(HeaderNames.HOST,
-                       it -> http2Headers.authority(it.value()));
+                       it -> http2Headers.authority(it.get()));
         http2Headers.scheme("http"); // TODO need to get if https (ctx)?
 
         HttpPrologue newPrologue = HttpPrologue.create(Http2Connection.FULL_PROTOCOL,
@@ -104,4 +102,18 @@ public class Http2Upgrader implements Http1Upgrader {
         return connection;
     }
 
+    /**
+     * <a href="https://datatracker.ietf.org/doc/html/rfc7540#section-3.2.1">RFC7540 3.2.1.</a>
+     * <pre>{@code
+     * HTTP2-Settings    = token68
+     * token68           = 1*( ALPHA / DIGIT / "-" / "." / "_" / "~" / "+" / "/" ) *"="
+     * }</pre>
+     *
+     * @param bytes Base64URL encoded bytes
+     * @return HTTP/2 settings
+     */
+    private static Http2Settings token68ToHttp2Settings(byte[] bytes) {
+        return Http2Settings.create(BufferData.create(Base64.getUrlDecoder().decode(bytes)));
+    }
+
 }


Test patch:
diff --git a/webserver/http2/src/test/java/io/helidon/webserver/http2/UpgradeSettingsTest.java b/webserver/http2/src/test/java/io/helidon/webserver/http2/UpgradeSettingsTest.java
new file mode 100644
index 0000000000..409350b646
--- /dev/null
+++ b/webserver/http2/src/test/java/io/helidon/webserver/http2/UpgradeSettingsTest.java
@@ -0,0 +1,108 @@
+/*
+ * Copyright (c) 2024 Oracle and/or its affiliates.
+ *
+ * Licensed 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 io.helidon.webserver.http2;
+
+import java.util.Base64;
+
+import io.helidon.common.buffers.BufferData;
+import io.helidon.common.buffers.DataWriter;
+import io.helidon.http.HeaderValues;
+import io.helidon.http.HttpPrologue;
+import io.helidon.http.Method;
+import io.helidon.http.WritableHeaders;
+import io.helidon.http.http2.Http2Flag;
+import io.helidon.http.http2.Http2Settings;
+import io.helidon.webserver.ConnectionContext;
+import io.helidon.webserver.ListenerContext;
+import io.helidon.webserver.Router;
+
+import org.junit.jupiter.api.Test;
+
+import static io.helidon.http.http2.Http2Setting.ENABLE_PUSH;
+import static io.helidon.http.http2.Http2Setting.HEADER_TABLE_SIZE;
+import static io.helidon.http.http2.Http2Setting.INITIAL_WINDOW_SIZE;
+import static io.helidon.http.http2.Http2Setting.MAX_CONCURRENT_STREAMS;
+import static io.helidon.http.http2.Http2Setting.MAX_FRAME_SIZE;
+import static io.helidon.http.http2.Http2Setting.MAX_HEADER_LIST_SIZE;
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.Matchers.is;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
+
+class UpgradeSettingsTest {
+
+    static final long MAX_UNSIGNED_INT = 0xFFFFFFFFL;
+
+    private final ConnectionContext ctx;
+    private final HttpPrologue prologue;
+
+    public UpgradeSettingsTest() {
+        ctx = mock(ConnectionContext.class);
+        prologue = HttpPrologue.create("http/1.1",
+                                       "http",
+                                       "1.1",
+                                       Method.GET,
+                                       "/resource.txt",
+                                       false);
+        DataWriter dataWriter = mock(DataWriter.class);
+        when(ctx.router()).thenReturn(Router.empty());
+        when(ctx.listenerContext()).thenReturn(mock(ListenerContext.class));
+        when(ctx.dataWriter()).thenReturn(dataWriter);
+    }
+
+    @Test
+    void urlEncodedSettingsGH8399() {
+        Http2Settings s = upgrade("AAEAABAAAAIAAAABAAN_____AAQAAP__AAUAAEAAAAYAACAA");
+        assertThat(s.presentValue(HEADER_TABLE_SIZE).orElseThrow(), is(4096L));
+        assertThat(s.presentValue(ENABLE_PUSH).orElseThrow(), is(true));
+        assertThat(s.presentValue(MAX_CONCURRENT_STREAMS).orElseThrow(), is(MAX_UNSIGNED_INT / 2));
+        assertThat(s.presentValue(INITIAL_WINDOW_SIZE).orElseThrow(), is(65_535L));
+        assertThat(s.presentValue(MAX_FRAME_SIZE).orElseThrow(), is(16_384L));
+        assertThat(s.presentValue(MAX_HEADER_LIST_SIZE).orElseThrow(), is(8192L));
+    }
+
+    @Test
+    void urlEncodedSettings() {
+        Http2Settings settings2 = Http2Settings.builder()
+                .add(HEADER_TABLE_SIZE, 4096L)
+                .add(ENABLE_PUSH, false)
+                .add(MAX_CONCURRENT_STREAMS, MAX_UNSIGNED_INT - 5)
+                .add(INITIAL_WINDOW_SIZE, 65535L)
+                .add(MAX_FRAME_SIZE, 16384L)
+                .add(MAX_HEADER_LIST_SIZE, 256L)
+                .build();
+        String encSett = Base64.getUrlEncoder().encodeToString(settingsToBytes(settings2));
+        Http2Settings s = upgrade(encSett);
+        assertThat(s.presentValue(MAX_CONCURRENT_STREAMS).orElseThrow(), is(MAX_UNSIGNED_INT - 5));
+        assertThat(s.presentValue(MAX_HEADER_LIST_SIZE).orElseThrow(), is(256L));
+    }
+
+    Http2Settings upgrade(String http2Settings) {
+        WritableHeaders<?> headers = WritableHeaders.create().add(HeaderValues.create("HTTP2-Settings", http2Settings));
+        Http2Upgrader http2Upgrader = Http2Upgrader.create(Http2Config.create());
+        Http2Connection connection = (Http2Connection) http2Upgrader.upgrade(ctx, prologue, headers);
+        return connection.clientSettings();
+    }
+
+    byte[] settingsToBytes(Http2Settings settings) {
+        BufferData settingsFrameData =
+                settings.toFrameData(null, 0, Http2Flag.SettingsFlags.create(0)).data();
+        byte[] b = new byte[settingsFrameData.available()];
+        settingsFrameData.read(b);
+        return b;
+    }
+}