Skip to content

Commit

Permalink
Preserve default Http2Settings when Http2SettingsBuilder is used (#…
Browse files Browse the repository at this point in the history
…3092)

Motivation:

By default we set values for `MAX_HEADER_LIST_SIZE` and
`INITIAL_WINDOW_SIZE` settings at `H2ProtocolConfigBuilder` level.
However, if users want to change only one setting (for example,
`MAX_CONCURRENT_STREAMS`), they lose our defaults when they use
`Http2SettingsBuilder`.

Modifications:

- Move defaults initialization from `H2ProtocolConfigBuilder` to
`Http2SettingsBuilder`.

Result:

Users always start with consistent default `Http2Settings` and can use
builder methods to override what they want.
  • Loading branch information
idelpivnitskiy authored Nov 11, 2024
1 parent 8fd62da commit 275623c
Show file tree
Hide file tree
Showing 2 changed files with 16 additions and 9 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ public final class Http2SettingsBuilder {
private static final long MAX_UNSIGNED_INT = 0xffffffffL;
private static final int MAX_FRAME_SIZE_LOWER_BOUND = 0x4000;
private static final int MAX_FRAME_SIZE_UPPER_BOUND = 0xffffff;

/**
* Identifier <a href="https://datatracker.ietf.org/doc/html/rfc7540#section-6.5.2">SETTINGS_HEADER_TABLE_SIZE</a>.
*/
Expand Down Expand Up @@ -57,13 +58,27 @@ public final class Http2SettingsBuilder {
* SETTINGS_MAX_HEADER_LIST_SIZE</a>.
*/
private static final char MAX_HEADER_LIST_SIZE = 0x6;

/**
* 1mb default {@link #INITIAL_WINDOW_SIZE}.
*/
private static final int DEFAULT_INITIAL_WINDOW_SIZE = 1_048_576;
/**
* The initial value of {@link #MAX_HEADER_LIST_SIZE} is
* <a href="https://tools.ietf.org/html/rfc7540#section-6.5.2">unlimited</a>.
* However, in practice we don't want to allow our peers to use unlimited memory by default.
*/
private static final long DEFAULT_MAX_HEADER_LIST_SIZE = 8192;

private final Map<Character, Long> settings;

/**
* Create a new instance.
*/
public Http2SettingsBuilder() {
settings = new HashMap<>(6);
initialWindowSize(DEFAULT_INITIAL_WINDOW_SIZE);
maxHeaderListSize(DEFAULT_MAX_HEADER_LIST_SIZE);
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@
import java.util.function.BooleanSupplier;
import javax.annotation.Nullable;

import static io.netty.handler.codec.http2.Http2CodecUtil.DEFAULT_HEADER_LIST_SIZE;
import static io.servicetalk.http.netty.H2KeepAlivePolicies.disabled;
import static io.servicetalk.http.netty.H2KeepAlivePolicies.validateKeepAlivePolicy;
import static io.servicetalk.utils.internal.NumberUtils.ensurePositive;
Expand All @@ -41,10 +40,6 @@
*/
public final class H2ProtocolConfigBuilder {
private static final BiPredicate<CharSequence, CharSequence> DEFAULT_SENSITIVITY_DETECTOR = (name, value) -> false;
/**
* 1mb default window size.
*/
private static final int INITIAL_FLOW_CONTROL_WINDOW = 1_048_576;
/**
* Netty currently doubles the connection window by default so a single stream doesn't exhaust all flow control
* bytes.
Expand All @@ -54,10 +49,7 @@ public final class H2ProtocolConfigBuilder {
* Default allocation quantum to use for the remote flow controller.
*/
private static final int DEFAULT_FLOW_CONTROL_QUANTUM = 1024 * 16;
private Http2Settings h2Settings = new Http2SettingsBuilder()
.initialWindowSize(INITIAL_FLOW_CONTROL_WINDOW)
.maxHeaderListSize(DEFAULT_HEADER_LIST_SIZE)
.build();
private Http2Settings h2Settings = new Http2SettingsBuilder().build();
private HttpHeadersFactory headersFactory = H2HeadersFactory.INSTANCE;
private BiPredicate<CharSequence, CharSequence> headersSensitivityDetector = DEFAULT_SENSITIVITY_DETECTOR;
@Nullable
Expand Down

0 comments on commit 275623c

Please sign in to comment.