Adjusts internal OgHttp2Session state when sending SETTINGS_INITIAL_WINDOW_SIZE to the peer. This implements the final piece of handling SETTINGS_INITIAL_WINDOW_SIZE. PiperOrigin-RevId: 429386425
diff --git a/http2/adapter/nghttp2_adapter_test.cc b/http2/adapter/nghttp2_adapter_test.cc index 5791692..0a7526c 100644 --- a/http2/adapter/nghttp2_adapter_test.cc +++ b/http2/adapter/nghttp2_adapter_test.cc
@@ -295,6 +295,83 @@ kInitialFlowControlWindowSize + 20000); } +TEST(NgHttp2AdapterTest, AckOfSettingInitialWindowSizeAffectsWindow) { + DataSavingVisitor visitor; + auto adapter = NgHttp2Adapter::CreateClientAdapter(visitor); + + testing::InSequence s; + + const std::vector<Header> headers = + ToHeaders({{":method", "GET"}, + {":scheme", "http"}, + {":authority", "example.com"}, + {":path", "/this/is/request/one"}}); + const int32_t stream_id1 = adapter->SubmitRequest(headers, nullptr, nullptr); + + EXPECT_CALL(visitor, OnBeforeFrameSent(HEADERS, stream_id1, _, 0x5)); + EXPECT_CALL(visitor, OnFrameSent(HEADERS, stream_id1, _, 0x5, 0)); + + int result = adapter->Send(); + EXPECT_EQ(0, result); + + const std::string initial_frames = + TestFrameSequence().ServerPreface().Serialize(); + EXPECT_CALL(visitor, OnFrameHeader(0, 0, SETTINGS, 0x0)); + EXPECT_CALL(visitor, OnSettingsStart()); + EXPECT_CALL(visitor, OnSettingsEnd()); + + int64_t parse_result = adapter->ProcessBytes(initial_frames); + EXPECT_EQ(initial_frames.size(), static_cast<size_t>(parse_result)); + + EXPECT_EQ(adapter->GetStreamReceiveWindowSize(stream_id1), + kInitialFlowControlWindowSize); + adapter->SubmitSettings({{INITIAL_WINDOW_SIZE, 80000u}}); + // No update for the first stream, yet. + EXPECT_EQ(adapter->GetStreamReceiveWindowSize(stream_id1), + kInitialFlowControlWindowSize); + + // Ack of server's initial settings. + EXPECT_CALL(visitor, OnBeforeFrameSent(SETTINGS, 0, _, 0x1)); + EXPECT_CALL(visitor, OnFrameSent(SETTINGS, 0, _, 0x1, 0)); + + // Outbound SETTINGS containing INITIAL_WINDOW_SIZE. + EXPECT_CALL(visitor, OnBeforeFrameSent(SETTINGS, 0, 6, 0x0)); + EXPECT_CALL(visitor, OnFrameSent(SETTINGS, 0, 6, 0x0, 0)); + + result = adapter->Send(); + EXPECT_EQ(0, result); + + // Still no update, as a SETTINGS ack has not yet been received. + EXPECT_EQ(adapter->GetStreamReceiveWindowSize(stream_id1), + kInitialFlowControlWindowSize); + + const std::string settings_ack = + TestFrameSequence().SettingsAck().Serialize(); + + EXPECT_CALL(visitor, OnFrameHeader(0, 0, SETTINGS, 0x1)); + EXPECT_CALL(visitor, OnSettingsAck); + + parse_result = adapter->ProcessBytes(settings_ack); + EXPECT_EQ(settings_ack.size(), static_cast<size_t>(parse_result)); + + // Stream window has been updated. + EXPECT_EQ(adapter->GetStreamReceiveWindowSize(stream_id1), 80000); + + const std::vector<Header> headers2 = + ToHeaders({{":method", "GET"}, + {":scheme", "http"}, + {":authority", "example.com"}, + {":path", "/this/is/request/two"}}); + const int32_t stream_id2 = adapter->SubmitRequest(headers, nullptr, nullptr); + + EXPECT_CALL(visitor, OnBeforeFrameSent(HEADERS, stream_id2, _, 0x5)); + EXPECT_CALL(visitor, OnFrameSent(HEADERS, stream_id2, _, 0x5, 0)); + result = adapter->Send(); + EXPECT_EQ(0, result); + + EXPECT_EQ(adapter->GetStreamReceiveWindowSize(stream_id2), 80000); +} + TEST(NgHttp2AdapterTest, ClientRejects100HeadersWithFin) { DataSavingVisitor visitor; auto adapter = NgHttp2Adapter::CreateClientAdapter(visitor);
diff --git a/http2/adapter/oghttp2_adapter_test.cc b/http2/adapter/oghttp2_adapter_test.cc index c00601e..0dc53ce 100644 --- a/http2/adapter/oghttp2_adapter_test.cc +++ b/http2/adapter/oghttp2_adapter_test.cc
@@ -379,6 +379,91 @@ kInitialFlowControlWindowSize + 20000); } +TEST(OgHttp2AdapterTest, AckOfSettingInitialWindowSizeAffectsWindow) { + DataSavingVisitor visitor; + OgHttp2Adapter::Options options{.perspective = Perspective::kClient}; + auto adapter = OgHttp2Adapter::Create(visitor, options); + + testing::InSequence s; + + const std::vector<Header> headers = + ToHeaders({{":method", "GET"}, + {":scheme", "http"}, + {":authority", "example.com"}, + {":path", "/this/is/request/one"}}); + const int32_t stream_id1 = adapter->SubmitRequest(headers, nullptr, nullptr); + + EXPECT_CALL(visitor, OnBeforeFrameSent(SETTINGS, 0, _, 0x0)); + EXPECT_CALL(visitor, OnFrameSent(SETTINGS, 0, _, 0x0, 0)); + EXPECT_CALL(visitor, OnBeforeFrameSent(HEADERS, stream_id1, _, 0x5)); + EXPECT_CALL(visitor, OnFrameSent(HEADERS, stream_id1, _, 0x5, 0)); + + int result = adapter->Send(); + EXPECT_EQ(0, result); + + const std::string initial_frames = + TestFrameSequence() + .ServerPreface() + .SettingsAck() // Ack of the client's initial settings. + .Serialize(); + EXPECT_CALL(visitor, OnFrameHeader(0, 0, SETTINGS, 0x0)); + EXPECT_CALL(visitor, OnSettingsStart()); + EXPECT_CALL(visitor, OnSettingsEnd()); + EXPECT_CALL(visitor, OnFrameHeader(0, 0, SETTINGS, 0x1)); + EXPECT_CALL(visitor, OnSettingsAck); + + int64_t parse_result = adapter->ProcessBytes(initial_frames); + EXPECT_EQ(initial_frames.size(), static_cast<size_t>(parse_result)); + + EXPECT_EQ(adapter->GetStreamReceiveWindowSize(stream_id1), + kInitialFlowControlWindowSize); + adapter->SubmitSettings({{INITIAL_WINDOW_SIZE, 80000u}}); + // No update for the first stream, yet. + EXPECT_EQ(adapter->GetStreamReceiveWindowSize(stream_id1), + kInitialFlowControlWindowSize); + + // Ack of server's initial settings. + EXPECT_CALL(visitor, OnBeforeFrameSent(SETTINGS, 0, _, 0x1)); + EXPECT_CALL(visitor, OnFrameSent(SETTINGS, 0, _, 0x1, 0)); + + // Outbound SETTINGS containing INITIAL_WINDOW_SIZE. + EXPECT_CALL(visitor, OnBeforeFrameSent(SETTINGS, 0, 6, 0x0)); + EXPECT_CALL(visitor, OnFrameSent(SETTINGS, 0, 6, 0x0, 0)); + + result = adapter->Send(); + EXPECT_EQ(0, result); + + // Still no update, as a SETTINGS ack has not yet been received. + EXPECT_EQ(adapter->GetStreamReceiveWindowSize(stream_id1), + kInitialFlowControlWindowSize); + + const std::string settings_ack = + TestFrameSequence().SettingsAck().Serialize(); + + EXPECT_CALL(visitor, OnFrameHeader(0, 0, SETTINGS, 0x1)); + EXPECT_CALL(visitor, OnSettingsAck); + + parse_result = adapter->ProcessBytes(settings_ack); + EXPECT_EQ(settings_ack.size(), static_cast<size_t>(parse_result)); + + // Stream window has been updated. + EXPECT_EQ(adapter->GetStreamReceiveWindowSize(stream_id1), 80000); + + const std::vector<Header> headers2 = + ToHeaders({{":method", "GET"}, + {":scheme", "http"}, + {":authority", "example.com"}, + {":path", "/this/is/request/two"}}); + const int32_t stream_id2 = adapter->SubmitRequest(headers, nullptr, nullptr); + + EXPECT_CALL(visitor, OnBeforeFrameSent(HEADERS, stream_id2, _, 0x5)); + EXPECT_CALL(visitor, OnFrameSent(HEADERS, stream_id2, _, 0x5, 0)); + result = adapter->Send(); + EXPECT_EQ(0, result); + + EXPECT_EQ(adapter->GetStreamReceiveWindowSize(stream_id2), 80000); +} + TEST(OgHttp2AdapterTest, ClientRejects100HeadersWithFin) { DataSavingVisitor visitor; OgHttp2Adapter::Options options{.perspective = Perspective::kClient};
diff --git a/http2/adapter/oghttp2_session.cc b/http2/adapter/oghttp2_session.cc index 563344b..45fecd9 100644 --- a/http2/adapter/oghttp2_session.cc +++ b/http2/adapter/oghttp2_session.cc
@@ -524,7 +524,9 @@ return; } - RunOnExit r; + const bool is_non_ack_settings = IsNonAckSettings(*frame); + MaybeSetupPreface(is_non_ack_settings); + if (frame->frame_type() == spdy::SpdyFrameType::GOAWAY) { queued_goaway_ = true; if (latched_error_) { @@ -544,6 +546,9 @@ UpdateReceiveWindow( frame->stream_id(), reinterpret_cast<spdy::SpdyWindowUpdateIR&>(*frame).delta()); + } else if (is_non_ack_settings) { + HandleOutboundSettings( + *reinterpret_cast<spdy::SpdySettingsIR*>(frame.get())); } if (frame->stream_id() != 0) { auto result = queued_frames_.insert({frame->stream_id(), 1}); @@ -567,7 +572,7 @@ return kSendError; } - MaybeSetupPreface(); + MaybeSetupPreface(/*sending_outbound_settings=*/false); SendResult continue_writing = SendQueuedFrames(); if (queued_immediate_goaway_) { @@ -1236,7 +1241,7 @@ Http2VisitorInterface::ConnectionError::kFlowControlError); return; } else { - UpdateInitialWindowSize(value); + UpdateStreamSendWindowSizes(value); } break; default: @@ -1497,19 +1502,18 @@ } } -void OgHttp2Session::MaybeSetupPreface() { +void OgHttp2Session::MaybeSetupPreface(bool sending_outbound_settings) { if (!queued_preface_) { + queued_preface_ = true; if (!IsServerSession()) { buffered_data_.assign(spdy::kHttp2ConnectionHeaderPrefix, spdy::kHttp2ConnectionHeaderPrefixSize); } - // First frame must be a non-ack SETTINGS. - if (frames_.empty() || !IsNonAckSettings(*frames_.front())) { - auto frame = PrepareSettingsFrame(GetInitialSettings()); - HandleOutboundSettings(*frame); - frames_.push_front(std::move(frame)); + if (!sending_outbound_settings) { + QUICHE_DCHECK(frames_.empty()); + // First frame must be a non-ack SETTINGS. + EnqueueFrame(PrepareSettingsFrame(GetInitialSettings())); } - queued_preface_ = true; } } @@ -1578,8 +1582,11 @@ case HEADER_TABLE_SIZE: decoder_.GetHpackDecoder()->ApplyHeaderTableSizeSetting(value); break; - case ENABLE_PUSH: case INITIAL_WINDOW_SIZE: + UpdateStreamReceiveWindowSizes(value); + initial_stream_receive_window_ = value; + break; + case ENABLE_PUSH: case MAX_FRAME_SIZE: case MAX_HEADER_LIST_SIZE: case ENABLE_CONNECT_PROTOCOL: @@ -1816,7 +1823,7 @@ } } -void OgHttp2Session::UpdateInitialWindowSize(uint32_t new_value) { +void OgHttp2Session::UpdateStreamSendWindowSizes(uint32_t new_value) { const int32_t delta = static_cast<int32_t>(new_value) - initial_stream_send_window_; initial_stream_send_window_ = new_value; @@ -1835,5 +1842,11 @@ } } +void OgHttp2Session::UpdateStreamReceiveWindowSizes(uint32_t new_value) { + for (auto& [stream_id, stream_state] : stream_map_) { + stream_state.window_manager.OnWindowSizeLimitChange(new_value); + } +} + } // namespace adapter } // namespace http2
diff --git a/http2/adapter/oghttp2_session.h b/http2/adapter/oghttp2_session.h index 6f02d2c..a714825 100644 --- a/http2/adapter/oghttp2_session.h +++ b/http2/adapter/oghttp2_session.h
@@ -289,8 +289,10 @@ struct QUICHE_EXPORT_PRIVATE ProcessBytesResultVisitor; - // Queues the connection preface, if not already done. - void MaybeSetupPreface(); + // Queues the connection preface, if not already done. If not + // `sending_outbound_settings` and the preface has not yet been queued, this + // method will generate and enqueue initial SETTINGS. + void MaybeSetupPreface(bool sending_outbound_settings); // Gets the settings to be sent in the initial SETTINGS frame sent as part of // the connection preface. @@ -405,7 +407,13 @@ // Invoked when sending a flow control window update to the peer. void UpdateReceiveWindow(Http2StreamId stream_id, int32_t delta); - void UpdateInitialWindowSize(uint32_t new_value); + // Updates stream send window accounting to respect the peer's advertised + // initial window setting. + void UpdateStreamSendWindowSizes(uint32_t new_value); + + // Updates stream receive window managers to use the newly advertised stream + // initial window. + void UpdateStreamReceiveWindowSizes(uint32_t new_value); // Receives events when inbound frames are parsed. Http2VisitorInterface& visitor_;