Fixes stream resumption on WINDOW_UPDATE when the send window size for a stream is negative in oghttp2.
PiperOrigin-RevId: 452286704
diff --git a/quiche/http2/adapter/nghttp2_adapter_test.cc b/quiche/http2/adapter/nghttp2_adapter_test.cc
index 9a61424..22c8f83 100644
--- a/quiche/http2/adapter/nghttp2_adapter_test.cc
+++ b/quiche/http2/adapter/nghttp2_adapter_test.cc
@@ -6793,6 +6793,96 @@
SpdyFrameType::RST_STREAM, SpdyFrameType::RST_STREAM}));
}
+TEST(NgHttp2AdapterTest, NegativeFlowControlStreamResumption) {
+ DataSavingVisitor visitor;
+ auto adapter = NgHttp2Adapter::CreateServerAdapter(visitor);
+
+ const std::string frames =
+ TestFrameSequence()
+ .ClientPreface({{INITIAL_WINDOW_SIZE, 128u * 1024u}})
+ .WindowUpdate(0, 1 << 20)
+ .Headers(1,
+ {{":method", "GET"},
+ {":scheme", "https"},
+ {":authority", "example.com"},
+ {":path", "/this/is/request/one"}},
+ /*fin=*/true)
+ .Serialize();
+ testing::InSequence s;
+
+ // Client preface (empty SETTINGS)
+ EXPECT_CALL(visitor, OnFrameHeader(0, 6, SETTINGS, 0));
+ EXPECT_CALL(visitor, OnSettingsStart());
+ EXPECT_CALL(visitor,
+ OnSetting(Http2Setting{Http2KnownSettingsId::INITIAL_WINDOW_SIZE,
+ 128u * 1024u}));
+ EXPECT_CALL(visitor, OnSettingsEnd());
+
+ EXPECT_CALL(visitor, OnFrameHeader(0, 4, WINDOW_UPDATE, 0));
+ EXPECT_CALL(visitor, OnWindowUpdate(0, 1 << 20));
+
+ // Stream 1
+ EXPECT_CALL(visitor, OnFrameHeader(1, _, HEADERS, 0x5));
+ EXPECT_CALL(visitor, OnBeginHeadersForStream(1));
+ EXPECT_CALL(visitor, OnHeaderForStream(1, _, _)).Times(4);
+ EXPECT_CALL(visitor, OnEndHeadersForStream(1));
+ EXPECT_CALL(visitor, OnEndStream(1));
+
+ const int64_t read_result = adapter->ProcessBytes(frames);
+ EXPECT_EQ(static_cast<size_t>(read_result), frames.size());
+
+ // Submit a response for the stream.
+ auto body = absl::make_unique<TestDataFrameSource>(visitor, true);
+ TestDataFrameSource& body_ref = *body;
+ body_ref.AppendPayload(std::string(70000, 'a'));
+ int submit_result = adapter->SubmitResponse(
+ 1, ToHeaders({{":status", "200"}}), std::move(body));
+ ASSERT_EQ(0, submit_result);
+
+ EXPECT_CALL(visitor, OnBeforeFrameSent(SETTINGS, 0, _, 0x1));
+ EXPECT_CALL(visitor, OnFrameSent(SETTINGS, 0, _, 0x1, 0));
+ EXPECT_CALL(visitor, OnBeforeFrameSent(HEADERS, 1, _, 0x4));
+ EXPECT_CALL(visitor, OnFrameSent(HEADERS, 1, _, 0x4, 0));
+ EXPECT_CALL(visitor, OnFrameSent(DATA, 1, _, 0x0, 0)).Times(5);
+
+ adapter->Send();
+ EXPECT_FALSE(adapter->want_write());
+
+ EXPECT_CALL(visitor, OnFrameHeader(0, 6, SETTINGS, 0));
+ EXPECT_CALL(visitor, OnSettingsStart());
+ EXPECT_CALL(visitor,
+ OnSetting(Http2Setting{Http2KnownSettingsId::INITIAL_WINDOW_SIZE,
+ 64u * 1024u}));
+ EXPECT_CALL(visitor, OnSettingsEnd());
+
+ // Processing these SETTINGS will cause stream 1's send window to become
+ // negative.
+ adapter->ProcessBytes(TestFrameSequence()
+ .Settings({{INITIAL_WINDOW_SIZE, 64u * 1024u}})
+ .Serialize());
+ EXPECT_TRUE(adapter->want_write());
+ // nghttp2 does not expose the fact that the send window size is negative.
+ EXPECT_EQ(adapter->GetStreamSendWindowSize(1), 0);
+
+ body_ref.AppendPayload("Stream should be resumed.");
+ adapter->ResumeStream(1);
+
+ EXPECT_CALL(visitor, OnBeforeFrameSent(SETTINGS, 0, _, 0x1));
+ EXPECT_CALL(visitor, OnFrameSent(SETTINGS, 0, _, 0x1, 0));
+ adapter->Send();
+ EXPECT_FALSE(adapter->want_write());
+
+ // Upon receiving the WINDOW_UPDATE, stream 1 should be ready to write.
+ EXPECT_CALL(visitor, OnFrameHeader(1, 4, WINDOW_UPDATE, 0));
+ EXPECT_CALL(visitor, OnWindowUpdate(1, 10000));
+ adapter->ProcessBytes(TestFrameSequence().WindowUpdate(1, 10000).Serialize());
+ EXPECT_TRUE(adapter->want_write());
+ EXPECT_GT(adapter->GetStreamSendWindowSize(1), 0);
+
+ EXPECT_CALL(visitor, OnFrameSent(DATA, 1, _, 0x0, 0));
+ adapter->Send();
+}
+
} // namespace
} // namespace test
} // namespace adapter
diff --git a/quiche/http2/adapter/oghttp2_adapter_test.cc b/quiche/http2/adapter/oghttp2_adapter_test.cc
index 077e33c..f09867c 100644
--- a/quiche/http2/adapter/oghttp2_adapter_test.cc
+++ b/quiche/http2/adapter/oghttp2_adapter_test.cc
@@ -7556,6 +7556,100 @@
EXPECT_EQ(frames.size(), static_cast<size_t>(result));
}
+TEST(OgHttp2AdapterTest, NegativeFlowControlStreamResumption) {
+ DataSavingVisitor visitor;
+ OgHttp2Adapter::Options options;
+ options.perspective = Perspective::kServer;
+ auto adapter = OgHttp2Adapter::Create(visitor, options);
+
+ const std::string frames =
+ TestFrameSequence()
+ .ClientPreface({{INITIAL_WINDOW_SIZE, 128u * 1024u}})
+ .WindowUpdate(0, 1 << 20)
+ .Headers(1,
+ {{":method", "GET"},
+ {":scheme", "https"},
+ {":authority", "example.com"},
+ {":path", "/this/is/request/one"}},
+ /*fin=*/true)
+ .Serialize();
+ testing::InSequence s;
+
+ // Client preface (empty SETTINGS)
+ EXPECT_CALL(visitor, OnFrameHeader(0, 6, SETTINGS, 0));
+ EXPECT_CALL(visitor, OnSettingsStart());
+ EXPECT_CALL(visitor,
+ OnSetting(Http2Setting{Http2KnownSettingsId::INITIAL_WINDOW_SIZE,
+ 128u * 1024u}));
+ EXPECT_CALL(visitor, OnSettingsEnd());
+
+ EXPECT_CALL(visitor, OnFrameHeader(0, 4, WINDOW_UPDATE, 0));
+ EXPECT_CALL(visitor, OnWindowUpdate(0, 1 << 20));
+
+ // Stream 1
+ EXPECT_CALL(visitor, OnFrameHeader(1, _, HEADERS, 0x5));
+ EXPECT_CALL(visitor, OnBeginHeadersForStream(1));
+ EXPECT_CALL(visitor, OnHeaderForStream(1, _, _)).Times(4);
+ EXPECT_CALL(visitor, OnEndHeadersForStream(1));
+ EXPECT_CALL(visitor, OnEndStream(1));
+
+ const int64_t read_result = adapter->ProcessBytes(frames);
+ EXPECT_EQ(static_cast<size_t>(read_result), frames.size());
+
+ // Submit a response for the stream.
+ auto body = absl::make_unique<TestDataFrameSource>(visitor, true);
+ TestDataFrameSource& body_ref = *body;
+ body_ref.AppendPayload(std::string(70000, 'a'));
+ int submit_result = adapter->SubmitResponse(
+ 1, ToHeaders({{":status", "200"}}), std::move(body));
+ ASSERT_EQ(0, submit_result);
+
+ EXPECT_CALL(visitor, OnBeforeFrameSent(SETTINGS, 0, _, 0x0));
+ EXPECT_CALL(visitor, OnFrameSent(SETTINGS, 0, _, 0x0, 0));
+
+ EXPECT_CALL(visitor, OnBeforeFrameSent(SETTINGS, 0, _, 0x1));
+ EXPECT_CALL(visitor, OnFrameSent(SETTINGS, 0, _, 0x1, 0));
+ EXPECT_CALL(visitor, OnBeforeFrameSent(HEADERS, 1, _, 0x4));
+ EXPECT_CALL(visitor, OnFrameSent(HEADERS, 1, _, 0x4, 0));
+ EXPECT_CALL(visitor, OnFrameSent(DATA, 1, _, 0x0, 0)).Times(5);
+
+ adapter->Send();
+ EXPECT_FALSE(adapter->want_write());
+
+ EXPECT_CALL(visitor, OnFrameHeader(0, 6, SETTINGS, 0));
+ EXPECT_CALL(visitor, OnSettingsStart());
+ EXPECT_CALL(visitor,
+ OnSetting(Http2Setting{Http2KnownSettingsId::INITIAL_WINDOW_SIZE,
+ 64u * 1024u}));
+ EXPECT_CALL(visitor, OnSettingsEnd());
+
+ // Processing these SETTINGS will cause stream 1's send window to become
+ // negative.
+ adapter->ProcessBytes(TestFrameSequence()
+ .Settings({{INITIAL_WINDOW_SIZE, 64u * 1024u}})
+ .Serialize());
+ EXPECT_TRUE(adapter->want_write());
+ EXPECT_LT(adapter->GetStreamSendWindowSize(1), 0);
+
+ body_ref.AppendPayload("Stream should be resumed.");
+ adapter->ResumeStream(1);
+
+ EXPECT_CALL(visitor, OnBeforeFrameSent(SETTINGS, 0, _, 0x1));
+ EXPECT_CALL(visitor, OnFrameSent(SETTINGS, 0, _, 0x1, 0));
+ adapter->Send();
+ EXPECT_FALSE(adapter->want_write());
+
+ // Upon receiving the WINDOW_UPDATE, stream 1 should be ready to write.
+ EXPECT_CALL(visitor, OnFrameHeader(1, 4, WINDOW_UPDATE, 0));
+ EXPECT_CALL(visitor, OnWindowUpdate(1, 10000));
+ adapter->ProcessBytes(TestFrameSequence().WindowUpdate(1, 10000).Serialize());
+ EXPECT_TRUE(adapter->want_write());
+ EXPECT_GT(adapter->GetStreamSendWindowSize(1), 0);
+
+ EXPECT_CALL(visitor, OnFrameSent(DATA, 1, _, 0x0, 0));
+ adapter->Send();
+}
+
} // namespace
} // namespace test
} // namespace adapter
diff --git a/quiche/http2/adapter/oghttp2_session.cc b/quiche/http2/adapter/oghttp2_session.cc
index 6cbe736..0f0d6bd 100644
--- a/quiche/http2/adapter/oghttp2_session.cc
+++ b/quiche/http2/adapter/oghttp2_session.cc
@@ -1416,12 +1416,13 @@
if (streams_reset_.contains(stream_id)) {
return;
}
- if (it->second.send_window == 0) {
+ const bool was_blocked = (it->second.send_window <= 0);
+ it->second.send_window += delta_window_size;
+ if (was_blocked && it->second.send_window > 0) {
// The stream was blocked on flow control.
QUICHE_VLOG(1) << "Marking stream " << stream_id << " ready to write.";
write_scheduler_.MarkStreamReady(stream_id, false);
}
- it->second.send_window += delta_window_size;
}
}
visitor_.OnWindowUpdate(stream_id, delta_window_size);
