[httpcomponents-core] 01/01: HTTPCORE-707: H2 connections incorrectly enforce the frame size max limit based on local settings instead of remote ones

[ol...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

olegk pushed a commit to branch HTTPCORE-707
in repository https://gitbox.apache.org/repos/asf/httpcomponents-core.git

commit e1c2e70810fb2f92a10efa63329a5a6c73b6a165
Author: Oleg Kalnichevski <ol...@apache.org>
AuthorDate: Fri Dec 31 15:00:42 2021 +0100

    HTTPCORE-707: H2 connections incorrectly enforce the frame size max limit based on local settings instead of remote ones
---
 .../http2/impl/nio/AbstractH2StreamMultiplexer.java |  5 +++++
 .../hc/core5/http2/impl/nio/FrameOutputBuffer.java  | 21 +++++++++++++--------
 .../core5/http2/impl/nio/TestFrameInOutBuffers.java |  2 +-
 3 files changed, 19 insertions(+), 9 deletions(-)

diff --git a/httpcore5-h2/src/main/java/org/apache/hc/core5/http2/impl/nio/AbstractH2StreamMultiplexer.java b/httpcore5-h2/src/main/java/org/apache/hc/core5/http2/impl/nio/AbstractH2StreamMultiplexer.java
index 2222f52..89976ef 100644
--- a/httpcore5-h2/src/main/java/org/apache/hc/core5/http2/impl/nio/AbstractH2StreamMultiplexer.java
+++ b/httpcore5-h2/src/main/java/org/apache/hc/core5/http2/impl/nio/AbstractH2StreamMultiplexer.java
@@ -1226,6 +1226,11 @@ abstract class AbstractH2StreamMultiplexer implements Identifiable, HttpConnecti
         final int delta = remoteConfig.getInitialWindowSize() - initOutputWinSize;
         initOutputWinSize = remoteConfig.getInitialWindowSize();
 
+        final int maxFrameSize = remoteConfig.getMaxFrameSize();
+        if (maxFrameSize > localConfig.getMaxFrameSize()) {
+            outputBuffer.expand(maxFrameSize);
+        }
+
         if (delta != 0) {
             if (!streamMap.isEmpty()) {
                 for (final Iterator<Map.Entry<Integer, H2Stream>> it = streamMap.entrySet().iterator(); it.hasNext(); ) {
diff --git a/httpcore5-h2/src/main/java/org/apache/hc/core5/http2/impl/nio/FrameOutputBuffer.java b/httpcore5-h2/src/main/java/org/apache/hc/core5/http2/impl/nio/FrameOutputBuffer.java
index aa76a13..f20edfb 100644
--- a/httpcore5-h2/src/main/java/org/apache/hc/core5/http2/impl/nio/FrameOutputBuffer.java
+++ b/httpcore5-h2/src/main/java/org/apache/hc/core5/http2/impl/nio/FrameOutputBuffer.java
@@ -31,8 +31,6 @@ import java.nio.ByteBuffer;
 import java.nio.channels.GatheringByteChannel;
 import java.nio.channels.WritableByteChannel;
 
-import org.apache.hc.core5.http2.H2ConnectionException;
-import org.apache.hc.core5.http2.H2Error;
 import org.apache.hc.core5.http2.H2TransportMetrics;
 import org.apache.hc.core5.http2.frame.FrameConsts;
 import org.apache.hc.core5.http2.frame.RawFrame;
@@ -47,8 +45,8 @@ import org.apache.hc.core5.util.Args;
 public final class FrameOutputBuffer {
 
     private final BasicH2TransportMetrics metrics;
-    private final int maxFramePayloadSize;
-    private final ByteBuffer buffer;
+    private volatile int maxFramePayloadSize;
+    private volatile ByteBuffer buffer;
 
     public FrameOutputBuffer(final BasicH2TransportMetrics metrics, final int maxFramePayloadSize) {
         Args.notNull(metrics, "HTTP2 transport metrics");
@@ -62,14 +60,21 @@ public final class FrameOutputBuffer {
         this(new BasicH2TransportMetrics(), maxFramePayloadSize);
     }
 
+    public void expand(final int maxFramePayloadSize) {
+        this.maxFramePayloadSize = maxFramePayloadSize;
+        final ByteBuffer newBuffer = ByteBuffer.allocate(FrameConsts.HEAD_LEN + maxFramePayloadSize);
+        if (buffer.position() > 0) {
+            buffer.flip();
+            newBuffer.put(buffer);
+        }
+        buffer = newBuffer;
+    }
+
     public void write(final RawFrame frame, final WritableByteChannel channel) throws IOException {
         Args.notNull(frame, "Frame");
 
         final ByteBuffer payload = frame.getPayload();
-        if (payload != null && payload.remaining() > maxFramePayloadSize) {
-            throw new H2ConnectionException(H2Error.FRAME_SIZE_ERROR, "Frame size exceeds maximum");
-        }
-
+        Args.check(payload == null || payload.remaining() <= maxFramePayloadSize, "Frame size exceeds maximum");
         buffer.putInt((payload != null ? payload.remaining() << 8 : 0) | (frame.getType() & 0xff));
         buffer.put((byte) (frame.getFlags() & 0xff));
         buffer.putInt(frame.getStreamId());
diff --git a/httpcore5-h2/src/test/java/org/apache/hc/core5/http2/impl/nio/TestFrameInOutBuffers.java b/httpcore5-h2/src/test/java/org/apache/hc/core5/http2/impl/nio/TestFrameInOutBuffers.java
index c9e8603..2d9cea9 100644
--- a/httpcore5-h2/src/test/java/org/apache/hc/core5/http2/impl/nio/TestFrameInOutBuffers.java
+++ b/httpcore5-h2/src/test/java/org/apache/hc/core5/http2/impl/nio/TestFrameInOutBuffers.java
@@ -251,7 +251,7 @@ public class TestFrameInOutBuffers {
         inBuffer.read(readableChannel);
     }
 
-    @Test(expected = H2ConnectionException.class)
+    @Test(expected = IllegalArgumentException.class)
     public void testWriteFrameExceedingLimit() throws Exception {
         final WritableByteChannelMock writableChannel = new WritableByteChannelMock(1024);
         final FrameOutputBuffer outbuffer = new FrameOutputBuffer(1024);