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);