Adds a boolean return value to Http2VisitorInterface::OnGoAway.
A visitor implementation can indicate a fatal error for the current HTTP/2 session by returning false from this method.
PiperOrigin-RevId: 392438796
diff --git a/http2/adapter/callback_visitor.cc b/http2/adapter/callback_visitor.cc
index 358bfea..a386f94 100644
--- a/http2/adapter/callback_visitor.cc
+++ b/http2/adapter/callback_visitor.cc
@@ -262,7 +262,7 @@
QUICHE_LOG(DFATAL) << "Not implemented";
}
-void CallbackVisitor::OnGoAway(Http2StreamId last_accepted_stream_id,
+bool CallbackVisitor::OnGoAway(Http2StreamId last_accepted_stream_id,
Http2ErrorCode error_code,
absl::string_view opaque_data) {
current_frame_.goaway.last_stream_id = last_accepted_stream_id;
@@ -270,8 +270,11 @@
current_frame_.goaway.opaque_data = ToUint8Ptr(opaque_data.data());
current_frame_.goaway.opaque_data_len = opaque_data.size();
if (callbacks_->on_frame_recv_callback) {
- callbacks_->on_frame_recv_callback(nullptr, ¤t_frame_, user_data_);
+ const int result = callbacks_->on_frame_recv_callback(
+ nullptr, ¤t_frame_, user_data_);
+ return result == 0;
}
+ return true;
}
void CallbackVisitor::OnWindowUpdate(Http2StreamId /*stream_id*/,
diff --git a/http2/adapter/callback_visitor.h b/http2/adapter/callback_visitor.h
index eb2e2b4..2f3b1e9 100644
--- a/http2/adapter/callback_visitor.h
+++ b/http2/adapter/callback_visitor.h
@@ -50,7 +50,7 @@
void OnPing(Http2PingId ping_id, bool is_ack) override;
void OnPushPromiseForStream(Http2StreamId stream_id,
Http2StreamId promised_stream_id) override;
- void OnGoAway(Http2StreamId last_accepted_stream_id,
+ bool OnGoAway(Http2StreamId last_accepted_stream_id,
Http2ErrorCode error_code,
absl::string_view opaque_data) override;
void OnWindowUpdate(Http2StreamId stream_id, int window_increment) override;
diff --git a/http2/adapter/http2_visitor_interface.h b/http2/adapter/http2_visitor_interface.h
index e04e58c..b281bd0 100644
--- a/http2/adapter/http2_visitor_interface.h
+++ b/http2/adapter/http2_visitor_interface.h
@@ -145,7 +145,7 @@
Http2StreamId promised_stream_id) = 0;
// Called when the connection receives a GOAWAY frame.
- virtual void OnGoAway(Http2StreamId last_accepted_stream_id,
+ virtual bool OnGoAway(Http2StreamId last_accepted_stream_id,
Http2ErrorCode error_code,
absl::string_view opaque_data) = 0;
diff --git a/http2/adapter/mock_http2_visitor.h b/http2/adapter/mock_http2_visitor.h
index 5498c02..739e840 100644
--- a/http2/adapter/mock_http2_visitor.h
+++ b/http2/adapter/mock_http2_visitor.h
@@ -20,6 +20,7 @@
.WillByDefault(testing::Return(true));
ON_CALL(*this, OnHeaderForStream).WillByDefault(testing::Return(HEADER_OK));
ON_CALL(*this, OnEndHeadersForStream).WillByDefault(testing::Return(true));
+ ON_CALL(*this, OnGoAway).WillByDefault(testing::Return(true));
ON_CALL(*this, OnInvalidFrame).WillByDefault(testing::Return(true));
ON_CALL(*this, OnMetadataForStream).WillByDefault(testing::Return(true));
ON_CALL(*this, OnMetadataEndForStream).WillByDefault(testing::Return(true));
@@ -84,10 +85,8 @@
(Http2StreamId stream_id, Http2StreamId promised_stream_id),
(override));
- MOCK_METHOD(void,
- OnGoAway,
- (Http2StreamId last_accepted_stream_id,
- Http2ErrorCode error_code,
+ MOCK_METHOD(bool, OnGoAway,
+ (Http2StreamId last_accepted_stream_id, Http2ErrorCode error_code,
absl::string_view opaque_data),
(override));
diff --git a/http2/adapter/nghttp2_adapter_test.cc b/http2/adapter/nghttp2_adapter_test.cc
index bc81936..ae9c7c1 100644
--- a/http2/adapter/nghttp2_adapter_test.cc
+++ b/http2/adapter/nghttp2_adapter_test.cc
@@ -778,6 +778,77 @@
EXPECT_THAT(visitor.data(), EqualsFrames({spdy::SpdyFrameType::SETTINGS}));
}
+TEST(NgHttp2AdapterTest, ClientFailsOnGoAway) {
+ DataSavingVisitor visitor;
+ auto adapter = NgHttp2Adapter::CreateClientAdapter(visitor);
+
+ testing::InSequence s;
+
+ const std::vector<const Header> headers1 =
+ ToHeaders({{":method", "GET"},
+ {":scheme", "http"},
+ {":authority", "example.com"},
+ {":path", "/this/is/request/one"}});
+
+ const char* kSentinel1 = "arbitrary pointer 1";
+ const int32_t stream_id1 =
+ adapter->SubmitRequest(headers1, nullptr, const_cast<char*>(kSentinel1));
+ ASSERT_GT(stream_id1, 0);
+ QUICHE_LOG(INFO) << "Created stream: " << stream_id1;
+
+ 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);
+ absl::string_view data = visitor.data();
+ EXPECT_THAT(data, testing::StartsWith(spdy::kHttp2ConnectionHeaderPrefix));
+ data.remove_prefix(strlen(spdy::kHttp2ConnectionHeaderPrefix));
+ EXPECT_THAT(data, EqualsFrames({spdy::SpdyFrameType::HEADERS}));
+ visitor.Clear();
+
+ const std::string stream_frames =
+ TestFrameSequence()
+ .ServerPreface()
+ .Headers(1,
+ {{":status", "200"},
+ {"server", "my-fake-server"},
+ {"date", "Tue, 6 Apr 2021 12:54:01 GMT"}},
+ /*fin=*/false)
+ .GoAway(1, Http2ErrorCode::INTERNAL_ERROR, "indigestion")
+ .Data(1, "This is the response body.")
+ .Serialize();
+
+ // Server preface (empty SETTINGS)
+ EXPECT_CALL(visitor, OnFrameHeader(0, 0, SETTINGS, 0));
+ EXPECT_CALL(visitor, OnSettingsStart());
+ EXPECT_CALL(visitor, OnSettingsEnd());
+
+ EXPECT_CALL(visitor, OnFrameHeader(1, _, HEADERS, 4));
+ EXPECT_CALL(visitor, OnBeginHeadersForStream(1));
+ EXPECT_CALL(visitor, OnHeaderForStream(1, ":status", "200"));
+ EXPECT_CALL(visitor, OnHeaderForStream(1, "server", "my-fake-server"));
+ EXPECT_CALL(visitor,
+ OnHeaderForStream(1, "date", "Tue, 6 Apr 2021 12:54:01 GMT"));
+ EXPECT_CALL(visitor, OnEndHeadersForStream(1));
+ EXPECT_CALL(visitor, OnFrameHeader(0, _, GOAWAY, 0));
+ EXPECT_CALL(visitor,
+ OnGoAway(1, Http2ErrorCode::INTERNAL_ERROR, "indigestion"))
+ .WillOnce(testing::Return(false));
+ EXPECT_CALL(visitor, OnConnectionError());
+
+ const int64_t stream_result = adapter->ProcessBytes(stream_frames);
+ EXPECT_EQ(NGHTTP2_ERR_CALLBACK_FAILURE, stream_result);
+
+ EXPECT_CALL(visitor, OnBeforeFrameSent(SETTINGS, 0, 0, 0x1));
+ EXPECT_CALL(visitor, OnFrameSent(SETTINGS, 0, 0, 0x1, 0));
+
+ EXPECT_TRUE(adapter->session().want_write());
+ result = adapter->Send();
+ EXPECT_EQ(0, result);
+ EXPECT_THAT(visitor.data(), EqualsFrames({spdy::SpdyFrameType::SETTINGS}));
+}
+
TEST(NgHttp2AdapterTest, ClientSubmitRequest) {
DataSavingVisitor visitor;
auto adapter = NgHttp2Adapter::CreateClientAdapter(visitor);
diff --git a/http2/adapter/nghttp2_callbacks.cc b/http2/adapter/nghttp2_callbacks.cc
index 17fb1a0..5a4acd8 100644
--- a/http2/adapter/nghttp2_callbacks.cc
+++ b/http2/adapter/nghttp2_callbacks.cc
@@ -128,9 +128,12 @@
absl::string_view opaque_data(
reinterpret_cast<const char*>(frame->goaway.opaque_data),
frame->goaway.opaque_data_len);
- visitor->OnGoAway(frame->goaway.last_stream_id,
- ToHttp2ErrorCode(frame->goaway.error_code),
- opaque_data);
+ const bool result = visitor->OnGoAway(
+ frame->goaway.last_stream_id,
+ ToHttp2ErrorCode(frame->goaway.error_code), opaque_data);
+ if (!result) {
+ return NGHTTP2_ERR_CALLBACK_FAILURE;
+ }
break;
}
case NGHTTP2_WINDOW_UPDATE: {
diff --git a/http2/adapter/oghttp2_adapter_test.cc b/http2/adapter/oghttp2_adapter_test.cc
index 4cffe2e..4967f31 100644
--- a/http2/adapter/oghttp2_adapter_test.cc
+++ b/http2/adapter/oghttp2_adapter_test.cc
@@ -613,6 +613,82 @@
spdy::SpdyFrameType::RST_STREAM}));
}
+TEST(OgHttp2AdapterClientTest, ClientFailsOnGoAway) {
+ DataSavingVisitor visitor;
+ OgHttp2Adapter::Options options{.perspective = Perspective::kClient};
+ auto adapter = OgHttp2Adapter::Create(visitor, options);
+
+ testing::InSequence s;
+
+ const std::vector<const Header> headers1 =
+ ToHeaders({{":method", "GET"},
+ {":scheme", "http"},
+ {":authority", "example.com"},
+ {":path", "/this/is/request/one"}});
+
+ const char* kSentinel1 = "arbitrary pointer 1";
+ const int32_t stream_id1 =
+ adapter->SubmitRequest(headers1, nullptr, const_cast<char*>(kSentinel1));
+ ASSERT_GT(stream_id1, 0);
+ QUICHE_LOG(INFO) << "Created stream: " << stream_id1;
+
+ EXPECT_CALL(visitor, OnBeforeFrameSent(SETTINGS, 0, 0, 0x0));
+ EXPECT_CALL(visitor, OnFrameSent(SETTINGS, 0, 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);
+ absl::string_view data = visitor.data();
+ EXPECT_THAT(data, testing::StartsWith(spdy::kHttp2ConnectionHeaderPrefix));
+ data.remove_prefix(strlen(spdy::kHttp2ConnectionHeaderPrefix));
+ EXPECT_THAT(data, EqualsFrames({spdy::SpdyFrameType::SETTINGS,
+ spdy::SpdyFrameType::HEADERS}));
+ visitor.Clear();
+
+ const std::string stream_frames =
+ TestFrameSequence()
+ .ServerPreface()
+ .Headers(1,
+ {{":status", "200"},
+ {"server", "my-fake-server"},
+ {"date", "Tue, 6 Apr 2021 12:54:01 GMT"}},
+ /*fin=*/false)
+ .GoAway(1, Http2ErrorCode::INTERNAL_ERROR, "indigestion")
+ .Data(1, "This is the response body.")
+ .Serialize();
+
+ // Server preface (empty SETTINGS)
+ EXPECT_CALL(visitor, OnFrameHeader(0, 0, SETTINGS, 0));
+ EXPECT_CALL(visitor, OnSettingsStart());
+ EXPECT_CALL(visitor, OnSettingsEnd());
+
+ EXPECT_CALL(visitor, OnFrameHeader(1, _, HEADERS, 4));
+ EXPECT_CALL(visitor, OnBeginHeadersForStream(1));
+ EXPECT_CALL(visitor, OnHeaderForStream(1, ":status", "200"));
+ EXPECT_CALL(visitor, OnHeaderForStream(1, "server", "my-fake-server"));
+ EXPECT_CALL(visitor,
+ OnHeaderForStream(1, "date", "Tue, 6 Apr 2021 12:54:01 GMT"));
+ EXPECT_CALL(visitor, OnEndHeadersForStream(1));
+ EXPECT_CALL(visitor, OnFrameHeader(0, _, GOAWAY, 0));
+ // TODO(birenroy): Pass the GOAWAY opaque data through the oghttp2 stack.
+ EXPECT_CALL(visitor, OnGoAway(1, Http2ErrorCode::INTERNAL_ERROR, ""))
+ .WillOnce(testing::Return(false));
+ EXPECT_CALL(visitor, OnConnectionError());
+
+ const int64_t stream_result = adapter->ProcessBytes(stream_frames);
+ EXPECT_GT(stream_result, 0);
+ EXPECT_LT(stream_result, static_cast<int64_t>(stream_frames.size()));
+
+ EXPECT_CALL(visitor, OnBeforeFrameSent(SETTINGS, 0, 0, 0x1));
+ EXPECT_CALL(visitor, OnFrameSent(SETTINGS, 0, 0, 0x1, 0));
+
+ EXPECT_TRUE(adapter->session().want_write());
+ result = adapter->Send();
+ EXPECT_EQ(0, result);
+ EXPECT_THAT(visitor.data(), EqualsFrames({spdy::SpdyFrameType::SETTINGS}));
+}
+
TEST_F(OgHttp2AdapterTest, SubmitMetadata) {
auto source = absl::make_unique<TestMetadataSource>(ToHeaderBlock(ToHeaders(
{{"query-cost", "is too darn high"}, {"secret-sauce", "hollandaise"}})));
diff --git a/http2/adapter/oghttp2_session.cc b/http2/adapter/oghttp2_session.cc
index 39703a4..8a0dcd4 100644
--- a/http2/adapter/oghttp2_session.cc
+++ b/http2/adapter/oghttp2_session.cc
@@ -684,8 +684,11 @@
void OgHttp2Session::OnGoAway(spdy::SpdyStreamId last_accepted_stream_id,
spdy::SpdyErrorCode error_code) {
received_goaway_ = true;
- visitor_.OnGoAway(last_accepted_stream_id, TranslateErrorCode(error_code),
- "");
+ const bool result = visitor_.OnGoAway(last_accepted_stream_id,
+ TranslateErrorCode(error_code), "");
+ if (!result) {
+ decoder_.StopProcessing();
+ }
}
bool OgHttp2Session::OnGoAwayFrameData(const char* /*goaway_data*/, size_t
diff --git a/http2/adapter/recording_http2_visitor.cc b/http2/adapter/recording_http2_visitor.cc
index 767f3a7..e4bba9b 100644
--- a/http2/adapter/recording_http2_visitor.cc
+++ b/http2/adapter/recording_http2_visitor.cc
@@ -105,12 +105,13 @@
promised_stream_id));
}
-void RecordingHttp2Visitor::OnGoAway(Http2StreamId last_accepted_stream_id,
+bool RecordingHttp2Visitor::OnGoAway(Http2StreamId last_accepted_stream_id,
Http2ErrorCode error_code,
absl::string_view opaque_data) {
events_.push_back(
absl::StrFormat("OnGoAway %d %s %s", last_accepted_stream_id,
Http2ErrorCodeToString(error_code), opaque_data));
+ return true;
}
void RecordingHttp2Visitor::OnWindowUpdate(Http2StreamId stream_id,
diff --git a/http2/adapter/recording_http2_visitor.h b/http2/adapter/recording_http2_visitor.h
index ee06b1e..2e9d983 100644
--- a/http2/adapter/recording_http2_visitor.h
+++ b/http2/adapter/recording_http2_visitor.h
@@ -48,7 +48,7 @@
void OnPing(Http2PingId ping_id, bool is_ack) override;
void OnPushPromiseForStream(Http2StreamId stream_id,
Http2StreamId promised_stream_id) override;
- void OnGoAway(Http2StreamId last_accepted_stream_id,
+ bool OnGoAway(Http2StreamId last_accepted_stream_id,
Http2ErrorCode error_code,
absl::string_view opaque_data) override;
void OnWindowUpdate(Http2StreamId stream_id, int window_increment) override;
