Add an `oghttp2` option to cap outbound DATA frame payload size. `oghttp2` sizes outbound DATA frames up to the peer's `SETTINGS_MAX_FRAME_SIZE`, while `nghttp2` always caps them at 16 KB. Envoy's `WINDOW_UPDATE` flood protection budgets inbound `WINDOW_UPDATE` frames per outbound DATA frame, so a client that advertises a large `MAX_FRAME_SIZE` and acknowledges data in small chunks gets far fewer DATA frames under `oghttp2` and trips the flood limit. Add `max_outbound_data_frame_payload` to `OgHttp2Session::Options`. When set, outbound DATA frames are capped at the smaller of this value and the peer's `MAX_FRAME_SIZE`. The option is unset by default (zero is treated as unset), so existing behavior is unchanged. Envoy can set it to 16 KB behind a runtime flag to match `nghttp2` framing. Context: https://docs.google.com/document/d/19f6B6MpH0VX-pEq2KPTutGqPd7w1CTrGw-iD_5x6Yn4/edit?tab=t.0#heading=h.rjkpiys4lyzb PiperOrigin-RevId: 992426536
diff --git a/quiche/http2/adapter/oghttp2_adapter_test.cc b/quiche/http2/adapter/oghttp2_adapter_test.cc index 6faf800..be9793e 100644 --- a/quiche/http2/adapter/oghttp2_adapter_test.cc +++ b/quiche/http2/adapter/oghttp2_adapter_test.cc
@@ -1,9 +1,12 @@ #include "quiche/http2/adapter/oghttp2_adapter.h" +#include <cstddef> #include <cstdint> #include <limits> #include <memory> +#include <optional> #include <string> +#include <utility> #include <vector> #include "absl/strings/str_join.h" @@ -3250,6 +3253,105 @@ SpdyFrameType::DATA, SpdyFrameType::DATA})); } +TEST(OgHttp2AdapterTest, MaxOutboundDataFramePayload) { + struct TestCase { + std::optional<uint32_t> max_outbound_data_frame_payload; + size_t expected_data_frame_size; + }; + // SETTINGS_MAX_FRAME_SIZE advertised by the client. + const uint32_t kPeerMaxFrameSize = 64 * 1024; + const size_t kBodySize = 48 * 1024; + const std::vector<TestCase> test_cases = { + // Without the option, DATA frames are limited only by the peer max. + {std::nullopt, 48 * 1024}, + // Zero is treated as unset. + {0u, 48 * 1024}, + // The option caps DATA frames below the peer's MAX_FRAME_SIZE. + {16 * 1024, 16 * 1024}, + {24 * 1024, 24 * 1024}, + }; + + for (const TestCase& test_case : test_cases) { + TestVisitor visitor; + OgHttp2Adapter::Options options; + options.perspective = Perspective::kServer; + options.max_outbound_data_frame_payload = + test_case.max_outbound_data_frame_payload; + auto adapter = OgHttp2Adapter::Create(visitor, options); + + testing::InSequence s; + + const std::string frames = + TestFrameSequence() + .ClientPreface({{MAX_FRAME_SIZE, kPeerMaxFrameSize}}) + .Headers(1, + {{":method", "GET"}, + {":scheme", "https"}, + {":authority", "example.com"}, + {":path", "/this/is/request/one"}}, + /*fin=*/true) + .Serialize(); + // Client preface (SETTINGS with MAX_FRAME_SIZE) + EXPECT_CALL(visitor, OnFrameHeader(0, 6, SETTINGS, 0)); + EXPECT_CALL(visitor, OnSettingsStart()); + EXPECT_CALL(visitor, + OnSetting(Http2Setting{MAX_FRAME_SIZE, kPeerMaxFrameSize})); + EXPECT_CALL(visitor, OnSettingsEnd()); + // Stream 1 + EXPECT_CALL(visitor, OnFrameHeader(1, _, HEADERS, 5)); + EXPECT_CALL(visitor, OnBeginHeadersForStream(1)); + EXPECT_CALL(visitor, OnHeaderForStream(1, ":method", "GET")); + EXPECT_CALL(visitor, OnHeaderForStream(1, ":scheme", "https")); + EXPECT_CALL(visitor, OnHeaderForStream(1, ":authority", "example.com")); + EXPECT_CALL(visitor, OnHeaderForStream(1, ":path", "/this/is/request/one")); + EXPECT_CALL(visitor, OnEndHeadersForStream(1)); + EXPECT_CALL(visitor, OnEndStream(1)); + + const int64_t read_result = adapter->ProcessBytes(frames); + EXPECT_EQ(frames.size(), static_cast<size_t>(read_result)); + + // Server SETTINGS and SETTINGS ack. + EXPECT_CALL(visitor, OnBeforeFrameSent(SETTINGS, 0, _, 0x0)); + EXPECT_CALL(visitor, OnFrameSent(SETTINGS, 0, _, 0x0, 0)); + EXPECT_CALL(visitor, OnBeforeFrameSent(SETTINGS, 0, _, ACK_FLAG)); + EXPECT_CALL(visitor, OnFrameSent(SETTINGS, 0, _, ACK_FLAG, 0)); + + int send_result = adapter->Send(); + EXPECT_EQ(0, send_result); + visitor.Clear(); + + visitor.AppendPayloadForStream(1, std::string(kBodySize, 'c')); + visitor.SetEndData(1, true); + const int submit_result = + adapter->SubmitResponse(1, ToHeaders({{":status", "200"}}), false); + EXPECT_EQ(0, submit_result); + + EXPECT_CALL(visitor, OnBeforeFrameSent(HEADERS, 1, _, 0x4)); + EXPECT_CALL(visitor, OnFrameSent(HEADERS, 1, _, 0x4, 0)); + const size_t num_data_frames = + kBodySize / test_case.expected_data_frame_size; + // Only the last DATA frame carries END_STREAM. + EXPECT_CALL( + visitor, + OnFrameSent(DATA, 1, test_case.expected_data_frame_size, 0x0, 0)) + .Times(static_cast<int>(num_data_frames) - 1); + EXPECT_CALL(visitor, + OnFrameSent(DATA, 1, test_case.expected_data_frame_size, + END_STREAM_FLAG, 0)); + EXPECT_CALL(visitor, OnCloseStream(1, Http2ErrorCode::HTTP2_NO_ERROR)); + + send_result = adapter->Send(); + EXPECT_EQ(0, send_result); + // The serialized bytes are HEADERS followed by the expected DATA frames. + std::vector<std::pair<SpdyFrameType, std::optional<size_t>>> + expected_frames = {{SpdyFrameType::HEADERS, std::nullopt}}; + expected_frames.insert( + expected_frames.end(), num_data_frames, + {SpdyFrameType::DATA, test_case.expected_data_frame_size}); + EXPECT_THAT(visitor.data(), EqualsFrames(expected_frames)); + } +} + TEST(OgHttp2AdapterTest, ClientReceivesInitialWindowSettingAfterStreamStart) { TestVisitor visitor; OgHttp2Adapter::Options options;
diff --git a/quiche/http2/adapter/oghttp2_session.cc b/quiche/http2/adapter/oghttp2_session.cc index c2d083b..83d2403 100644 --- a/quiche/http2/adapter/oghttp2_session.cc +++ b/quiche/http2/adapter/oghttp2_session.cc
@@ -898,7 +898,7 @@ bool wrote_data = false; int32_t available_window = std::min({connection_send_window_, state.send_window, - static_cast<int32_t>(max_frame_payload_)}); + static_cast<int32_t>(MaxOutboundDataFramePayload())}); while (connection_can_write == SendResult::SEND_OK && available_window > 0 && IsReadyToWriteData(state)) { wrote_data = true; @@ -939,8 +939,9 @@ } connection_send_window_ -= info.payload_length; state.send_window -= info.payload_length; - available_window = std::min({connection_send_window_, state.send_window, - static_cast<int32_t>(max_frame_payload_)}); + available_window = + std::min({connection_send_window_, state.send_window, + static_cast<int32_t>(MaxOutboundDataFramePayload())}); if (info.end_stream) { state.half_closed_local = true; MaybeFinWithRstStream(it); @@ -2253,6 +2254,15 @@ return {/*payload_length=*/0, /*end_data=*/true, /*end_stream=*/true}; } +uint32_t OgHttp2Session::MaxOutboundDataFramePayload() const { + if (options_.max_outbound_data_frame_payload.has_value() && + *options_.max_outbound_data_frame_payload > 0) { + return std::min(max_frame_payload_, + *options_.max_outbound_data_frame_payload); + } + return max_frame_payload_; +} + bool OgHttp2Session::SendDataFrame(Http2StreamId stream_id, absl::string_view frame_header, size_t payload_length,
diff --git a/quiche/http2/adapter/oghttp2_session.h b/quiche/http2/adapter/oghttp2_session.h index be7ceb4..714279c 100644 --- a/quiche/http2/adapter/oghttp2_session.h +++ b/quiche/http2/adapter/oghttp2_session.h
@@ -93,6 +93,10 @@ bool crumble_cookies = false; // If true, allows a GOAWAY to be sent even when acting as a client. bool send_goaway_as_client = false; + // If set, caps the payload length of outbound DATA frames at this many + // bytes, even if the peer's SETTINGS_MAX_FRAME_SIZE allows larger frames. + // A value of zero is treated as unset. + std::optional<uint32_t> max_outbound_data_frame_payload; // Specifies the behavior of the HPACK encoder when compressing headers. enum CompressionOption : uint8_t { ENABLE_COMPRESSION, // Dynamic table enabled, Huffman enabled. @@ -478,6 +482,11 @@ size_t flow_control_available, StreamState& stream_state); + // Returns the maximum payload length of an outbound DATA frame: the peer's + // SETTINGS_MAX_FRAME_SIZE, further capped by + // `Options::max_outbound_data_frame_payload` if set. + uint32_t MaxOutboundDataFramePayload() const; + // Invokes the appropriate API to send a DATA frame header and payload. bool SendDataFrame(Http2StreamId stream_id, absl::string_view frame_header, size_t payload_length, StreamState& stream_state);