Adds a bool return value to `Http2VisitorInterface::OnEndStream()` to indicate success. An implementation of that method will soon invoke nghttp2 callbacks, which can fail. PiperOrigin-RevId: 488470078
diff --git a/quiche/http2/adapter/callback_visitor.cc b/quiche/http2/adapter/callback_visitor.cc index 7cdb835..993a8c4 100644 --- a/quiche/http2/adapter/callback_visitor.cc +++ b/quiche/http2/adapter/callback_visitor.cc
@@ -255,8 +255,9 @@ return result == 0; } -void CallbackVisitor::OnEndStream(Http2StreamId stream_id) { +bool CallbackVisitor::OnEndStream(Http2StreamId stream_id) { QUICHE_VLOG(1) << "OnEndStream(stream_id=" << stream_id << ")"; + return true; } void CallbackVisitor::OnRstStream(Http2StreamId stream_id,
diff --git a/quiche/http2/adapter/callback_visitor.h b/quiche/http2/adapter/callback_visitor.h index 7c982e8..5132104 100644 --- a/quiche/http2/adapter/callback_visitor.h +++ b/quiche/http2/adapter/callback_visitor.h
@@ -45,7 +45,7 @@ size_t payload_length) override; bool OnDataForStream(Http2StreamId stream_id, absl::string_view data) override; - void OnEndStream(Http2StreamId stream_id) override; + bool OnEndStream(Http2StreamId stream_id) override; void OnRstStream(Http2StreamId stream_id, Http2ErrorCode error_code) override; bool OnCloseStream(Http2StreamId stream_id, Http2ErrorCode error_code) override;
diff --git a/quiche/http2/adapter/callback_visitor_test.cc b/quiche/http2/adapter/callback_visitor_test.cc index 3eb16c6..68a83d3 100644 --- a/quiche/http2/adapter/callback_visitor_test.cc +++ b/quiche/http2/adapter/callback_visitor_test.cc
@@ -168,7 +168,7 @@ EXPECT_CALL(callbacks, OnFrameRecv(IsData(1, _, NGHTTP2_FLAG_END_STREAM))); visitor.OnBeginDataForStream(1, 0); - visitor.OnEndStream(1); + EXPECT_TRUE(visitor.OnEndStream(1)); EXPECT_CALL(callbacks, OnStreamClose(1, NGHTTP2_NO_ERROR)); visitor.OnCloseStream(1, Http2ErrorCode::HTTP2_NO_ERROR); @@ -388,7 +388,7 @@ "This is the request body.")); EXPECT_CALL(callbacks, OnFrameRecv(IsData(1, _, NGHTTP2_FLAG_END_STREAM))); visitor.OnDataForStream(1, "This is the request body."); - visitor.OnEndStream(1); + EXPECT_TRUE(visitor.OnEndStream(1)); EXPECT_CALL(callbacks, OnStreamClose(1, NGHTTP2_NO_ERROR)); visitor.OnCloseStream(1, Http2ErrorCode::HTTP2_NO_ERROR); @@ -431,7 +431,7 @@ OnDataChunkRecv(kFlags, 1, "This is the request body.")); EXPECT_CALL(callbacks, OnFrameRecv(IsData(1, _, kFlags, kPaddingLength))); EXPECT_TRUE(visitor.OnDataForStream(1, "This is the request body.")); - visitor.OnEndStream(1); + EXPECT_TRUE(visitor.OnEndStream(1)); EXPECT_CALL(callbacks, OnStreamClose(1, NGHTTP2_NO_ERROR)); visitor.OnCloseStream(1, Http2ErrorCode::HTTP2_NO_ERROR); @@ -449,7 +449,7 @@ EXPECT_CALL(callbacks, OnFrameRecv(IsData(3, _, kFlags, kPaddingLength))); EXPECT_TRUE(visitor.OnDataPaddingLength(3, kPaddingLength)); - visitor.OnEndStream(3); + EXPECT_TRUE(visitor.OnEndStream(3)); EXPECT_CALL(callbacks, OnStreamClose(3, NGHTTP2_NO_ERROR)); visitor.OnCloseStream(3, Http2ErrorCode::HTTP2_NO_ERROR); @@ -468,7 +468,7 @@ EXPECT_CALL(callbacks, OnFrameRecv(IsData(5, _, kFlags, kPaddingLength))) .WillOnce(testing::Return(NGHTTP2_ERR_CALLBACK_FAILURE)); EXPECT_FALSE(visitor.OnDataPaddingLength(5, kPaddingLength)); - visitor.OnEndStream(3); + EXPECT_TRUE(visitor.OnEndStream(3)); EXPECT_CALL(callbacks, OnStreamClose(5, NGHTTP2_NO_ERROR)); visitor.OnCloseStream(5, Http2ErrorCode::HTTP2_NO_ERROR);
diff --git a/quiche/http2/adapter/http2_visitor_interface.h b/quiche/http2/adapter/http2_visitor_interface.h index 77bcd7f..fa8491d 100644 --- a/quiche/http2/adapter/http2_visitor_interface.h +++ b/quiche/http2/adapter/http2_visitor_interface.h
@@ -166,7 +166,7 @@ // Called when the peer sends the END_STREAM flag on a stream, indicating that // the peer will not send additional headers or data for that stream. - virtual void OnEndStream(Http2StreamId stream_id) = 0; + virtual bool OnEndStream(Http2StreamId stream_id) = 0; // Called when the connection receives a RST_STREAM for a stream. This call // will be followed by either OnCloseStream().
diff --git a/quiche/http2/adapter/mock_http2_visitor.h b/quiche/http2/adapter/mock_http2_visitor.h index d6f437b..86345d3 100644 --- a/quiche/http2/adapter/mock_http2_visitor.h +++ b/quiche/http2/adapter/mock_http2_visitor.h
@@ -23,6 +23,7 @@ ON_CALL(*this, OnDataPaddingLength).WillByDefault(testing::Return(true)); ON_CALL(*this, OnBeginDataForStream).WillByDefault(testing::Return(true)); ON_CALL(*this, OnDataForStream).WillByDefault(testing::Return(true)); + ON_CALL(*this, OnEndStream).WillByDefault(testing::Return(true)); ON_CALL(*this, OnCloseStream).WillByDefault(testing::Return(true)); ON_CALL(*this, OnGoAway).WillByDefault(testing::Return(true)); ON_CALL(*this, OnInvalidFrame).WillByDefault(testing::Return(true)); @@ -61,7 +62,7 @@ MOCK_METHOD(bool, OnDataForStream, (Http2StreamId stream_id, absl::string_view data), (override)); - MOCK_METHOD(void, OnEndStream, (Http2StreamId stream_id), (override)); + MOCK_METHOD(bool, OnEndStream, (Http2StreamId stream_id), (override)); MOCK_METHOD(void, OnRstStream, (Http2StreamId stream_id, Http2ErrorCode error_code), (override));
diff --git a/quiche/http2/adapter/nghttp2_callbacks.cc b/quiche/http2/adapter/nghttp2_callbacks.cc index 067c522..200a709 100644 --- a/quiche/http2/adapter/nghttp2_callbacks.cc +++ b/quiche/http2/adapter/nghttp2_callbacks.cc
@@ -73,7 +73,10 @@ visitor->OnDataPaddingLength(stream_id, frame->data.padlen); } if (frame->hd.flags & NGHTTP2_FLAG_END_STREAM) { - visitor->OnEndStream(stream_id); + const bool result = visitor->OnEndStream(stream_id); + if (!result) { + return NGHTTP2_ERR_CALLBACK_FAILURE; + } } break; case NGHTTP2_HEADERS: { @@ -84,7 +87,10 @@ } } if (frame->hd.flags & NGHTTP2_FLAG_END_STREAM) { - visitor->OnEndStream(stream_id); + const bool result = visitor->OnEndStream(stream_id); + if (!result) { + return NGHTTP2_ERR_CALLBACK_FAILURE; + } } break; }
diff --git a/quiche/http2/adapter/oghttp2_session.cc b/quiche/http2/adapter/oghttp2_session.cc index 52c0a12..c0c2c61 100644 --- a/quiche/http2/adapter/oghttp2_session.cc +++ b/quiche/http2/adapter/oghttp2_session.cc
@@ -1120,7 +1120,11 @@ return; } - visitor_.OnEndStream(stream_id); + const bool result = visitor_.OnEndStream(stream_id); + if (!result) { + fatal_visitor_callback_failure_ = true; + decoder_.StopProcessing(); + } } auto queued_frames_iter = queued_frames_.find(stream_id);
diff --git a/quiche/http2/adapter/recording_http2_visitor.cc b/quiche/http2/adapter/recording_http2_visitor.cc index 6321fd8..d55045f 100644 --- a/quiche/http2/adapter/recording_http2_visitor.cc +++ b/quiche/http2/adapter/recording_http2_visitor.cc
@@ -80,8 +80,9 @@ return true; } -void RecordingHttp2Visitor::OnEndStream(Http2StreamId stream_id) { +bool RecordingHttp2Visitor::OnEndStream(Http2StreamId stream_id) { events_.push_back(absl::StrFormat("OnEndStream %d", stream_id)); + return true; } void RecordingHttp2Visitor::OnRstStream(Http2StreamId stream_id,
diff --git a/quiche/http2/adapter/recording_http2_visitor.h b/quiche/http2/adapter/recording_http2_visitor.h index 4310e4c..d796fe7 100644 --- a/quiche/http2/adapter/recording_http2_visitor.h +++ b/quiche/http2/adapter/recording_http2_visitor.h
@@ -39,7 +39,7 @@ size_t payload_length) override; bool OnDataForStream(Http2StreamId stream_id, absl::string_view data) override; - void OnEndStream(Http2StreamId stream_id) override; + bool OnEndStream(Http2StreamId stream_id) override; void OnRstStream(Http2StreamId stream_id, Http2ErrorCode error_code) override; bool OnCloseStream(Http2StreamId stream_id, Http2ErrorCode error_code) override;