Adds a boolean return value to Http2VisitorInterface::OnEndHeadersForStream().
A visitor implementation can indicate a fatal error for the current HTTP/2 session by returning false from this method.
PiperOrigin-RevId: 392078979
diff --git a/http2/adapter/callback_visitor.cc b/http2/adapter/callback_visitor.cc
index 7849574..981180e 100644
--- a/http2/adapter/callback_visitor.cc
+++ b/http2/adapter/callback_visitor.cc
@@ -182,12 +182,13 @@
return HEADER_OK;
}
-void CallbackVisitor::OnEndHeadersForStream(Http2StreamId /*stream_id*/) {
+bool CallbackVisitor::OnEndHeadersForStream(Http2StreamId /*stream_id*/) {
if (callbacks_->on_frame_recv_callback) {
const int result = callbacks_->on_frame_recv_callback(
nullptr, ¤t_frame_, user_data_);
- QUICHE_DCHECK_EQ(0, result);
+ return result == 0;
}
+ return true;
}
void CallbackVisitor::OnBeginDataForStream(Http2StreamId /*stream_id*/,
diff --git a/http2/adapter/callback_visitor.h b/http2/adapter/callback_visitor.h
index f33df0e..6a15f0a 100644
--- a/http2/adapter/callback_visitor.h
+++ b/http2/adapter/callback_visitor.h
@@ -36,7 +36,7 @@
OnHeaderResult OnHeaderForStream(Http2StreamId stream_id,
absl::string_view name,
absl::string_view value) override;
- void OnEndHeadersForStream(Http2StreamId stream_id) override;
+ bool OnEndHeadersForStream(Http2StreamId stream_id) override;
void OnBeginDataForStream(Http2StreamId stream_id,
size_t payload_length) override;
void OnDataForStream(Http2StreamId stream_id,
diff --git a/http2/adapter/http2_visitor_interface.h b/http2/adapter/http2_visitor_interface.h
index a95f70a..535b174 100644
--- a/http2/adapter/http2_visitor_interface.h
+++ b/http2/adapter/http2_visitor_interface.h
@@ -104,7 +104,7 @@
// Called when the connection has received the complete header block for a
// logical HEADERS frame on a stream (which may contain CONTINUATION frames,
// transparent to the user).
- virtual void OnEndHeadersForStream(Http2StreamId stream_id) = 0;
+ virtual bool OnEndHeadersForStream(Http2StreamId stream_id) = 0;
// Called when the connection receives the beginning of a DATA frame. The data
// payload will be provided via subsequent calls to OnDataForStream().
diff --git a/http2/adapter/mock_http2_visitor.h b/http2/adapter/mock_http2_visitor.h
index a489270..2ca83a4 100644
--- a/http2/adapter/mock_http2_visitor.h
+++ b/http2/adapter/mock_http2_visitor.h
@@ -18,6 +18,7 @@
ON_CALL(*this, OnBeginHeadersForStream)
.WillByDefault(testing::Return(true));
ON_CALL(*this, OnHeaderForStream).WillByDefault(testing::Return(HEADER_OK));
+ ON_CALL(*this, OnEndHeadersForStream).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));
@@ -43,9 +44,7 @@
absl::string_view value),
(override));
- MOCK_METHOD(void,
- OnEndHeadersForStream,
- (Http2StreamId stream_id),
+ MOCK_METHOD(bool, OnEndHeadersForStream, (Http2StreamId stream_id),
(override));
MOCK_METHOD(void,
diff --git a/http2/adapter/nghttp2_adapter_test.cc b/http2/adapter/nghttp2_adapter_test.cc
index 4435810..ad66439 100644
--- a/http2/adapter/nghttp2_adapter_test.cc
+++ b/http2/adapter/nghttp2_adapter_test.cc
@@ -1257,6 +1257,7 @@
EXPECT_CALL(visitor, OnEndHeadersForStream(1))
.WillOnce(testing::InvokeWithoutArgs([&adapter, kSentinel1]() {
adapter->SetStreamUserData(1, const_cast<char*>(kSentinel1));
+ return true;
}));
EXPECT_CALL(visitor, OnFrameHeader(1, 4, WINDOW_UPDATE, 0));
EXPECT_CALL(visitor, OnWindowUpdate(1, 2000));
@@ -1379,6 +1380,54 @@
spdy::SpdyFrameType::RST_STREAM}));
}
+TEST(NgHttp2AdapterTest, ServerErrorAfterHandlingHeaders) {
+ DataSavingVisitor visitor;
+ auto adapter = NgHttp2Adapter::CreateServerAdapter(visitor);
+
+ const std::string frames = TestFrameSequence()
+ .ClientPreface()
+ .Headers(1,
+ {{":method", "POST"},
+ {":scheme", "https"},
+ {":authority", "example.com"},
+ {":path", "/this/is/request/one"}},
+ /*fin=*/false)
+ .WindowUpdate(1, 2000)
+ .Data(1, "This is the request body.")
+ .WindowUpdate(0, 2000)
+ .Serialize();
+ testing::InSequence s;
+
+ // Client 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, ":method", "POST"));
+ EXPECT_CALL(visitor, OnHeaderForStream(1, ":scheme", "https"));
+ EXPECT_CALL(visitor, OnHeaderForStream(1, ":authority", "example.com"));
+ EXPECT_CALL(visitor, OnHeaderForStream(1, ":path", "/this/is/request/one"));
+ EXPECT_CALL(visitor, OnEndHeadersForStream(1))
+ .WillOnce(testing::Return(false));
+ EXPECT_CALL(visitor, OnConnectionError());
+
+ const int64_t result = adapter->ProcessBytes(frames);
+ EXPECT_EQ(-902, result);
+
+ EXPECT_TRUE(adapter->session().want_write());
+
+ EXPECT_CALL(visitor, OnBeforeFrameSent(SETTINGS, 0, 0, 0x1));
+ EXPECT_CALL(visitor, OnFrameSent(SETTINGS, 0, 0, 0x1, 0));
+
+ int send_result = adapter->Send();
+ // Some bytes should have been serialized.
+ EXPECT_EQ(0, send_result);
+ // SETTINGS ack
+ EXPECT_THAT(visitor.data(), EqualsFrames({spdy::SpdyFrameType::SETTINGS}));
+}
+
TEST(NgHttp2AdapterTest, ServerSubmitResponse) {
DataSavingVisitor visitor;
auto adapter = NgHttp2Adapter::CreateServerAdapter(visitor);
@@ -1411,6 +1460,7 @@
EXPECT_CALL(visitor, OnEndHeadersForStream(1))
.WillOnce(testing::InvokeWithoutArgs([&adapter, kSentinel1]() {
adapter->SetStreamUserData(1, const_cast<char*>(kSentinel1));
+ return true;
}));
EXPECT_CALL(visitor, OnEndStream(1));
diff --git a/http2/adapter/nghttp2_callbacks.cc b/http2/adapter/nghttp2_callbacks.cc
index 8956b7a..e22d437 100644
--- a/http2/adapter/nghttp2_callbacks.cc
+++ b/http2/adapter/nghttp2_callbacks.cc
@@ -67,7 +67,10 @@
break;
case NGHTTP2_HEADERS: {
if (frame->hd.flags & NGHTTP2_FLAG_END_HEADERS) {
- visitor->OnEndHeadersForStream(stream_id);
+ const bool result = visitor->OnEndHeadersForStream(stream_id);
+ if (!result) {
+ return NGHTTP2_ERR_CALLBACK_FAILURE;
+ }
}
if (frame->hd.flags & NGHTTP2_FLAG_END_STREAM) {
visitor->OnEndStream(stream_id);
diff --git a/http2/adapter/oghttp2_adapter_test.cc b/http2/adapter/oghttp2_adapter_test.cc
index 053b2d5..75f6121 100644
--- a/http2/adapter/oghttp2_adapter_test.cc
+++ b/http2/adapter/oghttp2_adapter_test.cc
@@ -1010,6 +1010,59 @@
spdy::SpdyFrameType::RST_STREAM}));
}
+TEST(OgHttp2AdapterServerTest, ServerErrorAfterHandlingHeaders) {
+ DataSavingVisitor visitor;
+ OgHttp2Adapter::Options options{.perspective = Perspective::kServer};
+ auto adapter = OgHttp2Adapter::Create(visitor, options);
+
+ const std::string frames = TestFrameSequence()
+ .ClientPreface()
+ .Headers(1,
+ {{":method", "POST"},
+ {":scheme", "https"},
+ {":authority", "example.com"},
+ {":path", "/this/is/request/one"}},
+ /*fin=*/false)
+ .WindowUpdate(1, 2000)
+ .Data(1, "This is the request body.")
+ .WindowUpdate(0, 2000)
+ .Serialize();
+ testing::InSequence s;
+
+ // Client 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, ":method", "POST"));
+ EXPECT_CALL(visitor, OnHeaderForStream(1, ":scheme", "https"));
+ EXPECT_CALL(visitor, OnHeaderForStream(1, ":authority", "example.com"));
+ EXPECT_CALL(visitor, OnHeaderForStream(1, ":path", "/this/is/request/one"));
+ EXPECT_CALL(visitor, OnEndHeadersForStream(1))
+ .WillOnce(testing::Return(false));
+ EXPECT_CALL(visitor, OnConnectionError());
+
+ const int64_t result = adapter->ProcessBytes(frames);
+ EXPECT_GT(result, 0);
+ EXPECT_LT(result, static_cast<int64_t>(frames.size()));
+
+ EXPECT_TRUE(adapter->session().want_write());
+
+ EXPECT_CALL(visitor, OnBeforeFrameSent(SETTINGS, 0, 0, 0x0));
+ EXPECT_CALL(visitor, OnFrameSent(SETTINGS, 0, 0, 0x0, 0));
+ EXPECT_CALL(visitor, OnBeforeFrameSent(SETTINGS, 0, 0, 0x1));
+ EXPECT_CALL(visitor, OnFrameSent(SETTINGS, 0, 0, 0x1, 0));
+
+ int send_result = adapter->Send();
+ // Some bytes should have been serialized.
+ EXPECT_EQ(0, send_result);
+ // SETTINGS and SETTINGS ack
+ EXPECT_THAT(visitor.data(), EqualsFrames({spdy::SpdyFrameType::SETTINGS,
+ spdy::SpdyFrameType::SETTINGS}));
+}
+
} // namespace
} // namespace test
} // namespace adapter
diff --git a/http2/adapter/oghttp2_session.cc b/http2/adapter/oghttp2_session.cc
index bed7f4e..b3505b8 100644
--- a/http2/adapter/oghttp2_session.cc
+++ b/http2/adapter/oghttp2_session.cc
@@ -128,7 +128,10 @@
size_t /* uncompressed_header_bytes */,
size_t /* compressed_header_bytes */) {
if (result_ == Http2VisitorInterface::HEADER_OK) {
- visitor_.OnEndHeadersForStream(stream_id_);
+ const bool result = visitor_.OnEndHeadersForStream(stream_id_);
+ if (!result) {
+ session_.decoder_.StopProcessing();
+ }
} else {
session_.OnHeaderStatus(stream_id_, result_);
}
diff --git a/http2/adapter/oghttp2_session_test.cc b/http2/adapter/oghttp2_session_test.cc
index 3a96355..8e088ee 100644
--- a/http2/adapter/oghttp2_session_test.cc
+++ b/http2/adapter/oghttp2_session_test.cc
@@ -550,6 +550,7 @@
EXPECT_CALL(visitor, OnEndHeadersForStream(1))
.WillOnce(testing::InvokeWithoutArgs([&session, kSentinel1]() {
session.SetStreamUserData(1, const_cast<char*>(kSentinel1));
+ return true;
}));
EXPECT_CALL(visitor, OnFrameHeader(1, 4, WINDOW_UPDATE, 0));
EXPECT_CALL(visitor, OnWindowUpdate(1, 2000));
@@ -686,6 +687,7 @@
EXPECT_CALL(visitor, OnEndHeadersForStream(1))
.WillOnce(testing::InvokeWithoutArgs([&session, kSentinel1]() {
session.SetStreamUserData(1, const_cast<char*>(kSentinel1));
+ return true;
}));
EXPECT_CALL(visitor, OnEndStream(1));
diff --git a/http2/adapter/recording_http2_visitor.cc b/http2/adapter/recording_http2_visitor.cc
index 9407ce0..b66d5e0 100644
--- a/http2/adapter/recording_http2_visitor.cc
+++ b/http2/adapter/recording_http2_visitor.cc
@@ -53,8 +53,9 @@
return HEADER_OK;
}
-void RecordingHttp2Visitor::OnEndHeadersForStream(Http2StreamId stream_id) {
+bool RecordingHttp2Visitor::OnEndHeadersForStream(Http2StreamId stream_id) {
events_.push_back(absl::StrFormat("OnEndHeadersForStream %d", stream_id));
+ return true;
}
void RecordingHttp2Visitor::OnBeginDataForStream(Http2StreamId stream_id,
diff --git a/http2/adapter/recording_http2_visitor.h b/http2/adapter/recording_http2_visitor.h
index a443905..9627445 100644
--- a/http2/adapter/recording_http2_visitor.h
+++ b/http2/adapter/recording_http2_visitor.h
@@ -34,7 +34,7 @@
OnHeaderResult OnHeaderForStream(Http2StreamId stream_id,
absl::string_view name,
absl::string_view value) override;
- void OnEndHeadersForStream(Http2StreamId stream_id) override;
+ bool OnEndHeadersForStream(Http2StreamId stream_id) override;
void OnBeginDataForStream(Http2StreamId stream_id,
size_t payload_length) override;
void OnDataForStream(Http2StreamId stream_id,
