Let OgHttp2Session invoke the visitor OnMetadataEndForStream() callback for empty metadata payloads.
blaze test //third_party/envoy/src/test/integration:multiplexed_integration_test --test_sharding_strategy=disabled --test_filter=IpVersions/Http2MetadataIntegrationTest.UpstreamSendingEmptyMetadata/IPv6_Http2Downstream_Http2UpstreamWrappedHttp2
http://sponge2/0e30db4a-ede6-4d29-9110-24cb144b78a8
PiperOrigin-RevId: 421049675
diff --git a/http2/adapter/nghttp2_adapter_test.cc b/http2/adapter/nghttp2_adapter_test.cc
index ced8a82..4e010b1 100644
--- a/http2/adapter/nghttp2_adapter_test.cc
+++ b/http2/adapter/nghttp2_adapter_test.cc
@@ -594,6 +594,66 @@
EXPECT_THAT(visitor.data(), EqualsFrames({spdy::SpdyFrameType::SETTINGS}));
}
+TEST(NgHttp2AdapterTest, ClientHandlesMetadataWithEmptyPayload) {
+ DataSavingVisitor visitor;
+ auto adapter = NgHttp2Adapter::CreateClientAdapter(visitor);
+
+ testing::InSequence s;
+
+ const std::vector<Header> headers1 =
+ ToHeaders({{":method", "GET"},
+ {":scheme", "http"},
+ {":authority", "example.com"},
+ {":path", "/this/is/request/one"}});
+
+ const int32_t stream_id = adapter->SubmitRequest(headers1, nullptr, nullptr);
+ ASSERT_GT(stream_id, 0);
+
+ EXPECT_CALL(visitor, OnBeforeFrameSent(HEADERS, stream_id, _, 0x5));
+ EXPECT_CALL(visitor, OnFrameSent(HEADERS, stream_id, _, 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)
+ .Metadata(1, "")
+ .Data(1, "This is the response body.", true)
+ .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, _, _)).Times(3);
+ EXPECT_CALL(visitor, OnEndHeadersForStream(1));
+ EXPECT_CALL(visitor, OnFrameHeader(1, _, kMetadataFrameType, 4));
+ EXPECT_CALL(visitor, OnBeginMetadataForStream(1, _));
+ EXPECT_CALL(visitor, OnMetadataEndForStream(1));
+ EXPECT_CALL(visitor, OnFrameHeader(1, _, DATA, 1));
+ EXPECT_CALL(visitor, OnBeginDataForStream(1, _));
+ EXPECT_CALL(visitor, OnDataForStream(1, "This is the response body."));
+ EXPECT_CALL(visitor, OnEndStream(1));
+ EXPECT_CALL(visitor, OnCloseStream(1, Http2ErrorCode::HTTP2_NO_ERROR));
+
+ const int64_t stream_result = adapter->ProcessBytes(stream_frames);
+ EXPECT_EQ(stream_frames.size(), static_cast<size_t>(stream_result));
+}
+
TEST(NgHttp2AdapterTest, ClientHandlesMetadataWithError) {
DataSavingVisitor visitor;
auto adapter = NgHttp2Adapter::CreateClientAdapter(visitor);
diff --git a/http2/adapter/oghttp2_adapter_test.cc b/http2/adapter/oghttp2_adapter_test.cc
index 2d87254..e9b863c 100644
--- a/http2/adapter/oghttp2_adapter_test.cc
+++ b/http2/adapter/oghttp2_adapter_test.cc
@@ -622,6 +622,70 @@
EXPECT_THAT(visitor.data(), EqualsFrames({spdy::SpdyFrameType::SETTINGS}));
}
+TEST(OgHttp2AdapterClientTest, ClientHandlesMetadataWithEmptyPayload) {
+ DataSavingVisitor visitor;
+ OgHttp2Adapter::Options options{.perspective = Perspective::kClient};
+ auto adapter = OgHttp2Adapter::Create(visitor, options);
+
+ testing::InSequence s;
+
+ const std::vector<Header> headers1 =
+ ToHeaders({{":method", "GET"},
+ {":scheme", "http"},
+ {":authority", "example.com"},
+ {":path", "/this/is/request/one"}});
+
+ const int32_t stream_id = adapter->SubmitRequest(headers1, nullptr, nullptr);
+ ASSERT_GT(stream_id, 0);
+
+ EXPECT_CALL(visitor, OnBeforeFrameSent(SETTINGS, 0, _, 0x0));
+ EXPECT_CALL(visitor, OnFrameSent(SETTINGS, 0, _, 0x0, 0));
+ EXPECT_CALL(visitor, OnBeforeFrameSent(HEADERS, stream_id, _, 0x5));
+ EXPECT_CALL(visitor, OnFrameSent(HEADERS, stream_id, _, 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)
+ .Metadata(1, "")
+ .Data(1, "This is the response body.", true)
+ .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, _, _)).Times(3);
+ EXPECT_CALL(visitor, OnEndHeadersForStream(1));
+ EXPECT_CALL(visitor, OnFrameHeader(1, _, kMetadataFrameType, 4));
+ EXPECT_CALL(visitor, OnBeginMetadataForStream(1, _));
+ EXPECT_CALL(visitor, OnMetadataEndForStream(1));
+ EXPECT_CALL(visitor, OnFrameHeader(1, _, DATA, 1));
+ EXPECT_CALL(visitor, OnBeginDataForStream(1, _));
+ EXPECT_CALL(visitor, OnDataForStream(1, "This is the response body."));
+ EXPECT_CALL(visitor, OnEndStream(1));
+ EXPECT_CALL(visitor, OnCloseStream(1, Http2ErrorCode::HTTP2_NO_ERROR));
+
+ const int64_t stream_result = adapter->ProcessBytes(stream_frames);
+ EXPECT_EQ(stream_frames.size(), static_cast<size_t>(stream_result));
+}
+
TEST(OgHttp2AdapterClientTest, ClientHandlesMetadataWithPayloadError) {
DataSavingVisitor visitor;
OgHttp2Adapter::Options options{.perspective = Perspective::kClient};
diff --git a/http2/adapter/oghttp2_session.cc b/http2/adapter/oghttp2_session.cc
index 09d3d93..016792a 100644
--- a/http2/adapter/oghttp2_session.cc
+++ b/http2/adapter/oghttp2_session.cc
@@ -1315,6 +1315,11 @@
metadata_stream_id_ = stream_id;
metadata_length_ = length;
end_metadata_ = flags & kMetadataEndFlag;
+
+ // Empty metadata payloads will not trigger OnFramePayload(), so handle
+ // that possibility here.
+ MaybeHandleMetadataEndForStream(metadata_stream_id_);
+
return true;
} else {
QUICHE_DLOG(INFO) << "Unexpected frame type " << static_cast<int>(type)
@@ -1330,16 +1335,7 @@
metadata_stream_id_, absl::string_view(data, len));
if (payload_success) {
metadata_length_ -= len;
- if (metadata_length_ == 0 && end_metadata_) {
- const bool completion_success =
- visitor_.OnMetadataEndForStream(metadata_stream_id_);
- if (!completion_success) {
- fatal_visitor_callback_failure_ = true;
- decoder_.StopProcessing();
- }
- metadata_stream_id_ = 0;
- end_metadata_ = false;
- }
+ MaybeHandleMetadataEndForStream(metadata_stream_id_);
} else {
fatal_visitor_callback_failure_ = true;
decoder_.StopProcessing();
@@ -1588,5 +1584,17 @@
}
}
+void OgHttp2Session::MaybeHandleMetadataEndForStream(Http2StreamId stream_id) {
+ if (metadata_length_ == 0 && end_metadata_) {
+ const bool completion_success = visitor_.OnMetadataEndForStream(stream_id);
+ if (!completion_success) {
+ fatal_visitor_callback_failure_ = true;
+ decoder_.StopProcessing();
+ }
+ metadata_stream_id_ = 0;
+ end_metadata_ = false;
+ }
+}
+
} // namespace adapter
} // namespace http2
diff --git a/http2/adapter/oghttp2_session.h b/http2/adapter/oghttp2_session.h
index a3075c0..97cff20 100644
--- a/http2/adapter/oghttp2_session.h
+++ b/http2/adapter/oghttp2_session.h
@@ -380,6 +380,9 @@
// Updates internal state to prepare for sending an immediate GOAWAY.
void PrepareForImmediateGoAway();
+ // Handles the potential end of received metadata for the given `stream_id`.
+ void MaybeHandleMetadataEndForStream(Http2StreamId stream_id);
+
// Receives events when inbound frames are parsed.
Http2VisitorInterface& visitor_;
