Fixes the conversion of a Span<const Header> to spdy::SpdyHeaderBlock in oghttp2 when the span contains multiple header fields with the same name.
PiperOrigin-RevId: 423107278
diff --git a/http2/adapter/nghttp2_adapter_test.cc b/http2/adapter/nghttp2_adapter_test.cc
index ff92a65..f2dbd67 100644
--- a/http2/adapter/nghttp2_adapter_test.cc
+++ b/http2/adapter/nghttp2_adapter_test.cc
@@ -3254,6 +3254,67 @@
absl::StrJoin(visitor.GetMetadata(1), ""));
}
+TEST(NgHttp2AdapterTest, RepeatedHeaderNames) {
+ 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"},
+ {"accept", "text/plain"},
+ {"accept", "text/html"}},
+ /*fin=*/true)
+ .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, 5));
+ 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, OnHeaderForStream(1, "accept", "text/plain"));
+ EXPECT_CALL(visitor, OnHeaderForStream(1, "accept", "text/html"));
+ EXPECT_CALL(visitor, OnEndHeadersForStream(1));
+ EXPECT_CALL(visitor, OnEndStream(1));
+
+ int64_t result = adapter->ProcessBytes(frames);
+ EXPECT_EQ(frames.size(), static_cast<size_t>(result));
+
+ const std::vector<Header> headers1 = ToHeaders(
+ {{":status", "200"}, {"content-length", "10"}, {"content-length", "10"}});
+ auto body1 = absl::make_unique<TestDataFrameSource>(visitor, true);
+ body1->AppendPayload("perfection");
+ body1->EndData();
+
+ 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));
+ EXPECT_CALL(visitor, OnFrameSent(DATA, 1, 10, 0x1, 0));
+ EXPECT_CALL(visitor, OnCloseStream(1, Http2ErrorCode::HTTP2_NO_ERROR));
+
+ int send_result = adapter->Send();
+ EXPECT_EQ(0, send_result);
+ EXPECT_THAT(visitor.data(), EqualsFrames({spdy::SpdyFrameType::SETTINGS,
+ spdy::SpdyFrameType::HEADERS,
+ spdy::SpdyFrameType::DATA}));
+}
+
TEST(NgHttp2AdapterTest, ServerRespondsToRequestWithTrailers) {
DataSavingVisitor visitor;
auto adapter = NgHttp2Adapter::CreateServerAdapter(visitor);
@@ -3638,6 +3699,60 @@
EXPECT_THAT(visitor.data(), EqualsFrames({spdy::SpdyFrameType::SETTINGS}));
}
+TEST(NgHttp2AdapterInteractionTest,
+ ClientServerInteractionRepeatedHeaderNames) {
+ DataSavingVisitor client_visitor;
+ auto client_adapter = NgHttp2Adapter::CreateClientAdapter(client_visitor);
+
+ client_adapter->SubmitSettings({});
+
+ const std::vector<Header> headers1 =
+ ToHeaders({{":method", "GET"},
+ {":scheme", "http"},
+ {":authority", "example.com"},
+ {":path", "/this/is/request/one"},
+ {"accept", "text/plain"},
+ {"accept", "text/html"}});
+
+ const int32_t stream_id1 =
+ client_adapter->SubmitRequest(headers1, nullptr, nullptr);
+ ASSERT_GT(stream_id1, 0);
+
+ EXPECT_CALL(client_visitor, OnBeforeFrameSent(SETTINGS, 0, _, 0x0));
+ EXPECT_CALL(client_visitor, OnFrameSent(SETTINGS, 0, _, 0x0, 0));
+ EXPECT_CALL(client_visitor, OnBeforeFrameSent(HEADERS, stream_id1, _, 0x5));
+ EXPECT_CALL(client_visitor, OnFrameSent(HEADERS, stream_id1, _, 0x5, 0));
+ int send_result = client_adapter->Send();
+ EXPECT_EQ(0, send_result);
+
+ DataSavingVisitor server_visitor;
+ auto server_adapter = NgHttp2Adapter::CreateServerAdapter(server_visitor);
+
+ testing::InSequence s;
+
+ // Client preface (empty SETTINGS)
+ EXPECT_CALL(server_visitor, OnFrameHeader(0, _, SETTINGS, 0));
+ EXPECT_CALL(server_visitor, OnSettingsStart());
+ EXPECT_CALL(server_visitor, OnSetting(_)).Times(testing::AnyNumber());
+ EXPECT_CALL(server_visitor, OnSettingsEnd());
+ // Stream 1
+ EXPECT_CALL(server_visitor, OnFrameHeader(1, _, HEADERS, 5));
+ EXPECT_CALL(server_visitor, OnBeginHeadersForStream(1));
+ EXPECT_CALL(server_visitor, OnHeaderForStream(1, ":method", "GET"));
+ EXPECT_CALL(server_visitor, OnHeaderForStream(1, ":scheme", "http"));
+ EXPECT_CALL(server_visitor,
+ OnHeaderForStream(1, ":authority", "example.com"));
+ EXPECT_CALL(server_visitor,
+ OnHeaderForStream(1, ":path", "/this/is/request/one"));
+ EXPECT_CALL(server_visitor, OnHeaderForStream(1, "accept", "text/plain"));
+ EXPECT_CALL(server_visitor, OnHeaderForStream(1, "accept", "text/html"));
+ EXPECT_CALL(server_visitor, OnEndHeadersForStream(1));
+ EXPECT_CALL(server_visitor, OnEndStream(1));
+
+ int64_t result = server_adapter->ProcessBytes(client_visitor.data());
+ EXPECT_EQ(client_visitor.data().size(), static_cast<size_t>(result));
+}
+
TEST(NgHttp2AdapterTest, ServerForbidsWindowUpdateOnIdleStream) {
DataSavingVisitor visitor;
auto adapter = NgHttp2Adapter::CreateServerAdapter(visitor);
diff --git a/http2/adapter/oghttp2_adapter_test.cc b/http2/adapter/oghttp2_adapter_test.cc
index ee67139..d785891 100644
--- a/http2/adapter/oghttp2_adapter_test.cc
+++ b/http2/adapter/oghttp2_adapter_test.cc
@@ -2637,6 +2637,71 @@
absl::StrJoin(visitor.GetMetadata(1), ""));
}
+TEST(OgHttp2AdapterServerTest, RepeatedHeaderNames) {
+ 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"},
+ {"accept", "text/plain"},
+ {"accept", "text/html"}},
+ /*fin=*/true)
+ .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, 5));
+ 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, OnHeaderForStream(1, "accept", "text/plain"));
+ EXPECT_CALL(visitor, OnHeaderForStream(1, "accept", "text/html"));
+ EXPECT_CALL(visitor, OnEndHeadersForStream(1));
+ EXPECT_CALL(visitor, OnEndStream(1));
+
+ int64_t result = adapter->ProcessBytes(frames);
+ EXPECT_EQ(frames.size(), static_cast<size_t>(result));
+
+ const std::vector<Header> headers1 = ToHeaders(
+ {{":status", "200"}, {"content-length", "10"}, {"content-length", "10"}});
+ auto body1 = absl::make_unique<TestDataFrameSource>(visitor, true);
+ body1->AppendPayload("perfection");
+ body1->EndData();
+
+ 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));
+ EXPECT_CALL(visitor, OnFrameSent(DATA, 1, 10, 0x1, 0));
+ EXPECT_CALL(visitor, OnCloseStream(1, Http2ErrorCode::HTTP2_NO_ERROR));
+
+ int send_result = adapter->Send();
+ EXPECT_EQ(0, send_result);
+ EXPECT_THAT(visitor.data(),
+ EqualsFrames(
+ {spdy::SpdyFrameType::SETTINGS, spdy::SpdyFrameType::SETTINGS,
+ spdy::SpdyFrameType::HEADERS, spdy::SpdyFrameType::DATA}));
+}
+
TEST(OgHttp2AdapterServerTest, ServerRespondsToRequestWithTrailers) {
DataSavingVisitor visitor;
OgHttp2Adapter::Options options{.perspective = Perspective::kServer};
@@ -3752,6 +3817,60 @@
client_adapter->Send();
}
+TEST(OgHttp2AdapterInteractionTest,
+ ClientServerInteractionRepeatedHeaderNames) {
+ DataSavingVisitor client_visitor;
+ OgHttp2Adapter::Options client_options{.perspective = Perspective::kClient};
+ auto client_adapter = OgHttp2Adapter::Create(client_visitor, client_options);
+
+ const std::vector<Header> headers1 =
+ ToHeaders({{":method", "GET"},
+ {":scheme", "http"},
+ {":authority", "example.com"},
+ {":path", "/this/is/request/one"},
+ {"accept", "text/plain"},
+ {"accept", "text/html"}});
+
+ const int32_t stream_id1 =
+ client_adapter->SubmitRequest(headers1, nullptr, nullptr);
+ ASSERT_GT(stream_id1, 0);
+
+ EXPECT_CALL(client_visitor, OnBeforeFrameSent(SETTINGS, 0, _, 0x0));
+ EXPECT_CALL(client_visitor, OnFrameSent(SETTINGS, 0, _, 0x0, 0));
+ EXPECT_CALL(client_visitor, OnBeforeFrameSent(HEADERS, stream_id1, _, 0x5));
+ EXPECT_CALL(client_visitor, OnFrameSent(HEADERS, stream_id1, _, 0x5, 0));
+ int send_result = client_adapter->Send();
+ EXPECT_EQ(0, send_result);
+
+ DataSavingVisitor server_visitor;
+ OgHttp2Adapter::Options server_options{.perspective = Perspective::kServer};
+ auto server_adapter = OgHttp2Adapter::Create(server_visitor, server_options);
+
+ testing::InSequence s;
+
+ // Client preface (empty SETTINGS)
+ EXPECT_CALL(server_visitor, OnFrameHeader(0, _, SETTINGS, 0));
+ EXPECT_CALL(server_visitor, OnSettingsStart());
+ EXPECT_CALL(server_visitor, OnSetting(_)).Times(testing::AnyNumber());
+ EXPECT_CALL(server_visitor, OnSettingsEnd());
+ // Stream 1
+ EXPECT_CALL(server_visitor, OnFrameHeader(1, _, HEADERS, 5));
+ EXPECT_CALL(server_visitor, OnBeginHeadersForStream(1));
+ EXPECT_CALL(server_visitor, OnHeaderForStream(1, ":method", "GET"));
+ EXPECT_CALL(server_visitor, OnHeaderForStream(1, ":scheme", "http"));
+ EXPECT_CALL(server_visitor,
+ OnHeaderForStream(1, ":authority", "example.com"));
+ EXPECT_CALL(server_visitor,
+ OnHeaderForStream(1, ":path", "/this/is/request/one"));
+ EXPECT_CALL(server_visitor, OnHeaderForStream(1, "accept", "text/plain"));
+ EXPECT_CALL(server_visitor, OnHeaderForStream(1, "accept", "text/html"));
+ EXPECT_CALL(server_visitor, OnEndHeadersForStream(1));
+ EXPECT_CALL(server_visitor, OnEndStream(1));
+
+ int64_t result = server_adapter->ProcessBytes(client_visitor.data());
+ EXPECT_EQ(client_visitor.data().size(), static_cast<size_t>(result));
+}
+
TEST(OgHttp2AdapterServerTest, ServerForbidsNewStreamBelowWatermark) {
DataSavingVisitor visitor;
OgHttp2Adapter::Options options{.perspective = Perspective::kServer};
diff --git a/http2/adapter/oghttp2_util.cc b/http2/adapter/oghttp2_util.cc
index 3ed13dc..ed0e6f6 100644
--- a/http2/adapter/oghttp2_util.cc
+++ b/http2/adapter/oghttp2_util.cc
@@ -8,7 +8,7 @@
for (const Header& header : headers) {
absl::string_view name = GetStringView(header.first).first;
absl::string_view value = GetStringView(header.second).first;
- block[name] = value;
+ block.AppendValueOrAddHeader(name, value);
}
return block;
}
diff --git a/http2/adapter/oghttp2_util_test.cc b/http2/adapter/oghttp2_util_test.cc
new file mode 100644
index 0000000..6f29b26
--- /dev/null
+++ b/http2/adapter/oghttp2_util_test.cc
@@ -0,0 +1,83 @@
+#include "http2/adapter/oghttp2_util.h"
+
+#include <utility>
+#include <vector>
+
+#include "http2/adapter/http2_protocol.h"
+#include "http2/adapter/test_frame_sequence.h"
+#include "common/platform/api/quiche_test.h"
+
+namespace http2 {
+namespace adapter {
+namespace test {
+namespace {
+
+using HeaderPair = std::pair<absl::string_view, absl::string_view>;
+
+TEST(ToHeaderBlock, EmptySpan) {
+ spdy::SpdyHeaderBlock block = ToHeaderBlock({});
+ EXPECT_TRUE(block.empty());
+}
+
+TEST(ToHeaderBlock, ExampleRequestHeaders) {
+ const std::vector<HeaderPair> pairs = {{":authority", "example.com"},
+ {":method", "GET"},
+ {":path", "/example.html"},
+ {":scheme", "http"},
+ {"accept", "text/plain, text/html"}};
+ const std::vector<Header> headers = ToHeaders(pairs);
+ spdy::SpdyHeaderBlock block = ToHeaderBlock(headers);
+ EXPECT_THAT(block, testing::ElementsAreArray(pairs));
+}
+
+TEST(ToHeaderBlock, ExampleResponseHeaders) {
+ const std::vector<HeaderPair> pairs = {
+ {":status", "403"},
+ {"content-length", "1023"},
+ {"x-extra-info", "humblest apologies"}};
+ const std::vector<Header> headers = ToHeaders(pairs);
+ spdy::SpdyHeaderBlock block = ToHeaderBlock(headers);
+ EXPECT_THAT(block, testing::ElementsAreArray(pairs));
+}
+
+TEST(ToHeaderBlock, RepeatedRequestHeaderNames) {
+ const std::vector<HeaderPair> pairs = {
+ {":authority", "example.com"}, {":method", "GET"},
+ {":path", "/example.html"}, {":scheme", "http"},
+ {"cookie", "chocolate_chips=yes"}, {"accept", "text/plain, text/html"},
+ {"cookie", "raisins=no"}};
+ const std::vector<HeaderPair> expected = {
+ {":authority", "example.com"},
+ {":method", "GET"},
+ {":path", "/example.html"},
+ {":scheme", "http"},
+ {"cookie", "chocolate_chips=yes; raisins=no"},
+ {"accept", "text/plain, text/html"}};
+ const std::vector<Header> headers = ToHeaders(pairs);
+ spdy::SpdyHeaderBlock block = ToHeaderBlock(headers);
+ EXPECT_THAT(block, testing::ElementsAreArray(expected));
+}
+
+TEST(ToHeaderBlock, RepeatedResponseHeaderNames) {
+ const std::vector<HeaderPair> pairs = {
+ {":status", "403"}, {"x-extra-info", "sorry"},
+ {"content-length", "1023"}, {"x-extra-info", "humblest apologies"},
+ {"content-length", "1024"}, {"set-cookie", "chocolate_chips=yes"},
+ {"set-cookie", "raisins=no"}};
+ const std::vector<HeaderPair> expected = {
+ {":status", "403"},
+ {"x-extra-info", absl::string_view("sorry\0humblest apologies", 24)},
+ {"content-length", absl::string_view("1023"
+ "\0"
+ "1024",
+ 9)},
+ {"set-cookie", absl::string_view("chocolate_chips=yes\0raisins=no", 30)}};
+ const std::vector<Header> headers = ToHeaders(pairs);
+ spdy::SpdyHeaderBlock block = ToHeaderBlock(headers);
+ EXPECT_THAT(block, testing::ElementsAreArray(expected));
+}
+
+} // namespace
+} // namespace test
+} // namespace adapter
+} // namespace http2
