quiche: handle multiple cookies and multiple set-cookie headers

include/envoy/http/header_map.h

@@ -294,7 +294,8 @@ class HeaderEntry {
   HEADER_FUNC(Protocol)                                                                            \
   HEADER_FUNC(Scheme)                                                                              \
   HEADER_FUNC(TE)                                                                                  \
-  HEADER_FUNC(UserAgent)
+  HEADER_FUNC(UserAgent)                                                                           \
+  HEADER_FUNC(Cookie)
 
 /**
  * Default O(1) response headers.

source/common/http/header_map_impl.cc

@@ -362,8 +362,10 @@ void HeaderMapImpl::insertByKey(HeaderString&& key, HeaderString&& value) {
     if (*lookup.value().entry_ == nullptr) {
       maybeCreateInline(lookup.value().entry_, *lookup.value().key_, std::move(value));
     } else {
+      const std::string delimiter =
+          (*lookup.value().key_ == Http::Headers::get().Cookie ? "; " : ",");
       const uint64_t added_size =
-          appendToHeader((*lookup.value().entry_)->value(), value.getStringView());
+          appendToHeader((*lookup.value().entry_)->value(), value.getStringView(), delimiter);
       addSize(added_size);
       value.clear();
     }

source/extensions/quic_listeners/quiche/envoy_quic_utils.cc

@@ -54,7 +54,8 @@ spdy::SpdyHeaderBlock envoyHeadersToSpdyHeaderBlock(const Http::HeaderMap& heade
   spdy::SpdyHeaderBlock header_block;
   headers.iterate([&header_block](const Http::HeaderEntry& header) -> Http::HeaderMap::Iterate {
     // The key-value pairs are copied.
-    header_block.insert({header.key().getStringView(), header.value().getStringView()});
+    header_block.AppendValueOrAddHeader(header.key().getStringView(),
+                                        header.value().getStringView());
     return Http::HeaderMap::Iterate::Continue;
   });
   return header_block;

test/common/http/header_map_impl_test.cc

@@ -741,6 +741,20 @@ TEST_P(HeaderMapImplTest, DoubleCookieAdd) {
   ASSERT_EQ(set_cookie_value[1]->value().getStringView(), "bar");
 }
 
+TEST_P(HeaderMapImplTest, CoalesceCookieHeadersWithSemicolon) {
+  TestRequestHeaderMapImpl headers;
+  const std::string foo("foo=1");
+  const std::string bar("bar=2");
+  const LowerCaseString& cookie = Http::Headers::get().Cookie;
+  headers.addReference(cookie, foo);
+  headers.addReference(cookie, bar);
+  EXPECT_EQ(1UL, headers.size());
+
+  const auto cookie_value = headers.get(LowerCaseString("cookie"));
+  ASSERT_EQ(cookie_value.size(), 1);
+  ASSERT_EQ(cookie_value[0]->value().getStringView(), "foo=1; bar=2");
+}
+
 TEST_P(HeaderMapImplTest, DoubleInlineSet) {
   TestRequestHeaderMapImpl headers;
   headers.setReferenceKey(Headers::get().ContentType, "blah");

test/extensions/quic_listeners/quiche/envoy_quic_server_stream_test.cc

@@ -183,6 +183,9 @@ TEST_P(EnvoyQuicServerStreamTest, GetRequestAndResponse) {
   request_headers.OnHeader(":authority", host_);
   request_headers.OnHeader(":method", "GET");
   request_headers.OnHeader(":path", "/");
+  // QUICHE stack doesn't coalesce Cookie headers for header compression optimization.
+  request_headers.OnHeader("cookie", "a=b");
+  request_headers.OnHeader("cookie", "c=d");
   request_headers.OnHeaderBlockEnd(/*uncompressed_header_bytes=*/0,
                                    /*compressed_header_bytes=*/0);
 
@@ -192,13 +195,18 @@ TEST_P(EnvoyQuicServerStreamTest, GetRequestAndResponse) {
         EXPECT_EQ(host_, headers->getHostValue());
         EXPECT_EQ("/", headers->getPathValue());
         EXPECT_EQ(Http::Headers::get().MethodValues.Get, headers->getMethodValue());
+        // Verify that the duplicated headers are handled correctly before passing to stream
+        // decoder.
+        EXPECT_EQ("a=b; c=d", headers->getCookieValue());
       }));
   if (quic::VersionUsesHttp3(quic_version_.transport_version)) {
     EXPECT_CALL(stream_decoder_, decodeData(BufferStringEqual(""), /*end_stream=*/true));
     spdy::SpdyHeaderBlock spdy_headers;
     spdy_headers[":authority"] = host_;
     spdy_headers[":method"] = "GET";
     spdy_headers[":path"] = "/";
+    spdy_headers.AppendValueOrAddHeader("cookie", "a=b");
+    spdy_headers.AppendValueOrAddHeader("cookie", "c=d");
     std::string payload = spdyHeaderToHttp3StreamPayload(spdy_headers);
     quic::QuicStreamFrame frame(stream_id_, true, 0, payload);
     quic_stream_->OnStreamFrame(frame);

test/extensions/quic_listeners/quiche/integration/quic_http_integration_test.cc

@@ -574,5 +574,35 @@ TEST_P(QuicHttpIntegrationTest, Reset101SwitchProtocolResponse) {
   EXPECT_FALSE(response->complete());
 }
 
+TEST_P(QuicHttpIntegrationTest, MultipleSetCookieAndCookieHeader) {
+  initialize();
+
+  codec_client_ = makeHttpConnection(lookupPort("http"));
+  auto encoder_decoder =
+      codec_client_->startRequest(Http::TestRequestHeaderMapImpl{{":method", "GET"},
+                                                                 {":path", "/dynamo/url"},
+                                                                 {":scheme", "http"},
+                                                                 {":authority", "host"},
+                                                                 {"cookie", "a=b"},
+                                                                 {"cookie", "c=d"}});
+  request_encoder_ = &encoder_decoder.first;
+  auto response = std::move(encoder_decoder.second);
+  codec_client_->sendData(*request_encoder_, 0, true);
+  waitForNextUpstreamRequest();
+
+  EXPECT_EQ(upstream_request_->headers().get(Http::Headers::get().Cookie)[0]->value(), "a=b; c=d");
+
+  upstream_request_->encodeHeaders(Http::TestResponseHeaderMapImpl{{":status", "200"},
+                                                                   {"set-cookie", "foo"},
+                                                                   {"set-cookie", "bar"}},
+                                   true);
+  response->waitForEndStream();
+  EXPECT_TRUE(response->complete());
+  const auto out = response->headers().get(Http::LowerCaseString("set-cookie"));
+  ASSERT_EQ(out.size(), 2);
+  ASSERT_EQ(out[0]->value().getStringView(), "foo");
+  ASSERT_EQ(out[1]->value().getStringView(), "bar");
+}
+
 } // namespace Quic
 } // namespace Envoy