Skip to content

Commit

Permalink
Proactively disconnect connections flooded with response trailers (#1…
Browse files Browse the repository at this point in the history
…3410)

Signed-off-by: Yan Avlasov <yavlasov@google.com>
  • Loading branch information
yanavlasov authored Oct 8, 2020
1 parent 9ae5adb commit 873f33b
Show file tree
Hide file tree
Showing 5 changed files with 80 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 @@ -247,6 +247,7 @@ void ConnectionImpl::StreamImpl::encodeTrailersBase(const HeaderMap& trailers) {
auto status = parent_.sendPendingFrames();
// See comment in the `encodeHeadersBase()` method about this RELEASE_ASSERT.
RELEASE_ASSERT(status.ok(), "sendPendingFrames() failure in non dispatching context");
parent_.checkProtocolConstraintViolation();
}
}

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 @@ -238,6 +238,7 @@ void ConnectionImpl::StreamImpl::encodeTrailersBase(const HeaderMap& trailers) {
} else {
submitTrailers(trailers);
parent_.sendPendingFrames();
parent_.checkProtocolConstraintViolation();
}
}

Expand Down
39 changes: 39 additions & 0 deletions test/common/http/http2/codec_impl_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -2198,6 +2198,45 @@ TEST_P(Http2CodecImplTest, PingStacksWithDataFlood) {
EXPECT_EQ(1, server_stats_store_.counter("http2.outbound_flood").value());
}

// Verify that codec detects flood of outbound trailers
TEST_P(Http2CodecImplTest, ResponseTrailersFlood) {
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_);
EXPECT_NO_THROW(response_encoder_->encodeTrailers(TestResponseTrailerMapImpl{{"foo", "bar"}}));

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());
}

TEST_P(Http2CodecImplTest, PriorityFlood) {
priorityFlood();
// Legacy codec does not propagate error details and uses generic error message
Expand Down
1 change: 1 addition & 0 deletions test/integration/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -321,6 +321,7 @@ envoy_cc_test(
shard_count = 4,
tags = ["flaky_on_windows"],
deps = [
":autonomous_upstream_lib",
":http_integration_lib",
"//source/common/buffer:buffer_lib",
"//source/common/http:header_map_lib",
Expand Down
38 changes: 38 additions & 0 deletions test/integration/http2_integration_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
#include "common/http/header_map_impl.h"
#include "common/network/socket_option_impl.h"

#include "test/integration/autonomous_upstream.h"
#include "test/integration/utility.h"
#include "test/mocks/http/mocks.h"
#include "test/test_common/network_utility.h"
Expand Down Expand Up @@ -1938,6 +1939,43 @@ name: send_local_reply_filter
// TODO(yanavlasov): add the same tests as above for the encoder filters.
// This is currently blocked by the https://github.com/envoyproxy/envoy/pull/13256

// Verify that the server can detect flood of response trailers.
TEST_P(Http2FloodMitigationTest, Trailers) {
// Set large buffer limits so the test is not affected by the flow control.
config_helper_.setBufferLimits(1024 * 1024 * 1024, 1024 * 1024 * 1024);
autonomous_upstream_ = true;
autonomous_allow_incomplete_streams_ = true;
beginSession();

// Do not read from the socket and send request that causes autonomous upstream
// to respond with 999 DATA frames and trailers. The Http2FloodMitigationTest::beginSession()
// sets 1000 flood limit for all frame types. Including 1 HEADERS response frame
// 999 DATA frames and trailers should trigger flood protection.
// Simulate TCP push back on the Envoy's downstream network socket, so that outbound frames start
// to accumulate in the transport socket buffer.
writev_matcher_->setWritevReturnsEgain();

static_cast<AutonomousUpstream*>(fake_upstreams_.front().get())
->setResponseTrailers(std::make_unique<Http::TestResponseTrailerMapImpl>(
Http::TestResponseTrailerMapImpl({{"foo", "bar"}})));

const auto request =
Http2Frame::makeRequest(Http2Frame::makeClientStreamId(0), "host", "/test/long/url",
{Http2Frame::Header("response_data_blocks", "999")});
sendFrame(request);

// Wait for connection to be flooded with outbound trailers and disconnected.
tcp_client_->waitForDisconnect();

// If the server codec had incorrectly thrown an exception on flood detection it would cause
// the entire upstream to be disconnected. Verify it is still active, and there are no destroyed
// connections.
ASSERT_EQ(1, test_server_->gauge("cluster.cluster_0.upstream_cx_active")->value());
ASSERT_EQ(0, test_server_->counter("cluster.cluster_0.upstream_cx_destroy")->value());
// Verify that the flood check was triggered
EXPECT_EQ(1, test_server_->counter("http2.outbound_flood")->value());
}

// Verify that the server can detect flood of RST_STREAM frames.
TEST_P(Http2FloodMitigationTest, RST_STREAM) {
// Use invalid HTTP headers to trigger sending RST_STREAM frames.
Expand Down

0 comments on commit 873f33b

Please sign in to comment.