improve testing of http2 client and refine default config (#10580)

* improve testing of http2 client and refine default config
* improve testing in HTTP2Test for hpack

Signed-off-by: Lachlan Roberts <lachlan@webtide.com>

jetty-http2/http2-client/src/main/java/org/eclipse/jetty/http2/client/HTTP2Client.java

@@ -113,7 +113,7 @@ public class HTTP2Client extends ContainerLifeCycle
     private int maxDecoderTableCapacity = HpackContext.DEFAULT_MAX_TABLE_CAPACITY;
     private int maxEncoderTableCapacity = HpackContext.DEFAULT_MAX_TABLE_CAPACITY;
     private int maxHeaderBlockFragment = 0;
-    private int maxResponseHeadersSize = -1;
+    private int maxResponseHeadersSize = 8 * 1024;
     private FlowControlStrategy.Factory flowControlStrategyFactory = () -> new BufferingFlowControlStrategy(0.5F);
     private long streamIdleTimeout;
     private boolean useInputDirectByteBuffers = true;

jetty-http2/http2-client/src/test/java/org/eclipse/jetty/http2/client/HTTP2Test.java

@@ -55,11 +55,14 @@ import org.eclipse.jetty.util.FuturePromise;
 import org.eclipse.jetty.util.IO;
 import org.eclipse.jetty.util.Jetty;
 import org.eclipse.jetty.util.Promise;
+import org.eclipse.jetty.util.StringUtil;
 import org.eclipse.jetty.util.component.Graceful;
 import org.junit.jupiter.api.Test;
 
 import static org.awaitility.Awaitility.await;
 import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.Matchers.containsString;
+import static org.hamcrest.Matchers.equalTo;
 import static org.hamcrest.Matchers.instanceOf;
 import static org.hamcrest.Matchers.is;
 import static org.junit.jupiter.api.Assertions.assertEquals;
@@ -1004,6 +1007,149 @@ public class HTTP2Test extends AbstractTest
         assertFalse(((HTTP2Session)serverSession).getEndPoint().isOpen());
     }
 
+    @Test
+    public void testClientSendsLargeHeader() throws Exception
+    {
+        CountDownLatch settingsLatch = new CountDownLatch(2);
+
+        CompletableFuture<Throwable> serverFailureFuture = new CompletableFuture<>();
+        CompletableFuture<String> serverCloseReasonFuture = new CompletableFuture<>();
+        start(new ServerSessionListener.Adapter()
+        {
+            @Override
+            public void onSettings(Session session, SettingsFrame frame)
+            {
+                settingsLatch.countDown();
+            }
+
+            @Override
+            public void onFailure(Session session, Throwable failure)
+            {
+                serverFailureFuture.complete(failure);
+            }
+
+            @Override
+            public void onClose(Session session, GoAwayFrame frame)
+            {
+                serverCloseReasonFuture.complete(frame.tryConvertPayload());
+            }
+        });
+
+        CompletableFuture<Throwable> clientFailureFuture = new CompletableFuture<>();
+        CompletableFuture<String> clientCloseReasonFuture = new CompletableFuture<>();
+        Session.Listener.Adapter listener = new Session.Listener.Adapter()
+        {
+            @Override
+            public void onSettings(Session session, SettingsFrame frame)
+            {
+                settingsLatch.countDown();
+            }
+
+            @Override
+            public void onFailure(Session session, Throwable failure)
+            {
+                clientFailureFuture.complete(failure);
+            }
+
+            @Override
+            public void onClose(Session session, GoAwayFrame frame)
+            {
+                clientCloseReasonFuture.complete(frame.tryConvertPayload());
+            }
+        };
+
+        HTTP2Session session = (HTTP2Session)newClient(listener);
+        assertTrue(settingsLatch.await(5, TimeUnit.SECONDS));
+        session.getGenerator().getHpackEncoder().setMaxHeaderListSize(1024 * 1024);
+
+        String value = StringUtil.stringFrom("x", 8 * 1024);
+        HttpFields requestFields = HttpFields.build()
+            .put("custom", value);
+        MetaData.Request metaData = newRequest("GET", requestFields);
+        HeadersFrame request = new HeadersFrame(metaData, null, true);
+        session.newStream(request, new FuturePromise<>(), new Stream.Listener.Adapter());
+
+        // Test failure and close reason on client.
+        String closeReason = clientCloseReasonFuture.get(5, TimeUnit.SECONDS);
+        assertThat(closeReason, equalTo("invalid_hpack_block"));
+        assertNull(clientFailureFuture.getNow(null));
+
+        // Test failure and close reason on server.
+        closeReason = serverCloseReasonFuture.get(5, TimeUnit.SECONDS);
+        assertThat(closeReason, equalTo("invalid_hpack_block"));
+        Throwable failure = serverFailureFuture.get(5, TimeUnit.SECONDS);
+        assertThat(failure, instanceOf(IOException.class));
+        assertThat(failure.getMessage(), containsString("invalid_hpack_block"));
+    }
+
+    @Test
+    public void testServerSendsLargeHeader() throws Exception
+    {
+        CompletableFuture<Throwable> serverFailureFuture = new CompletableFuture<>();
+        CompletableFuture<String> serverCloseReasonFuture = new CompletableFuture<>();
+        start(new ServerSessionListener.Adapter()
+        {
+            @Override
+            public Stream.Listener onNewStream(Stream stream, HeadersFrame frame)
+            {
+                HTTP2Session session = (HTTP2Session)stream.getSession();
+                session.getGenerator().getHpackEncoder().setMaxHeaderListSize(1024 * 1024);
+
+                String value = StringUtil.stringFrom("x", 8 * 1024);
+                HttpFields fields = HttpFields.build().put("custom", value);
+                MetaData.Response response = new MetaData.Response(HttpVersion.HTTP_2, HttpStatus.OK_200, fields);
+                stream.headers(new HeadersFrame(stream.getId(), response, null, true));
+                return null;
+            }
+
+            @Override
+            public void onFailure(Session session, Throwable failure)
+            {
+                serverFailureFuture.complete(failure);
+            }
+
+            @Override
+            public void onClose(Session session, GoAwayFrame frame)
+            {
+                serverCloseReasonFuture.complete(frame.tryConvertPayload());
+            }
+        });
+
+        CompletableFuture<Throwable> clientFailureFuture = new CompletableFuture<>();
+        CompletableFuture<String> clientCloseReasonFuture = new CompletableFuture<>();
+        Session.Listener.Adapter listener = new Session.Listener.Adapter()
+        {
+            @Override
+            public void onFailure(Session session, Throwable failure)
+            {
+                clientFailureFuture.complete(failure);
+            }
+
+            @Override
+            public void onClose(Session session, GoAwayFrame frame)
+            {
+                clientCloseReasonFuture.complete(frame.tryConvertPayload());
+            }
+        };
+
+        Session session = newClient(listener);
+        MetaData.Request metaData = newRequest("GET", HttpFields.EMPTY);
+        HeadersFrame request = new HeadersFrame(metaData, null, true);
+        session.newStream(request, new FuturePromise<>(), new Stream.Listener.Adapter());
+
+        // Test failure and close reason on server.
+        String closeReason = serverCloseReasonFuture.get(5, TimeUnit.SECONDS);
+        assertThat(closeReason, equalTo("invalid_hpack_block"));
+        assertNull(serverFailureFuture.getNow(null));
+
+        // Test failure and close reason on client.
+        closeReason = clientCloseReasonFuture.get(5, TimeUnit.SECONDS);
+        assertThat(closeReason, equalTo("invalid_hpack_block"));
+        Throwable failure = clientFailureFuture.get(5, TimeUnit.SECONDS);
+        assertThat(failure, instanceOf(IOException.class));
+        assertThat(failure.getMessage(), containsString("invalid_hpack_block"));
+    }
+
     private static void sleep(long time)
     {
         try

jetty-http2/http2-common/src/main/java/org/eclipse/jetty/http2/parser/HeaderBlockParser.java

@@ -45,6 +45,11 @@ public class HeaderBlockParser
         this.notifier = notifier;
     }
 
+    public int getMaxHeaderListSize()
+    {
+        return hpackDecoder.getMaxHeaderListSize();
+    }
+
     /**
      * Parses @{code blockLength} HPACK bytes from the given {@code buffer}.
      *

jetty-http2/http2-common/src/main/java/org/eclipse/jetty/http2/parser/HeadersBodyParser.java

@@ -170,6 +170,10 @@ public class HeadersBodyParser extends BodyParser
                 {
                     if (hasFlag(Flags.END_HEADERS))
                     {
+                        int maxLength = headerBlockParser.getMaxHeaderListSize();
+                        if (maxLength > 0 && length > maxLength)
+                            return connectionFailure(buffer, ErrorCode.REFUSED_STREAM_ERROR.code, "invalid_headers_frame");
+
                         MetaData metaData = headerBlockParser.parse(buffer, length);
                         if (metaData == HeaderBlockParser.SESSION_FAILURE)
                             return false;

jetty-http2/http2-common/src/main/java/org/eclipse/jetty/http2/parser/PushPromiseBodyParser.java

@@ -119,6 +119,10 @@ public class PushPromiseBodyParser extends BodyParser
                 }
                 case HEADERS:
                 {
+                    int maxLength = headerBlockParser.getMaxHeaderListSize();
+                    if (maxLength > 0 && length > maxLength)
+                        return connectionFailure(buffer, ErrorCode.REFUSED_STREAM_ERROR.code, "invalid_headers_frame");
+
                     MetaData.Request metaData = (MetaData.Request)headerBlockParser.parse(buffer, length);
                     if (metaData == HeaderBlockParser.SESSION_FAILURE)
                         return false;

jetty-http2/http2-hpack/src/main/java/org/eclipse/jetty/http2/hpack/HpackDecoder.java

@@ -92,6 +92,11 @@ public class HpackDecoder
         setMaxTableCapacity(maxTableSizeLimit);
     }
 
+    public int getMaxHeaderListSize()
+    {
+        return _builder.getMaxSize();
+    }
+
     public void setMaxHeaderListSize(int maxHeaderListSize)
     {
         _builder.setMaxSize(maxHeaderListSize);

jetty-http2/http2-hpack/src/main/java/org/eclipse/jetty/http2/hpack/MetaDataBuilder.java

@@ -74,7 +74,7 @@ public class MetaDataBuilder
     {
         HttpHeader header = field.getHeader();
         String name = field.getName();
-        if (name == null || name.length() == 0)
+        if (name == null || name.isEmpty())
             throw new SessionException("Header size 0");
         String value = field.getValue();
         int fieldSize = name.length() + (value == null ? 0 : value.length());
@@ -146,7 +146,7 @@ public class MetaDataBuilder
                 case C_PATH:
                     if (checkPseudoHeader(header, _path))
                     {
-                        if (value != null && value.length() > 0)
+                        if (value != null && !value.isEmpty())
                             _path = value;
                         else
                             streamException("No Path");
@@ -253,7 +253,7 @@ public class MetaDataBuilder
                 else
                     return new MetaData.Request(
                         _method,
-                        _scheme == null ? HttpScheme.HTTP.asString() : _scheme.asString(),
+                        _scheme.asString(),
                         _authority,
                         _path,
                         HttpVersion.HTTP_2,