Add testing for connection window flow control violations.
This CL adds oghttp2 and nghttp2 tests for the following scenario:
1. Server sends SETTINGS with a MAX_FRAME_SIZE value greater than the initial
connection flow control window.
2. Client sends a request with a DATA frame payload of the MAX_FRAME_SIZE.
The server should recognize that the client-sent DATA frame induces a
connection-level flow control error, and should send a GOAWAY.
[NGHTTP2]
Indeed, nghttp2 recognizes this error correctly when the DATA frame is
processed all at once. However, nghttp2 does not recognize this error when the
DATA frame is processed in multiple ProcessBytes() calls.
[OGHTTP2]
It is likely that oghttp2 would recognize the flow control error [*], but
oghttp2 does not handle outbound MAX_FRAME_SIZE. Thus, oghttp2 currently
GOAWAYs prematurely before exercising flow control validation. Addressing the
MAX_FRAME_SIZE feature gap will be the work of b/223471995.
This testing was inspired by attempts to enable GFE2 HTTP/2 flow control e2e
testing for GFE3 (WIP with pending cl/433228245).
[*] http://google3/third_party/http2/adapter/oghttp2_session.cc;l=1042-1049;rcl=432298189
PiperOrigin-RevId: 433539997
diff --git a/http2/adapter/nghttp2_adapter_test.cc b/http2/adapter/nghttp2_adapter_test.cc
index 0a7526c..55b1bbf 100644
--- a/http2/adapter/nghttp2_adapter_test.cc
+++ b/http2/adapter/nghttp2_adapter_test.cc
@@ -3129,6 +3129,181 @@
EXPECT_THAT(visitor.data(), EqualsFrames({SpdyFrameType::GOAWAY}));
}
+TEST(NgHttp2AdapterTest, ClientDisobeysConnectionFlowControlWithOneDataFrame) {
+ DataSavingVisitor visitor;
+ auto adapter = NgHttp2Adapter::CreateServerAdapter(visitor);
+
+ // Allow the client to send a DATA frame that exceeds the connection flow
+ // control window.
+ const uint32_t window_overflow_bytes = kInitialFlowControlWindowSize + 1;
+ adapter->SubmitSettings({{MAX_FRAME_SIZE, window_overflow_bytes}});
+
+ const std::string initial_frames =
+ TestFrameSequence()
+ .ClientPreface()
+ .Headers(1,
+ {{":method", "POST"},
+ {":scheme", "https"},
+ {":authority", "example.com"},
+ {":path", "/this/is/request/one"}},
+ /*fin=*/false)
+ .Serialize();
+
+ testing::InSequence s;
+
+ // Client preface (empty SETTINGS)
+ EXPECT_CALL(visitor, OnFrameHeader(0, 0, SETTINGS, 0));
+ EXPECT_CALL(visitor, OnSettingsStart());
+ EXPECT_CALL(visitor, OnSettingsEnd());
+
+ EXPECT_CALL(visitor, OnFrameHeader(1, _, HEADERS, 4));
+ EXPECT_CALL(visitor, OnBeginHeadersForStream(1));
+ EXPECT_CALL(visitor, OnHeaderForStream).Times(4);
+ EXPECT_CALL(visitor, OnEndHeadersForStream(1));
+
+ int64_t process_result = adapter->ProcessBytes(initial_frames);
+ EXPECT_EQ(initial_frames.size(), static_cast<size_t>(process_result));
+
+ EXPECT_TRUE(adapter->want_write());
+
+ // Outbound SETTINGS containing MAX_FRAME_SIZE.
+ EXPECT_CALL(visitor, OnBeforeFrameSent(SETTINGS, 0, 6, 0x0));
+ EXPECT_CALL(visitor, OnFrameSent(SETTINGS, 0, 6, 0x0, 0));
+
+ // Ack of client's initial settings.
+ EXPECT_CALL(visitor, OnBeforeFrameSent(SETTINGS, 0, _, 0x1));
+ EXPECT_CALL(visitor, OnFrameSent(SETTINGS, 0, _, 0x1, 0));
+
+ int send_result = adapter->Send();
+ EXPECT_EQ(0, send_result);
+ EXPECT_THAT(visitor.data(),
+ EqualsFrames({SpdyFrameType::SETTINGS, SpdyFrameType::SETTINGS}));
+ visitor.Clear();
+
+ // Now let the client ack the MAX_FRAME_SIZE SETTINGS and send a DATA frame to
+ // overflow the connection-level window. The result should be a GOAWAY.
+ const std::string overflow_frames =
+ TestFrameSequence()
+ .SettingsAck()
+ .Data(1, std::string(window_overflow_bytes, 'a'))
+ .Serialize();
+
+ EXPECT_CALL(visitor, OnFrameHeader(0, 0, SETTINGS, 0x1));
+ EXPECT_CALL(visitor, OnSettingsAck());
+ EXPECT_CALL(visitor, OnFrameHeader(1, window_overflow_bytes, DATA, 0x0));
+ EXPECT_CALL(visitor, OnBeginDataForStream(1, window_overflow_bytes));
+ // No further frame data is delivered.
+
+ process_result = adapter->ProcessBytes(overflow_frames);
+ EXPECT_EQ(overflow_frames.size(), static_cast<size_t>(process_result));
+
+ EXPECT_CALL(visitor, OnBeforeFrameSent(GOAWAY, 0, _, 0x0));
+ EXPECT_CALL(
+ visitor,
+ OnFrameSent(GOAWAY, 0, _, 0x0,
+ static_cast<int>(Http2ErrorCode::FLOW_CONTROL_ERROR)));
+
+ send_result = adapter->Send();
+ EXPECT_EQ(0, send_result);
+ EXPECT_THAT(visitor.data(), EqualsFrames({SpdyFrameType::GOAWAY}));
+}
+
+TEST(NgHttp2AdapterTest, ClientDisobeysConnectionFlowControlAcrossReads) {
+ DataSavingVisitor visitor;
+ auto adapter = NgHttp2Adapter::CreateServerAdapter(visitor);
+
+ // Allow the client to send a DATA frame that exceeds the connection flow
+ // control window.
+ const uint32_t window_overflow_bytes = kInitialFlowControlWindowSize + 1;
+ adapter->SubmitSettings({{MAX_FRAME_SIZE, window_overflow_bytes}});
+
+ const std::string initial_frames =
+ TestFrameSequence()
+ .ClientPreface()
+ .Headers(1,
+ {{":method", "POST"},
+ {":scheme", "https"},
+ {":authority", "example.com"},
+ {":path", "/this/is/request/one"}},
+ /*fin=*/false)
+ .Serialize();
+
+ testing::InSequence s;
+
+ // Client preface (empty SETTINGS)
+ EXPECT_CALL(visitor, OnFrameHeader(0, 0, SETTINGS, 0));
+ EXPECT_CALL(visitor, OnSettingsStart());
+ EXPECT_CALL(visitor, OnSettingsEnd());
+
+ EXPECT_CALL(visitor, OnFrameHeader(1, _, HEADERS, 4));
+ EXPECT_CALL(visitor, OnBeginHeadersForStream(1));
+ EXPECT_CALL(visitor, OnHeaderForStream).Times(4);
+ EXPECT_CALL(visitor, OnEndHeadersForStream(1));
+
+ int64_t process_result = adapter->ProcessBytes(initial_frames);
+ EXPECT_EQ(initial_frames.size(), static_cast<size_t>(process_result));
+
+ EXPECT_TRUE(adapter->want_write());
+
+ // Outbound SETTINGS containing MAX_FRAME_SIZE.
+ EXPECT_CALL(visitor, OnBeforeFrameSent(SETTINGS, 0, 6, 0x0));
+ EXPECT_CALL(visitor, OnFrameSent(SETTINGS, 0, 6, 0x0, 0));
+
+ // Ack of client's initial settings.
+ EXPECT_CALL(visitor, OnBeforeFrameSent(SETTINGS, 0, _, 0x1));
+ EXPECT_CALL(visitor, OnFrameSent(SETTINGS, 0, _, 0x1, 0));
+
+ int send_result = adapter->Send();
+ EXPECT_EQ(0, send_result);
+ EXPECT_THAT(visitor.data(),
+ EqualsFrames({SpdyFrameType::SETTINGS, SpdyFrameType::SETTINGS}));
+ visitor.Clear();
+
+ // Now let the client ack the MAX_FRAME_SIZE SETTINGS and send a DATA frame to
+ // overflow the connection-level window. The result should be a GOAWAY, but
+ // because the processing is split across several calls, nghttp2 instead
+ // delivers the data payloads (which the visitor then consumes). This is a bug
+ // in nghttp2, which should recognize the flow control error.
+ const std::string overflow_frames =
+ TestFrameSequence()
+ .SettingsAck()
+ .Data(1, std::string(window_overflow_bytes, 'a'))
+ .Serialize();
+
+ EXPECT_CALL(visitor, OnFrameHeader(0, 0, SETTINGS, 0x1));
+ EXPECT_CALL(visitor, OnSettingsAck());
+ EXPECT_CALL(visitor, OnFrameHeader(1, window_overflow_bytes, DATA, 0x0));
+ EXPECT_CALL(visitor, OnBeginDataForStream(1, window_overflow_bytes));
+ // BUG: The visitor should not have received the data.
+ EXPECT_CALL(visitor, OnDataForStream(1, _))
+ .WillRepeatedly(
+ [&adapter](Http2StreamId stream_id, absl::string_view data) {
+ adapter->MarkDataConsumedForStream(stream_id, data.size());
+ return true;
+ });
+
+ const size_t chunk_length = 16384;
+ ASSERT_GE(overflow_frames.size(), chunk_length);
+ absl::string_view remaining = overflow_frames;
+ while (!remaining.empty()) {
+ absl::string_view chunk = remaining.substr(0, chunk_length);
+ process_result = adapter->ProcessBytes(chunk);
+ EXPECT_EQ(chunk.length(), static_cast<size_t>(process_result));
+
+ remaining.remove_prefix(chunk.length());
+ }
+
+ EXPECT_CALL(visitor, OnBeforeFrameSent(WINDOW_UPDATE, 0, 4, 0x0));
+ EXPECT_CALL(visitor, OnFrameSent(WINDOW_UPDATE, 0, 4, 0x0, 0));
+ EXPECT_CALL(visitor, OnBeforeFrameSent(WINDOW_UPDATE, 1, 4, 0x0));
+ EXPECT_CALL(visitor, OnFrameSent(WINDOW_UPDATE, 1, 4, 0x0, 0));
+
+ send_result = adapter->Send();
+ EXPECT_EQ(0, send_result);
+ EXPECT_THAT(visitor.data(), EqualsFrames({SpdyFrameType::WINDOW_UPDATE,
+ SpdyFrameType::WINDOW_UPDATE}));
+}
+
TEST(NgHttp2AdapterTest, ClientDisobeysStreamFlowControl) {
DataSavingVisitor visitor;
auto adapter = NgHttp2Adapter::CreateServerAdapter(visitor);
diff --git a/http2/adapter/oghttp2_adapter_test.cc b/http2/adapter/oghttp2_adapter_test.cc
index 8f0aaba..7fbf6e4 100644
--- a/http2/adapter/oghttp2_adapter_test.cc
+++ b/http2/adapter/oghttp2_adapter_test.cc
@@ -4126,6 +4126,170 @@
EqualsFrames({SpdyFrameType::SETTINGS, SpdyFrameType::GOAWAY}));
}
+TEST(OgHttp2AdapterTest, ClientDisobeysConnectionFlowControlWithOneDataFrame) {
+ DataSavingVisitor visitor;
+ OgHttp2Adapter::Options options{.perspective = Perspective::kServer};
+ auto adapter = OgHttp2Adapter::Create(visitor, options);
+
+ // Allow the client to send a DATA frame that exceeds the connection flow
+ // control window.
+ const uint32_t window_overflow_bytes = kInitialFlowControlWindowSize + 1;
+ adapter->SubmitSettings({{MAX_FRAME_SIZE, window_overflow_bytes}});
+
+ const std::string initial_frames =
+ TestFrameSequence()
+ .ClientPreface()
+ .Headers(1,
+ {{":method", "POST"},
+ {":scheme", "https"},
+ {":authority", "example.com"},
+ {":path", "/this/is/request/one"}},
+ /*fin=*/false)
+ .Serialize();
+
+ testing::InSequence s;
+
+ // Client preface (empty SETTINGS)
+ EXPECT_CALL(visitor, OnFrameHeader(0, 0, SETTINGS, 0));
+ EXPECT_CALL(visitor, OnSettingsStart());
+ EXPECT_CALL(visitor, OnSettingsEnd());
+
+ EXPECT_CALL(visitor, OnFrameHeader(1, _, HEADERS, 4));
+ EXPECT_CALL(visitor, OnBeginHeadersForStream(1));
+ EXPECT_CALL(visitor, OnHeaderForStream).Times(4);
+ EXPECT_CALL(visitor, OnEndHeadersForStream(1));
+
+ int64_t process_result = adapter->ProcessBytes(initial_frames);
+ EXPECT_EQ(initial_frames.size(), static_cast<size_t>(process_result));
+
+ EXPECT_TRUE(adapter->want_write());
+
+ // Outbound SETTINGS containing MAX_FRAME_SIZE.
+ EXPECT_CALL(visitor, OnBeforeFrameSent(SETTINGS, 0, 6, 0x0));
+ EXPECT_CALL(visitor, OnFrameSent(SETTINGS, 0, 6, 0x0, 0));
+
+ // Ack of client's initial settings.
+ EXPECT_CALL(visitor, OnBeforeFrameSent(SETTINGS, 0, _, 0x1));
+ EXPECT_CALL(visitor, OnFrameSent(SETTINGS, 0, _, 0x1, 0));
+
+ int send_result = adapter->Send();
+ EXPECT_EQ(0, send_result);
+ EXPECT_THAT(visitor.data(),
+ EqualsFrames({SpdyFrameType::SETTINGS, SpdyFrameType::SETTINGS}));
+ visitor.Clear();
+
+ // Now let the client ack the MAX_FRAME_SIZE SETTINGS and send a DATA frame to
+ // overflow the connection-level window. The result should be a GOAWAY.
+ // TODO(b/223471995): The result is a GOAWAY, but not for the right reason.
+ // Fix oghttp2 to handle outbound SETTINGS with MAX_FRAME_SIZE.
+ const std::string overflow_frames =
+ TestFrameSequence()
+ .SettingsAck()
+ .Data(1, std::string(window_overflow_bytes, 'a'))
+ .Serialize();
+
+ EXPECT_CALL(visitor, OnFrameHeader(0, 0, SETTINGS, 0x1));
+ EXPECT_CALL(visitor, OnSettingsAck());
+ EXPECT_CALL(visitor, OnFrameHeader(1, window_overflow_bytes, DATA, 0x0));
+ EXPECT_CALL(visitor, OnConnectionError(ConnectionError::kParseError));
+ // No further frame data is delivered.
+
+ process_result = adapter->ProcessBytes(overflow_frames);
+ EXPECT_EQ(overflow_frames.size(), static_cast<size_t>(process_result));
+
+ EXPECT_CALL(visitor, OnBeforeFrameSent(GOAWAY, 0, _, 0x0));
+ EXPECT_CALL(visitor,
+ OnFrameSent(GOAWAY, 0, _, 0x0,
+ static_cast<int>(Http2ErrorCode::FRAME_SIZE_ERROR)));
+
+ send_result = adapter->Send();
+ EXPECT_EQ(0, send_result);
+ EXPECT_THAT(visitor.data(), EqualsFrames({SpdyFrameType::GOAWAY}));
+}
+
+TEST(OgHttp2AdapterTest, ClientDisobeysConnectionFlowControlAcrossReads) {
+ DataSavingVisitor visitor;
+ OgHttp2Adapter::Options options{.perspective = Perspective::kServer};
+ auto adapter = OgHttp2Adapter::Create(visitor, options);
+
+ // Allow the client to send a DATA frame that exceeds the connection flow
+ // control window.
+ const uint32_t window_overflow_bytes = kInitialFlowControlWindowSize + 1;
+ adapter->SubmitSettings({{MAX_FRAME_SIZE, window_overflow_bytes}});
+
+ const std::string initial_frames =
+ TestFrameSequence()
+ .ClientPreface()
+ .Headers(1,
+ {{":method", "POST"},
+ {":scheme", "https"},
+ {":authority", "example.com"},
+ {":path", "/this/is/request/one"}},
+ /*fin=*/false)
+ .Serialize();
+
+ testing::InSequence s;
+
+ // Client preface (empty SETTINGS)
+ EXPECT_CALL(visitor, OnFrameHeader(0, 0, SETTINGS, 0));
+ EXPECT_CALL(visitor, OnSettingsStart());
+ EXPECT_CALL(visitor, OnSettingsEnd());
+
+ EXPECT_CALL(visitor, OnFrameHeader(1, _, HEADERS, 4));
+ EXPECT_CALL(visitor, OnBeginHeadersForStream(1));
+ EXPECT_CALL(visitor, OnHeaderForStream).Times(4);
+ EXPECT_CALL(visitor, OnEndHeadersForStream(1));
+
+ int64_t process_result = adapter->ProcessBytes(initial_frames);
+ EXPECT_EQ(initial_frames.size(), static_cast<size_t>(process_result));
+
+ EXPECT_TRUE(adapter->want_write());
+
+ // Outbound SETTINGS containing MAX_FRAME_SIZE.
+ EXPECT_CALL(visitor, OnBeforeFrameSent(SETTINGS, 0, 6, 0x0));
+ EXPECT_CALL(visitor, OnFrameSent(SETTINGS, 0, 6, 0x0, 0));
+
+ // Ack of client's initial settings.
+ EXPECT_CALL(visitor, OnBeforeFrameSent(SETTINGS, 0, _, 0x1));
+ EXPECT_CALL(visitor, OnFrameSent(SETTINGS, 0, _, 0x1, 0));
+
+ int send_result = adapter->Send();
+ EXPECT_EQ(0, send_result);
+ EXPECT_THAT(visitor.data(),
+ EqualsFrames({SpdyFrameType::SETTINGS, SpdyFrameType::SETTINGS}));
+ visitor.Clear();
+
+ // Now let the client ack the MAX_FRAME_SIZE SETTINGS and send a DATA frame to
+ // overflow the connection-level window. The result should be a GOAWAY.
+ // TODO(b/223471995): The result is a GOAWAY, but not for the right reason.
+ // Fix oghttp2 to handle outbound SETTINGS with MAX_FRAME_SIZE.
+ const std::string overflow_frames =
+ TestFrameSequence()
+ .SettingsAck()
+ .Data(1, std::string(window_overflow_bytes, 'a'))
+ .Serialize();
+
+ EXPECT_CALL(visitor, OnFrameHeader(0, 0, SETTINGS, 0x1));
+ EXPECT_CALL(visitor, OnSettingsAck());
+ EXPECT_CALL(visitor, OnFrameHeader(1, window_overflow_bytes, DATA, 0x0));
+ EXPECT_CALL(visitor, OnConnectionError(ConnectionError::kParseError));
+
+ const size_t chunk_length = 16384;
+ ASSERT_GE(overflow_frames.size(), chunk_length);
+ process_result =
+ adapter->ProcessBytes(overflow_frames.substr(0, chunk_length));
+ EXPECT_EQ(chunk_length, static_cast<size_t>(process_result));
+
+ EXPECT_CALL(visitor, OnBeforeFrameSent(GOAWAY, 0, _, 0x0));
+ EXPECT_CALL(visitor,
+ OnFrameSent(GOAWAY, 0, _, 0x0,
+ static_cast<int>(Http2ErrorCode::FRAME_SIZE_ERROR)));
+
+ send_result = adapter->Send();
+ EXPECT_EQ(0, send_result);
+ EXPECT_THAT(visitor.data(), EqualsFrames({SpdyFrameType::GOAWAY}));
+}
+
TEST(OgHttp2AdapterTest, ClientDisobeysStreamFlowControl) {
DataSavingVisitor visitor;
OgHttp2Adapter::Options options{.perspective = Perspective::kServer};
