Skip to content

Commit

Permalink
http2: Proactively disconnect connections flooded in drain timeout (#…
Browse files Browse the repository at this point in the history
…13420)

Signed-off-by: Yan Avlasov <yavlasov@google.com>
  • Loading branch information
yanavlasov authored Oct 8, 2020
1 parent c6afbe4 commit 9e4845a
Show file tree
Hide file tree
Showing 4 changed files with 68 additions and 0 deletions.
1 change: 1 addition & 0 deletions source/common/http/http2/codec_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -696,6 +696,7 @@ void ConnectionImpl::goAway() {
auto status = sendPendingFrames();
// See comment in the `encodeHeadersBase()` method about this RELEASE_ASSERT.
RELEASE_ASSERT(status.ok(), "sendPendingFrames() failure in non dispatching context");
checkProtocolConstraintViolation();
}

void ConnectionImpl::shutdownNotice() {
Expand Down
1 change: 1 addition & 0 deletions source/common/http/http2/codec_impl_legacy.cc
Original file line number Diff line number Diff line change
Expand Up @@ -665,6 +665,7 @@ void ConnectionImpl::goAway() {
ASSERT(rc == 0);

sendPendingFrames();
checkProtocolConstraintViolation();
}

void ConnectionImpl::shutdownNotice() {
Expand Down
40 changes: 40 additions & 0 deletions test/common/http/http2/codec_impl_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -2299,6 +2299,46 @@ TEST_P(Http2CodecImplTest, EmptyDataFloodOverride) {
EXPECT_TRUE(status.ok());
}

// Verify that codec detects flood of outbound frames caused by goAway() method
TEST_P(Http2CodecImplTest, GoAwayCausesOutboundFlood) {
initialize();

TestRequestHeaderMapImpl request_headers;
HttpTestUtility::addDefaultHeaders(request_headers);
EXPECT_CALL(request_decoder_, decodeHeaders_(_, false));
request_encoder_->encodeHeaders(request_headers, false);

int frame_count = 0;
Buffer::OwnedImpl buffer;
ON_CALL(server_connection_, write(_, _))
.WillByDefault(Invoke([&buffer, &frame_count](Buffer::Instance& frame, bool) {
++frame_count;
buffer.move(frame);
}));

auto* violation_callback =
new NiceMock<Event::MockSchedulableCallback>(&server_connection_.dispatcher_);

TestResponseHeaderMapImpl response_headers{{":status", "200"}};
response_encoder_->encodeHeaders(response_headers, false);
// Account for the single HEADERS frame above
for (uint32_t i = 0; i < CommonUtility::OptionsLimits::DEFAULT_MAX_OUTBOUND_FRAMES - 1; ++i) {
Buffer::OwnedImpl data("0");
EXPECT_NO_THROW(response_encoder_->encodeData(data, false));
}

EXPECT_FALSE(violation_callback->enabled_);

server_->goAway();

EXPECT_TRUE(violation_callback->enabled_);
EXPECT_CALL(server_connection_, close(Envoy::Network::ConnectionCloseType::NoFlush));
violation_callback->invokeCallback();

EXPECT_EQ(frame_count, CommonUtility::OptionsLimits::DEFAULT_MAX_OUTBOUND_FRAMES + 1);
EXPECT_EQ(1, server_stats_store_.counter("http2.outbound_flood").value());
}

// Verify that codec detects flood of outbound frames caused by shutdownNotice() method
TEST_P(Http2CodecImplTest, ShudowNoticeCausesOutboundFlood) {
initialize();
Expand Down
26 changes: 26 additions & 0 deletions test/integration/http2_integration_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -2011,6 +2011,32 @@ TEST_P(Http2FloodMitigationTest, RST_STREAM) {
test_server_->counter("http.config_test.downstream_cx_delayed_close_timeout")->value());
}

// Verify detection of frame flood when sending second GOAWAY frame on drain timeout
TEST_P(Http2FloodMitigationTest, GoAwayOverflowOnDrainTimeout) {
config_helper_.addConfigModifier(
[](envoy::extensions::filters::network::http_connection_manager::v3::HttpConnectionManager&
hcm) {
auto* drain_time_out = hcm.mutable_drain_timeout();
std::chrono::milliseconds timeout(1000);
auto seconds = std::chrono::duration_cast<std::chrono::seconds>(timeout);
drain_time_out->set_seconds(seconds.count());

auto* http_protocol_options = hcm.mutable_common_http_protocol_options();
auto* idle_time_out = http_protocol_options->mutable_idle_timeout();
idle_time_out->set_seconds(seconds.count());
});
// pre-fill two away from overflow
prefillOutboundDownstreamQueue(AllFrameFloodLimit - 2);

// connection idle timeout will send first GOAWAY frame and start drain timer
// drain timeout will send second GOAWAY frame which should trigger flood protection
// Wait for connection to be flooded with outbound GOAWAY frame and disconnected.
tcp_client_->waitForDisconnect();

// Verify that the flood check was triggered
EXPECT_EQ(1, test_server_->counter("http2.outbound_flood")->value());
}

// Verify detection of overflowing outbound frame queue with the GOAWAY frames sent after the
// downstream idle connection timeout disconnects the connection.
// The test verifies protocol constraint violation handling in the
Expand Down

0 comments on commit 9e4845a

Please sign in to comment.