Fixes handling of DATA frame padding in OgHttp2Session. Mishandling of padding can lead to a flow control leak, potentially causing the kind of connection stalls observed in https://github.com/envoyproxy/envoy/issues/32401. Thanks to eagle-eyed https://github.com/diannahu for the root cause! PiperOrigin-RevId: 607455607
diff --git a/quiche/http2/adapter/oghttp2_adapter_test.cc b/quiche/http2/adapter/oghttp2_adapter_test.cc index be2b1ce..ddaffae 100644 --- a/quiche/http2/adapter/oghttp2_adapter_test.cc +++ b/quiche/http2/adapter/oghttp2_adapter_test.cc
@@ -8123,7 +8123,6 @@ const int64_t result = adapter->ProcessBytes(frames); EXPECT_EQ(result, frames.size()); - EXPECT_LT(adapter->GetReceiveWindowSize(), 2048); EXPECT_TRUE(adapter->want_write()); @@ -8131,17 +8130,19 @@ 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); + // 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::SETTINGS})); + EqualsFrames({SpdyFrameType::SETTINGS, SpdyFrameType::SETTINGS, + SpdyFrameType::WINDOW_UPDATE, + SpdyFrameType::WINDOW_UPDATE})); } // Verifies that NoopHeaderValidator allows several header combinations that
diff --git a/quiche/http2/adapter/oghttp2_session.cc b/quiche/http2/adapter/oghttp2_session.cc index bd9464f..6527c1c 100644 --- a/quiche/http2/adapter/oghttp2_session.cc +++ b/quiche/http2/adapter/oghttp2_session.cc
@@ -1147,12 +1147,16 @@ void OgHttp2Session::OnStreamPadLength(spdy::SpdyStreamId stream_id, size_t value) { - bool result = visitor_.OnDataPaddingLength(stream_id, 1 + value); + const size_t padding_length = 1 + value; + const bool result = visitor_.OnDataPaddingLength(stream_id, padding_length); if (!result) { fatal_visitor_callback_failure_ = true; decoder_.StopProcessing(); } - MarkDataBuffered(stream_id, 1 + value); + connection_window_manager_.MarkWindowConsumed(padding_length); + if (auto it = stream_map_.find(stream_id); it != stream_map_.end()) { + it->second.window_manager.MarkWindowConsumed(padding_length); + } } void OgHttp2Session::OnStreamPadding(spdy::SpdyStreamId /*stream_id*/, size_t