HTTP/2: Should we restrict value range for SETTINGS_MAX_CONCURRENT_STREAMS?

[idelpivnitskiy]

Http2Settings validate the value range based on values in Http2CodecUtil:

netty/codec-http2/src/main/java/io/netty/handler/codec/http2/Http2CodecUtil.java

Line 92 in 6a18f8f

public static final long MAX_CONCURRENT_STREAMS = MAX_UNSIGNED_INT;

It accepts any value from 0 to an unsigned 32-bit integer (4,294,967,295). This seems legal because RFC reserves 32-bits for a value in Settings frame: https://datatracker.ietf.org/doc/html/rfc9113#section-6.5.1

Following Section 6.5.2 Defined Settings applies additional restrictions to some settings, like SETTINGS_ENABLE_PUSH, SETTINGS_INITIAL_WINDOW_SIZE, SETTINGS_MAX_FRAME_SIZE, and clarifies what to do when value is out of the specified range. There are no explicit clarifications about the value range for SETTINGS_MAX_CONCURRENT_STREAMS.

However, Section 5.1.1 Stream Identifiers says:

Streams are identified by an unsigned 31-bit integer. Streams initiated by a client MUST use odd-numbered stream identifiers; those initiated by the server MUST use even-numbered stream identifiers.

This means there could be no streams with an id greater than 2,147,483,647 and each side can have no more than 1,073,741,824 streams total.

Http2Stream#id() return type is int. DefaultHttp2Connection.canOpenStream() also operates with integers.

Since RFC doesn't define behavior for SETTINGS_MAX_CONCURRENT_STREAMS values outside this range, what would be your expected behavior?

1. Netty silently overrides the user-specified or received value that is out of reasonable range (current behavior):
   

   netty/codec-http2/src/main/java/io/netty/handler/codec/http2/DefaultHttp2ConnectionDecoder.java

   Lines 476 to 479 in 6a18f8f

   	Long maxConcurrentStreams = settings.maxConcurrentStreams();
   	if (maxConcurrentStreams != null) {
   	connection.remote().maxActiveStreams((int) min(maxConcurrentStreams, MAX_VALUE));
   	}

2. Netty should throw an exception if users set (or it receives) a value greater than 2^31 - 1 or maybe even 2^30.

The main issue with (1) is that every Netty-based library should be aware of this silent behavior and do the same if it reads Http2Settings.maxConcurrentStreams() as a Long value.

On the other hand, there are deployed systems (etcd) that use unsigned 32-bit integer (4,294,967,295) as the default value. Making this change in Netty will mean it won't be able to connect until those systems change the default value.

WDYT?

[idelpivnitskiy]

cc @normanmaurer @chrisvest @ejona86

[chrisvest]

Sounds like setting SETTINGS_MAX_CONCURRENT_STREAMS to a value greater than the max stream identifier value effectively means the number of concurrent streams are unbounded, and clamping it from one unbounded value to a smaller but still unbounded value will make no behavioral difference.

I don't understand how netty is prevented from connecting to etcd?

[chrisvest]

For reference, this is the discussion that etcd had about this default value: https://github.com/etcd-io/etcd/pull/14169/files#r913763470
And the HTTP/2 server in the Go standard library: https://github.com/grpc/grpc-go/blob/5b509df1e30a25b14dba119c09295924c9e17abe/internal/transport/http2_server.go#L168-L172

[ejona86]

Is there a problem today? It sounds like "everything is fine, but if we change something it could break things." I don't see any proposed gain for adding validation.

The main issue with (1) is that every Netty-based library should be aware of this silent behavior and do the same if it reads Http2Settings.maxConcurrentStreams() as a Long value.

Well, sorta. If it is null it means "no limit" and implementations could choose to replace that with MAX_UINT to simplify handling the case.

[idelpivnitskiy]

I don't understand how netty is prevented from connecting to etcd?

It sounds like "everything is fine, but if we change something it could break things."

Exactly. Today Netty behaves like (1), changing the behavior to (2) will not allow connecting to etcd until etcd changes the value to the range we require.

Well, sorta. If it is null it means "no limit" and implementations could choose to replace that with MAX_UINT to simplify handling the case.

No issues with null values. In this case libs on top of Netty can either ignore the new Http2Settings frame or use some pre-defined default.

Confusion is when it receives values above 2^31 - 1 (Integer.MAX_VALUE). Because all other Netty API uses int when working with number of streams, it's not intuitive that the value returned from Http2Settings.maxConcurrentStreams() can result in a negative value if users cast it or use Long.intValue(). In comparison, initialWindowSize() and maxFrameSize() have Integer as return type.

If changing the validation can break things, will it make sense to add maxConcurrentStreamsAsInt() method that returns Integer? It can have adjustment logic internally, similar to:

netty/codec-http2/src/main/java/io/netty/handler/codec/http2/DefaultHttp2ConnectionDecoder.java

Line 478 in 6a18f8f

connection.remote().maxActiveStreams((int) min(maxConcurrentStreams, MAX_VALUE));