You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by re...@apache.org on 2026/10/13 18:40:59 UTC
(tomcat) 04/09: Cap the header frame payload buffer expansion at the maximum frame size
This is an automated email from the ASF dual-hosted git repository.
rmaucher pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/tomcat.git
commit 83f2b53732a9e0e62d63c58ae4a16f027a8b0d1c
Author: opencode <re...@apache.org>
AuthorDate: Tue Oct 13 15:18:59 2026 +0200
Cap the header frame payload buffer expansion at the maximum frame size
Both implementations of HeaderFrameBuffers expanded the payload buffer
by doubling the current size on every expansion, with no upper bound
and no overflow guard. For a single header field too large to fit in
the current buffer this resulted in allocations of one gigabyte or more
(probable OutOfMemoryError) and, once the doubled size passed
Integer.MAX_VALUE, an integer overflow to a negative value causing
ByteBuffer.allocate() to throw IllegalArgumentException. Worse, for
single fields in the range of 16 MiB to 1 GiB the old code continued
until the field fit and then wrote the frame with the payload size
truncated modulo 2^24 by setThreeBytes(), silently corrupting the
framing on the wire.
Cap the expansion at the maximum frame size that can be represented in
the frame header (2^24-1, already defined as MAX_MAX_FRAME_SIZE for
SETTINGS validation). The payload buffer can now never exceed what the
framing can represent. If the encoded headers still do not fit in a
buffer of that size, an IllegalStateException with an i18n message is
thrown between frames. The resulting failure is contained: the stream
is closed and a GOAWAY is sent, matching the handling of other
unchecked exceptions on the write path. Behaviour for every buffer size
below the cap is unchanged. Add a unit test that drives the async
expansion logic to the cap via reflection and verifies the cap and the
clean failure beyond it.
---
.../coyote/http2/Http2AsyncUpgradeHandler.java | 10 +++-
.../apache/coyote/http2/Http2UpgradeHandler.java | 11 +++-
.../apache/coyote/http2/LocalStrings.properties | 1 +
.../coyote/http2/TestHttp2AsyncUpgradeHandler.java | 67 ++++++++++++++++++++++
4 files changed, 87 insertions(+), 2 deletions(-)
diff --git a/java/org/apache/coyote/http2/Http2AsyncUpgradeHandler.java b/java/org/apache/coyote/http2/Http2AsyncUpgradeHandler.java
index f34e8168c7..8eaa6eeaad 100644
--- a/java/org/apache/coyote/http2/Http2AsyncUpgradeHandler.java
+++ b/java/org/apache/coyote/http2/Http2AsyncUpgradeHandler.java
@@ -583,7 +583,15 @@ public class Http2AsyncUpgradeHandler extends Http2UpgradeHandler {
@Override
public void expandPayload() {
- payloadSize = payloadSize * 2;
+ // The encoded headers are written as the payload of a single
+ // frame so a buffer larger than the largest frame size that can
+ // be represented is never usable. Fail rather than continue
+ // doubling, which would eventually overflow the buffer size.
+ if (payloadSize >= ConnectionSettingsBase.MAX_MAX_FRAME_SIZE) {
+ throw new IllegalStateException(sm.getString("upgradeHandler.headerPayloadTooBig",
+ Integer.toString(ConnectionSettingsBase.MAX_MAX_FRAME_SIZE)));
+ }
+ payloadSize = Integer.min(payloadSize * 2, ConnectionSettingsBase.MAX_MAX_FRAME_SIZE);
payload = ByteBuffer.allocate(payloadSize);
}
}
diff --git a/java/org/apache/coyote/http2/Http2UpgradeHandler.java b/java/org/apache/coyote/http2/Http2UpgradeHandler.java
index 17b3679893..53ab4950bb 100644
--- a/java/org/apache/coyote/http2/Http2UpgradeHandler.java
+++ b/java/org/apache/coyote/http2/Http2UpgradeHandler.java
@@ -2248,7 +2248,16 @@ class Http2UpgradeHandler extends AbstractStream implements InternalHttpUpgradeH
@Override
public void expandPayload() {
- payload = ByteBuffer.allocate(payload.capacity() * 2);
+ // The encoded headers are written as the payload of a single
+ // frame so a buffer larger than the largest frame size that can
+ // be represented is never usable. Fail rather than continue
+ // doubling, which would eventually overflow the buffer size.
+ if (payload.capacity() >= ConnectionSettingsBase.MAX_MAX_FRAME_SIZE) {
+ throw new IllegalStateException(sm.getString("upgradeHandler.headerPayloadTooBig",
+ Integer.toString(ConnectionSettingsBase.MAX_MAX_FRAME_SIZE)));
+ }
+ payload = ByteBuffer.allocate(
+ Integer.min(payload.capacity() * 2, ConnectionSettingsBase.MAX_MAX_FRAME_SIZE));
}
}
}
diff --git a/java/org/apache/coyote/http2/LocalStrings.properties b/java/org/apache/coyote/http2/LocalStrings.properties
index a7ef71f8c4..cab0b658b4 100644
--- a/java/org/apache/coyote/http2/LocalStrings.properties
+++ b/java/org/apache/coyote/http2/LocalStrings.properties
@@ -148,6 +148,7 @@ upgradeHandler.enableRfc7540Priorities=Connection [{0}], RFC 7540 priorities may
upgradeHandler.fallToDebug=\n\
\ Note: further occurrences of HTTP/2 stream errors will be logged at DEBUG level.
upgradeHandler.goaway.debug=Connection [{0}], Goaway, Last stream [{1}], Error code [{2}], Debug data [{3}]
+upgradeHandler.headerPayloadTooBig=The encoded headers do not fit in a buffer with a size equal to the maximum permitted frame size of [{0}] bytes
upgradeHandler.init=Connection [{0}], State [{1}]
upgradeHandler.invalidPreface=Connection [{0}], Invalid connection preface
upgradeHandler.ioerror=Connection [{0}]
diff --git a/test/org/apache/coyote/http2/TestHttp2AsyncUpgradeHandler.java b/test/org/apache/coyote/http2/TestHttp2AsyncUpgradeHandler.java
new file mode 100644
index 0000000000..2043a57a23
--- /dev/null
+++ b/test/org/apache/coyote/http2/TestHttp2AsyncUpgradeHandler.java
@@ -0,0 +1,67 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You 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 org.apache.coyote.http2;
+
+import java.lang.reflect.Constructor;
+import java.lang.reflect.Field;
+import java.lang.reflect.InvocationTargetException;
+import java.lang.reflect.Method;
+
+import org.junit.Assert;
+import org.junit.Test;
+
+public class TestHttp2AsyncUpgradeHandler {
+
+ /*
+ * AsyncHeaderFrameBuffers is a private, static nested class so reflection is used to test that payload expansion is
+ * capped at the maximum frame size that can be represented in a frame header. Previously the payload size was
+ * simply doubled on every expansion which, for very large header blocks, eventually overflowed the size resulting
+ * in an attempt to allocate a buffer with a negative capacity.
+ */
+ @Test
+ public void testPayloadExpansionIsCapped() throws Exception {
+ Class<?> clazz = Class.forName("org.apache.coyote.http2.Http2AsyncUpgradeHandler$AsyncHeaderFrameBuffers");
+ Constructor<?> constructor = clazz.getDeclaredConstructor(int.class);
+ constructor.setAccessible(true);
+ Object buffers = constructor.newInstance(Integer.valueOf(1024));
+ Method expandPayload = clazz.getDeclaredMethod("expandPayload");
+ expandPayload.setAccessible(true);
+
+ int expectedSize = 1024;
+ while (expectedSize < ConnectionSettingsBase.MAX_MAX_FRAME_SIZE) {
+ expandPayload.invoke(buffers);
+ expectedSize = Integer.min(expectedSize * 2, ConnectionSettingsBase.MAX_MAX_FRAME_SIZE);
+ Assert.assertEquals(expectedSize, readPayloadSize(clazz, buffers));
+ }
+
+ // A buffer that is already as large as the largest permitted frame
+ // can not be expanded further
+ try {
+ expandPayload.invoke(buffers);
+ Assert.fail("Expansion beyond the maximum permitted frame size was expected to fail.");
+ } catch (InvocationTargetException e) {
+ Assert.assertEquals(IllegalStateException.class, e.getCause().getClass());
+ }
+ }
+
+
+ private int readPayloadSize(Class<?> clazz, Object buffers) throws Exception {
+ Field field = clazz.getDeclaredField("payloadSize");
+ field.setAccessible(true);
+ return ((Integer) field.get(buffers)).intValue();
+ }
+}
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org