[httpcomponents-core] branch master updated: 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 master
in repository https://gitbox.apache.org/repos/asf/httpcomponents-core.git


The following commit(s) were added to refs/heads/master by this push:
     new a3c77ef  HTTPCORE-707: H2 connections incorrectly enforce the frame size max limit based on local settings instead of remote ones
a3c77ef is described below

commit a3c77ef69d4449881a55f43d600e40029a2ee6fd
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 02ae8b9..f6ce609 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
@@ -1218,6 +1218,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 6465ad3..f1500f1 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
@@ -259,7 +259,7 @@ public class TestFrameInOutBuffers {
 
         final RawFrame frame = new RawFrame(FrameType.DATA.getValue(), 0, 1,
                 ByteBuffer.wrap(new byte[2048]));
-        Assertions.assertThrows(H2ConnectionException.class, () ->
+        Assertions.assertThrows(IllegalArgumentException.class, () ->
                 outbuffer.write(frame, writableChannel));
     }