Allows request trailers in oghttp2.
PiperOrigin-RevId: 414560203
diff --git a/http2/adapter/header_validator.cc b/http2/adapter/header_validator.cc
index c68ac35..e37b737 100644
--- a/http2/adapter/header_validator.cc
+++ b/http2/adapter/header_validator.cc
@@ -29,6 +29,10 @@
return pseudo_headers == *kRequiredHeaders;
}
+bool ValidateRequestTrailers(const std::vector<std::string>& pseudo_headers) {
+ return pseudo_headers.empty();
+}
+
bool ValidateResponseHeaders(const std::vector<std::string>& pseudo_headers) {
static const std::vector<std::string>* kRequiredHeaders =
new std::vector<std::string>({":status"});
@@ -91,6 +95,8 @@
switch (type) {
case HeaderType::REQUEST:
return ValidateRequestHeaders(pseudo_headers_);
+ case HeaderType::REQUEST_TRAILER:
+ return ValidateRequestTrailers(pseudo_headers_);
case HeaderType::RESPONSE_100:
case HeaderType::RESPONSE:
return ValidateResponseHeaders(pseudo_headers_);
diff --git a/http2/adapter/header_validator.h b/http2/adapter/header_validator.h
index dd8ab59..a27fdde 100644
--- a/http2/adapter/header_validator.h
+++ b/http2/adapter/header_validator.h
@@ -12,6 +12,7 @@
enum class HeaderType : uint8_t {
REQUEST,
+ REQUEST_TRAILER,
RESPONSE_100,
RESPONSE,
RESPONSE_TRAILER,
diff --git a/http2/adapter/nghttp2_adapter_test.cc b/http2/adapter/nghttp2_adapter_test.cc
index 23322cb..8b7d694 100644
--- a/http2/adapter/nghttp2_adapter_test.cc
+++ b/http2/adapter/nghttp2_adapter_test.cc
@@ -402,6 +402,54 @@
EXPECT_THAT(visitor.data(), EqualsFrames({spdy::SpdyFrameType::SETTINGS}));
}
+TEST(NgHttp2AdapterTest, ClientSendsTrailers) {
+ 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 std::string kBody = "This is an example request body.";
+ auto body1 = absl::make_unique<TestDataFrameSource>(visitor, false);
+ body1->AppendPayload(kBody);
+ // nghttp2 does not require that the data source indicate the end of data
+ // before trailers are enqueued.
+
+ const int32_t stream_id1 =
+ adapter->SubmitRequest(headers1, std::move(body1), nullptr);
+ ASSERT_GT(stream_id1, 0);
+
+ EXPECT_CALL(visitor, OnBeforeFrameSent(HEADERS, stream_id1, _, 0x4));
+ EXPECT_CALL(visitor, OnFrameSent(HEADERS, stream_id1, _, 0x4, 0));
+ EXPECT_CALL(visitor, OnFrameSent(DATA, stream_id1, _, 0x0, 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,
+ spdy::SpdyFrameType::DATA}));
+ visitor.Clear();
+
+ const std::vector<const Header> trailers1 =
+ ToHeaders({{"extra-info", "Trailers are weird but good?"}});
+ adapter->SubmitTrailer(stream_id1, trailers1);
+
+ EXPECT_CALL(visitor, OnBeforeFrameSent(HEADERS, stream_id1, _, 0x5));
+ EXPECT_CALL(visitor, OnFrameSent(HEADERS, stream_id1, _, 0x5, 0));
+
+ result = adapter->Send();
+ EXPECT_EQ(0, result);
+ data = visitor.data();
+ EXPECT_THAT(data, EqualsFrames({spdy::SpdyFrameType::HEADERS}));
+}
+
TEST(NgHttp2AdapterTest, ClientHandlesMetadata) {
DataSavingVisitor visitor;
auto adapter = NgHttp2Adapter::CreateClientAdapter(visitor);
@@ -2756,6 +2804,87 @@
absl::StrJoin(visitor.GetMetadata(1), ""));
}
+TEST(NgHttp2AdapterTest, ServerRespondsToRequestWithTrailers) {
+ DataSavingVisitor visitor;
+ auto adapter = NgHttp2Adapter::CreateServerAdapter(visitor);
+ 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"}})
+ .Data(1, "Example data, woohoo.")
+ .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, ":method", "GET"));
+ 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));
+ EXPECT_CALL(visitor, OnFrameHeader(1, _, DATA, 0));
+ EXPECT_CALL(visitor, OnBeginDataForStream(1, _));
+ EXPECT_CALL(visitor, OnDataForStream(1, _));
+
+ int64_t result = adapter->ProcessBytes(frames);
+ EXPECT_EQ(frames.size(), static_cast<size_t>(result));
+
+ const std::vector<const Header> headers1 = ToHeaders({{":status", "200"}});
+ auto body1 = absl::make_unique<TestDataFrameSource>(visitor, true);
+ TestDataFrameSource* body1_ptr = body1.get();
+
+ int submit_result = adapter->SubmitResponse(1, headers1, std::move(body1));
+ ASSERT_EQ(0, submit_result);
+
+ EXPECT_CALL(visitor, OnBeforeFrameSent(SETTINGS, 0, _, 0x1));
+ EXPECT_CALL(visitor, OnFrameSent(SETTINGS, 0, _, 0x1, 0));
+ EXPECT_CALL(visitor, OnBeforeFrameSent(HEADERS, 1, _, 0x4));
+ EXPECT_CALL(visitor, OnFrameSent(HEADERS, 1, _, 0x4, 0));
+
+ int send_result = adapter->Send();
+ EXPECT_EQ(0, send_result);
+ EXPECT_THAT(visitor.data(), EqualsFrames({spdy::SpdyFrameType::SETTINGS,
+ spdy::SpdyFrameType::HEADERS}));
+ visitor.Clear();
+
+ const std::string more_frames =
+ TestFrameSequence()
+ .Headers(1, {{"extra-info", "Trailers are weird but good?"}},
+ /*fin=*/true)
+ .Serialize();
+
+ EXPECT_CALL(visitor, OnFrameHeader(1, _, HEADERS, 5));
+ EXPECT_CALL(visitor, OnBeginHeadersForStream(1));
+ EXPECT_CALL(visitor, OnHeaderForStream(1, "extra-info",
+ "Trailers are weird but good?"));
+ EXPECT_CALL(visitor, OnEndHeadersForStream(1));
+ EXPECT_CALL(visitor, OnEndStream(1));
+
+ result = adapter->ProcessBytes(more_frames);
+ EXPECT_EQ(more_frames.size(), static_cast<size_t>(result));
+
+ body1_ptr->EndData();
+ EXPECT_EQ(true, adapter->ResumeStream(1));
+
+ EXPECT_CALL(visitor, OnFrameSent(DATA, 1, 0, 0x1, 0));
+ EXPECT_CALL(visitor, OnCloseStream(1, Http2ErrorCode::HTTP2_NO_ERROR));
+
+ send_result = adapter->Send();
+ EXPECT_EQ(0, send_result);
+ EXPECT_THAT(visitor.data(), EqualsFrames({spdy::SpdyFrameType::DATA}));
+}
+
TEST(NgHttp2AdapterTest, ServerSubmitsResponseWithDataSourceError) {
DataSavingVisitor visitor;
auto adapter = NgHttp2Adapter::CreateServerAdapter(visitor);
diff --git a/http2/adapter/oghttp2_adapter_test.cc b/http2/adapter/oghttp2_adapter_test.cc
index ea6eaf3..13d79a8 100644
--- a/http2/adapter/oghttp2_adapter_test.cc
+++ b/http2/adapter/oghttp2_adapter_test.cc
@@ -418,6 +418,57 @@
EXPECT_THAT(visitor.data(), EqualsFrames({spdy::SpdyFrameType::SETTINGS}));
}
+TEST(OgHttp2AdapterClientTest, ClientSendsTrailers) {
+ 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 std::string kBody = "This is an example request body.";
+ auto body1 = absl::make_unique<TestDataFrameSource>(visitor, false);
+ body1->AppendPayload(kBody);
+ body1->EndData();
+
+ const int32_t stream_id1 =
+ adapter->SubmitRequest(headers1, std::move(body1), nullptr);
+ ASSERT_GT(stream_id1, 0);
+
+ EXPECT_CALL(visitor, OnBeforeFrameSent(SETTINGS, 0, _, 0x0));
+ EXPECT_CALL(visitor, OnFrameSent(SETTINGS, 0, _, 0x0, 0));
+ EXPECT_CALL(visitor, OnBeforeFrameSent(HEADERS, stream_id1, _, 0x4));
+ EXPECT_CALL(visitor, OnFrameSent(HEADERS, stream_id1, _, 0x4, 0));
+ EXPECT_CALL(visitor, OnFrameSent(DATA, stream_id1, _, 0x0, 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,
+ spdy::SpdyFrameType::DATA}));
+ visitor.Clear();
+
+ const std::vector<const Header> trailers1 =
+ ToHeaders({{"extra-info", "Trailers are weird but good?"}});
+ adapter->SubmitTrailer(stream_id1, trailers1);
+
+ EXPECT_CALL(visitor, OnBeforeFrameSent(HEADERS, stream_id1, _, 0x5));
+ EXPECT_CALL(visitor, OnFrameSent(HEADERS, stream_id1, _, 0x5, 0));
+
+ result = adapter->Send();
+ EXPECT_EQ(0, result);
+ data = visitor.data();
+ EXPECT_THAT(data, EqualsFrames({spdy::SpdyFrameType::HEADERS}));
+}
+
TEST(OgHttp2AdapterClientTest, ClientHandlesMetadata) {
DataSavingVisitor visitor;
OgHttp2Adapter::Options options{.perspective = Perspective::kClient};
@@ -1955,6 +2006,91 @@
absl::StrJoin(visitor.GetMetadata(1), ""));
}
+TEST(OgHttp2AdapterServerTest, ServerRespondsToRequestWithTrailers) {
+ DataSavingVisitor visitor;
+ OgHttp2Adapter::Options options{.perspective = Perspective::kServer};
+ 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"}})
+ .Data(1, "Example data, woohoo.")
+ .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, ":method", "GET"));
+ 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));
+ EXPECT_CALL(visitor, OnFrameHeader(1, _, DATA, 0));
+ EXPECT_CALL(visitor, OnBeginDataForStream(1, _));
+ EXPECT_CALL(visitor, OnDataForStream(1, _));
+
+ int64_t result = adapter->ProcessBytes(frames);
+ EXPECT_EQ(frames.size(), static_cast<size_t>(result));
+
+ const std::vector<const Header> headers1 = ToHeaders({{":status", "200"}});
+ auto body1 = absl::make_unique<TestDataFrameSource>(visitor, true);
+ TestDataFrameSource* body1_ptr = body1.get();
+
+ int submit_result = adapter->SubmitResponse(1, headers1, std::move(body1));
+ ASSERT_EQ(0, submit_result);
+
+ 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, _, 0x4));
+ EXPECT_CALL(visitor, OnFrameSent(HEADERS, 1, _, 0x4, 0));
+
+ int send_result = adapter->Send();
+ EXPECT_EQ(0, send_result);
+ EXPECT_THAT(visitor.data(), EqualsFrames({spdy::SpdyFrameType::SETTINGS,
+ spdy::SpdyFrameType::SETTINGS,
+ spdy::SpdyFrameType::HEADERS}));
+ visitor.Clear();
+
+ const std::string more_frames =
+ TestFrameSequence()
+ .Headers(1, {{"extra-info", "Trailers are weird but good?"}},
+ /*fin=*/true)
+ .Serialize();
+
+ EXPECT_CALL(visitor, OnFrameHeader(1, _, HEADERS, 5));
+ EXPECT_CALL(visitor, OnBeginHeadersForStream(1));
+ EXPECT_CALL(visitor, OnHeaderForStream(1, "extra-info",
+ "Trailers are weird but good?"));
+ EXPECT_CALL(visitor, OnEndHeadersForStream(1));
+ EXPECT_CALL(visitor, OnEndStream(1));
+
+ result = adapter->ProcessBytes(more_frames);
+ EXPECT_EQ(more_frames.size(), static_cast<size_t>(result));
+
+ body1_ptr->EndData();
+ EXPECT_EQ(true, adapter->ResumeStream(1));
+
+ EXPECT_CALL(visitor, OnFrameSent(DATA, 1, 0, 0x1, 0));
+ EXPECT_CALL(visitor, OnCloseStream(1, Http2ErrorCode::HTTP2_NO_ERROR));
+
+ send_result = adapter->Send();
+ EXPECT_EQ(0, send_result);
+ EXPECT_THAT(visitor.data(), EqualsFrames({spdy::SpdyFrameType::DATA}));
+}
+
TEST(OgHttp2AdapterServerTest, ServerSubmitsResponseWithDataSourceError) {
DataSavingVisitor visitor;
OgHttp2Adapter::Options options{.perspective = Perspective::kServer};
diff --git a/http2/adapter/oghttp2_session.cc b/http2/adapter/oghttp2_session.cc
index 645a38d..a313755 100644
--- a/http2/adapter/oghttp2_session.cc
+++ b/http2/adapter/oghttp2_session.cc
@@ -210,6 +210,8 @@
result_ = Http2VisitorInterface::HEADER_OK;
const bool status = visitor_.OnBeginHeadersForStream(stream_id_);
if (!status) {
+ QUICHE_VLOG(1)
+ << "Visitor rejected header block, returning HEADER_CONNECTION_ERROR";
result_ = Http2VisitorInterface::HEADER_CONNECTION_ERROR;
}
validator_.StartHeaderBlock();
@@ -241,12 +243,14 @@
size_t /* compressed_header_bytes */) {
if (result_ == Http2VisitorInterface::HEADER_OK) {
if (!validator_.FinishHeaderBlock(type_)) {
+ QUICHE_VLOG(1)
+ << "FinishHeaderBlock returned false; returning HEADER_RST_STREAM";
result_ = Http2VisitorInterface::HEADER_RST_STREAM;
}
}
if (frame_contains_fin_ && IsResponse(type_) &&
StatusIs1xx(status_header())) {
- // Unexpected end of stream without final headers.
+ QUICHE_VLOG(1) << "Unexpected end of stream without final headers";
result_ = Http2VisitorInterface::HEADER_HTTP_MESSAGING;
}
if (result_ == Http2VisitorInterface::HEADER_OK) {
@@ -586,9 +590,14 @@
connection_can_write = SendMetadata(stream_id, state.outbound_metadata);
}
- if (state.outbound_body == nullptr) {
+ if (state.outbound_body == nullptr ||
+ (!options_.trailers_require_end_data && state.data_deferred)) {
// No data to send, but there might be trailers.
if (state.trailers != nullptr) {
+ // Trailers will include END_STREAM, so the data source can be discarded.
+ // Since data_deferred is true, there is no data waiting to be flushed for
+ // this stream.
+ state.outbound_body = nullptr;
auto block_ptr = std::move(state.trailers);
if (state.half_closed_local) {
QUICHE_LOG(ERROR) << "Sent fin; can't send trailers.";
@@ -775,10 +784,7 @@
// Save trailers so they can be written once data is done.
state.trailers =
absl::make_unique<spdy::SpdyHeaderBlock>(ToHeaderBlock(trailers));
- if (!options_.trailers_require_end_data) {
- iter->second.data_deferred = false;
- }
- if (!iter->second.data_deferred) {
+ if (!options_.trailers_require_end_data || !iter->second.data_deferred) {
write_scheduler_.MarkStreamReady(stream_id, false);
}
}
@@ -1022,6 +1028,10 @@
}
if (options_.perspective == Perspective::kServer) {
const auto new_stream_id = static_cast<Http2StreamId>(stream_id);
+ if (stream_map_.find(new_stream_id) != stream_map_.end() && fin) {
+ // Not a new stream, must be trailers.
+ return;
+ }
if (new_stream_id <= highest_processed_stream_id_) {
// A new stream ID lower than the watermark is a connection error.
LatchErrorAndNotify(Http2ErrorCode::PROTOCOL_ERROR,
@@ -1341,7 +1351,12 @@
HeaderType OgHttp2Session::NextHeaderType(
absl::optional<HeaderType> current_type) {
if (IsServerSession()) {
- return HeaderType::REQUEST;
+ if (!current_type) {
+ return HeaderType::REQUEST;
+ } else {
+ QUICHE_DCHECK(current_type == HeaderType::REQUEST);
+ return HeaderType::REQUEST_TRAILER;
+ }
} else if (!current_type ||
current_type.value() == HeaderType::RESPONSE_100) {
return HeaderType::RESPONSE;
