This is an automated email from the ASF dual-hosted git repository. rmaucher pushed a commit to branch 9.0.x in repository https://gitbox.apache.org/repos/asf/tomcat.git
commit 7e08402c20707ec0de26b8df544d40072fd7f3ed Author: opencode <[email protected]> AuthorDate: Tue Oct 13 14:54:42 2026 +0200 Validate the range of HTTP/2 settings provided by the administrator The base connection settings validation performed no checks for MAX_CONCURRENT_STREAMS and MAX_HEADER_LIST_SIZE. Since these settings are serialized in a SETTINGS frame as an unsigned 32-bit integer, out-of-range values were silently truncated when the frame was written. For example, an administrator configured maxConcurrentStreams of -1 (the common Tomcat convention for unlimited) was advertised to clients as 4294967295 while Tomcat's own limit enforcement used -1, causing every new stream to be rejected with REFUSED_STREAM. A value of 2^32 happened to match the internal UNLIMITED sentinel and so silently disabled the limit, while 2^32+1 was advertised as 1. Validate these settings against the range that can be represented in a SETTINGS frame (0 to 2^32-1) so an invalid configuration is reported when the HTTP/2 connection is opened rather than silently mis-advertised, document the permitted range for maxConcurrentStreams on the Http2Protocol setter and in the HTTP/2 connector documentation, correct the ConnectionSettingsLocal class Javadoc, and extend the unit tests for the local settings validation paths. Values received from a peer are always in range since they are read as unsigned 32-bit integers, so there is no change in behaviour for peers. --- .../coyote/http2/ConnectionSettingsBase.java | 19 ++++++++-- .../coyote/http2/ConnectionSettingsLocal.java | 6 ++-- java/org/apache/coyote/http2/Http2Protocol.java | 4 ++- .../apache/coyote/http2/LocalStrings.properties | 1 + .../coyote/http2/TestConnectionSettingsLocal.java | 41 ++++++++++++++++++++++ webapps/docs/config/http2.xml | 4 +++ 6 files changed, 68 insertions(+), 7 deletions(-) diff --git a/java/org/apache/coyote/http2/ConnectionSettingsBase.java b/java/org/apache/coyote/http2/ConnectionSettingsBase.java index a6439202f1..703bc8c2c0 100644 --- a/java/org/apache/coyote/http2/ConnectionSettingsBase.java +++ b/java/org/apache/coyote/http2/ConnectionSettingsBase.java @@ -35,6 +35,7 @@ abstract class ConnectionSettingsBase<T extends Throwable> { static final int MIN_MAX_FRAME_SIZE = 1 << 14; static final int MAX_MAX_FRAME_SIZE = (1 << 24) - 1; static final long UNLIMITED = ((long) 1 << 32); // Internal "no limit" sentinel; not a valid 32-bit SETTINGS value + static final long MAX_SETTING_VALUE = 0xFFFFFFFFL; // Largest value that can be serialized in a SETTINGS frame static final int MAX_HEADER_TABLE_SIZE = 1 << 16; // Defaults (defined by the specification) @@ -84,7 +85,7 @@ abstract class ConnectionSettingsBase<T extends Throwable> { validateEnablePush(value); break; case MAX_CONCURRENT_STREAMS: - // No further validation required + validateUnsigned32BitSetting(setting, value); break; case INITIAL_WINDOW_SIZE: validateInitialWindowSize(value); @@ -93,7 +94,7 @@ abstract class ConnectionSettingsBase<T extends Throwable> { validateMaxFrameSize(value); break; case MAX_HEADER_LIST_SIZE: - // No further validation required + validateUnsigned32BitSetting(setting, value); break; case NO_RFC7540_PRIORITIES: validateNoRfc7540Priorities(value); @@ -254,6 +255,20 @@ abstract class ConnectionSettingsBase<T extends Throwable> { } + private void validateUnsigned32BitSetting(Setting setting, long value) throws T { + // Values are serialized in a SETTINGS frame as an unsigned 32-bit + // integer so anything outside that range can not be used. Values + // received from a peer via Http2Parser are always in range since they + // are read as unsigned 32-bit integers. This validation is primarily + // to guard against invalid values provided via local configuration. + if (value < 0 || value > MAX_SETTING_VALUE) { + String msg = sm.getString("connectionSettings.settingValueInvalid", connectionId, setting.name(), + Long.toString(value), "0", Long.toString(MAX_SETTING_VALUE)); + throwException(msg, Http2Error.PROTOCOL_ERROR); + } + } + + abstract void throwException(String msg, Http2Error error) throws T; abstract String getEndpointName(); diff --git a/java/org/apache/coyote/http2/ConnectionSettingsLocal.java b/java/org/apache/coyote/http2/ConnectionSettingsLocal.java index 00ba630465..a210adf005 100644 --- a/java/org/apache/coyote/http2/ConnectionSettingsLocal.java +++ b/java/org/apache/coyote/http2/ConnectionSettingsLocal.java @@ -23,10 +23,8 @@ import java.util.Map; * server. There will be a delay between calling a setter and the setting taking effect at the client. When a setter is * called, the new value is added to the set of pending settings. Once the ACK is received, the new value is moved to * the current settings. While waiting for the ACK, the getters will return the most lenient / generous / relaxed of the - * current setting and the pending setting. Values passed to the setters are validated by the base class, except for - * {@code MAX_CONCURRENT_STREAMS} and {@code MAX_HEADER_LIST_SIZE}. If an invalid value is used the client will respond - * (almost certainly by closing the connection) as defined in the HTTP/2 - * specification. + * current setting and the pending setting. Values passed to the setters are validated by the base class. If an invalid + * value is used an {@link IllegalArgumentException} is thrown. */ class ConnectionSettingsLocal extends ConnectionSettingsBase<IllegalArgumentException> { diff --git a/java/org/apache/coyote/http2/Http2Protocol.java b/java/org/apache/coyote/http2/Http2Protocol.java index a301cc692a..0b12f61d6a 100644 --- a/java/org/apache/coyote/http2/Http2Protocol.java +++ b/java/org/apache/coyote/http2/Http2Protocol.java @@ -342,7 +342,9 @@ public class Http2Protocol implements UpgradeProtocol { /** - * Sets the maximum number of concurrent streams. + * Sets the maximum number of concurrent streams. The value must be in the range {@code 0} to {@code 2^32-1} (both + * inclusive) as this value is advertised to the client as an unsigned 32-bit integer. Values outside of this range + * will cause HTTP/2 connections to be rejected. * * @param maxConcurrentStreams the maximum concurrent streams */ diff --git a/java/org/apache/coyote/http2/LocalStrings.properties b/java/org/apache/coyote/http2/LocalStrings.properties index f469dc7151..6fe7897867 100644 --- a/java/org/apache/coyote/http2/LocalStrings.properties +++ b/java/org/apache/coyote/http2/LocalStrings.properties @@ -30,6 +30,7 @@ connectionSettings.enablePushInvalid=Connection [{0}], The requested value for e connectionSettings.headerTableSizeLimit=Connection [{0}], Attempted to set a header table size of [{1}] but the limit is 16k connectionSettings.maxFrameSizeInvalid=Connection [{0}], The requested maximum frame size of [{1}] is outside the permitted range of [{2}] to [{3}] connectionSettings.noRfc7540PrioritiesInvalid=Connection [{0}], The requested no RFC 7540 priorities setting [{1}] was not zero or one +connectionSettings.settingValueInvalid=Connection [{0}], The requested value [{2}] for the setting [{1}] is outside the permitted range of [{3}] to [{4}] connectionSettings.unknown=Connection [{0}], An unknown setting with identifier [{1}] and value [{2}] was ignored connectionSettings.windowSizeTooBig=Connection [{0}], The requested window size of [{1}] is bigger than the maximum permitted value of [{2}] connectionSettings.windowSizeTooSmall=Connection [{0}], The requested window size of [{1}] is smaller than the minimum permitted value of [0] diff --git a/test/org/apache/coyote/http2/TestConnectionSettingsLocal.java b/test/org/apache/coyote/http2/TestConnectionSettingsLocal.java index dc48a5ee48..35536fe788 100644 --- a/test/org/apache/coyote/http2/TestConnectionSettingsLocal.java +++ b/test/org/apache/coyote/http2/TestConnectionSettingsLocal.java @@ -42,4 +42,45 @@ public class TestConnectionSettingsLocal { ConnectionSettingsLocal settings = new ConnectionSettingsLocal("0"); settings.set(Setting.INITIAL_WINDOW_SIZE, ConnectionSettingsBase.MAX_WINDOW_SIZE + 1L); } + + + @Test(expected = IllegalArgumentException.class) + public void testMaxConcurrentStreamsNegative() { + // Use of a negative value is not permitted by the specification and + // can not be represented in a SETTINGS frame + ConnectionSettingsLocal settings = new ConnectionSettingsLocal("0"); + settings.set(Setting.MAX_CONCURRENT_STREAMS, -1); + } + + + @Test(expected = IllegalArgumentException.class) + public void testMaxConcurrentStreamsTooBig() { + // Values larger than an unsigned 32-bit integer can not be + // represented in a SETTINGS frame + ConnectionSettingsLocal settings = new ConnectionSettingsLocal("0"); + settings.set(Setting.MAX_CONCURRENT_STREAMS, ConnectionSettingsBase.MAX_SETTING_VALUE + 1); + } + + + @Test + public void testMaxConcurrentStreamsMaxValue() { + // The largest value that can be represented in a SETTINGS frame is + // permitted + ConnectionSettingsLocal settings = new ConnectionSettingsLocal("0"); + settings.set(Setting.MAX_CONCURRENT_STREAMS, ConnectionSettingsBase.MAX_SETTING_VALUE); + } + + + @Test(expected = IllegalArgumentException.class) + public void testMaxHeaderListSizeNegative() { + ConnectionSettingsLocal settings = new ConnectionSettingsLocal("0"); + settings.set(Setting.MAX_HEADER_LIST_SIZE, -1); + } + + + @Test(expected = IllegalArgumentException.class) + public void testMaxHeaderListSizeTooBig() { + ConnectionSettingsLocal settings = new ConnectionSettingsLocal("0"); + settings.set(Setting.MAX_HEADER_LIST_SIZE, ConnectionSettingsBase.MAX_SETTING_VALUE + 1); + } } diff --git a/webapps/docs/config/http2.xml b/webapps/docs/config/http2.xml index d65a2e812e..71efd751c8 100644 --- a/webapps/docs/config/http2.xml +++ b/webapps/docs/config/http2.xml @@ -182,6 +182,10 @@ <p>The controls the maximum number of active streams permitted for any one connection. If a client attempts to open more active streams than this limit, the stream will be reset with a <code>STREAM_REFUSED</code> error. + The value must be in the range <code>0</code> to <code>2^32-1</code> + (both inclusive) as this value is advertised to the client as an + unsigned 32-bit integer. Values outside of this range will cause HTTP/2 + connections to be rejected. If not specified, the default value of <code>100</code> will be used.</p> </attribute> --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
