Disables Cookie header field crumbling in OgHttp2Session by default.
Addresses https://github.com/envoyproxy/envoy/issues/32611.
PiperOrigin-RevId: 613371306
diff --git a/quiche/http2/adapter/nghttp2_adapter_test.cc b/quiche/http2/adapter/nghttp2_adapter_test.cc
index d118ed8..27770f5 100644
--- a/quiche/http2/adapter/nghttp2_adapter_test.cc
+++ b/quiche/http2/adapter/nghttp2_adapter_test.cc
@@ -5884,6 +5884,64 @@
EXPECT_EQ(client_visitor.data().size(), static_cast<size_t>(result));
}
+TEST(NgHttp2AdapterInteractionTest, ClientServerInteractionWithCookies) {
+ 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"},
+ {"cookie", "a; b=2; c"},
+ {"cookie", "d=e, f, g; h"}});
+
+ 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, _,
+ END_STREAM_FLAG | END_HEADERS_FLAG));
+ EXPECT_CALL(client_visitor,
+ OnFrameSent(HEADERS, stream_id1, _,
+ END_STREAM_FLAG | END_HEADERS_FLAG, 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"));
+ // Cookie values are preserved verbatim.
+ EXPECT_CALL(server_visitor, OnHeaderForStream(1, "cookie", "a; b=2; c"));
+ EXPECT_CALL(server_visitor, OnHeaderForStream(1, "cookie", "d=e, f, g; h"));
+ 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/quiche/http2/adapter/oghttp2_adapter_test.cc b/quiche/http2/adapter/oghttp2_adapter_test.cc
index 9cc9f57..92b9b86 100644
--- a/quiche/http2/adapter/oghttp2_adapter_test.cc
+++ b/quiche/http2/adapter/oghttp2_adapter_test.cc
@@ -6676,6 +6676,67 @@
EXPECT_EQ(client_visitor.data().size(), static_cast<size_t>(result));
}
+TEST(OgHttp2AdapterInteractionTest, ClientServerInteractionWithCookies) {
+ DataSavingVisitor client_visitor;
+ OgHttp2Adapter::Options client_options;
+ client_options.perspective = Perspective::kClient;
+ auto client_adapter = OgHttp2Adapter::Create(client_visitor, client_options);
+
+ // The Cookie header field value will be consolidated during HEADERS frame
+ // serialization.
+ const std::vector<Header> headers1 =
+ ToHeaders({{":method", "GET"},
+ {":scheme", "http"},
+ {":authority", "example.com"},
+ {":path", "/this/is/request/one"},
+ {"cookie", "a; b=2; c"},
+ {"cookie", "d=e, f, g; h"}});
+
+ 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, _,
+ END_STREAM_FLAG | END_HEADERS_FLAG));
+ EXPECT_CALL(client_visitor,
+ OnFrameSent(HEADERS, stream_id1, _,
+ END_STREAM_FLAG | END_HEADERS_FLAG, 0));
+ int send_result = client_adapter->Send();
+ EXPECT_EQ(0, send_result);
+
+ DataSavingVisitor server_visitor;
+ OgHttp2Adapter::Options server_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, "cookie", "a; b=2; c; d=e, f, g; h"));
+ 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(OgHttp2AdapterTest, ServerForbidsNewStreamBelowWatermark) {
DataSavingVisitor visitor;
OgHttp2Adapter::Options options;
diff --git a/quiche/http2/adapter/oghttp2_session.cc b/quiche/http2/adapter/oghttp2_session.cc
index e43939f..c29c848 100644
--- a/quiche/http2/adapter/oghttp2_session.cc
+++ b/quiche/http2/adapter/oghttp2_session.cc
@@ -376,6 +376,11 @@
headers_handler_.SetMaxFieldSize(*options_.max_header_field_size);
}
headers_handler_.SetAllowObsText(options_.allow_obs_text);
+ if (!options_.crumble_cookies) {
+ // As seen in https://github.com/envoyproxy/envoy/issues/32611, some HTTP/2
+ // endpoints don't properly handle multiple `Cookie` header fields.
+ framer_.GetHpackEncoder()->DisableCookieCrumbling();
+ }
}
OgHttp2Session::~OgHttp2Session() {}
diff --git a/quiche/http2/adapter/oghttp2_session.h b/quiche/http2/adapter/oghttp2_session.h
index dead807..d61106a 100644
--- a/quiche/http2/adapter/oghttp2_session.h
+++ b/quiche/http2/adapter/oghttp2_session.h
@@ -84,6 +84,9 @@
// If true, allows different values for `host` and `:authority` headers to
// be present in request headers.
bool allow_different_host_and_authority = false;
+ // If true, crumbles `Cookie` header field values for potentially better
+ // HPACK compression.
+ bool crumble_cookies = false;
};
OgHttp2Session(Http2VisitorInterface& visitor, Options options);
