Adds a test case demonstrating that OgHttp2Adapter does not consider DATA frame padding as "consumed".
In the test, this shows up as a lack of WINDOW_UPDATE frames in the adapter's response. This bug could be implicated in https://github.com/envoyproxy/envoy/issues/32401.
PiperOrigin-RevId: 607420483
diff --git a/quiche/http2/adapter/nghttp2_adapter_test.cc b/quiche/http2/adapter/nghttp2_adapter_test.cc
index 0493c87..d118ed8 100644
--- a/quiche/http2/adapter/nghttp2_adapter_test.cc
+++ b/quiche/http2/adapter/nghttp2_adapter_test.cc
@@ -7250,6 +7250,62 @@
SpdyFrameType::RST_STREAM, SpdyFrameType::RST_STREAM}));
}
+TEST(OgHttp2AdapterTest, ServerConsumesDataWithPadding) {
+ DataSavingVisitor visitor;
+ auto adapter = NgHttp2Adapter::CreateServerAdapter(visitor);
+
+ TestFrameSequence seq = std::move(TestFrameSequence().ClientPreface().Headers(
+ 1,
+ {{":method", "POST"},
+ {":scheme", "https"},
+ {":authority", "example.com"},
+ {":path", "/this/is/request/one"}},
+ /*fin=*/false));
+ // Generates a bunch of DATA frames, with the bulk of the payloads consisting
+ // of padding.
+ size_t total_size = 0;
+ while (total_size < 62 * 1024) {
+ seq.Data(1, "a", /*fin=*/false, /*padding=*/254);
+ total_size += 255;
+ }
+ const std::string frames = seq.Serialize();
+
+ // 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(1, _, _)).Times(4);
+ EXPECT_CALL(visitor, OnEndHeadersForStream(1));
+ EXPECT_CALL(visitor, OnFrameHeader(1, _, DATA, 0x8))
+ .Times(testing::AtLeast(1));
+ EXPECT_CALL(visitor, OnBeginDataForStream(1, _)).Times(testing::AtLeast(1));
+ EXPECT_CALL(visitor, OnDataForStream(1, "a")).Times(testing::AtLeast(1));
+ EXPECT_CALL(visitor, OnDataPaddingLength(1, _)).Times(testing::AtLeast(1));
+
+ const int64_t result = adapter->ProcessBytes(frames);
+ EXPECT_EQ(result, frames.size());
+
+ EXPECT_TRUE(adapter->want_write());
+
+ EXPECT_CALL(visitor, OnBeforeFrameSent(SETTINGS, 0, 0, ACK_FLAG));
+ EXPECT_CALL(visitor, OnFrameSent(SETTINGS, 0, 0, ACK_FLAG, 0));
+ // Since most of the flow control window consumed is padding, the adapter
+ // generates window updates.
+ EXPECT_CALL(visitor, OnBeforeFrameSent(WINDOW_UPDATE, 1, _, 0x0)).Times(1);
+ EXPECT_CALL(visitor, OnFrameSent(WINDOW_UPDATE, 1, _, 0x0, 0)).Times(1);
+ EXPECT_CALL(visitor, OnBeforeFrameSent(WINDOW_UPDATE, 0, _, 0x0)).Times(1);
+ EXPECT_CALL(visitor, OnFrameSent(WINDOW_UPDATE, 0, _, 0x0, 0)).Times(1);
+
+ const int send_result = adapter->Send();
+ EXPECT_EQ(0, send_result);
+ EXPECT_THAT(visitor.data(), EqualsFrames({SpdyFrameType::SETTINGS,
+ SpdyFrameType::WINDOW_UPDATE,
+ SpdyFrameType::WINDOW_UPDATE}));
+}
+
TEST(NgHttp2AdapterTest, NegativeFlowControlStreamResumption) {
DataSavingVisitor visitor;
auto adapter = NgHttp2Adapter::CreateServerAdapter(visitor);
diff --git a/quiche/http2/adapter/oghttp2_adapter_test.cc b/quiche/http2/adapter/oghttp2_adapter_test.cc
index 6b570e2..be2b1ce 100644
--- a/quiche/http2/adapter/oghttp2_adapter_test.cc
+++ b/quiche/http2/adapter/oghttp2_adapter_test.cc
@@ -8084,6 +8084,66 @@
SpdyFrameType::WINDOW_UPDATE}));
}
+TEST(OgHttp2AdapterTest, ServerConsumesDataWithPadding) {
+ DataSavingVisitor visitor;
+ OgHttp2Adapter::Options options;
+ options.perspective = Perspective::kServer;
+ auto adapter = OgHttp2Adapter::Create(visitor, options);
+
+ TestFrameSequence seq = std::move(TestFrameSequence().ClientPreface().Headers(
+ 1,
+ {{":method", "POST"},
+ {":scheme", "https"},
+ {":authority", "example.com"},
+ {":path", "/this/is/request/one"}},
+ /*fin=*/false));
+ // Generates a bunch of DATA frames, with the bulk of the payloads consisting
+ // of padding.
+ size_t total_size = 0;
+ while (total_size < 62 * 1024) {
+ seq.Data(1, "a", /*fin=*/false, /*padding=*/254);
+ total_size += 255;
+ }
+ const std::string frames = seq.Serialize();
+
+ // 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(1, _, _)).Times(4);
+ EXPECT_CALL(visitor, OnEndHeadersForStream(1));
+ EXPECT_CALL(visitor, OnFrameHeader(1, _, DATA, 0x8))
+ .Times(testing::AtLeast(1));
+ EXPECT_CALL(visitor, OnBeginDataForStream(1, _)).Times(testing::AtLeast(1));
+ EXPECT_CALL(visitor, OnDataForStream(1, "a")).Times(testing::AtLeast(1));
+ EXPECT_CALL(visitor, OnDataPaddingLength(1, _)).Times(testing::AtLeast(1));
+
+ const int64_t result = adapter->ProcessBytes(frames);
+ EXPECT_EQ(result, frames.size());
+ EXPECT_LT(adapter->GetReceiveWindowSize(), 2048);
+
+ EXPECT_TRUE(adapter->want_write());
+
+ EXPECT_CALL(visitor, OnBeforeFrameSent(SETTINGS, 0, _, 0x0));
+ EXPECT_CALL(visitor, OnFrameSent(SETTINGS, 0, _, 0x0, 0));
+ EXPECT_CALL(visitor, OnBeforeFrameSent(SETTINGS, 0, 0, ACK_FLAG));
+ EXPECT_CALL(visitor, OnFrameSent(SETTINGS, 0, 0, ACK_FLAG, 0));
+ // BUG: Most of the flow control window consumed is padding, so the adapter
+ // should have generated window updates.
+ EXPECT_CALL(visitor, OnBeforeFrameSent(WINDOW_UPDATE, 1, _, 0x0)).Times(0);
+ EXPECT_CALL(visitor, OnFrameSent(WINDOW_UPDATE, 1, _, 0x0, 0)).Times(0);
+ EXPECT_CALL(visitor, OnBeforeFrameSent(WINDOW_UPDATE, 0, _, 0x0)).Times(0);
+ EXPECT_CALL(visitor, OnFrameSent(WINDOW_UPDATE, 0, _, 0x0, 0)).Times(0);
+
+ const int send_result = adapter->Send();
+ EXPECT_EQ(0, send_result);
+ EXPECT_THAT(visitor.data(),
+ EqualsFrames({SpdyFrameType::SETTINGS, SpdyFrameType::SETTINGS}));
+}
+
// Verifies that NoopHeaderValidator allows several header combinations that
// would otherwise be invalid.
TEST(OgHttp2AdapterTest, NoopHeaderValidatorTest) {
diff --git a/quiche/http2/adapter/test_frame_sequence.h b/quiche/http2/adapter/test_frame_sequence.h
index 47f8199..93c5536 100644
--- a/quiche/http2/adapter/test_frame_sequence.h
+++ b/quiche/http2/adapter/test_frame_sequence.h
@@ -22,6 +22,9 @@
public:
TestFrameSequence() = default;
+ TestFrameSequence(TestFrameSequence&& other) = default;
+ TestFrameSequence& operator=(TestFrameSequence&& other) = default;
+
TestFrameSequence& ClientPreface(
absl::Span<const Http2Setting> settings = {});
TestFrameSequence& ServerPreface(
