-
Notifications
You must be signed in to change notification settings - Fork 2.1k
Description
Version
4.5.24
Context
Hi,
I have found that Http2MaxRstFrameDecoder and Http2MaxRstFrameEncoder are being used by a Vertx HttpClient. According to netty class AbstractHttp2ConnectionHandlerBuilder I didn't expect this to be possible due to this code:
if (maxDecodedRstFramesPerWindow == null) {
// Only enable by default on the server.
if (isServer()) {
maxDecodedRstFrames = DEFAULT_MAX_RST_FRAMES_PER_CONNECTION_FOR_SERVER;
} else {
maxDecodedRstFrames = 0;
}
} else {
maxDecodedRstFrames = maxDecodedRstFramesPerWindow;
}
if (maxDecodedRstFrames > 0 && maxDecodedRstFramesSecondsPerWindow > 0) {
decoder = new Http2MaxRstFrameDecoder(decoder, maxDecodedRstFrames, maxDecodedRstFramesSecondsPerWindow);
}
However, when the code hits isServer() method, it returns true because the variable isServer is null. The same situation applies to the encoder.
The problem appears to be in the VertxHttp2ConnectionHandlerBuilder class that extends from AbstractHttp2ConnectionHandlerBuilder. The server(boolean isServer) is overridden, but the parent value isServer is never updated.
I have tried to call to super.server(...) but it seems that some pre-checks fail. Honestly, I don't fully understand this behavior.
That said, I was able to fix it, overriding the isServer() method in VertxHttp2ConnectionHandlerBuilder as follows:
@Override
protected boolean isServer() {
return this.server;
}
Maybe, another option is to do configurable these values when the VertxHttp2ConnectionHandler is created, something like this:
public static VertxHttp2ConnectionHandler<Http2ClientConnection> createHttp2ConnectionHandler(
HttpClientBase client,
ClientMetrics metrics,
ContextInternal context,
boolean upgrade,
Object socketMetric) {
HttpClientOptions options = client.options();
HttpClientMetrics met = client.metrics();
VertxHttp2ConnectionHandler<Http2ClientConnection> handler = new VertxHttp2ConnectionHandlerBuilder<Http2ClientConnection>()
.server(false)
.decoderEnforceMaxRstFramesPerWindow(0,0)
.encoderEnforceMaxRstFramesPerWindow(0,0)
....
However, I am not sure about the implications of this approach.
Could you please take a look?
Thx!
Steps to reproduce
No response
Do you have a reproducer?
No response