Adds optional RST_STREAM NO_ERROR behavior after sending a fin to an incomplete request as a server.
PiperOrigin-RevId: 410264655
diff --git a/http2/adapter/oghttp2_adapter_test.cc b/http2/adapter/oghttp2_adapter_test.cc
index efe0327..171f0b2 100644
--- a/http2/adapter/oghttp2_adapter_test.cc
+++ b/http2/adapter/oghttp2_adapter_test.cc
@@ -1677,7 +1677,7 @@
EXPECT_CALL(visitor, OnFrameSent(SETTINGS, 0, _, 0x1, 0));
EXPECT_CALL(visitor, OnBeforeFrameSent(HEADERS, 1, _, 0x5));
EXPECT_CALL(visitor, OnFrameSent(HEADERS, 1, _, 0x5, 0));
- // BUG: Should send RST_STREAM NO_ERROR as well.
+ // RST_STREAM NO_ERROR option is disabled.
int send_result = adapter->Send();
EXPECT_EQ(0, send_result);
@@ -1687,6 +1687,62 @@
EXPECT_FALSE(adapter->want_write());
}
+TEST(OgHttp2AdapterServerTest,
+ IncompleteRequestWithServerResponseRstStreamEnabled) {
+ DataSavingVisitor visitor;
+ OgHttp2Adapter::Options options{.perspective = Perspective::kServer,
+ .rst_stream_no_error_when_incomplete = true};
+ auto adapter = OgHttp2Adapter::Create(visitor, options);
+ EXPECT_FALSE(adapter->want_write());
+
+ const std::string frames = TestFrameSequence()
+ .ClientPreface()
+ .Headers(1,
+ {{":method", "GET"},
+ {":scheme", "https"},
+ {":authority", "example.com"},
+ {":path", "/this/is/request/one"}},
+ /*fin=*/false)
+ .Serialize();
+ testing::InSequence s;
+
+ // Client preface (empty SETTINGS)
+ EXPECT_CALL(visitor, OnFrameHeader(0, 0, SETTINGS, 0));
+ EXPECT_CALL(visitor, OnSettingsStart());
+ EXPECT_CALL(visitor, OnSettingsEnd());
+ // Stream 1
+ EXPECT_CALL(visitor, OnFrameHeader(1, _, HEADERS, 4));
+ EXPECT_CALL(visitor, OnBeginHeadersForStream(1));
+ EXPECT_CALL(visitor, OnHeaderForStream(1, _, _)).Times(4);
+ EXPECT_CALL(visitor, OnEndHeadersForStream(1));
+
+ const int64_t result = adapter->ProcessBytes(frames);
+ EXPECT_EQ(frames.size(), static_cast<size_t>(result));
+
+ int submit_result =
+ adapter->SubmitResponse(1, ToHeaders({{":status", "200"}}), nullptr);
+ EXPECT_EQ(submit_result, 0);
+ 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, _, 0x1));
+ EXPECT_CALL(visitor, OnFrameSent(SETTINGS, 0, _, 0x1, 0));
+ EXPECT_CALL(visitor, OnBeforeFrameSent(HEADERS, 1, _, 0x5));
+ EXPECT_CALL(visitor, OnFrameSent(HEADERS, 1, _, 0x5, 0));
+ EXPECT_CALL(visitor, OnBeforeFrameSent(RST_STREAM, 1, 4, 0x0));
+ EXPECT_CALL(visitor, OnFrameSent(RST_STREAM, 1, 4, 0x0, 0));
+ EXPECT_CALL(visitor, OnCloseStream(1, Http2ErrorCode::NO_ERROR));
+
+ int send_result = adapter->Send();
+ EXPECT_EQ(0, send_result);
+ EXPECT_THAT(visitor.data(), EqualsFrames({spdy::SpdyFrameType::SETTINGS,
+ spdy::SpdyFrameType::SETTINGS,
+ spdy::SpdyFrameType::HEADERS,
+ spdy::SpdyFrameType::RST_STREAM}));
+ EXPECT_FALSE(adapter->want_write());
+}
+
TEST(OgHttp2AdapterServerTest, ServerSendsInvalidTrailers) {
DataSavingVisitor visitor;
OgHttp2Adapter::Options options{.perspective = Perspective::kServer};
diff --git a/http2/adapter/oghttp2_session.cc b/http2/adapter/oghttp2_session.cc
index c46f338..cf820b8 100644
--- a/http2/adapter/oghttp2_session.cc
+++ b/http2/adapter/oghttp2_session.cc
@@ -387,6 +387,7 @@
}
void OgHttp2Session::EnqueueFrame(std::unique_ptr<spdy::SpdyFrameIR> frame) {
+ RunOnExit r;
if (frame->frame_type() == spdy::SpdyFrameType::GOAWAY) {
queued_goaway_ = true;
} else if (frame->fin() ||
@@ -397,6 +398,9 @@
}
if (frame->frame_type() == spdy::SpdyFrameType::RST_STREAM) {
streams_reset_.insert(frame->stream_id());
+ } else if (iter != stream_map_.end()) {
+ // Enqueue RST_STREAM NO_ERROR if appropriate.
+ r = RunOnExit{[this, iter]() { MaybeFinWithRstStream(iter); }};
}
}
if (frame->stream_id() != 0) {
@@ -579,6 +583,7 @@
static_cast<int32_t>(max_frame_payload_)});
if (fin) {
state.half_closed_local = true;
+ MaybeFinWithRstStream(it);
}
AfterFrameSent(/* DATA */ 0, stream_id, length, fin ? 0x1 : 0x0, 0);
if (!stream_map_.contains(stream_id)) {
@@ -1097,6 +1102,20 @@
EnqueueFrame(std::move(frame));
}
+void OgHttp2Session::MaybeFinWithRstStream(StreamStateMap::iterator iter) {
+ QUICHE_DCHECK(iter != stream_map_.end() && iter->second.half_closed_local);
+
+ if (options_.rst_stream_no_error_when_incomplete &&
+ options_.perspective == Perspective::kServer &&
+ !iter->second.half_closed_remote) {
+ // Since the peer has not yet ended the stream, this endpoint should
+ // send a RST_STREAM NO_ERROR. See RFC 7540 Section 8.1.
+ EnqueueFrame(absl::make_unique<spdy::SpdyRstStreamIR>(
+ iter->first, spdy::SpdyErrorCode::ERROR_CODE_NO_ERROR));
+ iter->second.half_closed_remote = true;
+ }
+}
+
void OgHttp2Session::MarkDataBuffered(Http2StreamId stream_id, size_t bytes) {
connection_window_manager_.MarkDataBuffered(bytes);
auto it = stream_map_.find(stream_id);
diff --git a/http2/adapter/oghttp2_session.h b/http2/adapter/oghttp2_session.h
index b11b57c..c6a3e0f 100644
--- a/http2/adapter/oghttp2_session.h
+++ b/http2/adapter/oghttp2_session.h
@@ -36,6 +36,9 @@
Perspective perspective = Perspective::kClient;
// Whether to automatically send PING acks when receiving a PING.
bool auto_ping_ack = true;
+ // Whether (as server) to send a RST_STREAM NO_ERROR when sending a fin on
+ // an incomplete stream.
+ bool rst_stream_no_error_when_incomplete = false;
};
OgHttp2Session(Http2VisitorInterface& visitor, Options options);
@@ -276,6 +279,10 @@
void SendTrailers(Http2StreamId stream_id, spdy::SpdyHeaderBlock trailers);
+ // Encapsulates the RST_STREAM NO_ERROR behavior described in RFC 7540
+ // Section 8.1.
+ void MaybeFinWithRstStream(StreamStateMap::iterator iter);
+
// Performs flow control accounting for data sent by the peer.
void MarkDataBuffered(Http2StreamId stream_id, size_t bytes);
